[danii1]

Why

Task local-fix-worker-pause-injected-and-retry-drai: fix(worker): pause injected and retry drains while the harness chain is exhausted

When every harness in the failover chain was usage-limited or cooling down, the injected-task and dashboard-retry drains still claimed their rows every 5 seconds, immediately got "deferred" back from the fleet executor, requeued the rows, and logged a running/deferred pair each cycle. One injected task produced over a thousand identical log pairs per pause window. The fix makes both short-interval drains idle while the chain is exhausted, leaving the existing onPause/onResume lines as the only per-window output.

What changed

• packages/code/src/lib/workspace/injected-acquirer.ts: added an injectable isPaused?: () => boolean option, defaulting to getWorkerFailover()?.allExhausted() ?? false. tick() now returns before claiming when paused, so it neither runs, requeues, nor logs anything. The existing per-task "deferred" handling (repo capacity/concurrency) is unchanged.
• packages/code/src/lib/workspace/retry-acquirer.ts: same isPaused option, default, and early return at the top of tick().
• packages/code/src/lib/workspace/workspace-worker.ts: added isHarnessChainPaused() resolving the live failover state and wired it into both drains. Extracted the inline retry-acquirer construction into buildRetryQueueAcquirer() (matching buildInjectedTaskAcquirer) so production wiring is explicit and runWorkspaceWorker stays under the lint line limit.
• packages/code/tests/injected-acquirer.test.ts and packages/code/tests/retry-queue-acquirer.test.ts: each gained a test that runs 50 paused ticks, asserts the executor is never called, console.log is never called, the row stays pending/queued, then verifies the first unpaused tick drains it to done.
• fleet-executor.ts was intentionally left untouched: its pre-git isHarnessExhausted() guard remains defense in depth.

Scope

Included: the two acquirers' tick gates, failover predicate plumbing in workspace-worker.ts, and the unit tests above.
Excluded: failover/usage-limit semantics, drain interval changes, suppressing the repo-capacity "deferred" log, worktree retention, and dashboard changes.

Tradeoffs

• The pause predicate is evaluated once at the start of each tick(). Work that starts between that check and the executor call is still caught by the executor's existing guard and may emit one running/deferred pair, which is the intended defense in depth rather than a second check per claimed row.
• The default predicate is duplicated in both acquirers so each module stays independently usable; workspace-worker.ts also wires the shared helper explicitly.
• Extracting buildRetryQueueAcquirer adds an exported builder and spreads FleetExecutorDeps instead of listing fields, but keeps the executor configuration identical and gives the worker function headroom under max-lines-per-function.

Blast Radius

• runWorkspaceWorker now builds both drains through the failover-aware builders; behavior changes only while allExhausted() is true.
• Direct constructors used by tests and other callers keep their old behavior because the default resolves to null failover → false when failover has not been started.
• buildInjectedTaskAcquirer callers (workspace worker and workspace-worker.test.ts) gain the pause gate; existing tests still pass.
• The fleet executor's "deferred" contract, supervisor capacity handling, injected-store requeue semantics, and retry-store queue semantics are unchanged.
• No dashboard, tracker, or failover code paths were modified.

Verification

• cd packages/code && bun run test injected-acquirer.test.ts retry-queue-acquirer.test.ts → 22 pass, 0 fail (63 expect calls).
• cd packages/code && bun run test → 2527 pass, 0 fail across 168 files.
• cd packages/code && bun run typecheck → passed.
• Root bun run typecheck → Turbo reported 12/12 packages successful. The chained typecheck:examples step initially hit a transient nono sandbox IO error: Permission denied / sandbox-state drift; rerunning bunx tsc --noEmit -p examples/pipeline-plugins succeeded with no output.
• bunx oxfmt --check on the five changed files → all correctly formatted.
• bunx oxlint packages/code/src/lib/workspace packages/code/tests → no findings.
• No commit, push, label, or PR was created, per the task instructions.

---

This PR was automatically created by @devintern/code

[danii1]

Artifacts & verification

Run: 39 · Commit: e960701d20f789404a5998a44b391cf498fe434a

Change explanation

Produced by pr-explainer-narrative, execution 1 at e960701d20f789404a5998a44b391cf498fe434a.

Pause injected and retry drains while the harness chain is exhausted

Why this change is needed

The task states that when every harness in the chain is usage-limited, fleet-executor.ts already returns "deferred" before touching git, but the two queue acquirers keep claiming and requeueing the same row on every 5-second drain tick. The task records a single injected task producing roughly 1,200 running plus roughly 1,200 deferred log lines in under two hours, which buried real failures. The needed state (getWorkerFailover()?.allExhausted()) already existed; the acquirers simply did not consult it.

What changed

