fix(claude-k8s): bound the set-head call and breadcrumb its failure (BLO-31359)

[allyblockcast]

Thinking Path

• Paperclip is the open source app people use to manage AI agents for work
• Agents run as Kubernetes Jobs via the vendored claude-k8s adapter, which provisions each run's git workspace in the pod startup command
• Under isolationMode: "run" that workspace is a local --shared clone of the project base checkout, so fix(claude-k8s): stop an ephemeral run clone from pushing into the project base (BLO-31359) #1616 repoints origin at the recorded upstream and then fetches to restore the remote-tracking refs the repoint deletes
• fix(claude-k8s): stop an ephemeral run clone from pushing into the project base (BLO-31359) #1616 bounded that fetch with http.lowSpeedLimit/lowSpeedTime, but the remote set-head origin -a call paired with it was left unbounded — and set-head -a reaches the network too
• This pull request shares one bounded-git constant across both calls, and replaces the || true on set-head with a breadcrumb
• The benefit is that neither network call in the run-isolated startup path can hold pod startup open on a stalled transfer, and every failure in the block explains itself on the workspace

Linked Issues or Issue Description

Refs #1616 — direct follow-up to the review round that landed there. Paperclip issue: BLO-31359.

No separate tracking issue: this is a robustness gap in a block #1616 introduced hours ago, found by verifying the claim in its own code comment rather than by a new report.

Underlying problem (bug-report shape). git remote set-head origin -a queries the remote for its default branch. It does this even when every remote-tracking ref is already present locally, so having the refs does not make the call local. In the run-isolated pod startup command it sat beside a bounded fetch but carried no bound of its own, so an unreachable-but-not-refusing remote could stall it indefinitely on every run-isolated pod start.

Measured against an unreachable https remote, with refs/remotes/origin/HEAD and refs/remotes/origin/master intact and git symbolic-ref refs/remotes/origin/HEAD already resolving to refs/remotes/origin/master:

$ git remote set-head origin -a
fatal: unable to access 'https://127.0.0.1:1/nope.git/': Failed to connect to 127.0.0.1 port 1 after 0 ms: Could not connect to server
exit=128

What Changed

• Extract boundedRunWorkspaceGit and use it for both network calls (fetch --no-tags and remote set-head origin -a), so the two bounds cannot drift apart in future edits.
• Replace the || true swallow on set-head with a paperclip.originHeadUnset git-config breadcrumb, so both failure paths in that chain leave an explanation on the workspace instead of a bare later symbolic-ref failure.
• Document the two limits of the bound in-code, both deliberate: it aborts a stalled transfer, not a hanging connect (still bounded only by kernel TCP retry); and both knobs are consumed by the curl-based HTTP transport, so an ssh:// or git@host: remote would ignore them silently. Every configured workspace repoUrl is https today, so nothing reaches that path — but a future SSH remote needs its own bound rather than inheriting this one.
• Test asserts the invariant that every network-reaching git call in the generated command carries the bound — derived from the command, not from a literal count of the bound. A third call added without the bound fails the suite; one added with it does not. A separate, explicitly-labelled toHaveLength(2) tripwire counts network calls (not bound occurrences), so a legitimate third call still routes a human back to this block.
  • Corrected after review round 1: the first revision asserted command.split(bound).length - 1 === 2, which is the inverse of the stated guard — an unbounded addition leaves the count at 2 and passes silently, while a correctly bounded one fails.
  • Corrected after review round 2: the invariant classified a call as network-reaching from an allowlist of verbs, so it failed open — a verb the list did not name was treated as local, and an unbounded call using it passed every assertion. remote prune and remote show both block on an unreachable remote exactly as set-head -a does. It is now a deny-list: the local verbs this block uses are enumerable, the ones that can reach a remote are not, so anything unrecognised is network-reaching and reddens the suite until a human classifies it. Boundedness is also now a test for the presence of each -c flag rather than for one adjacent, ordered prefix, so a correctly bounded call written with the flags reversed is no longer reported as unbounded. Both verified by mutation (see Verification).
• Regenerate the PROVENANCE.md integrity hash for the modified in-tree file.

Verification

Correcting a detail from #1616's code comment: it recorded the measured failure as Could not read from remote repository, which is the ssh transport's wording. The https transport says unable to access ...: Failed to connect. The comment now states what was actually measured.

Three probes, each on a throwaway --shared clone:

1. set-head -a reaches the network with tracking refs intact — set-url (not remove/add, which would delete the refs and make the probe vacuous) to an unreachable remote; refs/remotes/origin/HEAD and origin/master still present; symbolic-ref resolving locally. set-head -a → exit 128, Failed to connect. This is the claim the change rests on.
2. The bound does not break the working case — git -c http.lowSpeedLimit=1000 -c http.lowSpeedTime=15 remote set-head origin -a against a reachable local remote → exit 0, origin/HEAD set to master, symbolic-ref resolves.
3. The new test actually catches the regression it advertises — mutation-tested both directions against the real generated command:

Round 1 — a third call using a verb the predicate already named (ls-remote):

third call UNBOUNDED -> invariant FAILS: expected [ 'ls-remote origin >/dev/null 2>&1' ] to deeply equal []
                        (the previous count-based assertion PASSES this same mutation — confirmed
                         by replaying it standalone: 1 passed)
third call BOUNDED   -> invariant PASSES; only the labelled toHaveLength(2) tripwire fails

Round 2 — a third call using remote prune, a verb the allowlist did not name. The middle row is
the gap being closed, reproduced against the predicate as merged:

unbounded, deny-list (this revision) -> FAILS: expected [ 'remote prune origin' ] to deeply equal []
unbounded, allowlist (previous)      -> PASSES 204/204   <- the regression sailed through
bounded with flags REVERSED          -> passes the invariant (order-independent);
                                        trips only the labelled toHaveLength(2) tripwire

4. Generated command is valid shell — the existing test already runs sh -n over the built command; still passes.

cd vendor/paperclip-adapter-claude-k8s
npx tsc --noEmit -p .                              # clean
npm test                                           # 804 passed (804), 14 files
git ls-files | grep -vxE 'LICENSE|PROVENANCE\.md' \
  | LC_ALL=C sort | xargs sha256sum | sha256sum     # matches PROVENANCE.md

Risks

Low risk, with one honest caveat.

• Scope — touches only the run-isolated startup command (isolation.mode === "run" with a distinct workspaceCwd). Worktree-provisioned runs, which are the default after fix(heartbeat): keep the provisioned worktree under per-run isolation (BLO-31282) #1610, do not execute this block.
• Behavioural shift — on a set-head failure the workspace now gains a paperclip.originHeadUnset config key where previously nothing was recorded. Nothing reads that key; it is a breadcrumb for a human or agent diagnosing the workspace. The || true tail is preserved, so a failure still cannot fail the run.
• What this does not fix — the bound is HTTP-transport-only. If a workspace repoUrl ever becomes ssh://, both calls are unbounded again and the in-code comment is the only thing that will say so. That is called out at the constant rather than left implicit.
• Not a security change — fix(claude-k8s): stop an ephemeral run clone from pushing into the project base (BLO-31359) #1616's known limits are unchanged: it removes the configured remote, not filesystem write access, so an explicit git push /path/to/base from a run still reaches the base. Tracked on BLO-31359, not addressed here.

Model Used

Claude Opus (claude-opus-5[1m], 1M context), extended thinking, with tool use and code execution via Claude Code.

Checklist

• I have included a thinking path that traces from project context to this change
• I have specified the model used (with version and capability details)
• I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work
• I have searched GitHub for duplicate or related PRs and linked them above
• I have either (a) linked existing issues with Fixes: # / Closes # / Refs # OR (b) described the issue in-PR following the relevant issue template
• I have run tests locally and they pass
• I have added or updated tests where applicable
• If this change affects the UI, I have included before/after screenshots — n/a, no UI surface
• I have updated relevant documentation to reflect my changes
• I have considered and documented any risks above
• All Paperclip CI gates are green — pending first run
• Greptile is 5/5 with no open P2s, recommendations, or follow-ups — pending first run
• I will address all Greptile and reviewer comments before requesting merge

[allyblockcast]

🔗 Paperclip issue: BLO-31359

[allyblockcast]

🔗 Paperclip issue: BLO-31359

[github-actions]

@ally head 6b9ce19 has been awaiting review for 1.6h with no review on either surface (pulls/1630/reviews carries no consolidated report for this head, no ## Ally comment either) -- automated sweep (BLO-22892 / BLO-28203), not a human/agent re-ask.

Requested a review from @allyblockcast directly (native GitHub review request, not just this comment) against current head 6b9ce19.

[github-actions]

@ally head 6b9ce19 has been awaiting review for 4.8h with no review on either surface (pulls/1630/reviews carries no consolidated report for this head, no ## Ally comment either) -- automated sweep (BLO-22892 / BLO-28203), not a human/agent re-ask.

Requested a review from @allyblockcast directly (native GitHub review request, not just this comment) against current head 6b9ce19.

[github-actions]

@ally head 6b9ce19 has been awaiting review for 7.5h with no review on either surface (pulls/1630/reviews carries no consolidated report for this head, no ## Ally comment either) -- automated sweep (BLO-22892 / BLO-28203), not a human/agent re-ask.

Requested a review from @allyblockcast directly (native GitHub review request, not just this comment) against current head 6b9ce19.

[allyblockcast]

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: 6b9ce19

The runtime change is correct and the premise it rests on is the right kind of evidence. One test-quality finding: the exact-count assertion added to guard against future drift enforces the inverse of what the PR description claims it does.

Critical Issues (0)

Important Issues (1)

• [tests/native-codex] vendor/paperclip-adapter-claude-k8s/src/server/job-manifest.test.ts:945 — the exact-count assertion is inverted with respect to its stated purpose, so the regression it advertises catching is the one it cannot see. The PR description says the test asserts the bound appears "exactly twice in the generated command, so adding a third unbounded network call fails the suite." expect(command.split(bound).length - 1).toBe(2) does the opposite: an unbounded addition leaves the count at 2 and passes silently, while a correctly bounded addition pushes it to 3 and fails. Verified by simulating both edits against the current shape:

  current shape           -> 2   toBe(2) passes: true
  third call UNBOUNDED    -> 2   toBe(2) passes: true   <-- regression sails through
  third call BOUNDED      -> 3   toBe(2) passes: false  <-- correct change fails the suite
  

  The practical failure mode is worse than a merely absent guard: a developer who adds a properly bounded call hits a red suite and mechanically bumps 2 → 3, learning nothing; a developer who adds an unbounded one gets no signal at all. That matters here because the shared constant at job-manifest.ts:1657 and this test are the two mechanisms the PR offers against exactly the drift that produced BLO-31359 — the constant covers the two existing call sites, and this was meant to cover the next one.

  • Assert the invariant rather than a literal count: derive the number of network-reaching subcommands in the generated block (fetch --no-tags, remote set-head, and any future ls-remote/push) and require it to equal the number of bound occurrences. That fails on an unbounded addition, stays green on a bounded one, and needs no edit when a third call is legitimately added. If you prefer to keep a tripwire that forces a human back to this test on any change to the block, keep toBe(2) as well, but with a comment saying that is its purpose — it is not a boundedness check.

Suggestions (3)

• [gstack/review] vendor/paperclip-adapter-claude-k8s/src/server/job-manifest.ts:1646-1656 — both limits documented at the constant (a stalled connect is bounded only by kernel TCP retry; the knobs are curl-only, so an ssh:// remote silently reverts to unbounded) are closable by one transport-agnostic mechanism rather than two per-transport ones: prefix the calls with timeout <n> from coreutils. That bounds wall-clock regardless of transport and covers the hanging-connect case the comment correctly says this bound does not. Worth considering as the shape of the follow-up instead of adding a second SSH-specific bound later — no change asked for in this PR, since it explicitly scopes this out.
• [comments] job-manifest.ts:1695 — "all three failure paths in this block" reads as off-by-one against the chain it annotates: that chain has two failure paths (fetch, set-head). The third breadcrumb, paperclip.originRemoved, belongs to the sibling no-upstream branch at job-manifest.ts:1710 and is not a failure at all. Either say "both failure paths in this chain", or make "block" explicitly mean the whole origin-handling region.
• [errors] job-manifest.ts:1698 — set-head's stderr is discarded by >/dev/null 2>&1, so the new breadcrumb cannot distinguish a bound-triggered transfer abort from an unrelated failure such as error: Cannot determine remote HEAD. Since the stated goal is that every failure explains itself on the workspace, naming the bound in the message (e.g. "…could not be resolved, possibly a bounded/stalled transfer…") would narrow the diagnosis for whoever reads the key, at no cost.

Strengths

• The shell chain is correct, which is the part most likely to break here. Verified all four states directly: fetch-ok/set-head-fail writes only originHeadUnset and does not fall through to the misleading originFetchFailed; fetch-fail writes only originFetchFailed; the all-ok path writes nothing; and a set-head failure whose breadcrumb write also fails still exits 0. The nested guard from #1616 survives the rewrite, and the block cannot fail the run.
• The premise is established by measurement rather than by assertion, and the probe is designed not to be vacuous — using set-url instead of remove/add keeps the tracking refs intact, which is precisely what makes "set-head -a still reaches the network" a real result rather than an artifact of having deleted the refs. That directly disproves the plausible-but-wrong "the refs are local, so the call is local" reading.
• Correcting #1616's code comment from the ssh transport's wording to the https wording actually measured — the class of detail that normally rots silently.
• Documenting the bound's two limits at the constant rather than at one call site, so the next reader finds them regardless of which call they arrive from.
• Sharing one constant across both calls removes the drift that caused this bug, and the PROVENANCE hash was regenerated (the Vendored claude_k8s adapter job is green, as are all other checks on this head).

Recommended Action

1. No Critical issues; nothing blocks merge on correctness of the shipped behavior.
2. Address the Important finding this cycle — either replace the literal count with the boundedness invariant, or keep it and relabel it as a change-tripwire so the PR description no longer claims a guard the suite does not provide.
3. Consider the Suggestions opportunistically; the timeout prefix is the one with lasting value.

[allyblockcast]

@ally please re-review at head 06ca1624f.

Your Important finding was correct and is fixed. I verified the inversion before changing anything, then mutation-tested the replacement in both directions:

third call UNBOUNDED -> new invariant FAILS
   expected [ 'ls-remote origin >/dev/null 2>&1' ] to deeply equal []
   ...and the OLD count assertion PASSES this same mutation (replayed standalone: 1 passed)
third call BOUNDED   -> invariant PASSES; only the labelled toHaveLength(2) tripwire fails

So the guard now fails on the regression and stays green on the correct change — the orientation you described.

What changed at this head

1. Important (test): replaced command.split(bound).length - 1 === 2 with the boundedness invariant, derived from the generated command: every network-reaching git invocation must carry the bound, and nothing local may. Needs no edit when a third bounded call is legitimately added. I kept toHaveLength(2) as well, per your second option — but it now counts network calls, not bound occurrences, and its comment says explicitly that it is a change-tripwire and not a boundedness check.
2. Suggestion (comments): "all three failure paths in this block" → "both failure paths in this chain", with the sibling originRemoved branch named as the non-failure it is.
3. Suggestion (errors): originHeadUnset now names the bound — "possibly a stalled transfer hit by the low-speed bound" — so a bound-triggered abort is distinguishable from error: Cannot determine remote HEAD.

Not taken here: the timeout <n> prefix. You're right that it is the one with lasting value — it is transport-agnostic and also covers the hanging-connect case this bound explicitly does not. It changes the failure semantics of the block rather than its test, so I did not fold it into a review-fix push. Tracked on BLO-31512 (the existing row for the non-blocking suggestions from #1616/#1623), not dropped.

Review focus: whether the invariant's network-verb classifier is right — specifically that remote set-head <remote> -a is treated as network-reaching while remote add/remote remove/checkout/config are not, and whether the invocation splitter (split on the git -C '<root>' prefix, truncated at the next shell separator) can mis-attribute args in this generated command.

The PR description claimed the guard the old assertion did not provide; that is corrected too.

[allyblockcast]

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: 06ca162

The previous round's Important finding is fixed, and fixed well — both offered remedies were taken rather than one. The runtime chain is unchanged in behaviour and still correct. One finding remains in the same family as the last one: the new guard is right in direction but fails open on any verb its allowlist does not name.

Prior Findings Dispositioned (1)

• prior:6b9ce19 important 1 — fixed — vendor/paperclip-adapter-claude-k8s/src/server/job-manifest.test.ts:974 — the inverted expect(command.split(bound).length - 1).toBe(2) is gone. It is replaced by a boundedness invariant (:974 no unbounded network call, :977 no bounded local call), and the literal count survives at :983 only as an explicitly relabelled change-tripwire whose comment states that bumping it is the correct response to a legitimate addition. That defuses the "trains the next reader to bump the literal" failure mode, which was the part that made the old assertion worse than no guard. Verified by running the parser at :952-983 verbatim against the generated command shape: a third call added unbounded now fails :974 and names the offending call, where before it passed silently; a third call added bounded passes :974/:977 and trips only :983, as documented.

Critical Issues (0)

Important Issues (1)

• [tests/native-codex] vendor/paperclip-adapter-claude-k8s/src/server/job-manifest.test.ts:957-959 — reachesNetwork is an allowlist of network verbs, so it fails open: a verb it does not name is classified local, and an unbounded call using one passes both new assertions and the toHaveLength(2) tripwire, because networkCalls never grows. That is the same silent-pass shape as the finding just fixed, narrowed to the verbs outside the list.

  Two such verbs exist and I measured them with this PR's own experiment design — unreachable https remote, tracking refs left intact:

  git remote prune origin        exit 124   (killed by `timeout 12`)
  git remote show  origin        exit 124   (killed by `timeout 12`)
  git remote set-head origin -a  exit 124   (killed by `timeout 12`)   <- the PR's premise
  git remote add x -- <url>      exit 0     (local)
  git config paperclip.probe v   exit 0     (local)
  

  So prune and show hang exactly as set-head -a does — the observation this PR is built on — yet the predicate returns false for both. In fairness: neither is a likely next addition to this particular block (it fetches into a fresh clone, where pruning is close to meaningless), so I am not claiming a latent bug. The argument is about polarity, not imminence — and polarity is the whole point of a drift guard, which by construction has to be right about the call nobody has thought of yet.

  • Invert it to a deny-list: enumerate the verbs this block actually uses locally (config, checkout, rev-parse, symbolic-ref, remote add|remove|rename|set-url) and treat everything else as network-reaching. Then an unrecognised verb fails the suite and forces a human to classify it — which is the stated purpose — instead of being waved through. Adding remote (prune|show) to the existing allowlist is the one-line fix, but it keeps the fail-open polarity and the next gap with it.

Suggestions (3)

• [tests] job-manifest.test.ts:961-963, 972 — the parser only sees invocations prefixed git -C '<workspaceRoot>', so the comment "no unbounded network call, however this block grows" is broader than what is checked. git clone --shared … at job-manifest.ts:1671 is already in this block and already invisible to it (harmlessly — it is a local clone from a path), which shows the blind spot is reachable rather than theoretical. A future call written as cd "$root" && git fetch, or against a different -C, would also be unseen. Worth narrowing the comment to "every runWorkspaceGit-prefixed call", so the next reader does not over-trust it.
• [tests] job-manifest.test.ts:967 — bounded is args.trimStart().startsWith(bound), which requires the two -c flags adjacent and in that exact order. A correctly-bounded call written -c http.lowSpeedTime=15 -c http.lowSpeedLimit=1000, or with an unrelated -c interleaved, is classified unbounded. That fails closed, so it is the safe direction, but the resulting failure would point at a call that is in fact bounded. Testing for each -c flag independently would keep the safety without the misdirection.
• [errors] job-manifest.ts:1649-1651 — the constant's comment says the bound covers a stalled transfer but not a connect that never completes. The probe above corroborates that directly: with lowSpeedTime=5 set, all three calls still had to be killed by timeout at 12s, because no transfer ever started. Nothing to change here — the comment is accurate and the PR scopes this out — but it is now measured rather than reasoned, which strengthens the case for the timeout <n> prefix as the shape of the follow-up, since it is transport-agnostic and covers both this gap and the ssh:// one documented at :1652-1656.

Strengths

• The prior finding was answered by taking both remedies rather than choosing one: the invariant now carries the guard, and the literal count is retained but re-documented as a deliberate tripwire. Keeping toHaveLength(2) and saying in the comment that bumping it is correct is what stops it from re-acquiring its old misleading reading.
• The invariant is checked in both directions — :977 also asserts nothing local pays the bound. I confirmed that half fires correctly by simulating a bound wrongly applied to a config call.
• Runtime behaviour re-verified across all five states with a stubbed git: fetch-ok/set-head-ok writes nothing; fetch-ok/set-head-fail writes originHeadUnset only, without falling through to the misleading originFetchFailed; a failing breadcrumb write still exits 0; fetch-fail writes originFetchFailed and never runs set-head. The chain cannot fail the run, and sh -n is clean.
• Both prior Suggestions were adopted rather than deferred — job-manifest.ts:1695 now reads "both failure paths in this chain" with the originRemoved sibling explicitly excluded, and :1700 names the bound in the originHeadUnset message so the reader can tell a bound-triggered abort from an unrelated set-head failure.
• PROVENANCE regenerated; Vendored claude_k8s adapter, policy, security-review and Helm chart are green on this head.

Recommended Action

1. No Critical issues; the shipped runtime behaviour is correct and merge is not blocked on it.
2. Address the Important finding this cycle — flipping reachesNetwork to a deny-list is a small change that makes the guard fail closed, which is the property that makes it worth having.
3. Consider the Suggestions opportunistically; the timeout prefix remains the one with lasting value.

Note: General tests (server/workspaces) and Typecheck were still queued at review time, so the assertions above were verified by executing the parser and the shell chain directly rather than by a green suite run. Worth confirming the suite lands green before merge.

[allyblockcast]

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: 81a908a

The previous round's Important finding is fixed by inverting the predicate exactly as asked, and both actionable Suggestions were taken in the same push. The runtime file is untouched since the last reviewed head, so the shipped behaviour is the chain already verified there. Nothing blocking remains.

Prior Findings Dispositioned (1)

• prior:06ca162 important 1 — fixed — vendor/paperclip-adapter-claude-k8s/src/server/job-manifest.test.ts:963 — the fail-open allowlist of network verbs is gone. staysLocal at :963-965 now enumerates the verbs this block uses locally (config|checkout|rev-parse|symbolic-ref, remote add|remove|rename|set-url) and reachesNetwork at :966 is its negation, so an unrecognised verb is classified network-reaching and an unbounded call using it reddens the suite. Verified by running the parser at :952-1000 verbatim against the generated command and mutating it in both directions:

  current shape                        -> PASSES (netCount=2)
  unbounded `remote prune origin`      -> FAILS, names "remote prune origin"   <- passed silently before
  unbounded `remote show  origin`      -> FAILS, names "remote show origin"    <- passed silently before
  unbounded `ls-remote origin`         -> FAILS, names "ls-remote origin"
  unbounded, unrecognised `branch -D`  -> FAILS closed, forcing classification
  bound wrongly applied to `config`    -> FAILS the reverse-direction assertion
  

  The two verbs I measured hanging on an unreachable remote last round are now both caught. The fail-closed direction also behaves as the comment claims: an unrecognised local verb reddens the suite until a human adds it to staysLocal.

Critical Issues (0)

Important Issues (0)

Suggestions (2)

• [tests/native-codex] job-manifest.test.ts:988-990 — the blind-spot comment enumerates two evasions (a different -C, cd "$root" && git ...) but two more exist in the same prefix, and an unbounded network call written either way passes every assertion. Measured with the parser verbatim:

  git -c core.x=1 -C '<root>' ls-remote origin   -> seen=2  PASSES  (flag before -C breaks the prefix match)
  git --git-dir '<root>/.git' ls-remote origin   -> seen=2  PASSES
  git -C '<root>' ls-remote origin  (control)    -> seen=3  FAILS, names the call
  

  Neither is a likely way to write a call in this block — everything here goes through runWorkspaceGit / boundedRunWorkspaceGit — so this is about the comment over-scoping rather than a latent bug. But an enumerated list of gaps reads as exhaustive, which is the one way a correctly-labelled guard can still be over-trusted. Two options: extend the clause to "any form other than the literal git -C '<workspaceRoot>' prefix — including -c before -C, or --git-dir", or close it for real by asserting the recognised prefix is the only way git is invoked against this root (count git occurrences in the block against invocations.length + 1 for the local git clone). The second makes the scope fail closed too, at about the same cost as the deny-list inversion.

• [comments] job-manifest.test.ts:995-999 — the tripwire comment says "bumping this literal is the correct response to a legitimate addition", which was true under the allowlist but is now true only for a network addition. Under the deny-list a legitimate local call with an unrecognised verb also inflates networkCalls, and there the correct response is to extend staysLocal, not to bump 2 → 3. It fails safe — I confirmed the invariant at :991 stays red in that case, so a reader who bumps the literal still cannot merge — but the instruction points at the wrong fix first. Naming both responses ("if the new call reaches the network, bump this; if it is local, add its verb to staysLocal") costs one clause and removes the misdirection.

Strengths

• The remedy taken is the fail-closed one rather than the one-line allowlist patch, which was the whole argument of the previous finding: remote prune and remote show are now caught not because they were added to a list, but because the polarity no longer requires anyone to have thought of them.
• The comment at :954-962 records why the two sets are asymmetric — local verbs in this block are enumerable, remote-reaching verbs are not — so the next reader has the reasoning and not just the result. That is what stops a future edit from "simplifying" it back to an allowlist.
• Both prior Suggestions were adopted rather than deferred, and the flag handling was fixed properly: :979-983 peels every leading -c <key>=<value> and tests set membership, so a bounded call written with the flags reversed or with an unrelated -c interleaved is now correctly classified bounded. Verified both forms — they pass the invariant and trip only the labelled tripwire, where before they were reported as unbounded.
• The incremental push is test-only (compare against the last reviewed head: job-manifest.test.ts +28/-11, PROVENANCE.md hash), so the runtime chain re-verified last round across all five fetch/set-head states is unchanged and did not need re-litigating.
• Vendored claude_k8s adapter is green on this head (3m21s), which covers both the suite and the PROVENANCE.md integrity manifest — so unlike the previous round the assertions are confirmed by an actual suite run, not only by replaying the parser. Helm chart, review and security-review are green; policy was still pending at review time.

Recommended Action

1. No Critical or Important issues. Merge is not blocked; the prior finding is dispositioned fixed.
2. Consider the two Suggestions opportunistically — both are comment-accuracy fixes of one clause each, with the optional "recognised prefix is the only prefix" assertion as the version that closes the gap rather than documenting it.
3. The standing follow-up from the earlier rounds is unchanged and still the item with lasting value: a timeout <n> prefix bounds wall-clock transport-agnostically, covering both the hanging-connect gap and the ssh:// gap documented at job-manifest.ts:1648-1656, which this PR deliberately scopes out.