[JacobStephens2]

Summary

Git reads of origin (ls-remote, fetch) that fail in transport are now tried again, within a short budget, instead of failing the Pass. Every caller the issue names goes through Git::run/run_checked, so each one is covered: Issue branch selection, Pickup's reservation check, Worktree ownership checks, the remote-head probe and fetches.

 Git::retried_output_checked(args, check)
-  retry_output(attempt)                    # lock-wait only
+  if !reads_remote(args)                   # push and everything else
+    return retry_output(attempt)           # unchanged
+  for tried in 1..=3
+    output = retry_output(attempt)         # lock-wait still inside
+    if success || tried == 3 || !transport::is_transient(stderr)
+      return output                        # last attempt's cause, same shape
+    progress "<git cmd> failed: <git error> (attempt n/3); retrying in ns"
+    interruption.pause(n s)                # Ordinary: stops on interrupt

src/git/transport.rs   is_transient(stderr)
  permanent first: Authentication failed, could not read Username/Password,
                   Repository not found, Permission denied,
                   couldn't find remote ref, does not appear to be a git repository
  then HTTP status:  returned error: N / RPC failed; HTTP N  -> 429 or 5xx only
  then transient:    RPC failed; curl, HTTP2 framing layer, expected flush,
                     early EOF, remote end hung up, sideband disconnect,
                     Connection reset/timed out, Could not resolve host,
                     SSL_read, gnutls_handshake/recv
src/process.rs         Interruption::pause, now shared by the gh read retry

docs/guide.md documents the behaviour next to the CI read retries.

Evidence

• Before: a single dropped HTTP/2 stream failed the Pass on its first attempt: git ls-remote --heads origin refs/heads/issue-880 refs/heads/issue-880-branch-* failed: error: RPC failed; curl 16 Error in the HTTP2 framing layer (from the issue). Before the review fix, a missing ref over protocol v0 was retried: cargo test --bin thirdshift missing_ref FAILED with left: 3, right: 1.
  After: new tests, each against a real local origin whose uploadpack/receivepack fails on cue:

run_notification: a_dropped_stream_while_selecting_the_issue_branch_is_retried_without_a_failure
  origin drops 1st connection -> exit 0, retry line on stderr,
  session on issue-7, notification "Result: ready for review"            ok
git: an_ls_remote_a_dropped_stream_failed_is_run_again       2 connections ok
git: a_fetch_a_dropped_stream_failed_is_run_again            2 connections ok
git: ..._failing_in_transport_throughout_fails_with_the_last_cause
     3 connections, < 10s, cause is git's error                          ok
git: a_read_of_origin_failing_permanently_is_run_once (auth, not found,
     unclassified)                                            1 connection  ok
git: a_fetch_of_a_missing_ref_over_protocol_v0_is_run_once    1 connection  ok
git: a_push_failing_in_transport_is_attempted_once            1 connection  ok
git::execution_tests: interruption_ends_the_wait_before_a_read_of_origin_is_run_again
     SIGINT/SIGTERM/SIGHUP during wait -> "interrupted" < 500ms, 1 connection  ok
git: a_lock_another_git_holds_is_waited_for, a_ref_another_git_moved_meanwhile_... ok

Full suite cargo test --no-fail-fast: 2151 passed, 0 failed. An earlier full run had one failure in tests/console/stop.rs (console_stop_confirms_authenticates_and_triages_or_requeues_a_live_command). That test passes when run alone and in the full rerun, and it doesn't touch git.

Merge Danger

Door: two-way

Only the retry policy around existing commands changes. Reverting it brings back fail-on-first-attempt.

Blast Radius: small

At worst, a misclassified permanent failure costs 3s and two extra progress lines before it fails with the same cause. Pushes and other mutations are never retried.

Review

Both reports are under .thirdshift-review-gNunIZ/ (not committed). Addressed:

• Spec: a missing ref plus a hang-up counted as transient (fixed, with tests).
• Standards: the interruption-sleep switch repeated in the gh and git retries is now Interruption::pause; reads_origin was renamed reads_remote; the DROPPED_STREAM literal is reused; the integration test's upload-pack is installed by a child install, so a parallel test cannot hit "Text file busy".

Changed files the review left unread: src/process.rs was not read by the Spec reviewer. The review-fix commit's Interruption::pause change to src/process.rs and src/github.rs was made after both reviews and was not re-reviewed by either axis. The reviewers read every other changed file.

Unaddressed findings

Standards

• Duplicated Code: the git retry loop repeats GitHub::retry_read (src/github.rs:899). Partly addressed: the repeated wait is now Interruption::pause. The loops stay separate because they decide on different things. retry_read classifies an anyhow::Error carrying ReadFailure. The git loop classifies a completed Output's stderr, and only after the lock-wait loop inside it (src/git.rs, retried_output_checked). A generic helper would have to take both shapes and three closures, and that would make a shallower interface, not a deeper one.
• Duplicated Code: transport::is_transient is shaped like check_read::is_transient. Declined. The status grammars differ: gh writes HTTP 503 and is matched with split("HTTP ") at src/github/check_read.rs:46, while git/curl write returned error: 503 / RPC failed; HTTP 503 (src/git/transport.rs, http_status). The phrase lists overlap in only one entry. The issue's Key interfaces ask for a classifier "analogous to check_read::is_transient, matching on git's stderr", and a separate module is that.
• Primitive Obsession: the test helper origin_fails(pack: &str). Declined: it is test-only, and both call values are literals in the same file (src/git.rs, mod tests). A wrong value fails the test's own connection count.
• Placement: the end-to-end test lives in tests/run_notification.rs. Declined. The acceptance criterion requires that "the Pass continues with no failure notification", and that file holds the Resend stand-in and the the_one_request/text helpers the test uses to assert the notification's Result: ready for review.

Spec

• Completion views don't cut the retry wait short on interruption. Declined. The reviewer marked this as following the design, with no failing test. GLOSSARY.md:69 sets that completion work "finishes after Command interruption without clearing it; ordinary synchronization remains interruptible", and Git::completion documents that it ignores recorded interruption (src/git.rs:35). The issue's criterion "Interrupting the Command during a retry wait ends the wait promptly" holds for ordinary Commands; see interruption_ends_the_wait_before_a_read_of_origin_is_run_again.

Security

• None. The review covered the retry path in src/git.rs (reads_remote, retried_output_checked), src/git/transport.rs and Interruption::pause. Only ls-remote and fetch are retried; an argument shape that isn't recognized falls back to a single attempt. push is never retried. The retry progress line repeats the git error the failure cause already showed, and the delays are bounded.

Closes #880

🤖 Generated with Claude Code

Built with claude · claude-opus-5-5 · medium

Security review outcome

No unaddressed introduced findings.