Skip to content

[oss-candidate] fix(ws): forward background task lifecycle through the pre-turn mute gate - #1

Closed
askalf wants to merge 8 commits into
mainfrom
fix/forward-task-lifecycle-through-pre-turn-mute
Closed

askalf wants to merge 8 commits into
mainfrom
fix/forward-task-lifecycle-through-pre-turn-mute

Conversation

@askalf

@askalf askalf commented Sep 20, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • A background task that ends while a new user turn is being admitted never gets its terminal task_notification to the renderer. The task card stays "running", and the terminal is not persisted to session history either.
  • In bindClientSessionOutput, the pre-turn mute gate (options.shouldForward) drops every CLI message until userMessageSent flips. That includes background task lifecycle events, which have nothing to do with the new turn.
  • The sibling gate just above it, shouldSuppressCliOutputDuringStop, already exempts task lifecycle (if (taskLifecycle !== null) return false). The mute gate has no such exemption. That asymmetry is the bug.
  • Fix: one conjunct, taskLifecycle === null &&, on the mute gate in src/server/ws/handler.ts (+5/-1, no comment). No new dependencies, no version bumps.
  • Test: one new test in src/server/__tests__/websocket-handler.test.ts (+64/-0, 8 assertions), shaped like its neighbour for the same admission window. It fails on base with Received: [] and passes at head. The touched file is 100 pass / 0 fail at head c0053ad8.
$ bun test src/server/__tests__/websocket-handler.test.ts -t 'forwards background task lifecycle while a foreground admission awaits send acknowledgement'
# ---- BASE b8c7a115 handler.ts (fix present: 0) ----
bun test v1.3.14 (0d9b296a)

