fix(classifier): harden the README probe's error handling#55
Merged
Conversation
Follow-up to the vanished-star fix (#54), from a multi-model cross review (Codex GPT xhigh + Grok 4.5, three rounds to no findings). The GraphQL blob-OID probe now handles malformed and transient GitHub responses deterministically, and errors carry honest exit semantics: - isRepositoryNotFound swallows a GraphQL NOT_FOUND only when EVERY error is rooted at exactly the `repository` field (path ['repository'] or absent). A mixed array, a nested path, or a NOT_FOUND on another field now propagates instead of being masked as "repo gone". - A data-less 2xx envelope (raw @octokit/graphql yields undefined) or a record lacking the `repository` field is a transient glitch, not an absent README: it retries via the shared coordinator (github-client's requireResponse contract) instead of silently downgrading to REST. - classifyError treats HTTP 500 as retryable alongside 502/503/504 (a malformed request is already 400/422 -> terminal). - cli.ts fatal() honors ExporterError.exitCode, so a DeferredError (a probe that exhausted its bounded retries, a rate-limit cooldown) exits 20 -- "defer this run, keep last-known-good, retry next schedule" -- instead of masquerading as a terminal exit 10. Policy: a transient blip is absorbed by bounded retries; a sustained or persistent fault defers the whole run (exit 20) rather than degrading a repo to metadata, which would overwrite a good README annotation and be rejected by the provenance gate. The planner is unchanged. Verified end-to-end against the real deleted repo (returns null) and a live repo. Full suite: 699 tests green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YLoMBRETP2VWBxT1whUNc9
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
1 participant
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.
Summary
Follow-up hardening to the vanished-star fix (#54), driven by a three-round multi-model cross review (Codex GPT
xhigh+ Grok 4.5, iterated until both returned "No findings"). The GraphQL blob-OID README probe now handles malformed and transient GitHub responses deterministically, and errors carry honest exit semantics.planner.tsis unchanged — this is entirely in the probe seam,classifyError, and the CLI's error exit code.Changes
isRepositoryNotFound(readme-source.ts) — swallow a GraphQLNOT_FOUNDas "repo gone" only when every error is rooted at exactly therepositoryfield (path: ['repository'], or an absent path defensively). A mixed array, a nested path (['repository','object']), or a different-fieldNOT_FOUNDnow propagates instead of being masked.@octokit/graphqlyieldsundefined),{}, or[]lacks therepositoryfield and is a transient glitch, not an absent README: it retries via the sharedRetryCoordinator(matching github-client'srequireResponsecontract) instead of silently downgrading.classifyError(github-client/retry.ts) — HTTP 500 → retryable alongside 502/503/504 (a malformed request is already 400/422 → terminal).fatal()(cli.ts) — honorsExporterError.exitCode, so aDeferredError(a probe that exhausted its bounded retries, a rate-limit cooldown) exits 20 — "defer this run, keep last-known-good, retry next schedule" — instead of masquerading as a terminal exit 10.Design policy (chosen after review)
A transient blip is absorbed by bounded retries; a sustained or persistent fault defers the whole run (exit 20, keep last-known-good, retry next scheduled run) rather than degrading a repo to metadata. An earlier attempt to degrade-to-metadata / fall through to REST per repo was removed because the review showed it overwrites good README annotations and is rejected by the provenance gate.
Verification
curtischoutw/claude-institution) returnsnull; a live repo resolves normally.['repository']matching, mixed/nested/other-field propagation, path-less tolerance, empty/malformed-envelope retry, HTTP-500 retry, and DeferredError-on-exhaustion. Full repo suite: 699 tests green;format:check/lint/typecheckclean.🤖 Generated with Claude Code
https://claude.ai/code/session_01YLoMBRETP2VWBxT1whUNc9