Repository navigation
fix(backend): a plain 403 must not abort the rest of evidence collection (#621) - #640
Closed
Yuncan050115 wants to merge 2 commits into
Closed
Yuncan050115 wants to merge 2 commits into
Yuncan050115 wants to merge 2 commits into
Conversation
Yuncan050115
requested review from
AsperforMias,
KurosawaGeeker and
hikariming
as code owners
November 4, 2026 13:08
ghfind-review
Bot
added
the
review: low
Yuncan050115
force-pushed
the
fix/hydrate-evidence-plain-403
branch
from
November 4, 2026 16:38
2ff8f86 to
644e40a
Compare
Yuncan050115
changed the title
Yuncan050115
force-pushed
the
fix/hydrate-evidence-plain-403
branch
from
November 4, 2026 16:54
644e40a to
d69c03a
Compare
Yuncan050115
changed the title
Yuncan050115
force-pushed
the
fix/hydrate-evidence-plain-403
branch
from
November 5, 2026 16:28
a72d063 to
bb1f315
Compare
… abort hydrateTopRepoEvidence compared readmeErr/languageErr with `!=` against ErrGitHubUnavailable, but restGet wraps that sentinel with %w, so the comparison never matched and one unreadable README aborted the whole candidate loop. finishCollectedScan discards the returned error, so the scan still reported complete_public while later repositories silently lost their README, languages and root listing. Use errors.Is, which is already this package's idiom for wrapped sentinels. ErrGitHubRateLimited and the unwrapped 404 path are untouched, so a drained token pool still aborts and a missing README still means "no README". Adds three httptest cases; the first one fails on the pre-fix code. Fixes FindAny#621
FindAny#621 asks that a plain 403, a 403 carrying Retry-After, a rate-limit 403, a 429 and a cancellation are not swallowed into one outcome. The errors.Is change makes every wrapped ErrGitHubUnavailable skippable, so the visible difference between "skip this repository" and "abort the scan" now depends on how restGet classifies the response, not on this loop. Adds a table over the real request path covering all five classes, with request counts that also pin the retry budget (two attempts for a single-token pool): a plain 403 is requested once and skipped, Retry-After and rate-limit responses are retried before skipping or aborting, 429 and context.Canceled abort, and an aborted scan never touches a later repository. Refs FindAny#621
Yuncan050115
force-pushed
the
fix/hydrate-evidence-plain-403
branch
from
November 6, 2026 03:19
bb1f315 to
05b2edc
Compare
Collaborator
|
Since 2026-11-07 ghfind is split into three repositories. This repository now holds only the public scoring module, SDKs, CLI and the deploy workflows; the code this PR touches moved to a private repository, so it can no longer be merged here. The Go collector in 仓库已于 2026-11-07 拆分为三个仓库，本 PR 涉及的代码已迁入私有仓库，无法在此合并。Go 采集器已不再部署（生产评分运行在 Cloudflare Workers 的 TypeScript 实现上），此修复不会影响线上，关闭。感谢细致的分析与测试！ |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refs #621.
Summary
errors.Isfor the wrappedErrGitHubUnavailableinhydrateTopRepoEvidence, so a plain 403 skips one repository instead of aborting every later candidateOne README 403 previously ended the candidate loop while
finishCollectedScandiscarded the error and the scan still reportedcomplete_public. The second-pass merge that erases first-pass evidence is deliberately not in this PR (#645).Verification
go test ./internal/backend/ -run '^TestHydrateTopRepoEvidence' -count=1: 4 tests + 5 subtests pass. Withgithub_enrichment.goreverted to main, the two continue-cases and the two skip-class subtests fail and the three abort-cases still pass; green with the fix.go vet ./internal/backend/,go build ./internal/backend/,gofmt -l: clean.node ops/open-core/split.mjs check: 2292 files classified, 0 public->private references;blockers: 0.Package-wide
go test ./internal/backend/keeps a failure set identical to baseline (sorted-name diff empty both ways) — no C compiler here,go-sqlite3 requires cgo; full-modulego build ./...OOMs locally;pnpm scoring:boundaryhits the repo's ownC:\C:\...Windows path bug whilescoring/is untouched by this PR.