fix(watch): hold a secret-blocked run for a human instead of reverting the claim

[SebaBoler]

Part of #443 (the suspected re-pick bug from the architecture review; the Tracker/ReviewHost split stays open).

What

A run whose diff the outgoing secret scan withheld was re-picked by the watch loop forever on the sources that revert the claim on "no change". runSourcedIssue already returned { task, secretBlocked: true } when deliverChange hit the scan, but the per-runner result types narrowed it away, so watchOnce destructured only { prUrl, parked }, treated the run as a genuine no-change, called onNoChange, and the claim was reverted: next poll, same issue, same secret, same masked-findings comment again.

• WatchRunOutcome.secretBlocked is kept by RunGithubIssueResult / RunGitlabIssueResult / RunLinearIssueResult (type-level test pins it) and forwarded through triageAgentRun.
• watchOnce holds a secret-blocked run: a new onSecretBlocked primitive runs instead of onNoChange; the outcome is counted as tick.secretBlocked (own column in the summary line, omitted on a clean tick), never as no-change. A rejecting onSecretBlocked lands in tick.failed via onFailure, the claim stays, and the "secret blocked -> held for a human" line is logged before the edit so the block is on record either way.
• Each source posts its own release step (the shared adapters only post the masked findings, as before):
  • GitHub / GitLab issues: one edit swaps claimed -> secret-blocked, so the claim is never released without the hold label landing; the comment names the trigger label to re-apply. The label is ensured right before the swap (gh label create --force / glab label create, best-effort): gh issue edit --add-label fails on an unknown label and glab issue update --label is a silent no-op while --unlabel in the same call still lands.
  • GitHub Projects / Linear: the item keeps the claimed status/state (reverting to the trigger would re-list it); the comment names the status/state to move back to.
• Re-trigger clears the stale hold label on claim (GitHub issues, GitLab, Projects), only for items listReady saw carrying it, so there is no blind --remove-label. On Projects the cleanup runs after the status move and is best-effort: claimGate reads any throw from claim as "already claimed", which would strand the item in the claimed status with no run and no comment. The edit targets the issue's own repo (a board can hold issues from other repos).
• No white-label exception: the claim label is already the watch's own marker, and a white-label adapter posts no findings, so the hold label + comment are the human's only signal there.
• Adapters: signalSecretBlock ensures the label before adding it (GitHub: --add-label on an unknown label fails; GitLab: silent no-op), every step best-effort.

Tests

src/runners/watch.test.ts: engine (held, not no-change; counted apart; failure path; summary line), per-source hold text and the exact label edits (GitHub issues, GitLab, Projects, Linear), stale-label cleanup only on items seen with the label, Projects claim surviving a failing cleanup with the edit sent to the item's repo, white-label pins swap + comment + label ensured before the swap. github.test.ts / gitlab.test.ts: signalSecretBlock pre-creates the label.

Gates

pnpm lint clean, pnpm typecheck clean, pnpm test 2529 passed / 3 skipped.

[github-actions]

Vanguard Review

Verdict: NO BLOCKING FINDINGS

No blocking findings.

I verified the core claims against the base sources rather than the PR description: triageAgentRun does return runner(id) (src/runners/watch.ts:405), so widening its type to WatchRunOutcome really does let secretBlocked through at runtime on the needs-info paths too; no production code rebuilds a WatchPrimitives literal field-by-field (only {...primitives, listReady} in runLoopV1), so the new optional onSecretBlocked is never silently dropped; and the "not re-listed" argument holds for the default configs (listReady filters on the trigger label / trigger state type, and claim removed the trigger label). No CI-config paths are touched, so the workflowPaths() constraint is not in play.

CLAUDE.md defines no severity scale beyond blocking / non-blocking, so I label below with that plus a magnitude.

Non-blocking, major — the new hold state can be invisible: nothing creates vanguard:secret-blocked on an issue

githubAdapter.signalSecretBlock (src/runners/github.ts:45-50) and the GitLab equivalent (src/runners/gitlab.ts:45-50) add the label with .catch(() => undefined) and never label create --force first — unlike the PR path, which does exactly that at src/runners/revise-pr.ts:331. In a repo where the label was never created, the add fails silently. The new onSecretBlocked then removes the claimed label and the trigger label is already gone on claim, so the issue ends with no Vanguard label at all — only the masked comment. That contradicts the diff's own rationale ("The issue sits on vanguard:secret-blocked … with no running marker") and makes the held state un-greppable/un-filterable. Suggest a label create --force before the add (mirroring revise-pr), in the same PR since the new design depends on that label being present.

Non-blocking, major — white-label watch leaves a secret-blocked issue with no trace

runSourcedIssue skips signalSecretBlock entirely when deps.commitAuthor !== undefined (src/runners/source-adapter.ts:584-588), and --commit-author is accepted by the watch command (src/cli/args.ts; see args.test.ts:733). Combined with the new onSecretBlocked for GitHub/GitLab issues, a white-label secret-blocked run now: removes the trigger label (claim), removes the claimed label (onSecretBlocked), adds no label, posts no comment. The issue is indistinguishable from "never picked up", and nothing will re-pick it — previously it at least got a comment. Consider keeping the claimed marker when nothing was signalled (i.e. skip the release in white-label), so the hold is visible.

Non-blocking, minor — stale secret-blocked label survives the human re-trigger

Nothing ever removes GITHUB_SECRET_BLOCKED_LABEL / GITLAB_SECRET_BLOCKED_LABEL from an issue; the only removal is HAND_BACK_LABELS on a PR (src/runners/revise-pr.ts:66). Since this PR makes "human strips the secret and re-triggers" the designed flow, the issue keeps that label through the next successful run, and src/tasks/board.ts:34 keeps bucketing it as verify-failed. A one-liner in claim (which already edits labels) would clear it.

Non-blocking, minor — Linear's "stays claimed" guarantee is config-dependent

linearWatchPrimitives.listReady filters by state type (opts.triggerState ?? 'unstarted'), while claimedState is a state name. If an operator's claimed state resolves to the same type as the trigger, holding the issue in the claimed state still re-lists it next poll and the loop this PR fixes returns — now without even the no-change comment as a breadcrumb. The exposure is pre-existing (onFailure has it too), but the new comment at linearWatchPrimitives asserts the hold is safe; worth qualifying there, or warning when the two resolve to the same type.

Non-blocking, nit — coverage and public-type notes

• The two changed summary lines (runWatchLoop, runLoopV1) gain secretBlockedNote(...) with no test asserting , 1 secret-blocked appears and is absent at zero; the six new tests all stop at the tick/primitives level.
• WatchTick.secretBlocked is required and WatchTick is re-exported from src/index.ts:45, so external code constructing a tick (test doubles, custom loops) breaks on upgrade. Consistent with the other buckets, so arguably fine — just call it out in release notes.