• Both short-interval drains now honor a pause gate. InjectedTaskAcquirer.tick() and RetryQueueAcquirer.tick() return before claiming a row when the harness chain is exhausted. No claim, no requeue, and no running / re-running / deferred output is emitted while paused. Rows stay pending (injected) or queued (retry).
• The predicate is injectable and live. Both option types gain isPaused?: () => boolean, defaulting to the process-wide getWorkerFailover()?.allExhausted() ?? false. Production passes an explicit isHarnessChainPaused wrapper, which reads the failover state at call time rather than snapshotting it.
• Retry acquirer construction is extracted into buildRetryQueueAcquirer, mirroring the existing buildInjectedTaskAcquirer, so the retry drain can be wired and tested the same way. Behavior of the executor closure is otherwise unchanged.
• Defense in depth is preserved. The fleet executor's own isHarnessExhausted() pre-git guard and its "deferred" result for repo/concurrency deferrals remain intact, so a task that starts between the acquirer check and the executor call is still deferred correctly.
• Tests added for paused behavior, pause-to-resume draining, and live wiring against a real WorkerFailover. The acquirer test files also reset the failover singleton around each test.

Validation actually performed

Observed, from the pipeline evidence:

• The __verify-1 step reports running the targeted acquirer tests (22 pass), the full packages/code suite (2527 pass, 0 fail), package and root typecheck, oxfmt --check, and oxlint with no errors.
• The auto-review step reports running three targeted test files (64 pass) and the full packages/code suite (2530 pass, 0 fail). The differing full-suite totals (2527 vs 2530) come from separate recorded runs, not from this summary.
• The auto-review step also reports worker-add-command.test.ts at 40 pass, 0 fail.
• An auto-review experiment preloaded an exhausted WorkerFailover and ran the two acquirer test files: 2 pass, 20 fail; without the preload the same files reported 22 pass, 0 fail. This demonstrates test coupling to the singleton, not a production defect.

Not observed: no daemon run, no before/after log-line measurement, and no reproduction of the original 1,200-line flood.

Material risks

• Test coupling to the process-wide failover singleton (observed). worker-add-command.test.ts constructs InjectedTaskAcquirer without isPaused and has no resetWorkerFailover() hook, unlike the three test files updated here. A leaked or preloaded exhausted controller would make its tick() a silent no-op. It passes today, but the preload experiment above shows the failure mode is real.
• Silent idle drain if the failover state is wrong (inference). If allExhausted() stays true incorrectly, both drains remain idle with only the existing onPause line as signal. This is the intended behavior, but it shifts reliance onto failover-state correctness.
• Reduced per-tick visibility (inference). Operators lose the periodic deferred heartbeat for the duration of the pause; the pause/resume lines are now the only per-window output, as the task requested.
• Duplication (inference). The default predicate is implemented separately in each acquirer plus the isHarnessChainPaused wrapper; a future change to the pause definition must be applied in all three places.
• No row loss (observed in tests, inferred for production). Unit and wiring tests show rows remain pending/queued across many paused ticks and drain on the first unpaused tick. Production correctness rests on the same code path.

Where reviewers should focus

• packages/code/src/lib/workspace/injected-acquirer.ts — gate placement after the stopped / claiming / store-closed checks and before claiming.
• packages/code/src/lib/workspace/retry-acquirer.ts — same gate, and that busy is not set while paused.
• packages/code/src/lib/workspace/workspace-worker.ts — isHarnessChainPaused wiring into both builders and the retry-acquirer extraction preserving the original executor options.
• packages/code/tests/worker-add-command.test.ts — the one acquirer consumer not given a failover reset; worth deciding whether to add one.
• The retained fleet-executor.ts pre-git guard, to confirm it was not weakened.

Observed evidence vs inference

• Observed: the implementation's gates, defaults, and production wiring as described above; the recorded test, typecheck, format, and lint results; the preload experiment showing singleton-sensitive tests.
• Inference / suggested checks: that production drains behave identically under a real usage-limit window, that no row can be stranded if failover state misreports, and the operational impact of the quieter log. Suggested check: a manual or e2e run that forces allExhausted() and confirms only onPause/onResume lines appear while the row remains queued.

Visual explanation

Produced by pr-explainer-visual, execution 1 at e960701d20f789404a5998a44b391cf498fe434a.

Show artifact

Summary

When every harness in the chain is usage-limited, the injected and retry drains now leave their rows untouched instead of claiming, running, and requeueing the same row on every 5-second tick. This removes the repeated running / deferred log pair that filled roughly 1,200 lines per task in under two hours and buried real failures. The gate is injectable for tests and defaults to the same process-wide allExhausted() state the fleet executor already checks before creating a worktree.

Evidence

The change is one gate per acquirer, wired to the existing failover singleton:

runWorkspaceWorker
  │
  ├─ startWorkerFailover({ onPause, onResume })   ← already existed
  │
  ├─ buildRetryQueueAcquirer(..., isPaused: isHarnessChainPaused)
  └─ buildInjectedTaskAcquirer(..., isPaused: isHarnessChainPaused)
                                             │
                                             ▼
                    getWorkerFailover()?.allExhausted() ?? false
                                             │
                          ┌──────────────────┴──────────────────┐
                       true                                  false
                          │                                     │
                    tick() returns                    claim → execute
                  (no claim, no requeue,                       │
                   no running/deferred log)     fleet-executor pre-git guard:
                                                isHarnessExhausted → "deferred"
                                                (kept as defense in depth)

The retry acquirer also stops setting busy while paused, and buildRetryQueueAcquirer was extracted so the executable closure is constructed the same way the injected drain already was.

Validation observed in the supplied pipeline evidence:

Check	Result
Targeted acquirer tests (injected-acquirer, retry-queue-acquirer)	22 pass, 0 fail (__verify-1)
Full packages/code suite	2527 pass, 0 fail (__verify-1); 2530 pass, 0 fail (auto-review run)
Package and root typecheck, oxfmt --check, oxlint	reported clean
Three targeted test files (auto-review)	64 pass
worker-add-command.test.ts	40 pass, 0 fail
Preload an exhausted WorkerFailover, then run the two acquirer files	2 pass, 20 fail; without the preload, 22 pass, 0 fail

Before/after results for the actual daemon log flood were not captured. No live usage-limit window was reproduced, and the 1,200-line counts come from the task report, not from this change's validation.

Merge Danger

Reverting any one of the three source files restores the old per-tick claim/requeue loop and the log flood, but nothing else breaks: the rows are durable in SQLite, the executor guard is unchanged, and there is no schema, config, or API change. Blast radius is small and contained — only runWorkspaceWorker constructs these acquirers in production, and both builders pass the same predicate, so a revert is a straight rollback of the two gates plus the wiring.

The real risk is not the revert, it is the pause state itself: if allExhausted() stays true incorrectly, both drains go quiet and the only remaining signal is the existing onPause line, so a stuck failover would silently stall queued work. The gate is otherwise equivalent to work the executor was already deferring, so it is safe to ship without a staged rollout. One known test-hygiene edge: worker-add-command.test.ts builds InjectedTaskAcquirer without isPaused and without resetWorkerFailover(), so it depends on no other test leaking an exhausted controller; the preload experiment above shows that failure mode is real, though the file passes today.

Verification report

Produced by __verify-1, execution 1 at 97c263c6146d203b66a92603e5d2fa65d7dec51f.

Show artifact

{
  "summary": "Implementation satisfies the task. Both drains gate on the harness-chain exhaustion state before claiming rows, the predicate is injectable with a failover-backed default, production wiring uses the real failover, and the fleet-executor pre-git deferral guard remains intact. Verified by running the targeted tests (22 pass), the full packages/code suite (2527 pass, 0 fail), package and root typecheck, oxfmt --check, and oxlint with no errors. Blast-radius fact that makes this safe: getWorkerFailover() is process-wide and startWorkerFailover() runs at workspace-worker.ts:888 before the two acquirers are built at :936-953, and both builders pass isHarnessChainPaused (:520, :558), the same predicate fleet-executor.ts:245 already uses; the only other constructors are tests, so the new acquirer gate cannot disagree with the executor's defense-in-depth guard or bypass any production consumer.",
  "items": [
    {
      "priority": "info",
      "confidence": "high",
      "category": "bug",
      "file": "packages/code/src/lib/workspace/injected-acquirer.ts",
      "line": "127",
      "issue": "Requirement 1/5 met: tick() returns before claiming, requeueing, or logging when paused; isPaused is injectable and defaults to getWorkerFailover()?.allExhausted() ?? false.",
      "evidence": "ran: bun run test injected-acquirer.test.ts retry-queue-acquirer.test.ts -> 22 pass, 0 fail; file:127 gate and file:162-163 default predicate",
      "suggestion": "No change required."
    },
    {
      "priority": "info",
      "confidence": "high",
      "category": "bug",
      "file": "packages/code/src/lib/workspace/retry-acquirer.ts",
      "line": "82",
      "issue": "Requirement 1/5 met: retry drain returns before claiming/re-running/requeueing when paused, with the same injectable failover-backed predicate as the injected drain.",
      "evidence": "ran: targeted tests -> 22 pass, 0 fail; file:82 gate and file:102-103 default predicate",
      "suggestion": "No change required."
    },
    {
      "priority": "info",
      "confidence": "high",
      "category": "testing",
      "file": "packages/code/tests/injected-acquirer.test.ts",
      "line": "115",
      "issue": "Acceptance criteria covered: paused ticks never execute, never log console.log, row stays pending across 50 paused ticks, then the first unpaused tick drains it. Retry equivalent at retry-queue-acquirer.test.ts:127.",
      "evidence": "ran: bun run test injected-acquirer.test.ts retry-queue-acquirer.test.ts -> 22 pass, 0 fail, 63 expect() calls",
      "suggestion": "No change required."
    },
    {
      "priority": "info",
      "confidence": "high",
      "category": "bug",
      "file": "packages/code/src/lib/workspace/fleet-executor.ts",
      "line": "301",
      "issue": "Requirement 4 met: the executor's isHarnessExhausted() pre-git guard and its 'deferred' return remain unchanged, preserving defense in depth and the busy-repo/concurrency deferral log path.",
      "evidence": "read: fleet-executor.ts:244-245 and :301-303 still return 'deferred'; injected-acquirer runOne still logs at :215 and retry runOne at :127",
      "suggestion": "No change required."
    },
    {
      "priority": "info",
      "confidence": "high",
      "category": "code-quality",
      "file": "packages/code/src/lib/workspace/workspace-worker.ts",
      "line": "520",
      "issue": "Requirement 5 met: both production builders receive isPaused: isHarnessChainPaused; the retry builder extraction preserves the previous deps/options exactly and is used at :937.",
      "evidence": "read: workspace-worker.ts:488-489, :520, :558, :937-953; git show confirms only the inline retry construction was replaced with an equivalent builder",
      "suggestion": "No change required."
    },
    {
      "priority": "info",
      "confidence": "high",
      "category": "testing",
      "file": "packages/code/tests/injected-acquirer.test.ts",
      "line": "91",
      "issue": "No regression in unfrozen behavior: deferred results still requeue exactly once per tick and other existing injected/retry tests pass unchanged.",
      "evidence": "ran: bun run test -> 2527 pass, 0 fail across 168 files; existing deferred tests at injected-acquirer.test.ts:91 and retry-queue-acquirer.test.ts:110 passed",
      "suggestion": "No change required."
    },
    {
      "priority": "info",
      "confidence": "high",
      "category": "code-quality",
      "file": "packages/code/src/lib/workspace/workspace-worker.ts",
      "line": "488",
      "issue": "Formatting, lint, and type safety pass for the changed surface.",
      "evidence": "ran: bunx oxfmt --check on all five changed files -> all matched; bunx oxlint packages/code/src/lib/workspace packages/code/tests -> no output; bun run typecheck in packages/code and at repo root -> success",
      "suggestion": "No change required."
    },
    {
      "priority": "low",
      "confidence": "low",
      "category": "testing",
      "file": "packages/code/src/lib/workspace/workspace-worker.ts",
      "line": "520",
      "issue": "No test directly asserts the builder-to-acquirer isPaused plumbing or that getWorkerFailover() drives the default; coverage is indirect through the acquirer tests using an injected predicate.",
      "evidence": "unproven: grep shows buildInjectedTaskAcquirer tests in workspace-worker.test.ts do not start a failover or assert pause behavior",
      "suggestion": "Optional: add a builder-level test that stubs getWorkerFailover/allExhausted and asserts a pending row is untouched, though the acceptance criteria do not require it."
    }
  ],
  "approved": true
}

Auto-review summary

Produced by auto-review, execution 1 at e960701d20f789404a5998a44b391cf498fe434a.

Show artifact

{
  "iterations": 2,
  "success": true,
  "finalFeedback": {
    "summary": "The change is a read-only pause gate: InjectedTaskAcquirer and RetryQueueAcquirer now return before claiming when the process-wide WorkerFailover.allExhausted() is true, which is the same predicate the fleet executor already consults; because both builders pin an explicit repo, these rows always reach the executor's return \"deferred\" guard at fleet-executor.ts:301 rather than an earlier routing skip, so a paused tick only removes the old claim-defer-requeue churn. Only runWorkspaceWorker builds the acquirers (workspace-worker.ts:937,948), and both builders pass the same predicate, so there is no other production caller to update. I verified the behavior with the three targeted test files (64 pass) and the full packages/code suite (2530 pass, 0 fail); the remaining findings are test-hygiene, stale-comment, duplication, and timer-path coverage issues, not correctness bugs.",
    "items": [
      {
        "priority": "low",
        "confidence": "medium",
        "category": "testing",
        "file": "packages/code/tests/worker-add-command.test.ts",
        "line": "750-757",
        "issue": "This is a sibling consumer of the changed InjectedTaskAcquirer contract: it constructs the acquirer without isPaused, so the new default reads the process-wide failover singleton. The file has no resetWorkerFailover() hook, unlike the three test files this PR updated. A failover controller leaked or preloaded as exhausted would make acquirer.tick() a silent no-op and fail expect(calls).toEqual([enqueued.taskKey]). It is currently safe only because the other files that call startWorkerFailover reset it in afterEach.",
        "evidence": "packages/code/tests/worker-add-command.test.ts:750-757; `grep -n resetWorkerFailover packages/code/tests/worker-add-command.test.ts` returns no matches. `bun run test worker-add-command.test.ts` -> 40 pass, 0 fail; full `bun run test` -> 2530 pass, 0 fail.",
        "suggestion": "Add resetWorkerFailover() to this file's beforeEach/afterEach, or pass isPaused: () => false in this direct unit test, so it cannot be affected by process-wide failover state."
      },
      {
        "priority": "low",
        "confidence": "medium",
        "category": "documentation",
        "file": "packages/code/src/lib/workspace/injected-acquirer.ts",
        "line": "109-114",
        "issue": "The tick() doc still says a deferred row \"(busy repo or exhausted harness)\" is requeued. The new early return at 124-129 means an exhausted harness never claims or requeues a row; only a busy repo (and the executor's race guard at runOne 210-215) reaches requeue. The comment now describes the old behavior and could mislead a future change.",
        "evidence": "injected-acquirer.ts:111-112 versus the new early return at injected-acquirer.ts:124-129; the runOne deferral at 210-215 is now reachable only for executor-level deferrals.",
        "suggestion": "Reword the tick doc to say the drain returns before claiming while the chain is exhausted, and that only busy-repo/executor-race deferrals are requeued."
      },
      {
        "priority": "low",
        "confidence": "medium",
        "category": "code-quality",
        "file": "packages/code/src/lib/workspace/workspace-worker.ts",
        "line": "488-490",
        "issue": "The pause policy getWorkerFailover()?.allExhausted() ?? false is duplicated in isHarnessChainPaused (workspace-worker.ts:488-490), InjectedTaskAcquirer.isPaused (injected-acquirer.ts:163), and RetryQueueAcquirer.isPaused (retry-acquirer.ts:103). The fleet executor's guard is separately configurable via FleetExecutorDeps.isHarnessExhausted (fleet-executor.ts:244-245), so a caller that customizes the executor predicate without also passing isPaused gets two different pause decisions. Production currently leaves both at the global default, so no behavior change today, but this is a drift risk.",
        "evidence": "workspace-worker.ts:488-490, injected-acquirer.ts:163, retry-acquirer.ts:103, fleet-executor.ts:244-245; typecheck and full suite pass.",
        "suggestion": "Export one helper (ideally from worker/failover.ts) and use it as the default in both acquirers, or wire the builder's isPaused to the same deps.isHarnessExhausted predicate used by the executor."
      },
      {
        "priority": "info",
        "confidence": "medium",
        "category": "testing",
        "file": "packages/code/tests/workspace-worker.test.ts",
        "line": "925-936",
        "issue": "The new wiring tests exhaust the chain with failover.manager.reportUsageLimit(...) and clear it with windowElapsed(...), bypassing WorkerFailover.reportFromHint (which arms the resume timer at failover.ts:217-225) and the onPause/onResume callbacks. They prove the live predicate and the acquirer wiring, but not that the timer-driven resume actually unpauses a running drain. The per-tick re-evaluation makes this low risk, but the integration path is untested.",
        "evidence": "workspace-worker.test.ts:928-936 uses failover.manager.reportUsageLimit; the production path is failover.ts:217-225. Targeted tests: 64 pass; full suite: 2530 pass, 0 fail.",
        "suggestion": "Add a test that drives failover.reportFromHint(...) (or uses fake timers around the armed window) and asserts the same acquirer drains after the window elapses."
      }
    ],
    "approved": true
  },
  "history": [
    {
      "iteration": 1,
      "feedback": {
        "summary": "The change is sound and does what it claims: the two short-interval drains now skip claim/requeue churn while the harness chain is exhausted. The one fact that makes it safe is that the new gate reads exactly the same dynamically-evaluated predicate the fleet executor already applies before creating a worktree (`getWorkerFailover()?.allExhausted()`, fleet-executor.ts:244-245 and :301), so the acquirer can only skip work that would have been claimed and immediately deferred anyway — rows stay durable and pending. I proved this at `ran-it` level for both acquirers against a real exhausted `WorkerFailover` (0 claims, rows still pending, then drained after `windowElapsed`), and the existing suites, typecheck, lint, and format all pass; the remaining issues are test coverage and small maintainability edges, not functional defects.",
        "items": [
          {
            "priority": "medium",
            "confidence": "high",
            "category": "testing",
            "file": "packages/code/tests/injected-acquirer.test.ts",
            "line": "36-53, 115-148",
            "issue": "Existing acquirer tests now implicitly read the process-wide failover singleton, because `isPaused` defaults to `getWorkerFailover()?.allExhausted()` (injected-acquirer.ts:163, retry-acquirer.ts:103) and none of these tests pass `isPaused` or reset the singleton. Any test file (or in-process code) that leaves an exhausted `WorkerFailover` in the same bun process silently disables these drains and fails the unpaused tests — the opposite of how the tests read.",
            "evidence": "Ran `bun test --preload /tmp/opencode/preload-exhausted.ts injected-acquirer retry-queue-acquirer` (preload starts a one-entry failover and exhausts it via `manager.reportUsageLimit`): output `2 pass 20 fail`, e.g. `retry-queue-acquirer.test.ts:198` expected `[\"PROJ-1\"]` and received `[]`. Without the preload the same two files report `22 pass 0 fail`.",
            "suggestion": "Make these tests hermetic: pass `isPaused: () => false` in the existing constructors (or add a `resetWorkerFailover()` in `beforeEach`/`afterEach` plus a documented setup guard) so a leaked singleton cannot change their outcome."
          },
          {
            "priority": "medium",
            "confidence": "medium",
            "category": "testing",
            "file": "packages/code/src/lib/workspace/workspace-worker.ts",
            "line": "530-561",
            "issue": "The production wiring of this feature is not covered by any test. `buildRetryQueueAcquirer` is only referenced from `workspace-worker.ts` (no test imports it), and both new test cases inject a local `let paused` flag, so the actual default branch (`getWorkerFailover()?.allExhausted()` in injected-acquirer.ts:163 / retry-acquirer.ts:103) and the exported `isHarnessChainPaused` (workspace-worker.ts:488-490) are never exercised. A future change that stops wiring `isPaused` (or wires a stale snapshot) would leave every test green while the pause silently never engages.",
            "evidence": "`grep -rn \"buildRetryQueueAcquirer\" packages/code` returns only `src/lib/workspace/workspace-worker.ts:530,937`; `tests/injected-acquirer.test.ts:118-149` and `tests/retry-queue-acquirer.test.ts:127-160` use `isPaused: () => paused`. I verified the current behavior manually with /tmp/opencode/pause-check.ts and /tmp/opencode/pause-check-retry.ts: paused tick => 0 calls, row pending, then drained after `windowElapsed`.",
            "suggestion": "Add one test per acquirer that starts a real (or stubbed) `WorkerFailover`, exhausts it, and asserts no claim; plus a small `isHarnessChainPaused`/`buildRetryQueueAcquirer` wiring test so the option cannot be dropped unnoticed."
          },
          {
            "priority": "low",
            "confidence": "medium",
            "category": "bug",
            "file": "packages/code/src/lib/workspace/injected-acquirer.ts",
            "line": "124-129",
            "issue": "The acquirer-level guard runs before the executor performs routing, so while the chain is exhausted a task that cannot be routed is no longer recorded as a routing skip. Previously a tick reached fleet-executor.ts:265-288 (routing + `skips.record`/warning) and returned `true`; now the row simply stays `pending` for the whole limit window (which can be hours or a month for spend limits), delaying the operator-visible misrouting signal. Same for retry rows (retry-acquirer.ts:82-84) whose persisted repo no longer exists.",
            "evidence": "packages/code/src/lib/workspace/fleet-executor.ts:265-288 (routing and skip recording) is ordered before :297-303 (harness guard); packages/code/src/lib/workspace/injected-acquirer.ts:127 returns before `claimNext()`.",
            "suggestion": "Accept this as intended (deferred routing is arguably safer) but state it in the comment/docstring, or only short-circuit when `hasPending()` is false so idle ticks stay cheap while the drain still surfaces routing errors."
          },
          {
            "priority": "low",
            "confidence": "medium",
            "category": "code-quality",
            "file": "packages/code/src/lib/workspace/workspace-worker.ts",
            "line": "537-555",
            "issue": "The new exported `buildRetryQueueAcquirer` spreads `...deps` and only overrides `team` when the retry row has one (`...(retryTeam ? { team: retryTeam } : {})`). A caller that supplies `deps.team` (allowed by `FleetExecutorDeps`) would make a team-less retry run under that team's routing rules and actioned source, contradicting the comment that persisted repo/team keep retries deterministic. The current caller passes no team, so this cannot happen today.",
            "evidence": "packages/code/src/lib/workspace/workspace-worker.ts:539-544 spreads `deps` then conditionally overrides `team`; the only caller at :937-941 passes `{ config, workspaceDir, skips, repoManager, supervisor }` with no `team`.",
            "suggestion": "Set `team: retryTeam` explicitly (clearing any inherited value) or document that the builder requires a team-free `deps` object."
          },
          {
            "priority": "low",
            "confidence": "medium",
            "category": "code-quality",
            "file": "packages/code/src/lib/workspace/workspace-worker.ts",
            "line": "488-490, 520, 558",
            "issue": "The exhausted predicate now exists in four places: fleet-executor.ts:244-245, injected-acquirer.ts:162-164, retry-acquirer.ts:101-104, and the new `isHarnessChainPaused` helper. Because the two acquirers' default expressions are byte-for-byte the same call, the explicit `isPaused: isHarnessChainPaused` wiring at :520 and :558 can never change behavior — it is untested redundancy that invites drift (e.g. one site later gaining an extra condition).",
            "evidence": "packages/code/src/lib/workspace/injected-acquirer.ts:163 `this.options.isPaused?.() ?? getWorkerFailover()?.allExhausted() ?? false` is identical to packages/code/src/lib/workspace/workspace-worker.ts:489; the same pair is duplicated in retry-acquirer.ts:103 and fleet-executor.ts:245.",
            "suggestion": "Export a single `isHarnessChainExhausted()` from `lib/worker/failover.ts` and use it as the default in all three modules, dropping `isHarnessChainPaused` and the explicit wiring (the optional option stays for tests)."
          },
          {
            "priority": "low",
            "confidence": "medium",
            "category": "bug",
            "file": "packages/code/src/lib/workspace/retry-acquirer.ts",
            "line": "75-85",
            "issue": "The new `isPaused()` call is outside the `try` that protects the drain, and the interval is registered as `setInterval(() => void this.tick())` with no catch (retry-acquirer.ts:60, 82-84). A throwing `isPaused` (it is a caller-supplied callback) therefore becomes an unhandled rejection on every tick, unlike the injected acquirer whose `kick()` catches and logs (injected-acquirer.ts:100-107). The production predicate cannot throw today, so this is defensive only.",
            "evidence": "packages/code/src/lib/workspace/retry-acquirer.ts:60 `setInterval(() => void this.tick(), ...)`; :82-84 `if (this.isPaused()) return;` precedes `this.busy = true`/:86 `try`; contrast injected-acquirer.ts:100-107 `void this.tick().catch(...)`.",
            "suggestion": "Move the pause check inside the existing `try`, or make the interval callback mirror `kick()` (`void this.tick().catch(...)`) so a bad predicate is logged instead of crashing the drain silently."
          },
          {
            "priority": "info",
            "confidence": "medium",
            "category": "documentation",
            "file": "docs/code/configuration.md",
            "line": "573",
            "issue": "No doc change is required (the failover section already says new agent work is deferred while every harness is limited), but the user-visible consequence of this PR is not stated anywhere: queued `worker add` tasks and dashboard retries now stay `pending` with no per-tick log lines until the window ends, which is what an operator watching the log/dashboard will observe.",
            "evidence": "docs/code/configuration.md:573 documents deferral only as \"polling/review/automation runs return to their next tick\"; docs/code/worker.md:333-364 describes ad-hoc task lifecycle without a paused-harness state.",
            "suggestion": "Add one sentence to the failover bullet (and optionally the `worker add` \"Behavior to expect\" list) noting that ad-hoc tasks and scheduled retries stay queued silently until the earliest window ends, and that no work is lost."
          }
        ],
        "approved": false
      },
      "addressed": [
        {
          "priority": "medium",
          "confidence": "high",
          "category": "testing",
          "file": "packages/code/tests/injected-acquirer.test.ts",
          "line": "36-53, 115-148",
          "issue": "Existing acquirer tests now implicitly read the process-wide failover singleton, because `isPaused` defaults to `getWorkerFailover()?.allExhausted()` (injected-acquirer.ts:163, retry-acquirer.ts:103) and none of these tests pass `isPaused` or reset the singleton. Any test file (or in-process code) that leaves an exhausted `WorkerFailover` in the same bun process silently disables these drains and fails the unpaused tests — the opposite of how the tests read.",
          "evidence": "Ran `bun test --preload /tmp/opencode/preload-exhausted.ts injected-acquirer retry-queue-acquirer` (preload starts a one-entry failover and exhausts it via `manager.reportUsageLimit`): output `2 pass 20 fail`, e.g. `retry-queue-acquirer.test.ts:198` expected `[\"PROJ-1\"]` and received `[]`. Without the preload the same two files report `22 pass 0 fail`.",
          "suggestion": "Make these tests hermetic: pass `isPaused: () => false` in the existing constructors (or add a `resetWorkerFailover()` in `beforeEach`/`afterEach` plus a documented setup guard) so a leaked singleton cannot change their outcome."
        },
        {
          "priority": "medium",
          "confidence": "medium",
          "category": "testing",
          "file": "packages/code/src/lib/workspace/workspace-worker.ts",
          "line": "530-561",
          "issue": "The production wiring of this feature is not covered by any test. `buildRetryQueueAcquirer` is only referenced from `workspace-worker.ts` (no test imports it), and both new test cases inject a local `let paused` flag, so the actual default branch (`getWorkerFailover()?.allExhausted()` in injected-acquirer.ts:163 / retry-acquirer.ts:103) and the exported `isHarnessChainPaused` (workspace-worker.ts:488-490) are never exercised. A future change that stops wiring `isPaused` (or wires a stale snapshot) would leave every test green while the pause silently never engages.",
          "evidence": "`grep -rn \"buildRetryQueueAcquirer\" packages/code` returns only `src/lib/workspace/workspace-worker.ts:530,937`; `tests/injected-acquirer.test.ts:118-149` and `tests/retry-queue-acquirer.test.ts:127-160` use `isPaused: () => paused`. I verified the current behavior manually with /tmp/opencode/pause-check.ts and /tmp/opencode/pause-check-retry.ts: paused tick => 0 calls, row pending, then drained after `windowElapsed`.",
          "suggestion": "Add one test per acquirer that starts a real (or stubbed) `WorkerFailover`, exhausts it, and asserts no claim; plus a small `isHarnessChainPaused`/`buildRetryQueueAcquirer` wiring test so the option cannot be dropped unnoticed."
        }
      ]
    },
    {
      "iteration": 2,
      "feedback": {
        "summary": "The change is a read-only pause gate: InjectedTaskAcquirer and RetryQueueAcquirer now return before claiming when the process-wide WorkerFailover.allExhausted() is true, which is the same predicate the fleet executor already consults; because both builders pin an explicit repo, these rows always reach the executor's return \"deferred\" guard at fleet-executor.ts:301 rather than an earlier routing skip, so a paused tick only removes the old claim-defer-requeue churn. Only runWorkspaceWorker builds the acquirers (workspace-worker.ts:937,948), and both builders pass the same predicate, so there is no other production caller to update. I verified the behavior with the three targeted test files (64 pass) and the full packages/code suite (2530 pass, 0 fail); the remaining findings are test-hygiene, stale-comment, duplication, and timer-path coverage issues, not correctness bugs.",
        "items": [
          {
            "priority": "low",
            "confidence": "medium",
            "category": "testing",
            "file": "packages/code/tests/worker-add-command.test.ts",
            "line": "750-757",
            "issue": "This is a sibling consumer of the changed InjectedTaskAcquirer contract: it constructs the acquirer without isPaused, so the new default reads the process-wide failover singleton. The file has no resetWorkerFailover() hook, unlike the three test files this PR updated. A failover controller leaked or preloaded as exhausted would make acquirer.tick() a silent no-op and fail expect(calls).toEqual([enqueued.taskKey]). It is currently safe only because the other files that call startWorkerFailover reset it in afterEach.",
            "evidence": "packages/code/tests/worker-add-command.test.ts:750-757; `grep -n resetWorkerFailover packages/code/tests/worker-add-command.test.ts` returns no matches. `bun run test worker-add-command.test.ts` -> 40 pass, 0 fail; full `bun run test` -> 2530 pass, 0 fail.",
            "suggestion": "Add resetWorkerFailover() to this file's beforeEach/afterEach, or pass isPaused: () => false in this direct unit test, so it cannot be affected by process-wide failover state."
          },
          {
            "priority": "low",
            "confidence": "medium",
            "category": "documentation",
            "file": "packages/code/src/lib/workspace/injected-acquirer.ts",
            "line": "109-114",
            "issue": "The tick() doc still says a deferred row \"(busy repo or exhausted harness)\" is requeued. The new early return at 124-129 means an exhausted harness never claims or requeues a row; only a busy repo (and the executor's race guard at runOne 210-215) reaches requeue. The comment now describes the old behavior and could mislead a future change.",
            "evidence": "injected-acquirer.ts:111-112 versus the new early return at injected-acquirer.ts:124-129; the runOne deferral at 210-215 is now reachable only for executor-level deferrals.",
            "suggestion": "Reword the tick doc to say the drain returns before claiming while the chain is exhausted, and that only busy-repo/executor-race deferrals are requeued."
          },
          {
            "priority": "low",
            "confidence": "medium",
            "category": "code-quality",
            "file": "packages/code/src/lib/workspace/workspace-worker.ts",
            "line": "488-490",
            "issue": "The pause policy getWorkerFailover()?.allExhausted() ?? false is duplicated in isHarnessChainPaused (workspace-worker.ts:488-490), InjectedTaskAcquirer.isPaused (injected-acquirer.ts:163), and RetryQueueAcquirer.isPaused (retry-acquirer.ts:103). The fleet executor's guard is separately configurable via FleetExecutorDeps.isHarnessExhausted (fleet-executor.ts:244-245), so a caller that customizes the executor predicate without also passing isPaused gets two different pause decisions. Production currently leaves both at the global default, so no behavior change today, but this is a drift risk.",
            "evidence": "workspace-worker.ts:488-490, injected-acquirer.ts:163, retry-acquirer.ts:103, fleet-executor.ts:244-245; typecheck and full suite pass.",
            "suggestion": "Export one helper (ideally from worker/failover.ts) and use it as the default in both acquirers, or wire the builder's isPaused to the same deps.isHarnessExhausted predicate used by the executor."
          },
          {
            "priority": "info",
            "confidence": "medium",
            "category": "testing",
            "file": "packages/code/tests/workspace-worker.test.ts",
            "line": "925-936",
            "issue": "The new wiring tests exhaust the chain with failover.manager.reportUsageLimit(...) and clear it with windowElapsed(...), bypassing WorkerFailover.reportFromHint (which arms the resume timer at failover.ts:217-225) and the onPause/onResume callbacks. They prove the live predicate and the acquirer wiring, but not that the timer-driven resume actually unpauses a running drain. The per-tick re-evaluation makes this low risk, but the integration path is untested.",
            "evidence": "workspace-worker.test.ts:928-936 uses failover.manager.reportUsageLimit; the production path is failover.ts:217-225. Targeted tests: 64 pass; full suite: 2530 pass, 0 fail.",
            "suggestion": "Add a test that drives failover.reportFromHint(...) (or uses fake timers around the armed window) and asserts the same acquirer drains after the window elapses."
          }
        ],
        "approved": true
      },
      "addressed": []
    }
  ]
}