Skip to content

Retry transient GitHub reads before failing a Pass - #792

Merged
JacobStephens2 merged 6 commits into
mainfrom
issue-786
Oct 19, 2026
Merged

JacobStephens2 merged 6 commits into
mainfrom
issue-786

Conversation

@JacobStephens2

@JacobStephens2 JacobStephens2 commented Oct 19, 2026 •

Copy link
Copy Markdown
Owner

Summary

GitHub read
  transient failure → wait 1s, then 2s → at most 3 attempts
  permanent/quota failure → return immediately
  exhausted budget → return last cause
GitHub mutation → one attempt
CI observation → existing whole-observation retry budget

Retries cover issue, PR, label, paginated, and private record reads. Each retry produces a progress/Command log line. Ordinary interruption cancels backoff; completion operations retain their existing interruption policy. Private record diagnostics remain redacted.

Closes #786

Evidence

  • Before: cargo test --bin thirdshift github::execution_tests::a_transient_issue_read_recovers_without_exposing_the_failure -- --exact failed on the first HTTP 504.
    After: the retained regression passes; transient transport errors recover across read operations.
  • Before: the Spec reviewer's exact regression, cargo test --bin thirdshift github::execution_tests::a_transient_private_security_issue_listing_recovers -- --exact --nocapture, failed on a Security Pass issue listing. The related private-read regression also failed before its fix.
    After: both retained regressions pass. Read recovery issues the following mutation once, with private diagnostics redacted.
  • Persistent failures stop after three attempts with the last cause. Authentication, 404, validation, and quota errors stop after one. Tests cover cancellation during backoff and completion after interruption.
  • Before: the quota matrix retried RATE_LIMIT_EXCEEDED, and the classifier rejected a GraphQL secondary-limit HTTP 429 as quota exhaustion.
    After: the retained quota and classifier cases pass: primary quota errors stop immediately, while secondary-limit 429s remain transient. Request-count diagnostics also normalize padded wc output.
  • The Architect command test verifies a recovered 504 publishes its plan, sends one success notification, and retains the retry in its Command log.
  • Validation passed: cargo check, cargo fmt --check, cargo clippy --all-targets -- -D warnings, and the full cargo test suite (1,954 passed, 0 failed, 2 ignored across 49 test binaries).
  • One earlier full attempt hit the unchanged partial_stdout_is_reported_after_log_failure_with_a_blocked_prompt test. It then passed four isolated runs, the complete session execution group, and the successful full rerun above. No session code changed.

Tested seams

  • Public GitHub read and mutation operations through a fake gh, including private record operations.
  • thirdshift architect through fake GitHub and Resend, observing command results, Run notifications, and Command logs.

Review coverage

The refreshed independent review against main found one optional Standards P3 suggestion, declined below, and zero Spec findings. The earlier published Spec P2 finding was reproduced and fixed; its private-read regressions remain. Both reviewers read every changed file:

  • src/github.rs
  • src/github/advisories.rs
  • src/github/check_read.rs
  • src/github/execution_tests.rs
  • tests/architect_run.rs
  • tests/fakes/gh.rs

Unread changed files: none on either axis. Generated-output exclusions: none.

Unaddressed findings

Standards

  • P3, possible Duplicated Code: keep the small diagnostic conversions beside their readers. Plan decision: public reads explicitly use private_status: None at src/github.rs:767, while private reads use Some(output.status). ReadFailure::fmt enforces status-only private diagnostics, following ADR 0015. Keeping that choice local avoids a privacy-mode parameter in shared execution; retry/classification policy is already shared.

Spec

None.

Security

None. Single-session guidance-mode source review covered all six changed files in git diff 69ccac42ac98a454df8984ea7f9b329ad0e29d07...HEAD at 8bd23bad702b98eba95bdb088102718437fe36a6, plus supporting GitHub execution, CI, cancellation, logging, and private-record callers. Reviewed injection, disclosure, mutation replay, and retry amplification using the attack-class guidance and ADRs 0013 and 0015. No proof-of-concept candidate or source fix; tests were not rerun in this Security review.

Security review outcome

No unaddressed introduced findings.

Merge Danger

Door: two-way

Revert restores the previous read behavior; no stored-data migration.

Blast Radius: GitHub

Transient read failures can add up to three seconds of backoff per observation. Mutations remain single-attempt, and CI keeps its existing observation budget.

Built with codex · gpt-6.1-sol · high

@JacobStephens2
JacobStephens2 merged commit de3c1ed into main Oct 19, 2026
18 checks passed
@JacobStephens2
JacobStephens2 deleted the issue-786 branch October 19, 2026 06:32
@JacobStephens2 JacobStephens2 mentioned this pull request Oct 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Retry transient GitHub read failures before failing a Pass

1 participant