Not covered: I did not re-run pnpm typecheck / pnpm test (the gate results are the author's claim), and I did not review the pr-watch/revise secret-block path beyond confirming it is a separate tick type and not regressed by this diff.

[SebaBoler]

Round 2 — one finding per line, verified against the code before acting.

Major — nothing creates vanguard:secret-blocked on an issue. Confirmed: editGithubLabels is a bare gh issue edit --add-label (fails on a repo without the label) and editGitlabLabels a bare glab issue update --label (silent no-op, per the comment on addMrFailureLabel). Fixed: both adapters' signalSecretBlock now pre-create the label (gh label create --force / glab label create) before the add, mirroring addPrFailureLabel and revise-pr.ts:331; every step stays best-effort. githubAdapter takes an injectable GhRunner and is exported @internal, and src/runners/github.test.ts (new) asserts create-before-add, the masked comment, and that no failure escapes. Same two tests for gitlabAdapter in gitlab.test.ts.

Major — white-label leaves a secret-blocked issue with no trace. Confirmed (whiteLabel = deps.commitAuthor !== undefined skips signalSecretBlock). Fixed as suggested: isWhiteLabel(deps) is now the single definition in source-adapter.ts, and the GitHub/GitLab issue primitives omit onSecretBlocked in white-label mode, so the claimed marker stays as the one visible trace of the hold. Tests per source.

Minor — stale secret-blocked label survives the re-trigger. Not changed, deliberately. A blind --remove-label in claim would make gh issue edit fail in every repo where the label does not yet exist (gh resolves label names against the repo before editing), turning every claim into taken and stalling the loop; doing it safely needs the issue's current labels at claim time. The leftover label is pre-existing (nothing removed it from issues before this PR either) and belongs with the label lifecycle rather than this fix — I will note it on #443 as a follow-up.

Minor — Linear hold is config-dependent. Agreed; the comment on linearWatchPrimitives now states the assumption explicitly (claimedState must not resolve to the triggerState type that listReady filters on — the same assumption claim and onFailure already rest on). A runtime warning would need a Linear state-type lookup and is out of scope here.

Nit — summary line untested. Added a runLoopV1 test asserting watch: 1 PR(s), 0 no-change, 0 failed, 0 skipped, 1 secret-blocked. and the unchanged line on a clean tick.

Nit — WatchTick.secretBlocked is a required public field. Kept required, consistent with the other buckets; noted in the PR body as a release note.

Gates: pnpm lint clean, pnpm typecheck clean, pnpm test 2524 passed / 3 skipped.

[github-actions]

Vanguard Review

Verdict: NO BLOCKING FINDINGS

No blocking findings.

The core fix is sound and I verified the mechanics against the tree: runSourcedIssue really does return { task, secretBlocked: true } (src/runners/source-adapter.ts:588) and skips signalSecretBlock under white-label (:586), triageAgentRun passes the runner result through unchanged (src/runners/watch.ts:405), so widening its type to WatchRunOutcome genuinely propagates the flag on the needs-info path too. Label argv matches existing convention (gh label create … --force as in revise-pr.ts:149; glab label create --repo … --name … as in tasks/gitlab.ts:133), editGithubLabels/commentGithubIssue/editGitlabLabels all already accept an injected runner, auth is optional on RunIssueDeps so the new test fixtures typecheck, and renderSecretBlockComment does contain the blocked publish / masked strings the new tests assert. No new regex, no shell interpolation, no raw-secret path (SecretFinding.masked only).

Findings, by the repo's severity vocabulary (src/runners/review-prompt.ts:52 — only critical/high gate a merge; none of these are):

[medium] The hold tells the human nothing about how to release it, and on Linear/Projects looks identical to "still running".
renderSecretBlockComment ends at No PR was opened. (src/core/secret-scan.ts:192) — no retry instruction. The old (wrong) NO_CHANGE_MSG at least said "re-apply the trigger label to retry". Recovery now differs per source: GH/GL issues need the trigger label re-applied; Linear and Projects need a manual move out of the claimed state. For Linear there is no label at all, so columnFor (src/tasks/board.ts:34-41) keeps the issue in claimed forever with no board signal. Suggest a per-source "how to resume" line in the block comment (the adapter already owns that comment).

[medium] The hold can go dark when the label signal fails.
signalSecretBlock is end-to-end best-effort (every step .catch(() => undefined)), but onSecretBlocked for GH/GL issues unconditionally removes the claimed label. If the label never lands (token without issue-write, rate limit — the pre-create only covers "label missing"), the issue ends with no trigger label, no claimed label, no secret-blocked label and no comment: not re-listed next poll, and invisible on the board. Consider making the secret-blocked label the responsibility of onSecretBlocked (whose errors do surface via onFailure), or having signalSecretBlock report whether the signal landed so the claim is only released when it did.

[low] Stale vanguard:secret-blocked after a successful retry. claim removes only the trigger label (watch.ts:600, :855); nothing clears the secret-blocked label on the issue, and columnFor ranks secret-blocked → verify-failed ahead of review, so a re-run issue that opens a PR stays misclassified. Pre-existing, but this PR makes re-triggering the sanctioned recovery path — adding the label to the claim's remove: list closes the loop.

[low] Log line is emitted after the primitive. In watchOnce, await primitives.onSecretBlocked?.(id) precedes the secret blocked -> held for a human log; if the label edit throws, the run is reported as failed with a "Vanguard run failed" comment carrying a label-edit error and no record that a secret block occurred. Log first, then call.

[low] githubAdapter's gh injection is partial. Only signalSecretBlock/commentGithubIssue take it; prepare (new GitHubTaskFetcher), linkPr (linkPullRequest) and the proof-failure path (addPrFailureLabel, which execas directly) still reach the real CLI. Worth stating in the @internal comment so a future test using this seam doesn't silently hit the network.

[low] Breaking exported-type change under a fix( subject. WatchTick.secretBlocked is required and WatchTick is exported from src/index.ts:45; CHANGELOG is release-please-generated from commits, so the PR-body release note won't reach it. Add a BREAKING CHANGE: footer (or make the field optional).

Tests: coverage is good and the per-source assertions (updates).toEqual([['issue','update','ENG-1','--state','In Progress']]), expect(release).not.toContain('--add-label')) pin the actual regression. One weakness: the type test asserts the aliases (RunGithubIssueResult etc.), not the functions, so re-narrowing at runGithubIssue's own signature would still slip through — expectTypeOf(runGithubIssue).returns.resolves.toHaveProperty('secretBlocked') pins the real seam. I could not execute pnpm test/typecheck on the diff (the working tree is at the pre-PR baseline), so the stated gate results are unverified here.

Not covered: the pr-watch/mr-watch revise path, which has the same secretBlocked shape (revise-pr.ts:335) and is explicitly deferred by this PR; and the Projects fake-gh fixture internals.

[SebaBoler]

Round 3 — one line per finding, each verified against the code first.

[medium] The hold tells the human nothing about how to release it. Agreed. Each adapter now appends a resume line to the block comment it owns: GitHub/GitLab — "re-apply the trigger label (the running label was cleared)"; Linear — "move the issue back to its trigger state (it was left in the claimed state so the watch would not re-run it)". renderSecretBlockComment itself is unchanged (the revise path shares it). Asserted in github.test.ts / gitlab.test.ts.

[medium] The hold can go dark when the label signal fails. Agreed, and fixed per your first suggestion: onSecretBlocked for GitHub/GitLab issues is now a single edit swapping the claimed label for the secret-blocked label (--remove-label running --add-label secret-blocked in one gh issue edit / glab issue update). The claim is never released without the hold label landing; if the edit fails it throws into onFailure and the claimed marker stays. The adapter's best-effort pre-create + add remain (idempotent). Tests updated to pin the swap.

[low] Stale vanguard:secret-blocked after a successful retry. Done, without the blind remove that would fail gh issue edit in repos lacking the label: listReady already has each issue's labels, so the primitives remember which ready issues carry the hold label and claim removes it only for those. Tests cover both the re-triggered and the fresh issue on GitHub and GitLab.

[low] Log line emitted after the primitive. Done — logged first.

[low] githubAdapter's gh injection is partial. Done — the @internal note now says only signalSecretBlock goes through the injected runner; prepare, linkPr, addFailureLabel still reach the real CLI.

[low] BREAKING CHANGE: footer for WatchTick.secretBlocked. Not changed, deliberately. WatchTick is an output type: every consumer reads it from watchOnce/runLoopV1, and adding a field is additive for readers; only a hand-built WatchTick literal breaks, which is not a supported use. A BREAKING CHANGE footer would make release-please cut 2.0.0 for a bug fix, and making the bucket optional would force every reader to null-check a field the engine always fills. The PR body carries the note.

Tests — type test pins aliases, not functions. Done — added expectTypeOf(runGithubIssue / runGitlabIssue / runLinearIssue).returns.resolves.toHaveProperty('secretBlocked').

Gates: pnpm lint clean, pnpm typecheck clean, pnpm test 2526 passed / 3 skipped.

[github-actions]

Vanguard Review

Verdict: BLOCKING

Guidelines applied: /workspace/CLAUDE.md (review guidance section). It defines no severity ladder, so I label findings blocking / major / minor. The CI-config constraint is not engaged — the diff touches only src/** (no .github/workflows/**, .gitlab-ci.yml, .gitlab/**), so nothing here is a CI-config finding.

Blocking

1. The release instruction posted on GitHub Projects issues is wrong, and Projects has no other way out of the hold. src/runners/github.ts appends RESUME_HINT ("re-apply the trigger label (the running label was cleared)") inside githubAdapter.signalSecretBlock, unconditionally for every GitHub-backed run. But githubProjectWatchPrimitives (src/runners/watch.ts:779-805) lists ready items by item.status !== opts.triggerStatus → skip (the label is only an optional ownership filter), its claim is setStatus(id, claimedStatus), and this PR deliberately gives it no onSecretBlocked. So for Projects both clauses are false: there is no trigger label, and no running marker was cleared. A human who follows the comment re-applies a label, the item stays in In Progress, and the hold never lifts — the correct action is "move the item back to the trigger status". Projects is one of the two sources this PR set out to un-loop, and holding only works if the human-facing recovery text is right. The same text also lands on one-shot vanguard run issues, where neither label exists. Linear already solved this with its own LINEAR_RESUME_HINT; the fix is to make the hint source-aware (e.g. thread it in like SecretBlockStage, or have the watch primitive post the release line) rather than hard-coding the label procedure in the shared adapter.

Major (non-blocking)

2. White-label hold leaves no signal, and the stated reason doesn't hold. runSourcedIssue already skips signalSecretBlock in white-label mode (src/runners/source-adapter.ts:586), and the new isWhiteLabel(opts.deps) ? {} : { onSecretBlocked } branches (watch.ts, GitHub and GitLab issue primitives) drop the hold label too — so the issue sits on vanguard:running with no comment anywhere, indistinguishable from a run still in flight or one that crashed. The justifying comment ("a white-label run signals nothing on the issue, so there the claimed marker is the only visible trace") is inconsistent with the neighbouring code: those same primitives apply the branded vanguard:running on claim and still post branded onFailure / NO_CHANGE_MSG comments in white-label mode. The special case therefore buys no extra zero-trace guarantee while removing the operator's only readable signal. Either add the hold label there too (the claim is already branded) or emit something a human can see.

3. The stale-hold-label cleanup is missing for GitHub Projects. heldBefore + the conditional --remove-label only exist in githubIssueWatchPrimitives and gitlabWatchPrimitives. githubProjectWatchPrimitives.claim is setStatus(id, claimedStatus), so an item a human moves back to the trigger status keeps vanguard:secret-blocked forever — including after a later successful PR. That label is exactly what this PR designates as the hold signal for Projects, so it stops being trustworthy.

Minor

4. The load-bearing failure path is untested. The round-3 design rests on the comment "a failing edit throws into onFailure and the claimed marker stays" (watch.ts, both issue primitives). watchOnce does route it there (the onSecretBlocked?.() call is inside the try, catch → onFailure → kind: 'failed'), but no test covers a rejecting onSecretBlocked: tick.failed contains the id, tick.secretBlocked does not, and the claim is retained. Six new tests pin the happy paths; this is the one that protects against losing the claim.

5. gh label create <label> --force with no --color/--description. gh assigns a random colour when --color is omitted, and --force applies it to an already-existing label — so every secret block re-rolls the label's colour. It mirrors addPrFailureLabel (src/tasks/github.ts:107), so it is consistent with the repo, but pinning a colour/description (or dropping --force) avoids the churn. GitLab's variant matches addMrFailureLabel exactly — no issue there.

Verified clean

• triageAgentRun returns runner(id) unchanged, so widening its type to WatchRunOutcome really does surface secretBlocked through the needs-info-gated path.
• heldBefore is cleared-and-repopulated inside listReady, and both callers (runWatchLoop, and runLoopV1 via const listed = await agentPrimitives.listReady() before it overrides listReady) invoke it before any claim, so the stale-label clearing has no gap and the set cannot grow unbounded.
• Both summary-line sites (runWatchLoop:479, runLoopV1:536) got secretBlockedNote; no third printer exists.
• renderSecretBlockComment emits only masked findings; the appended hints add no untrusted or secret content, and the GitLab note path still goes through neutralizeQuickActions.
• WatchTick.secretBlocked being required breaks only external literal constructors; no in-repo consumer constructs one. The release note discloses it — consider a CHANGELOG.md entry if release-please doesn't derive it from the commit.
• The PR body's embedded verdict claims ("Verdict: real", gate results) are author assertions; I checked the code paths they describe rather than taking them as instructions.

Not covered: I did not run pnpm typecheck / pnpm test, and did not read the full source-adapter.ts delivery pipeline beyond the secret-block branch or the Linear CLI fetcher internals.

[SebaBoler]

Round 4 — one line per finding, each verified against the code first.

[blocking] Wrong release instruction on GitHub Projects (and one-shot runs). Confirmed: Projects lists by Status, claims by Status, and had no onSecretBlocked, so "re-apply the trigger label" was wrong there, and the same text reached vanguard run issues. Fixed the way you suggested: the procedure is no longer in the shared adapters at all (renderSecretBlockComment output is posted unchanged, as before this PR). Each watch primitive posts its own hold comment with the configured recovery: GitHub/GitLab issues — "Swapped running for secret-blocked… re-apply the <trigger label> label"; GitHub Projects — "stays in ""… move it back to """; Linear — "stays in ""… move it back to """. Projects and Linear now have an onSecretBlocked (comment only, no status/state change). Tests assert the exact per-source text.

[major] White-label hold leaves no signal. Agreed the special case was inconsistent with the branded claim label and failure/no-change comments. Removed: the hold is always labelled (swap) and explained, white-label included — and since a white-label adapter posts no findings, that comment is the human's only signal there. White-label tests now pin the swap + comment.

[major] Stale hold label not cleaned on GitHub Projects. Done: listReady records ready items carrying vanguard:secret-blocked, and claim removes it after the status move, only for those items (no blind --remove-label). Test covers a re-triggered item vs a fresh one.

[minor] Failure path untested. Done: a rejecting onSecretBlocked lands in tick.failed, not tick.secretBlocked, calls onFailure, never calls onNoChange, and the "secret blocked -> held for a human" line is logged before the edit.

[minor] gh label create --force without --color. Not changed, deliberately: it matches addPrFailureLabel and the revise path (revise-pr.ts:331), which create the very same label; pinning a colour only here would not stop the churn from those sites. Worth one follow-up that gives every Vanguard label a fixed colour/description in one place.

Gates: pnpm lint clean, pnpm typecheck clean, pnpm test 2528 passed / 3 skipped.

[github-actions]

Vanguard Review

Verdict: BLOCKING

Reviewed against CLAUDE.md (no review doc beyond it; no CI-config paths are touched, so that constraint doesn't apply). I verified the core fix really works: triageAgentRun returns runner(id) unchanged (watch.ts:406), so widening the three result types to RunIssueResult does get secretBlocked to watchOnce, and no WatchTick literal is constructed anywhere else in-repo (the required-field break is external-only and is release-noted). Secret handling is clean: renderSecretBlockComment masks structurally, both new hint strings are static with no interpolation, and the GitLab note still passes through neutralizeQuickActions. The new signalSecretBlock chains leave no unhandled rejection.

1. Blocking — correctness: the new resume hint is wrong (and a no-op) on the GitHub Projects path

src/runners/github.ts RESUME_HINT: "re-apply the trigger label (the running label was cleared)." githubAdapter is shared by both GitHub watch sources, and githubProjectWatchPrimitives runs through it (watch.ts:802 runOne: (id) => runGithubIssue(id, opts.deps)). On Projects the hold leaves the item in claimedStatus, readiness is item.status === opts.triggerStatus (watch.ts:~795), and claim only calls setStatus — the opts.label filter label was never removed. So both halves of the sentence are false there: nothing was cleared, and re-applying the label does not re-list the item. An operator who follows it leaves the item parked in In Progress indefinitely, which is the stuck state this PR exists to avoid. The same wording also reaches one-shot vanguard run --issue runs, where no watch labels were ever involved.
Fix: a source-specific hint (as already done for Linear via LINEAR_RESUME_HINT), or one sentence covering both releases ("re-apply the trigger label, or move the board item back to its trigger status").

2. Non-blocking — the hold label is never cleared on the Projects path

githubIssueWatchPrimitives / gitlabWatchPrimitives now strip a stale *secret-blocked label in claim via heldBefore, but githubProjectWatchPrimitives — whose documented signal is that label — never clears it. After a human fixes the secret and moves the item back, the successful re-run leaves vanguard:secret-blocked on the issue next to the review label. project item-list already returns content.labels, so the same guarded removal is cheap to add.

3. Non-blocking — the stale-clear rests on an implicit call-order contract

heldBefore is populated only by the primitives' own listReady, but runLoopV1 (watch.ts:~574) hands watchOnce a wrapper: { ...agentPrimitives, listReady: async () => agentReady }. It works only because runLoopV1 happens to call agentPrimitives.listReady() itself earlier in the tick; reordering that, or any external caller that wraps listReady, silently disables the cleanup with no test failing. Deriving the held set from the listed tasks handed to claim, or pinning the loop-v1 path in a test, would make it robust.

4. Non-blocking — untested invariant claimed in the round-3 comment

"a failing edit throws into onFailure and the claimed marker stays" is correct by inspection (the hold edit is inside watchOnce's try), but nothing pins it. A test with a rejecting onSecretBlocked asserting tick.failed === [id] and tick.secretBlocked === [] would lock in both the retained claim and the fact that such a run is reported as a generic failure.

5. Non-blocking — white-label hold is invisible

runSourcedIssue skips signalSecretBlock under white-label (source-adapter.ts:586) and the new primitives skip the hold edit, so a white-label secret block leaves the issue on the claimed label with no comment, label, or note anywhere — the operator log line is the only record. That is deliberate and better than the previous branded no-change comment, but it deserves a line in the release note so operators know recovery is fully manual.

Not covered: the Linear/Projects "stays claimed" choice beyond confirming it rests on the same trigger-state-type assumption the existing claim/onFailure already depend on, and I did not run pnpm test/typecheck (working tree is at the base commit, so the diff's tests aren't present locally).

[github-actions]

Vanguard Review

Verdict: BLOCKING

I read CLAUDE.md ("Review guidance") and the review document it points to (skills/code-review/SKILL.md), and use that vocabulary: severity low|medium|high|critical, kind security|perf|correctness|style; high/critical is blocking. No CI-config paths are touched by this diff, so the hard constraint is not in play. I reviewed the diff against the checked-out base (e581ef0); I did not run pnpm lint/typecheck/test, because the working tree is the base commit without this PR applied, so a local gate run would say nothing about the change (the author reports all three clean).

The core fix is right and the previous behaviour was genuinely broken: runSourcedIssue already returned { task, secretBlocked: true } at src/runners/source-adapter.ts:585, but the per-runner result types narrowed it away, so watchOnce destructured only { prUrl, parked } and ran onNoChange — reverting the claim and re-running the same secret forever on the state-reverting sources. Widening the result types, adding WatchRunOutcome/onSecretBlocked, and forwarding through triageAgentRun (return runner(id), so the field survives) all check out.

Findings

1. [high · correctness · verified] A failed stale-label cleanup in githubProjectWatchPrimitives.claim strands the item in the claimed status forever

src/runners/watch.ts, githubProjectWatchPrimitives.claim:

claim: async (id) => {
  await setStatus(id, opts.claimedStatus);
  if (heldBefore.has(id)) await editGithubLabels(repo, id, { remove: [GITHUB_SECRET_BLOCKED_LABEL] }, gh);
},

claimGate (src/runners/watch.ts:121-130) treats any throw from claim() as "someone else got it": catch { return 'taken'; } → watchOnce logs skipped -> already claimed and returns kind: 'skipped'. But setStatus has already moved the item to claimedStatus, and nothing reverts it. listReady only yields items with item.status === opts.triggerStatus, so the item is never listed, never run, never commented on — it is silently stranded until a human notices, which is the exact failure class this PR exists to remove.

Failure scenario: a human moves a held item back to Todo; the next poll's claim sets In Progress, then the gh issue edit --remove-label call hits a transient 5xx/rate limit (or, for a Projects board containing issues from another repository, issueRef is built from content.repository at src/runners/watch.ts:793 while the label edit is sent with --repo <deps.repoSlug> — a mismatch that fails or edits an unrelated issue number in the watch repo). Result: item parked in In Progress, reported as skipped, no retry.

This only affects items in heldBefore — precisely the re-triggered, previously secret-blocked ones. Suggested fix: make the cosmetic cleanup unable to abort the claim, e.g. if (heldBefore.has(id)) await editGithubLabels(...).catch(() => undefined); (and consider deriving the --repo for the label edit from the issueRef prefix rather than deps.repoSlug). The GitHub-issue and GitLab variants are fine here: they fold the extra --remove-label/--unlabel into the single existing edit, so a failure leaves the trigger label in place and the issue is re-listed next poll.

2. [medium · correctness · verified] The hold's "claim is never released without the hold label landing" invariant does not hold in white-label mode

Both new primitives document a fail-safe — githubIssueWatchPrimitives.onSecretBlocked: "the adapter pre-created the label and best-effort added it"; gitlabWatchPrimitives.onSecretBlocked: "One edit swaps claimed -> secret-blocked so the claim is never released without the hold label landing". The pre-create lives in the adapters (githubAdapter.signalSecretBlock / gitlabAdapter.signalSecretBlock), but runSourcedIssue skips signalSecretBlock entirely when commitAuthor is set (src/runners/source-adapter.ts:583: if (!whiteLabel) await adapter.signalSecretBlock(...)). GITHUB_SECRET_BLOCKED_LABEL is also absent from preflight's label check (githubLabelsFor, src/cli/preflight.ts:99-120), so nothing else guarantees the label exists.

Consequences on a white-label watch whose repo/project never created the label:

• GitHub: editGithubLabels(..., { add: [GITHUB_SECRET_BLOCKED_LABEL] }) fails (gh issue edit --add-label on an unknown label errors — this is the very reason addPrFailureLabel pre-creates at src/tasks/github.ts:107), so onSecretBlocked throws, watchOnce routes to onFailure, and the human gets Vanguard run failed instead of the hold text with the release step; tick.secretBlocked is empty, so the new summary counter under-reports.
• GitLab: glab issue update --label <unknown> is a silent no-op (the repo documents this at src/tasks/gitlab.ts:120-124) while --unlabel vanguard::running in the same call succeeds — the claim is released with no hold label and no trigger label, and the note posted claims a swap that never happened.

The three new white-label tests (watch.test.ts) use fake runners that resolve every call, so they pin the intended swap without exercising the missing-label path. Suggested fix: ensure the label inside the primitive right before the swap — the pattern already used at src/runners/revise-pr.ts:330 (gh label create … --force, best-effort) and addMrFailureLabel (glab label create --name) — and/or add the label to preflight's githubLabelsFor.

3. [low · style · verified] isWhiteLabel's docstring contradicts the behaviour it is introduced to share

src/runners/source-adapter.ts: the new export says white-label mode "must leave no Vanguard trace in the client repo (no branded comments, no secret-blocked label, plain PR body). One definition for runSourcedIssue and the watch primitives that must agree with it." No watch primitive calls it, and the primitives deliberately do the opposite: they add the hard-coded vanguard:secret-blocked / vanguard::secret-blocked label and post Vanguard held this issue: … regardless of commitAuthor — intentionally, per the PR's "No white-label exception". Worth noting the branded trace is more visible than the PR implies, since the claim label is operator-configurable (cmd.claimedState, src/cli/watch.ts:229) while the hold label and comment text are not; the pre-existing NO_CHANGE_MSG leaks branding the same way, so this is a documentation defect rather than a new leak. Either gate the branded text on isWhiteLabel or correct the docstring (and the "watch primitives that must agree with it" claim) to describe the hold exception.

4. [low · style · verified] The PR description describes a different change

The entire "What"/"Tests"/"Gates" body documents a src/cli/args.ts parseRunOptions/SharedRunOptions refactor, --fork/--spec-file rejection in watch/doctor, and usage-text consolidation. None of that is in this diff (no src/cli/ file is touched); only the "Round 4" section matches. Anyone reading the merge commit or doing a later bisect gets the wrong story. Rewrite the body to describe the secret-block hold.

Nothing in the PR text, commit messages, or code comments attempted to direct this review's outcome; the untrusted content was read as data only.

{"findings":[{"severity":"high","kind":"correctness","title":"githubProjectWatchPrimitives.claim: a failed stale-label cleanup after setStatus is classified as 'already claimed', stranding the item in claimedStatus with no retry and no comment","evidence":"src/runners/watch.ts githubProjectWatchPrimitives.claim awaits setStatus(id, claimedStatus) then editGithubLabels(repo, id, {remove:[GITHUB_SECRET_BLOCKED_LABEL]}); claimGate (src/runners/watch.ts:121-130) maps any throw to 'taken' -> kind 'skipped'. listReady only yields items whose status === triggerStatus, so the item is never re-listed. Cross-repo board items make this worse: issueRef uses content.repository (line 793) while the label edit uses --repo deps.repoSlug. Fix: .catch(() => undefined) on the cleanup (or run it before setStatus)."},{"severity":"medium","kind":"correctness","title":"The hold's 'claim never released without the hold label landing' invariant fails in white-label mode: the only label pre-create lives in signalSecretBlock, which runSourcedIssue skips when commitAuthor is set","evidence":"src/runners/source-adapter.ts:583 if (!whiteLabel) await adapter.signalSecretBlock(...); GITHUB_SECRET_BLOCKED_LABEL is not in preflight's githubLabelsFor (src/cli/preflight.ts:99-120). GitHub: gh issue edit --add-label on an uncreated label errors (cf. src/tasks/github.ts:107), so onSecretBlocked throws into onFailure -> 'Vanguard run failed' comment, no release instructions, tick.secretBlocked under-counts. GitLab: glab issue update --label is a silent no-op on a project lacking the label (documented at src/tasks/gitlab.ts:120-124) while --unlabel succeeds, so the claim is released with no marker and the note claims a swap that did not happen. The new white-label tests use always-succeeding fake runners and do not cover this. Fix: ensure the label in the primitive before the swap (pattern at src/runners/revise-pr.ts:330 / addMrFailureLabel) and/or add it to preflight."},{"severity":"low","kind":"style","title":"isWhiteLabel's docstring claims 'no secret-blocked label, no branded comments' and a definition shared with the watch primitives, but no primitive uses it and all of them label + post branded hold text in white-label mode","evidence":"src/runners/source-adapter.ts new isWhiteLabel export vs githubIssueWatchPrimitives/gitlabWatchPrimitives onSecretBlocked, which add the hard-coded secret-blocked label and 'Vanguard held this issue' text unconditionally; the claim label is configurable (src/cli/watch.ts:229) while the hold label/text are not."},{"severity":"low","kind":"style","title":"PR description documents an unrelated src/cli/args.ts parseRunOptions refactor that is not in this diff","evidence":"No src/cli/ file is modified; only the 'Round 4' section of the description corresponds to the diff's watch secret-block hold."}]}

[SebaBoler]

Round 5 — one line per finding, each verified against the code first.

[high] Projects claim: a failing stale-label cleanup after setStatus reads as "already claimed". Confirmed: claimGate maps any throw to 'taken', nothing reverts the status, and listReady only yields the trigger status, so the item would sit in the claimed status with no run and no comment. Fixed as suggested: the cleanup is best-effort (.catch(() => undefined)) and the edit now targets the issue's own repo (id.slice(0, id.indexOf('#')), i.e. content.repository), not the watch's repoSlug. Test: an item from other/repo whose gh issue edit throws 502 is still claimed and run, tick.skipped is empty, and the edit carried --repo other/repo. (The pre-existing comment calls in the Projects primitives still use repoSlug; that mismatch is older than this PR and separate — noted for a follow-up.)

[medium] White-label: the hold's label is never pre-created. Confirmed: signalSecretBlock is skipped under commitAuthor, so GitHub's swap would fail on an unknown label (-> onFailure, wrong comment, under-counted) and GitLab's swap would release the claim with no hold label. Fixed in the primitives, right before the swap: gh label create <label> --repo <repo> --force / glab label create --repo <project> --name <label>, best-effort, the same pre-step as addPrFailureLabel / addMrFailureLabel. Both white-label tests now assert the label create call lands before the swap edit.

[low] isWhiteLabel docstring. Deleted the export instead of rewording it: it had one caller, and main already had the inline deps.commitAuthor !== undefined there. Nothing in the watch primitives branches on white-label any more, which is the point of round 4.

[low] PR description. Rewritten; it now describes this change (the body was pasted from the sibling CLI PR).

Gates: pnpm lint clean, pnpm typecheck clean, pnpm test 2529 passed / 3 skipped.

[github-actions]

Vanguard Review

Verdict: NO BLOCKING FINDINGS

No blocking findings.

Guideline note: the repo's review guidance (CLAUDE.md → "Review guidance") defines only blocking vs not, plus the CI-config rule; it names no severity scale, so I label findings blocking / medium / low. This diff touches no CI-config path (src/ only), so the CI constraint is not in play. I reviewed against the base tree (the diff is not applied locally), so I could not execute pnpm typecheck/pnpm test myself — the gate claims are unverified by me.

Verified as sound: triageAgentRun returns runner(id) directly (src/runners/watch.ts:405), so widening its type really does forward secretBlocked; the GitHub swap is a single gh issue edit --add-label … --remove-label … so the claim can't be released without the hold label; GitLab's glab is already defaulted (opts.gl ?? defaultGlabRunner) so the new direct glab([...]) call can't be undefined; editGithubLabels/commentGithubIssue normalise owner/repo#n via issueNumber, so the Projects id.slice(...) repo override is well-formed; and labels is in both fetchers' list field sets, so heldBefore detection has real data.

1. White-label hold contradicts the documented white-label invariant (medium, non-blocking)

src/runners/source-adapter.ts:581-583 states the contract: "White-label runs keep the automation invisible in the client repo: no secret-blocked label, no branded comment" — hence if (!whiteLabel) await adapter.signalSecretBlock(...). The new onSecretBlocked in githubIssueWatchPrimitives / gitlabWatchPrimitives unconditionally runs gh|glab label create for the hardcoded vanguard:secret-blocked / vanguard::secret-blocked and posts "Vanguard held this issue: …", with tests pinning exactly that ("No white-label exception"). The author's argument holds only for configurable markers: label, claimedLabel, reviewLabel all come from --claimed-state/--review-state/--label, so a white-label operator can de-brand them; this hold label cannot be de-branded and is newly created in the client repo. Fix: add secretBlockedLabel?: string to WatchGithubOptions/WatchGitlabOptions (default the constant) and include it in githubLabelsFor() in src/cli/preflight.ts:99, where the other state labels are existence-checked, rather than relying on an inline label create --force (which also resets an existing label's colour/description).

2. Projects hold comment can land on an unrelated issue for cross-repo boards (low-medium, non-blocking)

githubProjectWatchPrimitives.onSecretBlocked posts via commentGithubIssue(repo, id, …) where repo is the watch's deps.repoSlug, but id may be other/repo#7 (listReady builds ${content.repository}#${number}). The same hunk derives the item's own repo one line earlier in claim (id.slice(0, id.indexOf('#'))) and even test-pins it, so the knowledge is already there. Consequence is worse than for the pre-existing onNoChange/onFailure instances: this comment is the human's only release instruction, so it is both lost and misdelivered to owner/repo#7. Fix: derive the repo from id for the comment too.

3. Vacuous assertion hides the GitLab "trigger label not restored" check (low, non-blocking)

src/runners/watch.test.ts, GitLab secret-blocked test: expect(release).not.toContain('--label vanguard') — release is the argv string[], and no single element ever equals "--label vanguard", so this passes unconditionally and tests nothing. The corresponding GitHub test does it correctly (expect(release).not.toContain('ready for agent')). Fix: reuse the unlabelled()-style helper to assert no --label value equals opts.label.

4. A rejecting onSecretBlocked reports only "Vanguard run failed" on the issue (low, non-blocking)

In watchOnce, a throwing hold edit routes to onFailure, which posts formatFailureComment('Vanguard run failed', error) — e.g. "label API down". The secret block and the fact that the claim is still held appear only in the operator log. A human reading the issue gets a misleading cause. Consider passing the block context into the failure comment on this path.

5. Nits (low)

• runLoopV1 --once carries spec.advanced ids into the agent pass without them appearing in listed (src/runners/watch.ts:526-531), so heldBefore misses them and a stale vanguard:secret-blocked label survives a re-run via the spec trigger. Cosmetic only.
• githubAdapter(deps, gh = defaultGhRunner) injects gh into signalSecretBlock only; prepare/linkPr still hit the real CLI. The doc comment says so, but GitHubTaskFetcher and linkPullRequest both already accept a runner — threading it through would remove the footgun for the next test.
• The new claim cleanup assumes content.repository is owner/repo. That matches existing code and fixtures (src/tasks/github-project.ts:65), but if gh project item-list emits a repository URL there, --repo https://github.com/owner/repo is what gets sent; the failure is swallowed, so worst case is a surviving cosmetic label. Worth a one-off check against real gh output.

Not covered: I did not audit secret-scan.ts / deliver-change.ts (unchanged by this diff) or the revise-pr secret-blocked path the description mentions as still open, and I did not run the test suite.