src/server/__tests__/websocket-handler.test.ts:
877 |     handleWebSocket.message(ws, JSON.stringify({ type: 'user_message', content: 'Ask while commands run' }))
878 |     await flushMicrotasks(30)
879 | 
880 |     for (const [taskId, status, taskType] of [['shell', 'completed', 'bash'], ['agent', 'failed', 'local_agent']]) {
881 |       for (const sent of await emit(task('task_notification', taskId, { status, task_type: taskType }))) {
882 |         expect(sent).toContainEqual({
                           ^
error: expect(received).toContainEqual(expected)

Expected to contain: {
  type: "system_notification",
  subtype: "task_notification",
  data: ObjectContaining {
    task_id: "shell",
    status: "completed",
  },
}
Received: []

(fail) WebSocket handler session isolation > forwards background task lifecycle while a foreground admission awaits send acknowledgement [15.54ms]

 0 pass
 99 filtered out
 1 fail
 1 expect() calls
Ran 1 test across 1 file. [2.69s]
# ---- HEAD c0053ad8 (fix present: 1) ----
(pass) WebSocket handler session isolation > forwards background task lifecycle while a foreground admission awaits send acknowledgement [14.85ms]
 1 pass
 99 filtered out
 0 fail
 8 expect() calls
Ran 1 test across 1 file. [2.67s]

Received: [] is the whole bug. The completion is not mis-shaped; it is dropped before it can be translated or sent.

Upstream

  • Repository: NanmiCoder/cc-haha
  • Default branch: main
  • Base sha: b8c7a11507c8da63f5c6745f7c27db99d6a313c0 (fix: complete AruHub model mappings and limit sponsor star)
  • Current head: c0053ad856ffd065b42a339d29756b5d85d76f44. It is test-only on top of 707f3119. src/server/ws/handler.ts differs from base only by the conjunct (sha1 d788d39b1736 at both 707f3119 and c0053ad8).
  • File changed: src/server/ws/handler.ts, bindClientSessionOutput (the options.shouldForward gate, base line 4273; origin/main 4615f9d4 line 4455)
  • Test file: src/server/__tests__/websocket-handler.test.ts
  • Related report: issue [BUG] 后台任务吞回调问题 NanmiCoder/cc-haha#1352 [BUG] 后台任务吞回调问题 is closed (COMPLETED, 2026-09-22T01:48:41Z). The maintainer's closing comment says it was fixed in v0.6.6: "任务结束时(即使模型还在继续输出)完成通知会立即推送到会话,不再被吞掉". That fix is 0f2c2b2c fix: stream background task completion while the model is working (Refs #1352, #1345). It touches only src/cli/print.ts, so the CLI now publishes a terminal promptly. This PR covers the next hop: a terminal that reaches the server while a user message is being admitted is still dropped by the WS mute gate on main 4615f9d4 (executed, see ## Verification method). The upstream description should start from that fact and should not cite [BUG] 后台任务吞回调问题 NanmiCoder/cc-haha#1352 as an open report. Issue 后台任务有时候状态不更新及时 NanmiCoder/cc-haha#1132 后台任务有时候状态不更新及时 is open and may be the user-visible form of this bug, but its body is the empty template, so it cannot be cited as a repro.

Bug

Trigger. A background task (bash/local_agent/local_workflow/remote_agent) is already running in a session, and the user sends a message from any renderer. sendUserMessage rebinds every client output with a pre-turn mute filter (handler.ts:930). It then awaits conversationService.sendMessage(...) (:952) and only sets userMessageSent = true at :993, after that await resolves. Every CLI message seen inside that admission window goes through options.shouldForward. Before userMessageSent, that filter returns true only for error results, agent_run_message frames and the current turn's own slash-command output.

Wrong outcome. If the background task ends inside that window, its terminal task_notification is discarded at handler.ts:4273. trackCliBackgroundTaskLifecycle has already run (:4250), so the server's own bookkeeping untracks the task. Two things are lost: live delivery to the renderer, and the persistThenForwardCliMessage write that records the terminal in session history. The renderer never learns the task ended, and the card stays "running".

Blast radius. Every renderer of this server: the Electron desktop app, the web UI and the H5/IM access paths all consume system_notification/task_notification over the same WebSocket. The window lasts as long as the CLI takes to admit a user message. Users send messages while background work runs by design. The loss is silent: nothing is logged and server-side state is consistent. The only symptom is a card that never closes. 0f2c2b2c makes terminals arrive while the model is still working, which is when a user is most likely to be typing the next message. That makes this collision more likely, not less.

Why the mute gate exists. Its comment at :921 says it keeps pre-turn SDK chatter out of the new turn's history. Task lifecycle is not chatter. A task that started before this turn still owes the renderer a terminal event.

Repro

Deterministic, using the repo's own WebSocket handler harness (no network, no model, no real CLI).

$ bun test src/server/__tests__/websocket-handler.test.ts -t 'forwards background task lifecycle while a foreground admission awaits send acknowledgement'

The test binds two client sockets to one session and starts two tasks, a plain bash task and a local_agent. It then sends a user message and holds the admission window open with a sendMessage promise that is not resolved until the end, the same technique as the neighbouring upstream test forwards directed Agent output while a foreground admission awaits send acknowledgement. With the window open it:

  1. emits a completed terminal for the plain task and a failed terminal for the Agent task, and asserts each reaches both clients (4 assertions);
  2. asserts appendSessionTaskNotification ran exactly 2 times, once per terminal and not once per client;
  3. starts a task inside the window and asserts task_started reaches both clients (2 assertions);
  4. emits task_progress and asserts nothing reaches either client.

The ## Summary console block has the verbatim output for both arms. On base the first assertion fails with Received: [].

Fix

--- a/src/server/ws/handler.ts
+++ b/src/server/ws/handler.ts
@@ -4270,7 +4270,11 @@ function bindClientSessionOutput(
       // Agents, and permission resolutions must pass so open prompts can close.
       return
     }
-    if (options?.shouldForward && !options.shouldForward(cliMsg)) {
+    if (
+      taskLifecycle === null &&
+      options?.shouldForward &&
+      !options.shouldForward(cliMsg)
+    ) {
       return
     }

Why this is minimal and correct. taskLifecycle is already computed above the gate (:4250). It is also the exemption key that shouldSuppressCliOutputDuringStop (:4329) uses. Reusing it makes the two adjacent gates agree without adding a new concept. The check is === null rather than a truthiness test because trackCliBackgroundTaskLifecycle returns either null (not a lifecycle message) or an object, so there is no falsy-but-valid lifecycle value.

Scope. Behaviour changes only for messages where getCliBackgroundTaskLifecycle returns non-null. Those are task_started, and task_notification with a non-empty trimmed task_id and a status of running/completed/failed/stopped/killed. task_progress, unknown statuses and empty ids still parse to null and stay muted. The same holds for everything else the gate was written to suppress.

Alternatives rejected (each built as a mutant; see ## Test evidence):

  • Flip userMessageSent = true earlier. This would unmute genuine pre-turn chatter, which is what the gate exists to stop.
  • Special-case subtype === 'task_notification' in the gate (mutant C). This duplicates classification that getCliBackgroundTaskLifecycle already owns and drifts from it. It drops task_started, and it lets paused and empty/whitespace ids through.
  • Exempt any system message carrying a task_id (mutant D) or the whole subtype family including task_progress (mutant E). Both are too wide: they let progress chatter through.
  • Exempt only terminals (mutant F). This is too narrow: it leaves heartbeats and starts inside the window muted.
  • Exempt only plain (non-Agent, unowned) task lifecycle (mutant G). This leaves Agent and teammate-owned terminals muted.
  • Exempt lifecycle inside createCurrentTurnLocalCommandForwarder. That forwarder is about slash-command echo and is also used for title binding. Handling tasks there would give one concept two owners.

Test evidence

One new test, forwards background task lifecycle while a foreground admission awaits send acknowledgement, sits in the existing WebSocket handler session isolation describe directly after its 57-line neighbour for the same gate. It uses the file's own makeClientSocket / flushMicrotasks and the same spy setup as that neighbour, plus two local closures (task payload builder, emit). Each payload carries a uuid, as the CLI stamps on every task event (src/cli/print.ts, uuid: randomUUID()), so persistence dedups on the event id and not on a per-callback timestamp.

Rework at c0053ad8 (Second Read at 707f3119: "test disproportionate to the fix"): the test went from +105 lines / 23 assertions to +64 lines / 8 assertions. It keeps only rows whose outcome the changed conjunct decides. The rows it dropped are the parser boundaries (paused, '', ' '), the authoritatively stopped Agent (returns at :4254, before this gate), and the Stop-fence phase (shouldSuppressCliOutputDuringStop already exempts lifecycle at :4329). None of them is decided by this diff. They are recorded in ## Boundaries as probe-measured at this head. The markTaskAuthoritativelyStopped import and the sendInterrupt spy went with them.

Per-assertion arms. A failure at assertion 1 would hide the later ones, so every assertion was also run in an untracked copy where expect records instead of throwing (/agent-output/oss/cc-haha/r8-make-probe.js; the copy was deleted afterwards, and git status --short showed only the committed file).

# What it pins Base b8c7a115 Head c0053ad8 Role
1-2 plain bash task's completed terminal reaches both clients FAIL (Received: []) ×2 pass discriminating (rows 5, 18, 22)
3-4 local_agent task's failed terminal reaches both clients FAIL ×2 pass discriminating (rows 6, 18, 22)
5 appendSessionTaskNotification called 2 times FAIL (0 calls) pass discriminating (row 21)
6-7 task_started inside the window reaches both clients FAIL ×2 pass discriminating (rows 8, 19)
8 task_progress inside the window stays muted pass pass control: the fix must not widen the exemption to non-lifecycle task chatter (killed by mutants B, D, E)
$ sh /agent-output/oss/cc-haha/r8-arms.sh <worktree>
=== HEAD (fix present: 1)
#1 pass #2 pass #3 pass #4 pass #5 pass #6 pass #7 pass #8 pass
=== BASE (fix present: 0)
#1 FAIL toContainEqual: Received: []
#2 FAIL toContainEqual: Received: []
#3 FAIL toContainEqual: Received: []
#4 FAIL toContainEqual: Received: []
#5 FAIL toHaveBeenCalledTimes: Expected number of calls: 2
#6 FAIL toContainEqual: Received: []
#7 FAIL toContainEqual: Received: []
#8 pass

Whole touched file (the base arm uses git checkout b8c7a115 -- src/server/ws/handler.ts and is restored with git checkout HEAD --):

$ bun test src/server/__tests__/websocket-handler.test.ts        # HEAD c0053ad8
 100 pass
 0 fail
 382 expect() calls
Ran 100 tests across 1 file. [2.93s]
$ bun test src/server/__tests__/websocket-handler.test.ts        # BASE handler.ts
(fail) WebSocket handler session isolation > forwards background task lifecycle while a foreground admission awaits send acknowledgement [2.21ms]
 99 pass
 1 fail
Ran 100 tests across 1 file. [3.03s]

The filtered test was also run 8 more times at head: 8/8 1 pass 0 fail.

Mutants at c0053ad8. Each was applied with git apply to src/server/ws/handler.ts, then the whole file and the per-assertion copy were run, then git checkout HEAD -- src/server/ws/handler.ts. Patches: /agent-output/oss/cc-haha/r4-mut-{A..F}.patch and v7-mut-G.patch. Transcripts: r8-arms.txt, r8-wholefile-mutants.txt.

=== mutant A   (neuter the authoritative-stop early return: `if (false && taskLifecycle?.suppressForward) return`)
 3 fail  97 pass   automatically retries a confirmed local stop until remote archive succeeds / keeps an archived remote Agent stopped when its local control response is lost / stops every active Agent task when generation is stopped
=== mutant B   (remove the gate: `false &&`)
#8 FAIL toEqual: + Received  + 42                         1 fail  99 pass
=== mutant C   (`cliMsg?.subtype !== 'task_notification' &&`)
#6 FAIL #7 FAIL toContainEqual: Received: []            1 fail  99 pass
=== mutant D   (any system message with a truthy task_id)
#8 FAIL toEqual: + Received  + 42                         1 fail  99 pass
=== mutant E   (subtype family incl. task_progress)
#8 FAIL toEqual: + Received  + 42                         1 fail  99 pass
=== mutant F   (terminals only: `!(taskLifecycle && taskLifecycle.running === false) &&`)
#6 FAIL #7 FAIL toContainEqual: Received: []            1 fail  99 pass
=== mutant G   (keeps muting Agent-typed or owned lifecycle: `(taskLifecycle === null || isAgentTaskType(cliMsg.task_type) || cliMsg.owner_agent_id) &&`)
#3 FAIL #4 FAIL toContainEqual: Received: []  #5 FAIL toHaveBeenCalledTimes   1 fail  99 pass

Every mutant is killed, each by a small named set. C and F kill the in-window start, B (gate removed), D and E kill the task_progress control, and G kills the Agent task's terminal and the write count. A is outside this diff; it is killed by three existing stop-latch tests that guard the same early return. Mutant G is why the fixture sends the terminals with explicit task_type: in the reviewer's sketch both terminals inherited bash from the builder, and G survived all 8 assertions until the Agent task's terminal carried local_agent.

Linters / typecheck. The repo has no lint, format or typecheck script for the server surface and no typescript dependency. check:desktop runs eslint only on desktop/. server-checks in pr-quality.yml is one bun run check:server step with no lint or tsc step chained. Style matches the surrounding file (TypeScript ESM, 2-space indent, no semicolons), and the added lines carry no comments.

live model: not run (untrusted fork / no provider).

Verification method

executed in the runner container: Alpine Linux x86_64 (musl), bun 1.3.14 (the version pinned by packageManager), node v24.19.0. The head worktree shares node_modules with a base worktree. Nothing is compiled: bun executes the TypeScript sources directly.

Current main arm. The head test file was copied into an origin/main 4615f9d4 worktree as an untracked probe and deleted afterwards:

$ bun test src/server/__tests__/r8-main.probe.test.ts -t 'forwards background task lifecycle while a foreground admission'
Received: []
(fail) WebSocket handler session isolation > forwards background task lifecycle while a foreground admission awaits send acknowledgement [14.75ms]
 0 pass
 99 filtered out
 1 fail
 1 expect() calls
Ran 1 test across 1 file. [3.37s]

origin/main:src/server/ws/handler.ts:4455 still reads if (options?.shouldForward && !options.shouldForward(cliMsg)) {, and git log b8c7a115..origin/main -S'taskLifecycle === null' -- src/server/ws/handler.ts is empty.

Fork CI at c0053ad8. PR Triage run 35966028733: success. PR Quality run 35966027909 completed failure (verified at this head): server-checks logged [server-tests] src/server/__tests__/websocket-handler.test.ts: passed and summary: files=447 passed-tests=5454 failed-tests=13 failed-files=7. Six are the same files red at 707f3119 and 5dacfa0f: connectorService, workspaceWatch, claudeBetas.integration, connectors/cliAdapter, connectors/managedRuntime, permissions/PermissionUpdate (none touched here; PermissionUpdate.test.ts fails with permissionSetupModule.transitionPermissionMode is not a function, unrelated to ws/handler.ts). The seventh, src/server/__tests__/conversations.test.ts (should return initial context for a prewarmed empty session on the first inspection request, a 10s CI timeout, TypeError: undefined is not an object (evaluating 'body.context.model')), is new at this head and not present at 707f3119. It reproduces nowhere else: run alone locally it passes in 307ms, and the file has no import path to ws/handler.ts or agentTaskState.ts (grep -n "handler" src/server/__tests__/conversations.test.ts is empty). It is a CI-load timing flake, the same class as the already-disclosed claudeRequiredThinking.test.ts flake, not a regression from this diff. coverage-checks and policy-enforcement pass. pr-quality-gate is red because server-checks is red. Upstream's own latest PR Quality on main is also failure.

Historical evidence, predicate unchanged since 64e4d638:

  • check:server at 64e4d638, run to completion on both arms. BASE failed-tests=36 failed-files=16, HEAD failed-tests=37 failed-files=17. The single head-only file, claudeRequiredThinking.test.ts, is a wall-clock flake: it also fails on base under CPU load, and its import graph never reaches ws/handler. Logs: /agent-output/oss/cc-haha/check-server.log, check-server-BASE.log.
  • Live end-to-end run at 05c8b24e: a real server, a real CLI child over --sdk-url and a real WebSocket client. BASE: terminals emitted inside the admission window never reach the client. HEAD: they are delivered before turn 2's message_complete. Frames are in the first ## Rework comment.

Not run in this container: check:policy, check:coverage (upstream CI runs them). Not verified anywhere: Windows. The code path is OS-independent (a JS predicate on an in-process callback).

Prior art

Search (re-run 2026-09-24 at head c0053ad) Result
gh search prs --repo NanmiCoder/cc-haha "shouldForward" --state open []
gh search prs --repo NanmiCoder/cc-haha "task_notification" --state open []
gh search prs --repo NanmiCoder/cc-haha "pre-turn mute" --state open / "background task" []
gh search prs --repo NanmiCoder/cc-haha "bindClientSessionOutput" (earlier round) NanmiCoder#861 (closed, unmerged, keep_alive model data; no overlap)
gh pr list --search "1352 in:body" --state all []
git log b8c7a115..origin/main -S'taskLifecycle === null' -- src/server/ws/handler.ts (main 4615f9d4) empty

Policy

Read in full: CONTRIBUTING.md, AGENTS.md, .github/pull_request_template.md. Absent (HTTP 404): AI_POLICY.md, AI.md, AGENT_POLICY.md, .github/AI_POLICY.md, .github/CONTRIBUTING.md, CODE_OF_CONDUCT.md. No CLA, no DCO sign-off requirement, no mandated AI trailer.

AI stance: permitted. AGENTS.md opens "This is a routing guide for coding agents." and the repo also ships .github/copilot-instructions.md. Nothing restricts AI-assisted contributions or AI-written PR text.

Quoted requirements and how this change complies:

  • CONTRIBUTING §1: "bun run check:impact 会列出这次改动选中的检查". Run earlier; it selected check:policy, check:server, check:chat-contract, check:agent-flow, check:coverage. check:server and check:chat-contract were executed locally at earlier heads. check:policy and check:coverage were not run here.
  • CONTRIBUTING §3: "每个 PR 的描述里必须包含:影响范围 / 测试说明 / 剩余风险". 影响范围: server (one predicate in src/server/ws/handler.ts). 测试说明: one regression test (8 assertions: 7 fail on base, 1 control killed by named mutants), touched file 100/100. 剩余风险: lanes not run here (check:policy, check:coverage); no Windows verification; the row-8 activity-status change (a task starting inside the window now also shows tool_executing).
  • CONTRIBUTING §3: "来自 fork 的 PR 不会获得仓库 secrets…贡献者只需在 PR 中明确写 live model: not run (untrusted fork / no provider)". Stated verbatim in ## Test evidence.
  • AGENTS.md: "Executable JS/TS production changes under src/, desktop/src/, or adapters/ require a same-area regression test". Added in src/server/__tests__/.
  • AGENTS.md: "For bugs, reproduce the failure or add a test that fails for the intended reason". It fails on base with Received: [].
  • AGENTS.md: "Reuse existing utilities, stores, services, and test harnesses; add dependencies or abstractions only when the task needs them". The test uses the file's harness and adds no shared helper.
  • AGENTS.md: "use Conventional Commit subjects and product branch prefixes such as fix/". Branch fix/forward-task-lifecycle-through-pre-turn-mute. The fork branch carries eight commits (fix(ws): …, test(ws): …), several of them review iterations. Upstream merges with merge commits, so squash to the single fix(ws): commit on submission.
  • AGENTS.md: "Required PR checks must be deterministic: no real models, public network, repository secrets…". Mock sockets and spied services only. Persistence dedup keys on a fixed uuid, not on the clock.
  • AGENTS.md: "Do not commit generated output". Only the two source files are committed.

Disclosure facts for the operator

Plain facts about what the AI assistant did, for you to write your own disclosure:

  • Chose the surface from the ticket's hypothesis (background-task callback delivery). Located the defect by reading src/server/ws/handler.ts and its callers, noticing that the stop fence exempts task lifecycle and the adjacent mute gate does not.
  • Wrote the regression test first and confirmed it fails on unmodified base (Received: []). Then wrote the one-conjunct fix and confirmed the test passes.
  • Independent AI runs attacked the candidate across several review rounds. They rebuilt the boundary ledger from the diff, ran every assertion against base, and built seven mutants of the gate, including each rejected alternative.
  • Ran bun run check:server to completion on both arms at an earlier head, plus a live end-to-end run with a real CLI child process.
  • Over the review rounds, removed the production comment and test narration and fixed a clock-dependent persistence assertion. In the final round, cut the test to the rows the changed conjunct decides (+105 to +64 lines, 23 to 8 assertions), kept every mutant killed, and re-proved the defect on current upstream main.
  • Searched upstream PRs and issues for prior art. Found that [BUG] 后台任务吞回调问题 NanmiCoder/cc-haha#1352 was closed as fixed on the CLI side and that the server-side drop remains.
  • Did not contact the upstream repository in any way. All work is on the sprayberry-code/cc-haha fork.
  • Did not verify on Windows and did not run check:policy or check:coverage.

Boundaries

The diff changes one condition: options?.shouldForward && !options.shouldForward(cliMsg) becomes taskLifecycle === null && options?.shouldForward && !options.shouldForward(cliMsg). The rows below cover the predicate's operands and the full value set of taskLifecycle, taken from getCliBackgroundTaskLifecycle (agentTaskState.ts:71-106) and trackCliBackgroundTaskLifecycle. #n refers to the assertion numbers in ## Test evidence. "Probe-measured" rows were run at c0053ad8 on both arms in an untracked copy of the test extended with those inputs (/agent-output/oss/cc-haha/r8-make-ledger-probe.js, transcript r8-ledger-arms.txt, deleted after). They are not committed because this diff does not decide their outcome.

# Input / state Fixed-code behaviour Pinned by
1 taskLifecycle === null, shouldForward returns false (pre-turn chatter) muted, unchanged existing allows the current slash command lifecycle through the pre-turn mute gate; NanmiCoder#8
2 taskLifecycle === null, shouldForward returns true (current slash-command echo) forwarded, unchanged existing allows direct /goal local command output through the pre-turn mute gate
3 options undefined (reconnect bind) short-circuits, forwarded, unchanged existing reconnect tests
4 options present, shouldForward undefined short-circuits, forwarded, unchanged existing tests (bindings other than :930 pass no filter)
5 terminal completed inside the window forwarded (the fix) #1-NanmiCoder#2
6 failed forwarded NanmiCoder#3-NanmiCoder#4
7 stopped, killed forwarded; same parser arm as completed/failed probe-measured: base Received: [], head pass
8 task_started inside the window forwarded, and tool_executing status emitted, the same status a task starting outside the window gets NanmiCoder#6-NanmiCoder#7 (kill mutants C, F)
9 status: 'running' heartbeat forwarded; same parser arm as task_started probe-measured: base Received: [], head pass
10 task_progress parser null, muted NanmiCoder#8 (killed by B, D, E)
11 task_id: '' parser null, muted probe-measured: [[], []] on both arms; decided by the parser, which this diff does not touch
12 task_id: ' ' trimmed to '', null, muted probe-measured, same as row 11
13 unknown status paused parser null, muted. A pre-existing parser gap, not widened here probe-measured, same as row 11
14 cliMsg null/undefined measured on both arms at 707f3119 (same handler.ts): the callback throws TypeError: null is not an object (evaluating 'cliMsg.type') at shouldForward (handler.ts:934), before and after the fix. Pre-existing in the base closure measured, not pinned; a fix belongs to the closure at :931, out of scope
15 task id authoritatively stopped returns at :4254 with suppressForward, before this gate unreachable by this diff; probe-measured [[], []] on both arms, killed by mutant A; also pinned by three existing stop-latch tests (mutant A)
16 Stop requested inside the window, then a terminal, then late assistant text fence exempts lifecycle at :4329, gate exempts it; each client gets exactly the terminal, the text stays fenced probe-measured: terminal base + Received + 1 fail ×2 / head pass; text [[], []] both arms. Existing lets directed Agent terminals through the stop fence after suppressing late content pins the fence
17 userMessageSent already true shouldForward returns true; the conjunct is a no-op after admission existing post-admission tests; the test ends with resolveSend(true)
18 two clients bound, one mid-admission lifecycle reaches both #1-NanmiCoder#4, NanmiCoder#6-NanmiCoder#7 assert per client
19 reverse order: message admitted before the task starts the start reaches the renderer NanmiCoder#6-NanmiCoder#7 (admit first, then task_started)
20 short-circuit: shouldForward(cliMsg) is no longer called for lifecycle messages safe: the forwarder's only mutable state is set only by local_command/local_command_output subtypes (handler.ts:4014-4037, :4044, :4055), which a lifecycle message never has unreachable by construction; the skipped call had no effect on base either
21 persistence of a terminal that now reaches persistThenForwardCliMessage with two clients written once per event (dedup on uuid), both sockets forward after the write NanmiCoder#5 (base: 0 calls, head: 2)
22 task shape: plain bash / local_agent (also remote_agent, owner_agent_id) all parse non-null and are forwarded #1-NanmiCoder#4 (plain and Agent terminals; NanmiCoder#3-NanmiCoder#4 kill mutant G, which keeps the Agent shapes muted). remote_agent and owner_agent_id probe-measured with row 7

Every row the conjunct decides is pinned by a named assertion in the committed test. Rows 7, 9 and 11-13 are other inputs to the same parser arms as pinned rows, and rows 15-16 are decided by gates this diff does not touch. Those are probe-measured at this head. Row 14 is measured (same throw on both arms), and row 20 is argued from source.

Suggested upstream PR title

fix(ws): forward background task lifecycle through the pre-turn mute gate

…gate

A background task started before a new user turn never delivered its
terminal task_notification to the renderer. bindClientSessionOutput's
pre-turn mute gate (shouldForward) dropped every message until
userMessageSent flipped, including task lifecycle, so a task that
completed inside the admission window left its card running forever and
the awaited callback never arrived.

Exempt task lifecycle from that gate, matching the exemption
shouldSuppressCliOutputDuringStop already makes for the same reason.

Tested: bun test src/server/__tests__/websocket-handler.test.ts
@askalf askalf added the oss-candidate Sprayberry Code candidate for upstream label Sep 20, 2026
@askalf
askalf marked this pull request as ready for review September 20, 2026 11:50
…te gate

Pin the full surface the exemption opens: the failed/stopped/killed and
running notification variants, a task that starts inside the admission
window, and delivery to every client bound to the session. Add two
controls proving the gate still mutes non-lifecycle task chatter and
that an authoritatively stopped Agent cannot be revived by a late
terminal.
@askalf askalf added the verified Adversarially verified by a fresh run label Sep 20, 2026
@askalf

askalf commented Sep 20, 2026

Copy link
Copy Markdown
Collaborator Author

Verification

Adversarial verification by a fresh run, at head 05c8b24ebcda604eaf3ade0bf8384047f0faf0d8. The ## Boundaries ledger was rebuilt from the diff and from the lifecycle parser, not from the PR body. Production source (src/server/ws/handler.ts) is byte-identical to the reviewed head 64e4d638 — this run added test code only (+261/-1 in the test file).

What the rebuild changed. The exemption surface is whatever makes taskLifecycle non-null, i.e. exactly what getCliBackgroundTaskLifecycle (agentTaskState.ts:71-106) parses: task_started; task_notification with status:'running'; task_notification with status in completed|failed|stopped|killed. Enumerating the parser rather than the prose produced 8 new tests and two ledger rows the body did not have (row 19, reverse ordering; row 20, the short-circuit that stops shouldForward from being called for lifecycle messages).

One behaviour change the previous body understated. A task that starts inside the admission window also emits {type:'status',state:'tool_executing',verb:'bun test'}, not only the task_started notification. My first draft of that test asserted only the notification and failed on head. I widened the test to pin both messages rather than drop the assertion. It is the same verb a task starting outside the window already produces, so it is consistent — but it is renderer-visible and now stated in ## Verification method.

Head arm — whole touched file at 05c8b24e

$ bun test src/server/__tests__/websocket-handler.test.ts
 108 pass
 0 fail
 385 expect() calls
Ran 108 tests across 1 file. [2.87s]

Base arm — b8c7a115, guard absent (grep -c 'taskLifecycle === null' src/server/ws/handler.ts → 0)

The head test file was dropped into a clean base worktree as an untracked websocket-handler.probe.test.ts (the new tests do not exist on base, so -t against the tracked file matches nothing and reads as a false negative), then removed; the base worktree is clean afterwards.

$ bun test src/server/__tests__/websocket-handler.probe.test.ts -t "admission"
(pass) WebSocket handler session isolation > forwards directed Agent output while a foreground admission awaits send acknowledgement [10.68ms]
(fail) WebSocket handler session isolation > forwards background task completion while a foreground admission awaits send acknowledgement [1.48ms]
(fail) WebSocket handler session isolation > forwards a background task 'failed' terminal while a foreground admission awaits send acknowledgement [1.22ms]
(fail) WebSocket handler session isolation > forwards a background task 'stopped' terminal while a foreground admission awaits send acknowledgement [0.38ms]
(fail) WebSocket handler session isolation > forwards a background task 'killed' terminal while a foreground admission awaits send acknowledgement [0.36ms]
(fail) WebSocket handler session isolation > forwards a background task heartbeat notification during a foreground admission [0.51ms]
(fail) WebSocket handler session isolation > forwards a background task that starts inside the foreground admission window [0.50ms]
(fail) WebSocket handler session isolation > forwards a background task terminal to every client bound to the session during one admission [0.62ms]
(pass) WebSocket handler session isolation > keeps a stopped Agent unrevived by a late terminal during a foreground admission (control) [0.60ms]
(pass) WebSocket handler session isolation > keeps non-lifecycle task chatter muted during a foreground admission (control) [0.77ms]
(pass) WebSocket handler session isolation > drains a user admission already waiting on CLI startup before clear commits [2.45ms]
(pass) WebSocket handler session isolation > cancels the same pending user admission when Stop arrives before CLI startup [2.05ms]
(pass) WebSocket handler session isolation > does not let an old pending-send fallback kill a replacement admission [1.08ms]
 6 pass
 7 fail
Ran 13 tests across 1 file. [2.75s]

7 fails on base = the 7 discriminating tests (the Hunter's original plus my 6). The 2 controls pass on base, as controls must. Every test was run against base before being committed.

The controls are not decoration

Both were proved by mutating the code they claim to guard. Both mutations reverted; git diff --stat src/server/ws/handler.ts empty, guard grep count back to 1.

# Mutation A — neuter the authoritativeStoppedTaskIds early return at :4254
-    if (taskLifecycle?.suppressForward) return
+    if (false && taskLifecycle?.suppressForward) return

$ bun test src/server/__tests__/websocket-handler.test.ts -t "unrevived"
error: expect(received).not.toContainEqual(expected)
Expected to not contain: ObjectContaining {
  type: "system_notification",
  subtype: "task_notification",
}
(fail) WebSocket handler session isolation > keeps a stopped Agent unrevived by a late terminal during a foreground admission (control) [10.99ms]
 0 pass / 1 fail
# Mutation B — neuter the pre-turn mute gate entirely at :4273
-      taskLifecycle === null &&
+      false &&

$ bun test src/server/__tests__/websocket-handler.test.ts -t "(control)"
(fail) WebSocket handler session isolation > keeps non-lifecycle task chatter muted during a foreground admission (control) [4.09ms]
 5 pass / 1 fail

Mutation A is the high-value attack from the ledger (row 15): if the fix could revive an authoritatively stopped Agent it would be wrong. It cannot — trackCliBackgroundTaskLifecycle returns suppressForward:true and the callback returns at :4254, upstream of the changed gate — and the control detects it if that ever stops being true. Mutation B shows the gate still mutes progress, unknown statuses and empty/whitespace task_id, i.e. the fix narrowed it rather than removing it.

Checks

$ gh pr checks 1 --repo askalf/cc-haha
no checks reported on the 'fix/forward-task-lifecycle-through-pre-turn-mute' branch

Actions have never been enabled on this fork (operator card 00MU9O4ZX2FF131BDBEFB48F56). Absence of CI, not a failing check. No lint/format/typecheck was run because the repo ships none for the server surface and has no typescript dependency at all; style was checked mechanically on the diff (no semicolons, no tabs, 2-space indent).

Verdict: verified. Every reachable ledger row is pinned by a named test or by an existing test identified in the body; row 13 is a pre-existing parser gap left deliberately untouched, and row 20 is unreachable by construction and argued from source.

@sprayberry-redline sprayberry-redline left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review from the Sprayberry Labs fleet code reviewer.

Reviewed by the GPT gating lane (gating review).

Verdict: CHANGES REQUESTED — upstream-required manual verification for this user-visible WebSocket change is missing.

Blocking finding — policy / test evidence

src/server/ws/handler.ts:4273

taskLifecycle === null &&
options?.shouldForward &&
!options.shouldForward(cliMsg)

This changes which task-lifecycle WebSocket messages reach connected renderers during a user-message admission. The candidate's quoted upstream CONTRIBUTING policy requires manual verification for “user-visible UI” or “cross WebSocket/process” changes in addition to automated checks, but its verification section explicitly says Windows behavior was not verified and provides only mock-socket evidence. A real renderer/session path can still differ in message timing, reconnection, and task-card handling, so the required evidence is absent.

Please perform and record the required manual browser/desktop verification of a background task completing while a user message is being admitted (or document a maintainer-approved exception), including the observed terminal task-card/callback outcome.

# No production-code change is required here.
# Add the required real renderer/desktop smoke-test result to the PR evidence.

What's good: the production change is minimal and reuses the adjacent lifecycle classification; the regression coverage enumerates the lifecycle variants and preserves the stopped-task and non-lifecycle mute controls. I also verified the base path contains the pre-admission shouldForward gate, the await-before-userMessageSent ordering, and the sibling stop-fence lifecycle exemption. Fork CI reports no checks, rather than a failing check.

@sprayberry-secondread sprayberry-secondread left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review from the Sprayberry Labs fleet code reviewer.

Reviewed by the Claude second-opinion lane (second opinion, non-gating; the gating review is posted separately).

Verdict: the delivery fix is sound at 05c8b24ebcda604eaf3ade0bf8384047f0faf0d8; no blocking code findings.

Correctness and scope

I can confirm the bug by tracing the base code (not by executing tests): sendUserMessage installs the client filter before awaiting sendMessage; a valid background completion arriving before userMessageSent = true fails that filter. Bookkeeping happens before the return, but persistence/translation/client delivery happens after it. The changed src/server/ws/handler.ts:4274 line, taskLifecycle === null &&, lets that completion reach the existing delivery path.

Reusing the lifecycle classification is preferable to moving the admission boundary or duplicating subtype checks. It retains the authoritative-stop early return at :4254. Skipping the callback for lifecycle messages is safe for the current caller: the slash-command forwarder's mutable flag changes only for local-command subtypes, not task lifecycle. Unknown statuses remaining muted is a reasonable scope boundary, not a reason to expand this parser here. Forwarding starts and their existing activity verb is coherent with forwarding lifecycle uniformly, and the new start test explicitly pins both messages.

Boundaries — rebuilt from the changed gate and its operands

The production diff changes one compound guard, with three operands and no numeric limit or index. Its input is an object-or-null classification, not an arbitrary truthy/falsy value.

Predicate / boundary Fixed behavior Test evidence / limitation
taskLifecycle === null: completed task Bypasses mute Completion test asserts the terminal notification
Same comparison: failed / stopped / killed Bypasses mute for each status Parameterized terminal test asserts each actual status; all three are discriminating
Same comparison: running notification Bypasses mute Heartbeat test asserts notification
Same comparison: task starts during admission Bypasses mute; emits notification and activity status Start-inside-window test asserts both
Same comparison: progress / unknown status Classification is null; mute still applies Chatter control includes progress and paused
Same comparison: empty / whitespace task ID Classification is null; mute still applies Chatter control includes both strings
Same comparison: absent/null/non-string ID, including zero, negative or maximum number, empty collection Existing parser rejects non-string IDs; no new exemption No new dedicated cases; unchanged parser branch. Numeric limits/equality/one-past-limit do not apply to this diff
Same comparison: suppressed lifecycle Earlier suppressForward return wins Stopped-Agent control covers authoritative stopped ID; the existing local-stop-confirmed guard is also upstream of the new gate
options?.shouldForward: no options / no callback Does not mute Unchanged binding behavior; new tests use ordinary opening before admission, but do not separately assert every no-filter combination
!options.shouldForward(cliMsg): null lifecycle and callback false Returns without delivery Chatter control
Same negation: callback true, including admission settled Does not mute Existing slash-command/admission behavior; settling the new tests is cleanup, not an additional post-admission assertion
New first-operand short circuit: lifecycle skips callback No mutation of slash-command state is lost Source trace through :4009–4066; no new spy assertion for call count
Multiple clients / other delivery path Each bound client independently passes lifecycle; disconnected watcher is untouched Two-client test asserts both sockets; not a disconnected-replay test

Null/undefined entire frames are not a new supported protocol variant: other callback code may dereference frames. The narrow parser's null-safe classification should not be read as an end-to-end null-frame guarantee. No platform-specific condition is introduced; Windows itself was not exercised in this review.

Assertion audit

I read every assertion in the added tests. The seven delivery regressions require observable messages that the base mute gate drops, including each of the failed/stopped/killed variants and both observers in the multi-client case. The start test also checks tool_executing, rather than silently tolerating it. The two negative controls intentionally hold on base: they protect the earlier stop fence and continued chatter muting, not proof of the defect. The body reports mutations for those controls; I did not rerun them. I found no claimed positive regression whose assertion is vacuous for one of its supplied variants.

Maintainer's-eye notes (not findings)

  • Idiomatic scope and reuse: the existing sibling stop fence uses the same lifecycle object. Recent upstream history also treats terminal delivery as the means of converging stale activity entries: 491f4492. This patch appropriately fixes the loss before delivery rather than adding a second recovery mechanism.
  • Test shape: same-area deterministic lifecycle regressions fit the approach in merged outside contributions #1299 and #1301. The existing socket harness and deferred admission are a better fit than a new integration framework. The original completion setup could share openAdmissionWindow, but the duplication is not a correctness concern.
  • Submission text: the Conventional Commit title fits those merged examples; drop the fork-only candidate prefix upstream. The upstream bodies are materially shorter. Lead with the race, small fix, deterministic regression, remaining verification gaps and the required impact/testing/risk sections; keep the extensive mutation transcript and ledger as supporting evidence. This is an operator presentation note, not a code finding or a claim about authorship. CONTRIBUTING also asks for relevant UI/cross-process manual evidence and selected checks; disclose what was not performed rather than equating the unit file with full upstream validation. I found no reason here to request a changelog-only edit.
  • Prior art: independently repeated PR searches for shouldForward and task notification (no results), read recent touched-module history, and inspected #1342's handler diff. That PR persists notifications in the disconnected watcher and adds replay plumbing; it does not change this connected-client mute gate. The recent task-stop recovery commit is related, but not the same fix.

What's good: this is a small production change with meaningful status-variant tests, a two-client regression, and explicit protection against reviving stopped tasks. I read the complete candidate diff and the relevant caller/parser/forwarder context, not the full application. GitHub reports no checks on this fork branch; I ran no test suite and make no independent green-CI claim.

SECOND READ: READY

@askalf

askalf commented Sep 23, 2026

Copy link
Copy Markdown
Collaborator Author

Rework

Answers the blocking review at head 05c8b24e, which asked for evidence from a real session rather than only the mock-socket tests. No code changed; the head is the same. This is evidence only.

Live end-to-end run

A throwaway harness (source below, not committed) starts the real server (startServer), a real CLI child process speaking the repo's own stream-json protocol over --sdk-url, and a real WebSocket client that sends exactly what the renderer sends (user_message) and records every frame the renderer would receive. Nothing under src/ is stubbed or spied.

Scenario: turn 1 starts a background task and turn 2 is sent. While turn 2 is being admitted (after the shouldForward rebind, before userMessageSent = true), the CLI emits the task's terminal notification. To hit that window deterministically, the harness clears the terminal-shell-environment cache before turn 2 and points SHELL at a login-shell stand-in that sleeps ~3 s. sendMessage() awaits that capture, so the window is ~3 s wide. On a user's machine the same window is opened by a slow shell profile, an OAuth refresh before the turn, or attachment materialisation.

arm result
BASE, completed inside the window never delivered: the task card stays running
BASE, failed inside the window never delivered: the task card stays running
BASE control, terminal after admission delivered at +6963 ms: the harness works, so the window is the cause
HEAD, completed inside the window delivered at +4385 ms, before turn 2's message_complete
HEAD, failed inside the window delivered at +4348 ms, before turn 2's message_complete
BASE `b8c7a115`, terminal `completed` inside the window: frames after the turn-2 admission opened
--- frames the renderer received after the turn-2 admission opened ---
  +3223ms status/thinking
  +6232ms message_complete

--- result ---
turn 1 completed at +3222ms; turn 2 admission opened at +3222ms
TERMINAL TASK NOTIFICATION NEVER REACHED THE CLIENT — the task card stays "running" forever
BASE `b8c7a115`, terminal `failed` inside the window: frames after the turn-2 admission opened
--- frames the renderer received after the turn-2 admission opened ---
  +3220ms status/thinking
  +6233ms message_complete

--- result ---
turn 1 completed at +3219ms; turn 2 admission opened at +3219ms
TERMINAL TASK NOTIFICATION NEVER REACHED THE CLIENT — the task card stays "running" forever
BASE `b8c7a115`, control: terminal lands after admission settles: frames after the turn-2 admission opened
--- frames the renderer received after the turn-2 admission opened ---
  +3204ms status/thinking
  +6211ms message_complete
  +6963ms system_notification/task_notification {"type":"system","subtype":"task_notification","task_id":"live-bash-task","tool_use_id":"live-bash-tool","status":"completed","summary":"Background command \"bun test\" finished","result":"exit 0","task_type":"bash","session_id":"live-admission-dd2cb642-d12d-4dc8-b83c-465bba19f84a"}

--- result ---
turn 1 completed at +3200ms; turn 2 admission opened at +3200ms
TERMINAL TASK NOTIFICATION DELIVERED at +6963ms: {"type":"system_notification","subtype":"task_notification","data":{"type":"system","subtype":"task_notification","task_id":"live-bash-task","tool_use_id":"live-bash-tool","status":"completed","summary":"Background command \"bun test\" finished","result":"exit 0","task_type":"bash","session_id":"live-admission-dd2cb642-d12d-4dc8-b83c-465bba19f84a"}}
ordering: delivered after turn 2 completed -> NOT inside the admission window (inconclusive run)
HEAD `05c8b24e`, terminal `completed` inside the window: frames after the turn-2 admission opened
--- frames the renderer received after the turn-2 admission opened ---
  +3232ms status/thinking
  +4385ms system_notification/task_notification {"type":"system","subtype":"task_notification","task_id":"live-bash-task","tool_use_id":"live-bash-tool","status":"completed","summary":"Background command \"bun test\" finished","result":"exit 0","task_type":"bash","session_id":"live-admission-a1dad8f6-b6cb-4826-80b5-dab3156bfe94"}
  +6241ms message_complete

--- result ---
turn 1 completed at +3231ms; turn 2 admission opened at +3232ms
TERMINAL TASK NOTIFICATION DELIVERED at +4385ms: {"type":"system_notification","subtype":"task_notification","data":{"type":"system","subtype":"task_notification","task_id":"live-bash-task","tool_use_id":"live-bash-tool","status":"completed","summary":"Background command \"bun test\" finished","result":"exit 0","task_type":"bash","session_id":"live-admission-a1dad8f6-b6cb-4826-80b5-dab3156bfe94"}}
ordering: delivered BEFORE turn 2 completed -> it was emitted inside the admission window
HEAD `05c8b24e`, terminal `failed` inside the window: frames after the turn-2 admission opened
--- frames the renderer received after the turn-2 admission opened ---
  +3191ms status/thinking
  +4348ms system_notification/task_notification {"type":"system","subtype":"task_notification","task_id":"live-bash-task","tool_use_id":"live-bash-tool","status":"failed","summary":"Background command \"bun test\" finished","result":"exit 0","task_type":"bash","session_id":"live-admission-3cd8f838-8687-4c24-b27c-6c2a0ef490ff"}
  +6199ms message_complete

--- result ---
turn 1 completed at +3190ms; turn 2 admission opened at +3190ms
TERMINAL TASK NOTIFICATION DELIVERED at +4348ms: {"type":"system_notification","subtype":"task_notification","data":{"type":"system","subtype":"task_notification","task_id":"live-bash-task","tool_use_id":"live-bash-tool","status":"failed","summary":"Background command \"bun test\" finished","result":"exit 0","task_type":"bash","session_id":"live-admission-3cd8f838-8687-4c24-b27c-6c2a0ef490ff"}}
ordering: delivered BEFORE turn 2 completed -> it was emitted inside the admission window

Test files re-run at 05c8b24e

$ bun test src/server/__tests__/websocket-handler.test.ts

 108 pass
 0 fail
 385 expect() calls
Ran 108 tests across 1 file. [2.48s]
$ bun test src/server/__tests__/conversations.test.ts

 117 pass
 0 fail
 444 expect() calls
Ran 117 tests across 1 file. [13.99s]

Not run

bun run check:desktop-ui-smoke, the repo's Electron desktop lane, does not run in this container:

$ bun run scripts/quality-gate/desktop-smoke/deterministic-cli.ts
[desktop-ui-smoke] real desktop UI against the mock SDK CLI — no provider, no credentials, no network
[desktop-ui-smoke] SKIPPED: desktop dependencies are not installed (run `bun install` in desktop/)

So the renderer here is the protocol client, not the Electron window. The harness sends and records the same WebSocket messages the renderer does, against the real server and a real CLI process, and that is the path this change touches. Whether this meets the CONTRIBUTING manual-verification bar for a cross-WebSocket change, or a desktop run is still wanted before submission, is the reviewers' call.

Harness source: live-admission-verify.ts
/**
 * Live end-to-end verification of the pre-turn mute gate.
 *
 * Runs the REAL server (startServer), a REAL CLI child process speaking the SDK
 * stream-json protocol over a real socket, and a REAL WebSocket client that
 * speaks exactly what the renderer speaks (`user_message`) and records exactly
 * what the renderer receives. Nothing in src/ is stubbed or spied.
 *
 * Scenario: a background task is started in turn 1; while turn 2 is being
 * admitted (the window between the client-output rebind and `userMessageSent`),
 * the CLI emits the task's terminal notification. The admission window is
 * widened to a few seconds by clearing the terminal-shell-environment cache
 * before turn 2 — `sendMessage` refreshes the network environment and awaits
 * that capture, which is exactly the kind of pre-send await that makes the race
 * reachable in production.
 */
import * as fs from 'node:fs/promises'
import * as path from 'node:path'
import { fileURLToPath } from 'node:url'
import { resetTerminalShellEnvironmentCacheForTests } from './src/utils/terminalShellEnvironment.js'

const configDir = await fs.mkdtemp(path.join(process.env.LIVE_TMP_ROOT || '/tmp', 'cc-haha-live-'))
process.env.CLAUDE_CONFIG_DIR = configDir
process.env.CLAUDE_CLI_PATH = fileURLToPath(new URL('./live-admission-cli.ts', import.meta.url))
await fs.mkdir(path.join(configDir, 'projects'), { recursive: true })

const { startServer, stopServerRuntimeForShutdown } = await import('./src/server/index.js')
const server = startServer(0, '127.0.0.1')
const wsUrl = `ws://127.0.0.1:${server.port}`
const sessionId = `live-admission-${crypto.randomUUID()}`

const t0 = Date.now()
const received: Array<{ at: number; msg: any }> = []
function log(line: string) {
  console.log(`[+${String(Date.now() - t0).padStart(5, ' ')}ms] ${line}`)
}

const workDir = await fs.mkdtemp(path.join(configDir, 'work-'))
const created = await fetch(`http://127.0.0.1:${server.port}/api/sessions`, {
  method: 'POST',
  headers: { 'Content-Type': 'application/json' },
  body: JSON.stringify({ workDir, sessionId }),
})
log(`POST /api/sessions -> ${created.status}`)

const ws = new WebSocket(`${wsUrl}/ws/${sessionId}`)
let connected!: () => void
const connectedPromise = new Promise<void>((resolve) => { connected = resolve })

ws.onmessage = (event) => {
  const msg = JSON.parse(event.data as string)
  received.push({ at: Date.now() - t0, msg })
  if (msg.type === 'connected') {
    log('client <- connected')
    connected()
    return
  }
  if (msg.type === 'system_notification') {
    log(`client <- system_notification ${msg.subtype} ${JSON.stringify(msg.data)}`)
    return
  }
  if (msg.type === 'message_complete') {
    log('client <- message_complete')
    return
  }
  if (msg.type === 'error') log(`client <- error ${msg.message}`)
}

await connectedPromise

function waitFor(predicate: (msg: any) => boolean, label: string, timeoutMs = 30000) {
  return new Promise<void>((resolve, reject) => {
    const started = Date.now()
    const timer = setInterval(() => {
      if (received.some((entry) => predicate(entry.msg))) {
        clearInterval(timer)
        resolve()
        return
      }
      if (Date.now() - started > timeoutMs) {
        clearInterval(timer)
        reject(new Error(`timed out waiting for ${label}`))
      }
    }, 25)
  })
}

// Turn 1 — start the background task.
log('client -> user_message "START_BG_TASK ..."')
ws.send(JSON.stringify({ type: 'user_message', content: 'START_BG_TASK please run bun test in the background' }))
await waitFor((msg) => msg.type === 'system_notification' && msg.subtype === 'task_started', 'task_started')
await waitFor((msg) => msg.type === 'message_complete', 'turn 1 completion')
const turn1DoneAt = Date.now() - t0
log('turn 1 complete; background task is running')

// Widen the next admission window: the pre-send network refresh awaits a fresh
// terminal-shell-environment capture, which the slow SHELL wrapper stretches.
resetTerminalShellEnvironmentCacheForTests()
const before2 = received.length

// Turn 2 — the terminal notification lands while this is being admitted.
log('client -> user_message "second turn" (admission window opens)')
const turn2SentAt = Date.now() - t0
ws.send(JSON.stringify({ type: 'user_message', content: 'second turn while the task finishes' }))

await waitFor(
  (msg) => msg.type === 'message_complete' && received.filter((e) => e.msg.type === 'message_complete').length >= 2,
  'turn 2 completion',
)
await new Promise((resolve) => setTimeout(resolve, 1500))

const turn2Frames = received.slice(before2)
const terminal = turn2Frames.find(
  (entry) =>
    entry.msg.type === 'system_notification' &&
    entry.msg.subtype === 'task_notification' &&
    entry.msg.data?.status === (process.env.LIVE_TASK_TERMINAL_STATUS || 'completed'),
)
const turn2Echo = turn2Frames.find(
  (entry) => entry.msg.type === 'message_complete',
)

console.log('\n--- frames the renderer received after the turn-2 admission opened ---')
for (const entry of turn2Frames) {
  const { type, subtype, state, data } = entry.msg
  console.log(`  +${entry.at}ms ${type}${subtype ? `/${subtype}` : ''}${state ? `/${state}` : ''}${data ? ` ${JSON.stringify(data)}` : ''}`)
}

console.log('\n--- result ---')
console.log(`turn 1 completed at +${turn1DoneAt}ms; turn 2 admission opened at +${turn2SentAt}ms`)
if (terminal) {
  console.log(`TERMINAL TASK NOTIFICATION DELIVERED at +${terminal.at}ms: ${JSON.stringify(terminal.msg)}`)
  console.log(
    terminal.at < (turn2Echo?.at ?? Infinity)
      ? 'ordering: delivered BEFORE turn 2 completed -> it was emitted inside the admission window'
      : 'ordering: delivered after turn 2 completed -> NOT inside the admission window (inconclusive run)',
  )
} else {
  console.log('TERMINAL TASK NOTIFICATION NEVER REACHED THE CLIENT — the task card stays "running" forever')
}

ws.close()
server.stop(true)
await stopServerRuntimeForShutdown()
await fs.rm(configDir, { recursive: true, force: true })
process.exit(terminal ? 0 : 1)
Harness source: live-admission-cli.ts, the CLI stand-in
/**
 * Stand-in Claude CLI for the live admission-window verification.
 *
 * Speaks the same newline-delimited SDK stream-json protocol over --sdk-url as
 * the repository's own src/server/__tests__/fixtures/mock-sdk-cli.ts, so the
 * server, the WebSocket handler and the renderer protocol are all real; only
 * the model process is replaced (CONTRIBUTING: fork PRs must not run a live
 * model). Two behaviours beyond the repo fixture:
 *   - `START_BG_TASK` emits a background task_started and schedules its
 *     terminal task_notification LIVE_TASK_TERMINAL_DELAY_MS later.
 *   - every other prompt is echoed back as a normal turn.
 */
const args = process.argv.slice(2)

function getArg(name: string): string | undefined {
  const index = args.indexOf(name)
  return index >= 0 ? args[index + 1] : undefined
}

function emit(ws: WebSocket, payload: Record<string, unknown>) {
  ws.send(JSON.stringify(payload) + '\n')
}

function extractUserText(message: any): string {
  const content = message?.message?.content
  if (!Array.isArray(content)) return ''
  return content
    .filter((block: any) => block?.type === 'text' && typeof block.text === 'string')
    .map((block: any) => block.text)
    .join(' ')
}

const sdkUrl = getArg('--sdk-url')
const sessionId = getArg('--session-id') || getArg('--resume') || crypto.randomUUID()
const terminalDelayMs = Number(process.env.LIVE_TASK_TERMINAL_DELAY_MS || '1500')
const terminalStatus = process.env.LIVE_TASK_TERMINAL_STATUS || 'completed'

if (!sdkUrl) {
  console.error('Missing --sdk-url')
  process.exit(1)
}

const ws = new WebSocket(sdkUrl)
let initSent = false

function sendInit() {
  if (initSent) return
  initSent = true
  emit(ws, {
    type: 'system',
    subtype: 'init',
    model: 'live-admission-mock',
    slash_commands: [{ name: 'help', description: 'Show help' }],
    session_id: sessionId,
  })
}

ws.addEventListener('open', () => {
  sendInit()
})

ws.addEventListener('message', (event) => {
  const payload = typeof event.data === 'string' ? event.data : String(event.data)
  for (const line of payload.split('\n').map((entry) => entry.trim()).filter(Boolean)) {
    const parsed = JSON.parse(line)
    if (parsed.type !== 'user') continue
    sendInit()
    const text = extractUserText(parsed)

    if (text.includes('START_BG_TASK')) {
      emit(ws, {
        type: 'system',
        subtype: 'task_started',
        task_id: 'live-bash-task',
        tool_use_id: 'live-bash-tool',
        description: 'bun test',
        task_type: 'bash',
        session_id: sessionId,
      })
      setTimeout(() => {
        console.error(`[live-cli] emitting ${terminalStatus} terminal for live-bash-task`)
        emit(ws, {
          type: 'system',
          subtype: 'task_notification',
          task_id: 'live-bash-task',
          tool_use_id: 'live-bash-tool',
          status: terminalStatus,
          summary: 'Background command "bun test" finished',
          result: 'exit 0',
          task_type: 'bash',
          session_id: sessionId,
        })
      }, terminalDelayMs)
    }

    emit(ws, {
      type: 'result',
      subtype: 'success',
      is_error: false,
      result: `Echo: ${text}`,
      usage: { input_tokens: 3, output_tokens: 2 },
      session_id: sessionId,
    })
  }
})

@sprayberry-redline sprayberry-redline left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review from the Sprayberry Labs fleet code reviewer.
Reviewed by the GPT gating lane (gating review).

REQUEST CHANGES — the implementation and regression coverage are well-scoped, but public added comments narrate the patch rather than documenting code; remove them before upstream submission. rule:reads-as-generated

Blocking — generated patch narration: src/server/ws/handler.ts:4278-4283

// The pre-turn mute gate exists to keep pre-turn SDK chatter out of the
// new turn's history. Background task lifecycle is not chatter: a task
// started before this turn still owes the renderer its terminal
// notification, and dropping it here strands the task card as running
// forever. The stop fence above already exempts lifecycle for the same
// reason.

This newly added comment explains why this patch is correct, describes prior behavior, and repeats the surrounding condition instead of stating a durable local contract. It is a generated-writing tell in production code and would make this small upstream fix read as generated. Delete it; the condition and existing nearby stop-fence comment already express the relevant behavior.

Blocking — generated patch narration: src/server/__tests__/websocket-handler.test.ts:1050-1051

// A task starting inside the admission window now also drives the activity
// verb, exactly as one starting outside it already did.

This comment narrates the change and compares it with earlier behavior rather than documenting test setup or a stable test invariant. That is another generated-writing tell in a public test. Remove the comment and retain the assertion.

    expect(sent).toContainEqual({

What's good: I traced the base gate at handler.ts:4250-4274, and the lifecycle/non-lifecycle split is correctly exercised by the new focused admission-window tests. The evidence supplies base/head A/B results; GitHub currently reports no CI checks for this branch. I also re-ran upstream prior-art searches and found no matching open pull request.

@sprayberry-secondread sprayberry-secondread left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review from the Sprayberry Labs fleet code reviewer.

Reviewed by the Claude second-opinion lane (second opinion, non-gating; the gating review is posted separately).

The production fix is sound by inspection; this evidence-only rework still needs committed test/comment cleanup before upstream submission.

Finding

Low | Submission-readiness: remove patch narration and compress the regression additions.

  • src/server/__tests__/websocket-handler.test.ts:1100,1126: test titles contain during a foreground admission (control). These labels describe the verification exercise rather than observable product behavior.
  • src/server/__tests__/websocket-handler.test.ts:1050-1051: // A task starting inside the admission window now also drives the activity / // verb, exactly as one starting outside it already did. This narrates the patch's history above an assertion that already names the expected status and verb.
  • src/server/ws/handler.ts:4278-4283: // The pre-turn mute gate exists to keep pre-turn SDK chatter out of the through // reason. This six-line argument for correctness belongs in the PR explanation, not inside the branch that handles non-lifecycle messages.
  • src/server/__tests__/websocket-handler.test.ts:838-1157: the 323-line test addition starts with it('forwards background task completion while a foreground admission awaits send acknowledgement' and then duplicates its setup in async function openAdmissionWindow(sessionId: string). It is many times the size of the one-conjunct production change. Preserve the cases, but share the setup and parameterize lifecycle payloads instead of carrying both implementations.

The concrete submission problem is unchanged by adding evidence to the body: these artifacts remain in the code the maintainer will review. This is not a newly discovered runtime defect.

Suggested fix:

Remove the two '(control)' suffixes and the activity-verb narration.
Remove or shorten the production explanation to a neutral invariant outside the rejection branch.
Use one admission-window setup for completed/failed/stopped/killed/running/start cases;
retain explicit assertions for both clients, the activity status, and suppression cases.
Keep A/B and mutation discussion in the PR evidence, not in test names or comments.

Independent correctness and boundary pass

I can confirm the base-code bug by tracing the unchanged caller and the removed condition: bindAllClientSessionOutputs installs the filter before awaiting sendMessage; userMessageSent remains false until that await settles. A valid system/task_notification with status: 'completed' is tracked, then rejected by the old filter before persistence/forwarding. The added conjunct lets it reach persistThenForwardCliMessage. The earlier authoritative-stop suppression remains effective.

Rebuilt from the changed condition (there are no added numeric comparisons or index expressions):

Changed operand / input Fixed behavior Test coverage read
taskLifecycle === null: completed, failed, stopped, killed Bypass mute filter Completion test plus all three terminal parameters assert exact task ID/status
Same operand: running notification, task start Bypass mute filter Heartbeat test; start test asserts notification and activity verb
Same operand: empty/whitespace ID, progress, unknown status Parser returns null; remain muted Four payloads in chatter test; final empty-frame assertion covers each
Same operand: missing/null/non-string ID, including zero, negative, maximum number, empty collection Existing string parser returns null; no new exemption Not separately asserted by the new tests; same pre-existing parser branch, not a newly changed boundary
options?.shouldForward: options/filter absent No mute rejection Existing socket-open path exercised during setup; no dedicated new absent-filter assertion
!options.shouldForward(cliMsg): false/true result for non-lifecycle frames Reject/continue as before Chatter test covers rejection; existing slash-command tests cover acceptance
Lifecycle after authoritative stop Earlier suppressForward return wins Stopped-Agent negative assertion
Other client/protocol path Each session-bound socket receives lifecycle Both sender and observer assertions in two-client test; no Windows/Electron execution claimed

Limit equality/one-past-limit cases do not apply to this predicate. Null/undefined messages are not equivalent to null lifecycle: upstream callback code also accesses message fields, so I do not infer whole-callback null safety from this change.

I read every new assertion. The seven positive variants distinguish base from fixed behavior by the forwarding trace. The two negative tests intentionally also pass on base; the supplied mutation evidence targets their independent suppression invariants, so they are not positive regression variants masquerading as coverage. Resolving the send promise at the end is cleanup, not an assertion of post-admission forwarding.

Upstream fit and evidence

  • Reusing the existing lifecycle classifier and socket harness is appropriate. The neighboring admission test already uses a pending send promise; no new production abstraction is needed.
  • Recent handler history, including 1be51c9 and b3c09a1, uses conventional scoped subjects. The candidate's underlying fix(ws): subject fits; remove the fork-only candidate prefix when submitting upstream.
  • Recent outside merged PRs #1371 and #1369 pair behavior changes with same-area tests. I inspected NanmiCoder#1371's test diff: behavioral names and direct service assertions, not verification-role labels. No changelog requirement was established from these examples.
  • Independently repeated searches for task_notification, background task, shouldForward, pre-turn mute, and PR bodies mentioning 1352 returned no competing fix. I also inspected the relevant NanmiCoder#1342 hunks: replay and disconnected notification persistence, not this mute gate.
  • The new Rework source and frame logs provide useful server/child-process/WebSocket integration evidence using a CLI stand-in. They do not demonstrate an Electron window or a real model, and I did not rerun them. CONTRIBUTING section 2 prefers real desktop mock-runtime verification for this class of cross-WebSocket change; keep the missing desktop smoke/Windows coverage explicit as an operator submission note. Required upstream description sections are likewise operator notes, not fork-code findings.

Scope: full two-file diff, admission caller/classifier context, current body and Rework evidence, commit messages, upstream history and outside PR context. No gating review read; no local test suite run. gh pr checks reports no checks, so reported 108/0 and 117/0 results are author evidence, not independently observed CI.

SECOND READ: NOT READY — committed control labels, patch-history commentary, and disproportionate duplicated test setup still need cleanup.

Fold the completed/failed/stopped/killed/running notification cases into
one parameterized test over a shared setup, and name the two muting
cases by the behaviour they pin.
@askalf askalf removed the verified Adversarially verified by a fresh run label Sep 23, 2026
@github-actions

Copy link
Copy Markdown

PR quality triage

Changed areas: area:server

CLI core policy: No CLI-core policy block detected.

Missing-test policy: No missing-test policy block detected.

Coverage baseline policy: No coverage-baseline policy block detected.

CLI core files:

  • none

Coverage policy files:

  • none

Required check plan:

  • See PR Quality / scope-plan; it is the single source of truth for selected jobs.

Test coverage signals:

  • No obvious missing-test signal from changed paths.

Risk notes:

  • Session runtime changed: review reconnect, startup diagnostics, provider selection, and thinking settings.

Hard merge gates come from the deterministic GitHub Actions contract lanes above.

@askalf

askalf commented Sep 23, 2026

Copy link
Copy Markdown
Collaborator Author

Rework

This answers the 2026-09-23 reviews at 05c8b24e: Redline's CHANGES_REQUESTED (rule:reads-as-generated) and the Second Read. New head: 05917ba138842b7fc0b54325b477da67dd7653cf.

  • 3a0586a1 fix(ws): drop the explanatory comment from the pre-turn mute gate deletes the six comment lines at handler.ts:4278-4283. The production diff against base is now only the taskLifecycle === null && conjunct, and the predicate is unchanged.
  • 05917ba1 test(ws): share one admission-window setup across the lifecycle cases:
    • Deletes the activity-verb narration comment and both (control) suffixes. The tests are now keeps a stopped Agent unrevived by a late terminal during a foreground admission and keeps non-lifecycle task chatter muted during a foreground admission.
    • Folds the original completion test's duplicated setup into openAdmissionWindow, and folds completed/failed/stopped/killed/running into one parameterized test built from two payload builders.
    • The test addition goes from +326 to +167 lines. Every assertion is kept: both clients, the tool_executing status, the stopped-Agent suppression and the four muted payloads.

Re-measured at 05917ba1:

# HEAD 05917ba1
$ bun test src/server/__tests__/websocket-handler.test.ts
 108 pass
 0 fail
 385 expect() calls
Ran 108 tests across 1 file. [2.68s]

# BASE b8c7a115 source + this head's test file
$ bun test src/server/__tests__/websocket-handler.test.ts
 101 pass
 7 fail
 383 expect() calls
Ran 108 tests across 1 file. [2.88s]

The 7 base failures are the 5 parameterized status cases, the inside-window start and the two-client case. The 2 keeps … cases pass on base. Each still fails under its mutation: mutation A (neutering suppressForward at :4254) kills the stopped-Agent case, and mutation B (neutering the new conjunct to false) kills the chatter case. The per-test listings are in the body's ## Test evidence.

No added line in the diff contains //, (control) or a non-ASCII byte.

A terminal emitted after Stop is requested mid-admission still reaches
the renderer, and a terminal delivered to two bound clients is persisted
once.
@askalf askalf added the verified Adversarially verified by a fresh run label Sep 23, 2026
@askalf

askalf commented Sep 23, 2026

Copy link
Copy Markdown
Collaborator Author

Verification

Head 7721f8bcc045b2d4f413ce8c971d57d5917f9804 (test-only commit on 05917ba1; src/server/ws/handler.ts byte-identical to 05917ba1, cmp clean after every mutant restore). Base b8c7a115. Container: bun 1.3.14, two worktrees sharing one node_modules.

What this round attacked. The rework at 05917ba1 was a test fold plus a comment deletion. I re-ran both arms on the folded file (head 108/0, base 101/7, identical to the hand-off), then rebuilt the ledger from the diff and the lifecycle parser. Two rows were closed by prose: row 16 (stop fence and mute gate both active) and the persistence side effect of a terminal now reaching persistThenForwardCliMessage, which no row carried. Both are now executed. The ## Fix section rejects three alternatives in prose; each was built as a mutant.

Added at this head (+31/-1).

  1. forwards a background task terminal past the stop fence during a foreground admission: task_started, admit, stop_generation, then the terminal and a late assistant text. Expects exactly one sent message, the terminal.
  2. On forwards a background task terminal to every client bound to the session during one admission: appendSessionTaskNotification called exactly once with {taskId:'two-client-task', status:'completed'}.

Head arm, whole file:

$ bun test src/server/__tests__/websocket-handler.test.ts
 109 pass
 0 fail
 388 expect() calls
Ran 109 tests across 1 file. [2.73s]

Base arm (b8c7a115 worktree, grep -c 'taskLifecycle === null &&' src/server/ws/handler.ts = 0, head test file copied in as an untracked *.probe.test.ts, deleted after, git status --short empty):

(fail) WebSocket handler session isolation > forwards a background task 'completed' notification while a foreground admission awaits send acknowledgement [2.23ms]
(fail) WebSocket handler session isolation > forwards a background task 'failed' notification while a foreground admission awaits send acknowledgement [0.53ms]
(fail) WebSocket handler session isolation > forwards a background task 'killed' notification while a foreground admission awaits send acknowledgement [0.45ms]
(fail) WebSocket handler session isolation > forwards a background task 'running' notification while a foreground admission awaits send acknowledgement [0.37ms]
(fail) WebSocket handler session isolation > forwards a background task 'stopped' notification while a foreground admission awaits send acknowledgement [0.50ms]
(fail) WebSocket handler session isolation > forwards a background task terminal past the stop fence during a foreground admission [1.21ms]
(fail) WebSocket handler session isolation > forwards a background task terminal to every client bound to the session during one admission [1.20ms]
(fail) WebSocket handler session isolation > forwards a background task that starts inside the foreground admission window [0.66ms]
 101 pass
 8 fail
Ran 109 tests across 1 file. [2.96s]

The persistence assertion alone on base (throwaway probe, same setup): append calls 0 on base, 1 on head, so the dropped terminal never reached history either.

Controls (pass on both arms, marked in the body table, never in a name): keeps a stopped Agent unrevived by a late terminal during a foreground admission, keeps non-lifecycle task chatter muted during a foreground admission.

Mutants (whole file each, /agent-output/oss/cc-haha/rv6-mutate.js):

# A  if (false && taskLifecycle?.suppressForward) return
 105 pass / 4 fail: keeps a stopped Agent unrevived...; automatically retries a confirmed local stop...; keeps an archived remote Agent stopped...; stops every active Agent task when generation is stopped
# B  false &&                                             (gate removed)
 108 pass / 1 fail: keeps non-lifecycle task chatter muted...
# C  cliMsg?.subtype !== 'task_notification' &&          (rejected alternative: subtype special case)
 107 pass / 2 fail: forwards a background task that starts inside...; keeps non-lifecycle task chatter muted...
# D  !(cliMsg?.type === 'system' && typeof cliMsg.task_id === 'string' && cliMsg.task_id) &&   (any task-scoped system message)
 108 pass / 1 fail: keeps non-lifecycle task chatter muted...
# E  !(type==='system' && subtype in task_started|task_notification|task_progress) &&           (widening to progress)
 108 pass / 1 fail: keeps non-lifecycle task chatter muted...
# F  !(taskLifecycle && taskLifecycle.running === false) &&                                     (terminals only)
 107 pass / 2 fail: forwards a background task 'running' notification...; forwards a background task that starts inside...

No mutant survives. Each rejected alternative is killed by a named test rather than by argument.

Probed, not committed (identical on both arms): a stale task_started for a local_agent whose activeAgentTasks entry has stopIntent and localStopConfirmed, emitted inside the window, sends [] on both arms; trackCliBackgroundTaskLifecycle turns it into suppressForward:true at :297 and :4254 returns before the gate. A terminal emitted after settle() is forwarded on both arms (window closed, row 17).

Fork CI. At 05917ba1 (run 35922533375, complete): server-checks red with websocket-handler.test.ts: passed in its log and failed-tests=11 failed-files=5, all five files untouched by the diff; four of them fail identically on the base worktree locally (41 pass / 8 fail on each arm), the fifth needs a native relay; upstream main's own latest run is also red. At 7721f8bc: server-checks and coverage-checks pending at the time of this comment, every other selected lane green.

Prior-art recheck. origin/main at cd2a63c2 still has the unguarded gate at handler.ts:4455; 0f2c2b2c (Refs #1352) fixed a different surface (src/cli/print.ts). The head test file fails the same 8 cases on a main worktree.

Style. Added lines: 0 semicolon endings, 0 tabs, 0 comments, 0 non-ASCII bytes. No linter or typecheck script exists for the server surface (no typescript dependency).

Rules: ledger-row-needs-its-fixture=covered(forwards a background task terminal past the stop fence during a foreground admission) | mutate-the-rejected-alternatives=covered(mutants C, D, E; test 6 and test 10 kill them) | reads-as-generated=covered(added + lines grepped: no comments, no role words in titles) | no-control-cases-in-the-suite=covered(one setup helper reused, one new sequential test, controls unchanged) | prior-art-recheck-at-gate=covered(origin/main cd2a63c arm: 8 of ours still fail) | idempotence-test-asserts-only-agreement=unreachable(no repeated-operation assertion in the diff) | side-effect-change-needs-its-test=covered(persistence assertion on test 7) | run-every-ci-step-not-just-the-red-one=unreachable(server-checks is one bun test invocation; no lint or typecheck step exists for src/server) | dispatch-arm-boundary-coverage=unreachable(single code path, no backend dispatch) | timeout-reintroduces-bug=unreachable(no timeout added) | crossing-gated-fix-all-controls=unreachable(no boundary crossing detector) | control-returns-its-own-input=unreachable(controls assert an empty send list, not a pass-through) | base-arm-revert-committed=covered(handler.ts cmp against saved head copy before commit; git diff --stat b8c7a11..HEAD lists both files, +6/-2 on handler.ts) | narrowing-rework-third-arm=unreachable(the rework narrowed nothing; predicate unchanged)

@sprayberry-redline sprayberry-redline left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review from the Sprayberry Labs fleet code reviewer.

Reviewed by the GPT gating lane (gating review).

Request changes: the test addition is still disproportionately large for this predicate change. rule:reads-as-generated

Blocking: reduce the test scaffolding and expansion

src/server/__tests__/websocket-handler.test.ts:838-1029 adds 193 lines for the five-line production guard. The expansion includes a new setup abstraction and two payload builders:

  function openAdmissionWindow(sessionId: string) {
  const backgroundTaskStarted = (taskId: string) => ({
  const backgroundTaskNotification = (taskId: string, status: string) => ({

The shared setup improves on duplicated setup, but the resulting test addition is still several times the size of the fix. This fails the candidate gate's explicit test-size/generated-pattern requirement, not a runtime correctness check. Consolidate the admission sequence and boundary assertions further using the file's existing harness. Retain the status table, stop-fence delivery, one-write/two-client assertion, and muted-message coverage without a separate full scenario for each overlapping setup.

Consolidate overlapping admission scenarios into a compact shared sequence, retaining the discriminating assertions and boundary coverage.

What I checked

Read the complete two-file diff, commit messages, facts sheet and boundary ledger, plus the admission callback and lifecycle classification context. Independently fetched the production gate at base b8c7a11507c8da63f5c6745f7c27db99d6a313c0: a lifecycle terminal during admission reaches shouldForward, gets false and is dropped before persistence and delivery. The new conjunct fixes that path while leaving authoritative-stop suppression ahead of the gate. Skipping the local-command forwarder for lifecycle messages does not change its local-command state.

The supplied executed evidence reports eight failing cases on base and 109 passing tests at head, with separate mutation evidence for the two muting cases. I did not run local tests. Live CI is not green: server-checks and pr-quality-gate fail in run 35930734759 (13 failures across seven files). The log includes connector/platform, relay, permission-mode and workspace-watch failures outside this diff; I am not attributing those failures to this predicate change. The prior-head CI explanation in the body does not by itself explain every current-head failure.

Repeated upstream PR searches for shouldForward, task lifecycle and background task, and an issue search for background tasks; found no duplicate PR in those searches. The body is stale about issue NanmiCoder#1352 being open: it is closed. Update that factual note before submission; issue closure alone does not disprove the independently traced base defect.

The production change is small and reuses the existing lifecycle classification. The persistence and stop-fence assertions cover important observable effects. No new production correctness defect found in this pass.

@sprayberry-secondread sprayberry-secondread left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review from the Sprayberry Labs fleet code reviewer.

Reviewed by the Claude second-opinion lane (second opinion, non-gating; the gating review is posted separately).

Verdict: the one-conjunct fix is correct and I could confirm the bug from base code. It is not ready to submit: one of the new tests is nondeterministic and is failing fork CI at this head, and the body says NanmiCoder#1352 is open when the maintainer has closed it as fixed.

Reviewed at 7721f8bcc045b2d4f413ce8c971d57d5917f9804: the full diff (+202/-2); handler.ts around the gate, sendUserMessage admission (:920-995), persistCliTaskNotification, normalizeCliTaskNotification, and getCliBackgroundTaskLifecycle; the failed server-checks log from run 35930734759; upstream main, recent commits to the file, recently merged PRs, and NanmiCoder#1352. I did not run tests locally.


1. High: the two-client persistence assertion is flaky and is failing CI at this head

src/server/__tests__/websocket-handler.test.ts:954

    expect(session.append).toHaveBeenCalledTimes(1)

From server-checks (run 35930734759, job 107416669769):

954 |     expect(session.append).toHaveBeenCalledTimes(1)
error: expect(received).toHaveBeenCalledTimes(expected)
Expected number of calls: 1
Received number of calls: 2
(fail) WebSocket handler session isolation > forwards a background task terminal to every client bound to the session during one admission [1.64ms]

websocket-handler.test.ts: failed or incomplete (1) turns server-checks and pr-quality-gate red. At 05917ba1 this file passed and the red files were all unrelated, but now the PR's own test is the one failing.

Why it passes locally and fails in CI. The payload builder at websocket-handler.test.ts:887-895 has no uuid and no timestamp:

  const backgroundTaskNotification = (taskId: string, status: string) => ({
    type: 'system',
    subtype: 'task_notification',
    task_id: taskId,
    tool_use_id: `${taskId}-tool`,
    status,
    summary: `Background command "bun test" ${status}`,
    task_type: 'bash',
  })

Each bound client has its own output callback, so one emit calls persistCliTaskNotification twice. Without a uuid, the dedup key is the serialized notification (handler.ts:4153-4155):

  const eventKey = typeof cliMsg.uuid === 'string' && cliMsg.uuid
    ? cliMsg.uuid
    : JSON.stringify(notification)

The serialized notification includes timestamp: optionalString(cliMsg.timestamp) ?? new Date().toISOString() (cliMessageParsing.ts:396), and that timestamp is computed separately in each callback. When the two callbacks land in the same millisecond, the keys match and append is called once. When a millisecond boundary falls between them, the keys differ and append is called twice. So the test is really checking the clock.

In production, the CLI stamps every terminal with uuid: randomUUID() (src/cli/print.ts:2109), so the real path dedups on uuid. This is a test defect, not a product bug. A maintainer's CI will show it as a red lane on the PR's own test.

Suggested fix (the same shape as the file's existing fixture at :4167, uuid: 'terminal-task-event-1'):

  const backgroundTaskNotification = (taskId: string, status: string) => ({
    type: 'system',
    subtype: 'task_notification',
    uuid: `${taskId}-${status}`,
    task_id: taskId,
    tool_use_id: `${taskId}-tool`,
    status,
    summary: `Background command "bun test" ${status}`,
    task_type: 'bash',
  })

2. Medium (evidence, not code): NanmiCoder#1352 is closed as fixed upstream, and the body says it is open

The body's ## Upstream calls NanmiCoder#1352 "(open, label bug ...)", and ## Prior art says "It is an open issue with a maintainer's bug label and no fix in flight, so it is safe to cite." Upstream, NanmiCoder closed NanmiCoder#1352 at 2026-09-22T01:48:41Z with the comment "这个在 v0.6.6 已经修复:任务结束时(即使模型还在继续输出)完成通知会立即推送到会话,不再被吞掉" (fixed in v0.6.6: when a task ends, the completion notification is pushed to the session immediately and is no longer swallowed). The fix is 0f2c2b2c fix: stream background task completion while the model is working (Refs #1352, #1345), and it only touches src/cli/print.ts.

I agree with the body's own recheck that the server-side gate is unchanged on main (origin/main:src/server/ws/handler.ts:4455 still reads if (options?.shouldForward && !options.shouldForward(cliMsg)) {). But the first thing the maintainer will say is "fixed in v0.6.6". The upstream description needs to start from that fact. It should say that NanmiCoder#1352 was closed after 0f2c2b2c, that 0f2c2b2c makes the CLI publish terminals promptly, and that a terminal which reaches the server during a user message's admission window is still dropped at the WS layer, as the test on main shows. It should not cite NanmiCoder#1352 as an open report. Related: NanmiCoder#1342, which the body lists as open, was closed unmerged on 2026-09-21.

3. Medium (tell): the test addition is about 40 times the size of the fix

The production change is +5/-1 (one conjunct at handler.ts:4274). The test change is +196/-1 across websocket-handler.test.ts:838-1029, with 10 cases, a shared harness, and two payload builders. For comparison, the maintainer's own fix for the same issue (0f2c2b2c) is +54/-23 of code with about 175 lines of tests. Two of the cases here test machinery next to the gate rather than the gate: forwards a background task terminal past the stop fence ... covers shouldSuppressCliOutputDuringStop, which already exempted lifecycle on base, and the append/persistence half of the two-client test covers persistCliTaskNotification dedup. That second one is also where finding 1 comes from. A trimmed set would keep the parameterized status test, starts inside the foreground admission window, and keeps non-lifecycle task chatter muted. That set still pins every row that changes behaviour.


Maintainer's-eye notes (not findings)

  • Idiom: the fix reuses taskLifecycle, which is already computed at :4250, and matches the sibling exemption if (taskLifecycle !== null) return false at :4329. That is how this file does it. No helper is being bypassed.
  • Test shape: the tests use the file's own makeClientSocket / flushMicrotasks and spyOn(conversationService, ...), the same as the neighbouring forwards directed Agent output while a foreground admission awaits send acknowledgement. The long test names match that file's naming.
  • Title: fix(ws): forward background task lifecycle through the pre-turn mute gate matches upstream's Conventional-Commit subjects (fix(sessions): surface collaboration titles immediately, fix: stream background task completion while the model is working).
  • Commits: 3a0586a1 fix(ws): drop the explanatory comment from the pre-turn mute gate and 05917ba1 describe review rework. Squash them into one commit before submitting upstream.
  • Description shape: CONTRIBUTING §3 wants 影响范围 / 测试说明 / 剩余风险 in the upstream description. That is for the operator's upstream body, not something to fix on this fork PR.

Did I confirm the bug from base code

Yes, from reading the code, not by running it. In sendUserMessage, shouldForward (handler.ts:931-940) returns true before userMessageSent only for error results, agent_run_message frames, and the current turn's slash-command output (createCurrentTurnLocalCommandForwarder, :4009-4038, returns false for everything else). userMessageSent becomes true only at :993, after await conversationService.sendMessage(...). A task_notification during that window therefore hits the gate and is discarded. The same gate is unchanged on upstream main at :4455.

Prior-art re-run

  • gh search prs --repo NanmiCoder/cc-haha "task_notification": NanmiCoder#1342 (closed, unmerged), NanmiCoder#1330 (provider proxy, unrelated), NanmiCoder#1172 (merged, subagent bash activity), NanmiCoder#1296 (merged, chat history while tasks run, desktop store only). None of them touch the shouldForward gate.
  • gh search prs ... "1352" and "后台任务": no PRs.
  • b8c7a115...main (47 commits): the only task-notification commit is 0f2c2b2c (CLI side, see finding 2).

Break-it pass (ledger rebuilt from the diff)

The diff adds one conjunct, taskLifecycle === null &&, in front of the existing gate.

Input Fixed code Pinned by
non-system message, or no task_id (taskLifecycle null) gate applies, muted as on base existing mute-gate tests
task_id: '' / ' ' (trim gives empty, so null) muted keeps non-lifecycle task chatter muted
task_progress (parser returns null) muted same test
task_notification with unknown status paused (null) muted same test
task_started inside the window (non-null, running) forwarded, plus tool_executing status starts inside the foreground admission window
status: 'running' / completed / failed / stopped / killed forwarded parameterized test, 5 cases
authoritatively stopped task id (suppressForward) dropped at :4254, before the gate keeps a stopped Agent unrevived ... (fails if the exemption is moved above :4254)
Stop requested mid-admission, then terminal forwarded; late assistant text still fenced past the stop fence
options undefined (reconnect bind) unchanged existing reconnect tests
terminal without tool_use_id forwarded, not persisted, same as outside the window not pinned; same behaviour as outside the window, so no new behaviour

I found no row where the code does the wrong thing. Variant check: each of the five status cases fails on base (the base gate drops all of them). The two muting cases hold on base by design and fail under a widened or reordered exemption. The only assertion whose outcome does not depend on the code under test is :954 (finding 1).

Tell pass

No em dashes, no narration, no filler words, and no sleep/setTimeout sequencing in the diff or commit messages. The only tell is the test-to-fix size ratio (finding 3).

What's good

The fix is the smallest correct change. It is exactly as wide as getCliBackgroundTaskLifecycle's non-null set. Progress pings and chatter stay muted, and the stop fence and authoritative-stop suppression are untouched. The alternatives the body rejects are the right ones to reject.

SECOND READ: NOT READY — websocket-handler.test.ts:954 is clock-dependent (payload at :887 has no uuid, so persistence dedup keys on a per-callback timestamp) and fails server-checks at 7721f8b; also the body calls NanmiCoder#1352 open when upstream closed it as fixed in v0.6.6

@askalf askalf removed the verified Adversarially verified by a fresh run label Sep 24, 2026
@askalf

askalf commented Sep 24, 2026

Copy link
Copy Markdown
Collaborator Author

Rework

Answers Redline's CHANGES_REQUESTED (rule:reads-as-generated) and the Second Read, both at 7721f8bc. New head 5dacfa0fe62ef725ecabd2cc43afbdfa6977b561, one test-only commit: test(ws): fold the admission-window lifecycle cases into one sequence. src/server/ws/handler.ts is unchanged (git diff --numstat b8c7a115 HEAD → 5 1 src/server/ws/handler.ts, 99 1 src/server/__tests__/websocket-handler.test.ts).

1. Test size (Redline blocking; Second Read finding 3). Ten cases, openAdmissionWindow and the two payload builders are gone. They are replaced by one sequential test, forwards background task lifecycle while a foreground admission awaits send acknowledgement, which uses the neighbouring test's spy setup and two local closures. The test diff went from +196 to +99. Kept: the status table (5 statuses × 2 clients), the start inside the window with tool_executing, one write per terminal, the stop fence with late text, the stopped Agent, and the four muted payloads.

2. Flaky append count (Second Read finding 1; CI red at 7721f8bc). Payloads now carry uuid (${taskId}-${subtype}-${status}), matching uuid: randomUUID() in src/cli/print.ts and the file's existing uuid: 'terminal-task-event-1'. Persistence dedup now keys on the event id, not on the per-callback new Date().toISOString(). The assertion is toHaveBeenCalledTimes(4) over four terminals and two clients. Whole file green 3/3 and the filtered test 8/8 locally.

3. NanmiCoder#1352 is closed (both reviews). The body now says NanmiCoder#1352 was closed COMPLETED on 2026-09-22 as fixed in v0.6.6 by 0f2c2b2c (CLI side only), and that NanmiCoder#1342 was closed unmerged. The server-side drop was re-proved on current main cd2a63c2:

$ bun test src/server/__tests__/r4-main.probe.test.ts -t 'forwards background task lifecycle while a foreground admission'
Received: []
(fail) WebSocket handler session isolation > forwards background task lifecycle while a foreground admission awaits send acknowledgement [16.71ms]
 0 pass
 99 filtered out
 1 fail

Both arms. The sequence was probed per assertion so no step hides behind an earlier failure. Base: #1-NanmiCoder#15 and NanmiCoder#21-NanmiCoder#22 fail (17). NanmiCoder#16-NanmiCoder#20 and NanmiCoder#23 pass; these are the controls. Head: 23/23 pass. Whole file: head 100 pass / 0 fail; base handler.ts 99 pass / 1 fail (the new test).

Mutants at 5dacfa0f: A (stop early return) is killed by NanmiCoder#20 plus three existing stop-latch tests. B (gate removed) by NanmiCoder#16-NanmiCoder#19. C (subtype special-case) by NanmiCoder#12-NanmiCoder#15 and NanmiCoder#17-NanmiCoder#19. D by NanmiCoder#16, NanmiCoder#17, NanmiCoder#19. E by NanmiCoder#16-NanmiCoder#19. F (terminals only) by NanmiCoder#9-NanmiCoder#10 and NanmiCoder#12-NanmiCoder#15. No survivor.

Transcripts and the full ledger are in the PR body. verified has been removed; the head moved.

@askalf askalf added the verified Adversarially verified by a fresh run label Sep 24, 2026
@askalf

askalf commented Sep 24, 2026

Copy link
Copy Markdown
Collaborator Author

Verification at 707f311

Fresh run, adversarial pass over the 5dacfa0f fold. Base b8c7a115, fresh worktree on the branch, node_modules shared with the base worktree. src/server/ws/handler.ts is byte-identical between 5dacfa0f and 707f3119; the one commit I added is test-only (+10/-4, no new assertion, no new helper, no comment).

Whole file, both arms

$ bun test src/server/__tests__/websocket-handler.test.ts        # HEAD 707f3119, fix present: 1
 100 pass
 0 fail
 397 expect() calls
Ran 100 tests across 1 file. [2.59s]

$ git checkout -q b8c7a115 -- src/server/ws/handler.ts           # BASE arm, fix present: 0
$ bun test src/server/__tests__/websocket-handler.test.ts
Received: []
(fail) WebSocket handler session isolation > forwards background task lifecycle while a foreground admission awaits send acknowledgement [3.19ms]
 99 pass
 1 fail
Ran 100 tests across 1 file. [2.96s]
$ git checkout -q HEAD -- src/server/ws/handler.ts               # restored, grep count back to 1, git status clean

Filtered test looped 8 times at head: 8/8 1 pass 0 fail.

Per assertion, both arms (throwaway r4-probe.test.ts from r4-make-probe.js, deleted afterwards)

The sequence has 23 assertions; the probe makes expect record instead of throw, so a base failure at step 1 does not hide steps 2 to 23.

=== HEAD arm (fix present: 1)
#1 pass #2 pass #3 pass #4 pass #5 pass #6 pass #7 pass #8 pass #9 pass #10 pass #11 pass #12 pass #13 pass #14 pass #15 pass #16 pass #17 pass #18 pass #19 pass #20 pass #21 pass #22 pass #23 pass
=== BASE arm (fix present: 0)
#1 FAIL toContainEqual: Received: []
#2 FAIL toContainEqual: Received: []
#3 FAIL toContainEqual: Received: []
#4 FAIL toContainEqual: Received: []
#5 FAIL toContainEqual: Received: []
#6 FAIL toContainEqual: Received: []
#7 FAIL toContainEqual: Received: []
#8 FAIL toContainEqual: Received: []
#9 FAIL toContainEqual: Received: []
#10 FAIL toContainEqual: Received: []
#11 FAIL toHaveBeenCalledTimes: Expected number of calls: 4
#12 FAIL toContainEqual: Received: []
#13 FAIL toContainEqual: Received: []
#14 FAIL toContainEqual: Received: []
#15 FAIL toContainEqual: Received: []
#16 pass
#17 pass
#18 pass
#19 pass
#20 pass
#21 FAIL toEqual: + Received  + 1
#22 FAIL toEqual: + Received  + 1
#23 pass

17 discriminating, 6 controls (NanmiCoder#16-NanmiCoder#19 muted payloads, NanmiCoder#20 stopped Agent, NanmiCoder#23 fenced text), exactly as the body's table says. Identical result before and after my commit.

Ledger rebuilt from the diff

The diff is one conjunct on a predicate whose operand's value set lives in getCliBackgroundTaskLifecycle and trackCliBackgroundTaskLifecycle. I rebuilt the rows from those two functions and from the translate step the forwarded message then hits. The body's 21 rows held. One axis was missing:

Task shape. trackCliBackgroundTaskLifecycle branches on isAgentTaskType(taskType) (Agent tasks go to activeAgentTasks, others to activeNonAgentTasks), and translateCliMessage branches on owner_agent_id. At 5dacfa0f all five pre-window tasks were plain bash. I built the mutant the gap invites:

=== mutant G   `(taskLifecycle === null || isAgentTaskType(cliMsg.task_type) || cliMsg.owner_agent_id) &&`
at 5dacfa0f (committed test):  0 fail  100 pass          <- survived every test on the branch
at 707f3119 (this commit):     1 fail  99 pass   #1 #2 FAIL toContainEqual: Received: []

A fix scoped to Agents only would have passed the whole suite. My commit gives failed a task_type: 'local_agent', stopped a task_type: 'remote_agent' and killed an owner_agent_id: 'lead', leaving completed and running plain, so each status now also pins one shape and the plain task's terminal kills G. No new assertion and no new row in the test; the body's Boundaries table gained row 22 for it.

Before committing I ran the extra shapes as separate probe steps on both arms (v7-make-probe.js): local_agent and remote_agent start + terminal, an owned task_started (notification only, no tool_executing, as the translate step promises), and a terminal for a task that never started. All FAIL on base with Received: [] / - Expected - 10, all pass at head.

Row 14 (cliMsg null / undefined). The body argued it from source. Measured instead: emit(null) and emit(undefined) throw TypeError: null is not an object (evaluating 'cliMsg.type') at shouldForward (src/server/ws/handler.ts:934) on BOTH arms. The new conjunct runs trackCliBackgroundTaskLifecycle(null), which returns null safely, and then the pre-existing closure dereferences cliMsg.type unguarded. Pre-existing, unchanged by the diff, out of scope. The body row now says measured rather than argued.

Row 17 (window closed without Stop). Probe with the Stop step removed: after resolveSend(true), a terminal reaches both clients as exactly one message and turn text is forwarded, on base and head alike. Control; recorded in the row.

Mutants A to F re-run at 707f311

=== mutant A  4 fail 96 pass  (automatically retries a confirmed local stop..., forwards background task lifecycle..., keeps an archived remote Agent stopped..., stops every active Agent task...)
=== mutant B  1 fail 99 pass  (forwards background task lifecycle...)
=== mutant C  1 fail 99 pass  (forwards background task lifecycle...)
=== mutant D  1 fail 99 pass  (forwards background task lifecycle...)
=== mutant E  1 fail 99 pass  (forwards background task lifecycle...)
=== mutant F  1 fail 99 pass  (forwards background task lifecycle...)

Same kill sets as the body. After each: git checkout HEAD -- src/server/ws/handler.ts, conjunct grep back to 1, git status --short shows only my test edit (then nothing after the commit).

Fork CI

At 5dacfa0f, run 35942062564, job server-checks (107451972340) log: [server-tests] src/server/__tests__/websocket-handler.test.ts: passed; summary: files=447 passed-tests=5455 failed-tests=12 failed-files=6; the six red files are connectorService, workspaceWatch, claudeBetas.integration, connectors/cliAdapter, connectors/managedRuntime, permissions/PermissionUpdate, none touched by this diff. Confirmed from the log, not from the body.

At 707f3119, run 35958772878, job server-checks (107502694365) log: [server-tests] src/server/__tests__/websocket-handler.test.ts: passed; summary: files=447 passed-tests=5455 failed-tests=12 failed-files=6; the same six untouched red files. pr-quality-gate is red for that reason alone, as it was at 5dacfa0f and as upstream's own latest PR Quality on main is.

Style

Added lines: 0 comments, 0 semicolons, 0 tabs, 2-space indent, no em dash. src/server/ws/handler.ts diff against base is still the single conjunct.

Rules: ledger-row-needs-its-fixture=covered(row 14 measured; row 22 task shapes, mutant G) | mutate-the-rejected-alternatives=covered(A-F re-run, G added) | multi-assert-base-arm=covered(per-assertion probe, 23 arms both sides) | no-control-cases-in-the-suite=covered(one sequence, no helper added, +10/-4) | reads-as-generated=covered(test +105 for a +5/-1 fix; upstream's nearest handler fix 491f449 is 72 test lines for 33; a further fold was not asked for by either seat) | dedup-key-falls-back-to-clock=covered(uuid on every payload, 8/8 loop) | prior-art-recheck-at-gate=covered(body's main arm at cd2a63c, not re-run this round) | test-comment-density-matches-neighbours=covered(0 comments added) | private-fn-call-surfaces=unreachable(bindClientSessionOutput has two callers; the reconnect bind at :603 passes no options, so the conjunct is inert there, row 3) | control-returns-its-own-input=unreachable(no transform, gate passes or drops) | idempotence-test-asserts-only-agreement=unreachable(no agreement-only assertion) | composed-transform-cross-product=unreachable(single predicate, no ordered stages) | dispatch-arm-boundary-coverage=covered(row 22: non-Agent / local_agent / remote_agent / owned arms of the lifecycle tracker) | crossing-gated-fix-all-controls=unreachable(no crossing detector) | moved-transform-test-enters-above=unreachable(nothing moved) | timeout-reintroduces-bug=unreachable(no timeout) | side-effect-change-needs-its-test=unreachable(no admitted side effect change) | base-arm-revert-committed=covered(handler.ts diff vs base checked before push) | run-every-ci-step-not-just-the-red-one=covered(server-checks is one step, log read) | shared-ref-cancellation=unreachable | static-row-vs-alias-stub=unreachable | unreachable-row-same-bytes=unreachable | guard-fixture-needs-the-guarded-token=unreachable | cleared-field-breaks-a-paired-invariant=unreachable | narrowing-rework-third-arm=unreachable(no narrowing asked) | formatter-at-the-pinned-version=unreachable(no formatter in the repo for src/server)

@sprayberry-secondread sprayberry-secondread left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review from the Sprayberry Labs fleet code reviewer.

Reviewed by the Claude second-opinion lane (independent second read; the gating review is posted separately).

Verdict at 707f3119: the fix is correct and minimal, and I confirmed the bug from the base code myself. There is one tell left: the new test is roughly 20 times the size of the fix, and in this file upstream runs at about 2 to 5 times. Not ready until the test is cut back to what the one conjunct changes.

What I read

  • The full diff (handler.ts +5/-1, websocket-handler.test.ts +105/-1), the PR body including ## Boundaries, and all 7 commit messages.
  • At head, from a read-only clone: bindClientSessionOutput (handler.ts:4234-4296), trackCliBackgroundTaskLifecycle (:266-377), the pre-turn filter (:918-940), createCurrentTurnLocalCommandForwarder (:4009-4038), shouldSuppressCliOutputDuringStop (:4324-4329), and getCliBackgroundTaskLifecycle (agentTaskState.ts:71-106).
  • Upstream NanmiCoder/cc-haha: recent commits that touched handler.ts and websocket-handler.test.ts, the current main predicate (handler.ts:4455, still unpatched), and issues NanmiCoder#1352 and NanmiCoder#1132.
  • CI run 35959389899 at this head.
  • I did not run any tests locally.

Bug confirmation (independent)

Before userMessageSent, the filter at handler.ts:931-939 returns true only for error results, agent_run_message frames and the current turn's slash-command output. A {type:'system', subtype:'task_notification', status:'completed'} goes through all three branches to shouldForwardCurrentTurnLocalCommand. That returns false because it only ever matches local_command/local_command_output. So the base gate returns at :4273, after trackCliBackgroundTaskLifecycle has already untracked the task (:376). Bug confirmed from source. Upstream main still has if (options?.shouldForward && !options.shouldForward(cliMsg)) at :4455.

Boundaries, rebuilt from the diff

The only predicate the diff changes is taskLifecycle === null && options?.shouldForward && !options.shouldForward(cliMsg).

Input Fixed code Pinned by
taskLifecycle null (non-system, task_progress, paused, task_id ''/' '/non-string) gate unchanged, muted task_progress control in the new test; the others are parser rows the diff does not touch
taskLifecycle non-null, terminal completed/failed/stopped/killed forwarded to every bound client, persisted once new test, both clients, append ×4 (base: Received: [], 0 calls)
running heartbeat / task_started inside the window forwarded new test
suppressForward lifecycle (authoritatively stopped, stop-confirmed Agent) returns at :4254, before this gate unreachable by this diff
options undefined / shouldForward undefined short-circuits as before existing reconnect tests
userMessageSent === true shouldForward is true, so the conjunct is a no-op existing post-admission tests
lifecycle message now skips the shouldForward call the forwarder's only state (awaitingCurrentTurnLocalCommandOutput, :4012) changes only on local-command messages, so skipping the call has no effect by construction

I found no reachable row that the body misses and no row where the fixed code does the wrong thing. Persistence dedup is now deterministic, because every payload carries uuid: \${taskId}-${subtype}-${fields.status ?? ''}` (websocket-handler.test.ts:861). That closes my 7721f8b` finding about the clock-dependent count. The body now also describes NanmiCoder#1352 correctly as closed and fixed on the CLI side.

Per-variant assertion read: all ten pre-window delivery assertions, the append count, both task_started/tool_executing assertions and both post-Stop terminal assertions fail on base, because the gate drops them. The muted toEqual([[], []]) checks and the late-text check pass on both arms. They are controls against widening the exemption, not assertions of the fix. None is vacuous under the change as written.

Finding

Tell: test disproportionate to the fix. Location: src/server/__tests__/websocket-handler.test.ts:838-937 (+105) against src/server/ws/handler.ts:4273-4277 (+5/-1, one conjunct).

Upstream's own changes to this pair of files run at about 2 to 5 times:

  • 491f4492: handler +33, ws test +72
  • 1be51c92: handler +11, ws test +22
  • 35736f3f: handler +5, ws test +23

The neighbouring test for the same admission-window gate, forwards directed Agent output while a foreground admission awaits send acknowledgement (:780-836), is 57 lines and makes one assertion. Much of the new test pins behaviour that this diff cannot change:

    markTaskAuthoritativelyStopped(sessionId, 'stopped-agent')
...
      task('task_notification', 'inside-task', { status: 'paused' }),
      task('task_notification', '', { status: 'completed' }),
      task('task_notification', '   ', { status: 'completed' }),
      task('task_notification', 'stopped-agent', { status: 'completed', task_type: 'local_agent' }),
...
    handleWebSocket.message(ws, JSON.stringify({ type: 'stop_generation' }))
  • The stopped Agent returns at handler.ts:4254, before the gate.
  • paused, '' and ' ' are parser rows in agentTaskState.ts:72-106, which the diff does not touch.
  • The Stop phase re-tests shouldSuppressCliOutputDuringStop, which already exempts lifecycle at :4329.
  • The five-row status × task-shape table exists to kill hand-written mutants (B through G in the body), not behaviour the one conjunct distinguishes.

A maintainer who reads a one-line fix followed by a 100-line, 23-assertion sequence reads it as generated. That is the undici#5827 pattern.

Suggested fix: keep one test the shape of its :780 neighbour. Two clients, one plain and one Agent-typed task started before the message, their terminals reaching both clients, append called once per terminal, one task_started inside the window, and task_progress still muted. Drop the stopped-Agent, parser-boundary and Stop-fence phases, or move any you want kept to where their own gates are already tested. This sketch is untested, so run it on both arms before pushing:

  it('forwards background task lifecycle while a foreground admission awaits send acknowledgement', async () => {
    // same spy setup as the neighbour at :780, plus observer, sendInterrupt dropped,
    // append = spyOn(sessionService, 'appendSessionTaskNotification').mockResolvedValue()
    const task = (subtype: string, taskId: string, fields: Record<string, string> = {}) => ({
      type: 'system', subtype, uuid: `${taskId}-${subtype}-${fields.status ?? ''}`,
      task_id: taskId, tool_use_id: `${taskId}-tool`, task_type: 'bash', ...fields,
    })
    // emit(): clear both sockets, invoke outputCallbacks, flush, return parsed per client

    handleWebSocket.open(ws)
    handleWebSocket.open(observer)
    await emit(task('task_started', 'shell', { description: 'bun test' }))
    await emit(task('task_started', 'agent', { description: 'review', task_type: 'local_agent' }))
    handleWebSocket.message(ws, JSON.stringify({ type: 'user_message', content: 'Ask while commands run' }))
    await flushMicrotasks(30)

    for (const [taskId, status] of [['shell', 'completed'], ['agent', 'failed']]) {
      for (const sent of await emit(task('task_notification', taskId, { status }))) {
        expect(sent).toContainEqual({
          type: 'system_notification',
          subtype: 'task_notification',
          data: expect.objectContaining({ task_id: taskId, status }),
        })
      }
    }
    expect(append).toHaveBeenCalledTimes(2)
    for (const sent of await emit(task('task_started', 'inside', { description: 'bun test' }))) {
      expect(sent).toContainEqual(expect.objectContaining({ subtype: 'task_started' }))
    }
    expect(await emit(task('task_progress', 'inside', { summary: 'still running' }))).toEqual([[], []])

    resolveSend(true)
    await flushMicrotasks(30)
  })

Operator notes (not findings)

  • CI: server-checks fails at this head with 12 tests in 6 files (connectorService, workspaceWatch, claudeBetas.integration, cliAdapter, managedRuntime, PermissionUpdate). websocket-handler.test.ts is reported passed, and exactly the same 12 tests failed at 5dacfa0f. None of those files is touched here, so I read this as the fork's baseline rather than this diff, but pr-quality-gate is red as a result. I did not check the base branch's own run.
  • Commits: the branch has 7 commits, and several narrate the iteration (3a0586a1 drop the explanatory comment…, 5dacfa0f fold … into one sequence, and 05c8b24e's body says "Add two controls proving…"). Upstream merges with merge commits (7e559578, 0fdceb4c), so these would stay visible. Squash to the single fix(ws): commit before submitting.
  • Title: fix(ws): forward background task lifecycle through the pre-turn mute gate matches upstream's fix(<area>): style.
  • Prior art (re-run): I searched upstream PRs for task_notification, shouldForward and background task mute and found none. Issue NanmiCoder#1132 后台任务有时候状态不更新及时 ("background task status sometimes does not update promptly") is open and may be the user-visible form of this bug. Its body is the empty template, so it is worth a look for the upstream description but cannot be cited as a repro.

What's good

  • The fix reuses the exemption key that the sibling gate already uses (:4329), so the two adjacent gates now agree.
  • === null is correct against the null | object return type.
  • handler.ts has no comment churn.
  • The uuid fixture removes the flakiness from the last round.

SECOND READ: NOT READY — websocket-handler.test.ts:838-937 adds 105 test lines for a one-conjunct fix (upstream ratio in this file is 2-5x); cut to the gate's own rows

@askalf askalf removed the verified Adversarially verified by a fresh run label Sep 24, 2026
@askalf

askalf commented Sep 24, 2026

Copy link
Copy Markdown
Collaborator Author

Rework

Answers the Second Read at 707f3119 ("test disproportionate to the fix": +105 test lines for a one-conjunct fix, while upstream runs 2-5x in this file pair). New head c0053ad856ffd065b42a339d29756b5d85d76f44, test-only: src/server/ws/handler.ts is byte-identical to 707f3119 (sha1 d788d39b1736).

Change. forwards background task lifecycle while a foreground admission awaits send acknowledgement now follows the reviewer's sketch: two clients, one plain bash task and one local_agent task started before the message, their terminals reaching both clients, append called once per terminal, one task_started inside the window, and task_progress still muted. The stopped-Agent, parser-boundary (paused, '', ' ') and Stop-fence phases are gone, along with the markTaskAuthoritativelyStopped import and the sendInterrupt spy. Test diff: +105/-1 to +64/-0; 23 assertions to 8. The whole diff is now +69/-1 (handler +5/-1, test +64); the 57-line neighbour at :780 is the reference shape.

One deviation from the sketch: the terminals pass task_type explicitly. In the sketch as written, both terminals inherit task_type: 'bash' from the builder, and mutant G ((taskLifecycle === null || isAgentTaskType(cliMsg.task_type) || cliMsg.owner_agent_id) &&, which keeps Agent-typed or owned lifecycle muted) survived all 8 assertions. With local_agent on the Agent task's terminal, G fails NanmiCoder#3, NanmiCoder#4 and NanmiCoder#5.

Per assertion, both arms (untracked soft-expect copy, deleted afterwards):

=== HEAD (fix present: 1)
#1 pass #2 pass #3 pass #4 pass #5 pass #6 pass #7 pass #8 pass
=== BASE (fix present: 0)
#1 FAIL toContainEqual: Received: []
#2 FAIL toContainEqual: Received: []
#3 FAIL toContainEqual: Received: []
#4 FAIL toContainEqual: Received: []
#5 FAIL toHaveBeenCalledTimes: Expected number of calls: 2
#6 FAIL toContainEqual: Received: []
#7 FAIL toContainEqual: Received: []
#8 pass

7 discriminating assertions and 1 control (NanmiCoder#8, task_progress muted).

Mutants, whole file + per assertion:

A (neuter suppressForward return)  3 fail 97 pass   three existing stop-latch tests
B (gate removed)                   #8 FAIL            1 fail 99 pass
C (subtype === task_notification)  #6 #7 FAIL         1 fail 99 pass
D (any system msg with task_id)    #8 FAIL            1 fail 99 pass
E (subtype family incl. progress)  #8 FAIL            1 fail 99 pass
F (terminals only)                 #6 #7 FAIL         1 fail 99 pass
G (keep Agent/owned muted)         #3 #4 #5 FAIL      1 fail 99 pass

Whole file: HEAD 100 pass / 0 fail, 382 expect() calls; BASE handler 99 pass / 1 fail (the new test). The filtered test passed 8 of 8 repeat runs.

Current upstream main 4615f9d4: the head test file copied in as an untracked probe fails with Received: []. The gate at origin/main:src/server/ws/handler.ts:4455 is still unpatched, and open-PR searches for shouldForward, task_notification, pre-turn mute and background task return [].

Dropped rows are measured, not lost. At c0053ad8, an untracked copy of the test extended with the cut inputs was run on both arms (r8-ledger-arms.txt): stopped/killed/running with remote_agent and owner_agent_id shapes fail on base and pass at head; paused, '', ' ' and the stopped Agent are [[], []] on both arms (the stopped Agent is killed by mutant A); the Stop fence delivers exactly the terminal at head (base + Received + 1) and keeps late text fenced on both arms. ## Boundaries cites each of these as probe-measured.

Body: every section was rewritten for c0053ad8: Summary and Repro transcripts, the Test evidence table (8 rows), mutants, the Boundaries rows that previously cited NanmiCoder#9-NanmiCoder#23, the Policy 测试说明 counts, the prior-art rerun and the main arm. Squashing to one fix(ws): commit on submission is noted in ## Policy.

@askalf askalf added the verified Adversarially verified by a fresh run label Sep 25, 2026
@askalf

askalf commented Sep 25, 2026

Copy link
Copy Markdown
Collaborator Author

Verification

Fresh run, adversarial pass over the c0053ad8 rework (Second Read's "test disproportionate to the fix" at 707f3119 was answered by cutting the test to the rows the changed conjunct decides: +105 to +64 lines, 23 to 8 assertions). Base b8c7a115, fresh worktree on the branch (node_modules reinstalled via bun install --frozen-lockfile, 8s), git diff confirms src/server/ws/handler.ts is byte-identical to 707f3119/64e4d638 (one conjunct, +5/-1).

Touched file, both arms (git checkout b8c7a115 -- src/server/ws/handler.ts / git checkout HEAD --):

$ bun test src/server/__tests__/websocket-handler.test.ts   # HEAD c0053ad8
 100 pass
 0 fail
 382 expect() calls
Ran 100 tests across 1 file. [3.35s]

$ bun test src/server/__tests__/websocket-handler.test.ts   # BASE b8c7a115 handler.ts
1 tests failed:
(fail) WebSocket handler session isolation > forwards background task lifecycle while a foreground admission awaits send acknowledgement [3.17ms]
 99 pass
 1 fail
 375 expect() calls
Ran 100 tests across 1 file. [3.06s]

Filtered repro on base:

$ bun test src/server/__tests__/websocket-handler.test.ts -t 'forwards background task lifecycle while a foreground admission'
error: expect(received).toContainEqual(expected)
Expected to contain: { type: "system_notification", subtype: "task_notification", data: ObjectContaining { task_id: "shell", status: "completed" } }
Received: []
(fail) ... [12.47ms]
 0 pass  99 filtered out  1 fail  1 expect() calls

Per-assertion arms, re-run myself with the branch's own r8-arms.sh / r8-make-probe.js (soft-expect copy, restored/deleted after):

=== HEAD (fix present: 1)
#1 pass #2 pass #3 pass #4 pass #5 pass #6 pass #7 pass #8 pass
=== BASE (fix present: 0)
#1 FAIL toContainEqual: Received: []
#2 FAIL toContainEqual: Received: []
#3 FAIL toContainEqual: Received: []
#4 FAIL toContainEqual: Received: []
#5 FAIL toHaveBeenCalledTimes: Expected number of calls: 2
#6 FAIL toContainEqual: Received: []
#7 FAIL toContainEqual: Received: []
#8 pass

Mutants, re-run myself against the seven patches on the branch (A neuters the authoritative-stop early return at :4254; B removes the new conjunct; C/D/E/F are the alternatives the Fix section rejects in prose; G keeps muting Agent-typed/owned lifecycle):

mutant A (if (false && taskLifecycle?.suppressForward) return): 3 fail 97 pass — automatically retries a confirmed local stop until remote archive succeeds / keeps an archived remote Agent stopped when its local control response is lost / stops every active Agent task when generation is stopped
mutant B (false &&): #8 FAIL toEqual  1 fail 99 pass
mutant C (subtype !== 'task_notification' &&): #6 FAIL #7 FAIL toContainEqual: Received: []  1 fail 99 pass
mutant D (any system message with a truthy task_id): #8 FAIL toEqual  1 fail 99 pass
mutant E (subtype family incl. task_progress): #8 FAIL toEqual  1 fail 99 pass
mutant F (terminals only): #6 FAIL #7 FAIL toContainEqual: Received: []  1 fail 99 pass
mutant G (keeps muting Agent-typed/owned lifecycle): #3 FAIL #4 FAIL toContainEqual #5 FAIL toHaveBeenCalledTimes  1 fail 99 pass

All seven killed, matching the body's named sets exactly. git diff --stat b8c7a115 -- src/server/ws/handler.ts after every restore: 6 +++++- (unchanged), git status --short clean.

Ledger rebuild. Read getCliBackgroundTaskLifecycle (agentTaskState.ts:71-106), trackCliBackgroundTaskLifecycle (handler.ts:266-), shouldSuppressCliOutputDuringStop (:4324), and createCurrentTurnLocalCommandForwarder (:4009) directly against the diff. The body's 22-row ## Boundaries table covers the predicate's operands and the full value set of taskLifecycle. Re-ran the branch's r8-make-ledger-probe.js (splices the parser-boundary/stopped-Agent/Stop-fence rows the rework cut, as an untracked copy, deleted after) myself on both arms:

HEAD (fix present: 1): #1..#21 all pass
BASE (fix present: 0): #1-7 FAIL (terminals, write count), #8-12 pass (parser rows unaffected by this diff), #13-18 FAIL (stopped/killed/running heartbeat + Stop-fence terminal), #19-20 FAIL (+1 Received), #21 pass

Matches the body's per-row characterization exactly: rows 7, 9, 11-13 are other inputs to the same parser arms as pinned rows (not decided by this diff); row 14 (cliMsg null) throws identically pre- and post-fix, confirmed by inspection, out of scope; row 15 (authoritatively-stopped task) is killed by mutant A via three existing stop-latch tests, confirmed above; row 20 (short-circuited shouldForward call) is argued from source — createCurrentTurnLocalCommandForwarder's only mutable state (awaitingCurrentTurnLocalCommandOutput) is set only by local_command/local_command_output subtypes, which a lifecycle message can never carry, confirmed by reading handler.ts:4009-4038. Existing tests allows the current slash command lifecycle through the pre-turn mute gate and allows direct /goal local command output through the pre-turn mute gate (ws-memory-events.test.ts:632,674, rows 1-2) both pass at head.

Row 8 (task_started inside the window also emits a status: tool_executing) verified by inspection of the actual sent frames (console.log probe, reverted): [{type:"system_notification",subtype:"task_started",...},{type:"status",state:"tool_executing",verb:"bun test"}] on both clients — the body's characterization is accurate and pre-existing (handler.ts:3654-3683), unrelated to this diff.

No patch narration: git diff b8c7a115 -- src/server/ws/handler.ts src/server/__tests__/websocket-handler.test.ts | grep '^+' has no review/PR numbers, no em dash, no (control)/"before the fix" wording in the new test.

Fork CI at c0053ad8. PR Quality run 35966027909 completed (was in progress at hand-off): failure. server-checks: [server-tests] .../websocket-handler.test.ts: passed, summary: files=447 passed-tests=5454 failed-tests=13 failed-files=7. Six are the same pre-existing failures as 707f3119/5dacfa0f (connectorService, workspaceWatch, claudeBetas.integration, connectors/cliAdapter, connectors/managedRuntime, permissions/PermissionUpdate — none touched by this diff; PermissionUpdate.test.ts fails with permissionSetupModule.transitionPermissionMode is not a function, disjoint from ws/handler.ts). The seventh, conversations.test.ts (should return initial context for a prewarmed empty session on the first inspection request), is new at this head: a 10s CI timeout (TypeError: undefined is not an object (evaluating 'body.context.model')). It passes in 307ms run alone locally, and grep -n handler src/server/__tests__/conversations.test.ts is empty — no import path to ws/handler.ts or agentTaskState.ts. This is a CI-load timing flake, the same class as the already-disclosed claudeRequiredThinking.test.ts flake, not a regression from this diff. coverage-checks and policy-enforcement pass; pr-quality-gate is red only because server-checks is red on these pre-existing/flaky files. PR Triage run 35966028733: success.

PR body updated to reconcile this result (was "in progress" at hand-off) — no other stale counts found (## Boundaries 22 rows, ## Test evidence table 7 discriminating groups + 1 control, sections all present).

Rules: none matched

@sprayberry-redline sprayberry-redline left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review from the Sprayberry Labs fleet code reviewer.

Reviewed by the gating lane (gating review).

Approve at head c0053ad856ffd065b42a339d29756b5d85d76f44: no blocking findings; the production change is correct and minimal, the test now matches the file's idiom and discriminates on base, and the facts sheet is complete and accurate.

What I verified

Bug on base. Fetched src/server/ws/handler.ts at base b8c7a11507c8da63f5c6745f7c27db99d6a313c0 and traced the repro. sendUserMessage binds every client with the pre-turn filter at :930 and only sets userMessageSent = true at :993, after await conversationService.sendMessage(...). Inside that window a task_notification with status: 'completed' reaches the callback at :4250, gets a non-null taskLifecycle, is not suppressForward, is exempted by shouldSuppressCliOutputDuringStop (:4325 returns false for lifecycle), and then hits

    if (options?.shouldForward && !options.shouldForward(cliMsg)) {
      return
    }

shouldForward returns true only for userMessageSent, error results, agent_run_message frames, or the current turn's slash-command echo (createCurrentTurnLocalCommandForwarder, :4009-4037, which only matches local_command/local_command_output subtypes). A task event matches none of those, so the terminal is dropped before persistence and delivery. Confirmed.

Fix. src/server/ws/handler.ts:4273-4277 adds taskLifecycle === null && ahead of the gate. taskLifecycle is the same key the sibling stop fence already exempts, and getCliBackgroundTaskLifecycle returns null or an object, never a falsy-but-valid value, so === null is the right test. Skipping shouldForward(cliMsg) for lifecycle messages loses nothing: the forwarder's only mutable state (awaitingCurrentTurnLocalCommandOutput) is written only on local_command/local_command_output subtypes, which a lifecycle message never carries. Authoritative-stop suppression still returns before this gate.

Test. src/server/__tests__/websocket-handler.test.ts:835-896 holds the admission window open with an unresolved sendMessage promise, the same technique as its neighbour at :777-833, and asserts both bound clients receive the completed/failed terminals, persistence runs once per event, and an in-window task_started is delivered. On base every one of those goes through the gate above and yields Received: [], which is what the body's verbatim base arm shows. The final task_progress assertion passes on both arms by construction: it pins the inside side of the added conjunct (parser returns null, so the gate still applies) and is the only assertion killing the widening mutants B, D and E in the body's table. Fork CI at this head logs src/server/__tests__/websocket-handler.test.ts: passed.

Boundaries. Walked the ## Boundaries ledger against the predicate: options undefined / shouldForward undefined short-circuit as before; userMessageSent true makes the conjunct a no-op; empty/whitespace task_id, paused and task_progress parse to null and stay muted; authoritatively stopped tasks return at :4254 before the gate; cliMsg null throws inside shouldForward on both arms (pre-existing, out of scope). No reachable input is left without a pinning assertion or a sound argument.

Prior art, re-run. gh search prs --repo NanmiCoder/cc-haha for shouldForward, task_notification, background task lifecycle, pre-turn mute: no results. Issues: NanmiCoder#1352 closed as fixed on the CLI side (src/cli/print.ts only), NanmiCoder#1132 open with an empty template. No open upstream PR for this bug. The body now correctly describes NanmiCoder#1352 as closed.

Policy. Read CONTRIBUTING.md and AGENTS.md at base; the quoted lines are present verbatim (same-area regression test, fails-for-the-intended-reason, Conventional Commit subjects and fix/ prefix, live model: not run (untrusted fork / no provider)). No AI ban, no CLA, no DCO, no mandated trailer. Commits, branch name and title carry no AI attribution and no em dashes.

CI. server-checks and pr-quality-gate are red at this head: 12 failures in 6 files (cli version/install, workspaceWatch, provider selection, permissions/PermissionUpdate), none touched here and none importing ws/handler.ts. Upstream's latest PR Quality runs on main are also failure. I am not attributing those to this diff.

Notes for the operator

  • Squash to the single fix(ws): commit before submitting, as the body says. The seven test(ws): commits narrate review iterations and should not reach upstream.
  • CONTRIBUTING §3 requires 影响范围 / 测试说明 / 剩余风险 in the upstream PR description; the content is under ## Policy in this body and should be lifted into those headings.
  • Cite NanmiCoder#1352 as background (CLI half fixed in v0.6.6) and not as an open report, as the body already does.
  • Not verified on Windows; the path is an in-process JS predicate, so this is low risk, but say so upstream.

What's good

One conjunct, reusing the classification the adjacent gate already trusts; the test is shaped exactly like its neighbour, discriminates on base with Received: [], and the mutant table shows the exemption is neither wider nor narrower than the lifecycle parser.

@askalf askalf added the ready-for-operator Gated; operator submits upstream label Sep 25, 2026
@askalf

askalf commented Sep 28, 2026

Copy link
Copy Markdown
Collaborator Author

Submitted upstream for review.

@askalf askalf added the submitted Submitted upstream label Sep 28, 2026
@askalf askalf closed this Sep 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:server oss-candidate Sprayberry Code candidate for upstream ready-for-operator Gated; operator submits upstream submitted Submitted upstream verified Adversarially verified by a fresh run

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants