Skip to content

fix: a turn that strands a background task must not report success - #70

Merged
Lexus2016 merged 11 commits into
mainfrom
fix/background-task-continuation
Aug 25, 2026
Merged

Lexus2016 merged 11 commits into
mainfrom
fix/background-task-continuation

Conversation

@Lexus2016

Copy link
Copy Markdown
Owner

What this fixes

Two defects in the single-agent run loops, reported against v7.9.0.

1. A turn that strands a background task reported success

An agent that started something with run_in_background and then ended its turn — "I'll wait for it and continue" — reports a clean end_turn, i.e. subtype:'success'. That is the one rung the auto-continue ladder does not cover: the ladder fires only on NON-success. Nothing else resumes a headless claude -p — the process exits and takes the background shell with it — so the chat printed ✅ Done over work that had not happened, and the continuation the agent promised could never arrive.

run-continuation.js adds two pure decisions, and the loops spend at most one extra run collecting the task.

Detection is structural, never textual. A Bash call carrying run_in_background is the same JSON on every install; user-facing prose is written in the UI language (buildSystemPrompt pins that explicitly), so a regex over English phrases like "I'll check back" is dead on a French or Ukrainian one.

The debt is incremental and turn-level. Four simpler shapes were tried first and every one of them shipped green while still being broken — they are listed in the module header and each has a test named after it:

shape what it did wrong
one-way latch charged an extra run to every agent that obeyed the new instruction
per-run counters the rescue run started from zero, so a text-only second walk-away claimed success — the same bug, one run later
raw call counts two polls of one job cancelled a second, genuinely abandoned launch
batch total banked a surplus harvest, so reading a leftover tasks/<id>.output paid for a launch that came afterwards

applyBackgroundTool(state, name, input) folds each tool call into {debt, seen}: a harvest decrements only an existing debt, once per shell id, and a surplus is dropped.

The harvest side cannot key on BashOutput. Measured on CLI 2.1.231, a background launch answers Output is being written to: <hash>/<uuid>/tasks/<id>.output and the agent collects by reading that file — BashOutput is never called.

A bounded nudge needs an honest ending. When the rescue run also walks away, the turn says what is still running, as a real --- status line (statusLineKind() needs the fence, or the SPA paints its own Done badge over the warning).

2. error_max_turns far below the requested budget no longer says "raise Max turns"

Measured against CLI 2.1.231, a run capped at N reports num_turns === N + 1, so a genuine exhaustion lands at the cap. A stop at 3 turns against a 50-turn dial is a cap imposed somewhere else — a different CLI version, a settings.json, a hook on the machine the agent runs on — and sending that user to the dial is actively wrong advice. Applied to the first retry notice as well as the exhausted one: hit the 50-turn limit (used 3) contradicts itself.

The SSH auto-continue path also now logs (it logged nothing at all, on either branch) and names the budget and what was spent, which the local CLI loop has always done. Relates to #67.

Delivery channels

--system-prompt is dropped whenever there is a session to resume (claude-cli.js:252), and is absent entirely for a task with no bot. So each runner takes the channel that actually reaches it:

  • chat, Telegram — buildSystemPrompt
  • Kanban/scheduled (every engine) — the task prompt, next to TASK_VERIFICATION_SUFFIX
  • multi-agent workers — agentPrompt (they resume the orchestrator's session)
  • bots — botSp on a fresh session, the standing user block once resuming

The harvest rescue stays in runCliSingle/runSshSingle; duplicating the debt accounting into a third and fourth loop is how the local and remote paths drift apart. The unattended runners get the instruction only — which is the half that matters most there, since nobody is reading that chat.

Known limits, both documented in the module header

  • A harvest is not paired to a launch: the shell id exists only in the tool result, while onTool sees tool_use blocks. The mirror case (a leftover read before the launch) is closed by the no-banking rule.
  • Shell-syntax backgrounding (cmd &, nohup) inside a foreground Bash call is not detected — separating it from a && b and 2>&1 needs a shell parser. The system-prompt rule covers it in words.

Verification

  • npm test → 70 files, exit 0. New test/run-continuation.test.js (69 checks); every wrong shape above has a test named after it.
  • Six source-text pins on which expression carries BACKGROUND_TASK_INSTRUCTION, plus a negative pin that taskBotSp must not. Three commits in this branch were green no-ops before those pins existed.
  • Verified live against real CLI 2.1.231, all three shapes in one script: obedient agent → debt=0, no nudge; walk away then text-only rescue → debt=1, stranded notice instead of "Done"; launch two collect one → debt=1, nudge.
  • Reviewed across 8 rounds by three non-Claude models (codex, agy, grok). Four critical and six warning-level defects were found and fixed during review; the residual findings are the two documented limits above.

subtype:'success' is the one rung the auto-continue ladder does not cover.
A run that started something with run_in_background and then ended its turn
saying it would wait and continue reports a clean end_turn, so the ladder —
which fires only on NON-success — never saw it. Nothing resumes a headless
`claude -p`: the process exits and takes the background shell with it. The
chat printed "Done" over work that had not happened, and the continuation
the agent promised could never arrive.

run-continuation.js holds both post-run decisions, pure so they can be
pinned without a server or an SSH host:

- shouldNudgeBackgroundWait() spends ONE run harvesting the task, and only
  for a launch in the last run. Detection is structural (a Bash call
  carrying run_in_background), never textual — user-facing prose is written
  in the UI language, so an English regex is dead on a French install.
- describeTurnBudgetAnomaly() names an error_max_turns that did not come
  from our budget. Measured on CLI 2.1.231 a run capped at N reports
  num_turns === N + 1, so a stop at 3 of 50 is a cap imposed elsewhere and
  "raise Max turns" is actively wrong advice (#67 follow-up).

BACKGROUND_TASK_INSTRUCTION states the constraint in the system prompt, so
the harvest run is the second line of defence rather than the first.

Also: the SSH auto-continue path now logs (it logged nothing at all, on
either branch) and names the budget and what was spent, which is what the
local CLI loop has always done.
Two independent reviewers (codex, agy) and a live A/B run all landed on the
same defect in ebf439f: `bgLaunched` was a one-way latch, so every agent that
OBEYED BACKGROUND_TASK_INSTRUCTION — launch, collect, finish — was charged a
rescue run for its obedience. Worse than the bug it fixed.

- Both loops now count bgLaunches/bgHarvests per run and nudge only when
  launches > harvests. Verified live: an obedient agent reports
  launches=1 harvests=1 and is not nudged; one that walks away reports
  launches=1 harvests=0 and is.
- isBackgroundHarvest() cannot key on BashOutput alone. Measured on CLI
  2.1.231, a background launch answers "Output is being written to:
  <hash>/<uuid>/tasks/<id>.output" and the agent collects by READING that
  file; BashOutput is never called.
- isBackgroundLaunch() reads the flag off a PARSED object. As a substring
  match, `grep -rn '"run_in_background": true' *.js` — a real command in this
  repo — registered as a launch. The regex survives only for input that does
  not parse at all.
- A bounded nudge needs an honest ending: when the rescue run also walks away,
  describeStrandedBackgroundTask() says what was left running and the loop
  returns completed:false, so taskWorker does not mark a Kanban task done over
  unfinished work.
- describeTurnBudgetAnomaly() is applied to the FIRST retry notice too. "hit
  the 50-turn limit (used 3)" contradicts itself, and a user who stops reading
  after the first retry never reached the corrected sentence.

Known limit, documented: shell-syntax backgrounding (`cmd &`, `nohup`) inside a
foreground Bash call is not detected — separating it from `a && b` and `2>&1`
needs a shell parser. The system-prompt rule covers it in words.
Round 2 of independent review (codex, grok) found the bound from 87c8da8 was
dead in the loops, and a live two-run test reproduced it: the counters lived
inside runOnce, so the rescue run started from zero. Its most likely shape is a
text-only answer that touches no tool at all -> launches=0, harvests=0 ->
describeStrandedBackgroundTask() stayed silent and the turn reported success.
That is the original "Done over unfinished work", one run later.

- The debt now lives in the loop body: bgLaunches, a Set of harvested shell ids,
  and an anonymous-harvest counter, read through bgOutstanding(). Verified live:
  run 1 launches and walks away -> outstanding=1 -> nudge; run 2 answers in text
  and touches nothing -> outstanding still 1 -> stranded notice instead of Done.
- Harvests dedupe by shell id (from BashOutput/KillShell, or out of the
  tasks/<id>.output path). Two polls of one job used to cancel a second launch
  that really was abandoned. backgroundOutstanding() clamps at zero so a poll of
  something started in an earlier turn cannot mask a later launch.
- `View` is accepted alongside `Read`: it is the name in this project's own
  allowedTools lists, so an agent using it looked like it collected nothing.
- The stranded notice is a real `---` status line. statusLineKind()
  (public/index.html:7215) needs that fence, or the SPA paints its own Done badge
  over the warning.
- completed:false is REVERTED. It has no consumer that does the right thing: the
  chat path discards it, the API Kanban worker never calls these functions, and
  the one reader (server.js:1859, the subscription branch) turns it into
  {subtype:'error'} -> taskStatusForStop -> 'failed', which auto-retries a chain
  and re-runs every side effect. CLAUDE.md claimed otherwise; corrected.
Round 3 of independent review (codex, grok) found the batch total
max(0, launches - harvests) banked a harvest that had no launch behind it.
Reading a leftover tasks/<id>.output from an earlier turn — often the first
thing a resumed session does — then launching a shell and walking away came
out at zero owed: no rescue, no warning, "Done" over a running job. The
sibling case is launch A and B, collect only B.

The debt is now incremental and the whole rule lives in the module:
applyBackgroundTool(state, name, input) folds one tool call into {debt, seen},
a harvest decrements only an EXISTING debt and only once per shell id, and a
surplus is dropped. Both loops call that one function instead of open-coding
the accounting, which is how the local and remote paths drift apart.

An unidentifiable harvest (BashOutput with no readable bash_id) now returns
null and does not pay the debt. bash_id is required, so it only happens on a
malformed payload; crediting it re-opened the raw-call-count hole. One extra
rescue run is the cheaper mistake.

Verified live against the real CLI, all three shapes in one script:
  obedient (launch + collect)      -> debt=0, no nudge
  walk away, then text-only rescue -> debt=1, stranded notice, not "Done"
  launch two, collect one          -> debt=1, nudge

Also restores the KNOWN LIMIT note (shell-syntax backgrounding: `cmd &`,
nohup) to the module header, where it was lost in the previous rewrite.
Round 4 (codex FAIL / grok PASS) converged on one residual hole and one stale
sentence. Neither is a regression; both are now written down where the next
reader will hit them.

- A harvest is not PAIRED to a launch: the shell id lives only in the tool
  RESULT, and onTool sees tool_use blocks. So a Read of some OTHER
  tasks/<id>.output after a launch pays that launch's debt. The mirror case, a
  leftover read BEFORE the launch, is closed by the no-banking rule. Closing
  this one means threading tool_result through both transports and changing the
  stream contract, for a case that needs the agent to read an unrelated shell
  log in the same turn it abandons a real one.
- CLAUDE.md still named bgLaunches/bgHarvestIds/bgAnonHarvests in the
  ask-user-question closure list; the test passes bgState.
buildSystemPrompt() is called from exactly two places — the chat path and
processTelegramChat. taskWorker (Kanban/scheduled), the multi-agent DAG members
and the bots/dispatch path each build their own system prompt AND run their own
cli.send loop, so they got neither the instruction nor the harvest rescue.

An unattended task that walks away from a background job is the worst version of
this bug: nobody is reading that chat to notice the "Done". Those three now get
BACKGROUND_TASK_INSTRUCTION — one concatenation each, no behavioural risk.

The rescue itself stays in runCliSingle/runSshSingle: extending it would mean
duplicating the debt accounting into a third and fourth loop, which is exactly
how the local and remote paths would drift apart. Documented as uneven coverage
rather than left implicit.
Round 5 (codex) showed the previous commit was a no-op for two of the three
paths it claimed to cover. claude-cli.js:252 drops --system-prompt whenever
there is a session to resume, and:

- a multi-agent worker starts from the orchestrator's session id
  (agentSessionId = currentSessionId), so agentSp never reaches it;
- a bot keeps a persistent session per chat, so botSp reaches it on the first
  turn only. That path already had the fix for this shape — the `standing`
  block re-sent in the USER turn — and the instruction now goes there.

The multi-agent instruction moves from agentSp to agentPrompt for the same
reason. taskWorker's taskBotSp keeps it as a system prompt: that path does send
one on a fresh run.

Documented as still NOT covered: a Kanban task on the subscription engine
passes systemPrompt:'' to runInteractiveSingle, and changing that respawns the
tmux session — out of scope for this fix rather than silently claimed.

Not a defect, checked: BOT_PROMPT_MAX (8192) validates the user-supplied field
at the API boundary, not the assembled prompt, which has always been larger.
Round 6 (codex) found taskWorker coverage was true only for a bot-owned task on
a fresh session. taskBotSp is undefined for a task with no bot_id, and
claude-cli.js:252 drops --system-prompt when the task resumes an existing
session — so an ordinary scheduled task got nothing at all.

BACKGROUND_TASK_INSTRUCTION moves to the task prompt, next to
TASK_VERIFICATION_SUFFIX, which lives in that channel for exactly this reason.
Removed from taskBotSp, where it would now duplicate on the one path that has
both.

Net effect of rounds 5-6: no path gets this as a system prompt except chat and
Telegram, the two that go through buildSystemPrompt. Everything else takes the
user turn, which --resume always delivers.
Round 7 (grok) found the code holds but three doc claims did not, and — the
part that matters — that nothing pinned WHICH expression the constant is
concatenated onto. Rounds 5-7 each shipped a no-op green: appended to a system
prompt that --resume drops, to one that is undefined for a bot-less task, and
to one a bot sees only on its first turn.

test/run-continuation.test.js now reads server.js as source text (the technique
test/render/script-scope.test.mjs already uses, because the property is textual)
and pins all five channels: buildSystemPrompt, the task prompt, the multi-agent
worker prompt, the bots standing block, and the NEGATIVE case — taskBotSp must
NOT carry it, since its presence there reads as coverage it cannot deliver.

Doc corrections:
- "chat and Telegram are the only paths that get it as a system prompt" was
  false: a bot on a FRESH session gets it via botSp too.
- "the subscription Kanban engine is still not covered" became false with the
  round-6 move. It passes systemPrompt:'' but runInteractiveSingle types the
  prompt into the tmux pane (claude-interactive.js:441), so the user-turn
  channel reaches it like every other.
- server.js:1823 -> 1829 after the comment added in round 6.
Round 8 verdict was PASS, with one leftover of the same family it had just
confirmed closed: botSp is documented as a delivery channel and was the only
one not pinned. Deleting the constant from it leaves a brand-new bot with an
empty `standing` block and no copy anywhere, and the suite still went green —
which is exactly the failure mode rounds 5-7 kept producing.

Also corrects a stale citation: the `completed` reader is server.js:1865, not
1859 (that line is now the fullText branch above it).
Behaviour-preserving. runCliSingle/runSshSingle each carried thirteen copies of
the same three lines — append to fullText, append to chatBuffers with a
MAX_CHAT_BUFFER tail-trim, ws.send a {type:'text'} frame with a conditional
tabId spread. Every copy had to get the trim and the spread right; one that
missed either grew the buffer without bound or delivered into the wrong tab.

emitTurnNotice() now owns the buffer write and the frame. The caller still does
its own `fullText += notice` first, so the ORDER of effects is unchanged —
folding that in moved the append after the send, which two reviewers flagged as
a real (if currently unobservable) difference.

restartAvailable is a named boolean, not a free-form `extra` object. An extra
spread sat between `text` and `tabId` and would have let a future caller
overwrite `type`/`text` or leak a tabId the frame is not meant to carry; the
frame is now built key by key, so its order is fixed by construction.

Deliberately NOT converted, and said so in the header:
- the onText handlers, which stream model output rather than a status notice and
  also write stmts.setPartialText every fifth chunk;
- runners outside these two loops (taskWorker, multi-agent members, bots), which
  buffer into taskBuffers, tag frames with `agent:`, or persist only.

Also: _anom and _anomaly were two names for one value in one function; the
backgroundOutstanding(bgState) call is hoisted into _owed where it was read
twice on adjacent lines.

  notice boilerplate blocks   28 -> 2   (the two that remain are onText)
  MAX_CHAT_BUFFER repeats     56 -> 4
  lines across both loops    698 -> 674

Reviewed in two rounds by codex and grok (non-Claude). Round 1 found the leaky
`extra`, a half-done conversion leaving two styles in one function, the moved
fullText append, and a comment that claimed more than was done. All four fixed;
round 2 returned PASS from both.
@Lexus2016
Lexus2016 merged commit 00e0268 into main Aug 25, 2026
2 checks passed
@Lexus2016
Lexus2016 deleted the fix/background-task-continuation branch October 7, 2026 08:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant