E2E / verification report (M5c physical stop)

The engine is inert by design here (registration is M6), so "E2E" means the real-process harness: a paired Bridge spawning a real Managed qwen --acp child from source, which spawns the real managed-runtime-worker, running real Shell process groups.

Baseline discrimination (pre-implementation, M5a at 1a5aae80d9). The pre-existing process suite passed 5/5 on baseline; the two new F witnesses failed on it (Tests 2 failed | 5 passed) — the witnesses pin M5c behavior, not environment.

On this branch.

npm run typecheck exit 0; CLI build exit 0; eslint/prettier clean on touched files.

Mutation-witness evidence (replayed on this branch's base). Three witnesses are flip-pinned, i.e. their assertions fail when the mechanism is removed:

• two-sided ±120 s group-window with no leader check → the stale sweep SIGKILLs an unrelated young group: resolves a group whose live member is provably younger than its record goes red;
• no worker-side identity → waitForGroupExit settles against a recycled pgid's liveness: the hour-old-record witness goes red returning alive;
• naive resolve of the ambiguous shape → the leaderless-hold witness goes red: a group that might be session-backgrounded work would be silently dropped from the ledger.

Witness-selection finding to know while reading F. On POSIX the worker's Shells run on a PTY, so a killed worker takes its foreground Shell group down via the kernel's hangup — verified on baseline (~100 ms, no M5c code). Process death alone therefore does not discriminate M5c; the F witnesses assert the ledger lifecycle (written during the Shell run, names the group, gone after the sweep). Where the PTY model does not reach (TERM-ignoring members with a fast-exiting leader at cancel, Windows taskkill, anything the kernel won't hang up) the ledger is the mechanism, pinned by the unit sweeps and the cancel-evidence tests.

Not validated here. Windows real-machine behavior (unit doubles only; required before M6 per the design); the setsid/double-fork daemon residual; the spawn-to-pid-callback window. The acceptance criteria this maps to are in the design's "Physical stop (M5c)" section.

Real-environment verification — M5c physical stop @ 6cb680dca3

Merge reference: supports merging. I reproduced every M5c claim end to end with a real daemon, a real Managed child, its real Runtime worker and real PTY Shell process groups, as an A/B against the merge-base 2c591ecc08, on macOS arm64 and Linux arm64. Base leaks the process in every crash shape. Head stops it in every crash shape, or holds it and quarantines the engine when it cannot date the group. The cancel evidence and the quarantine/lift cycle behave exactly as the design describes.

None of the items below blocks merging, since the engine stays inert until M6. Two of them change what a user or operator will see once M6 enables the engine, and four mutants that survive the PR's own suites have consequences I could measure in the real stack. Those are worth settling before M6 relies on this code.

How it was run

• Trees: base = merge-base 2c591ecc08, head = 6cb680dca3, each built with npm run build && npm run bundle.
• Stack: each tree's own createServeApp (packages/cli/dist) with experimentalPairedEngines and the Managed engine injected through the managedExecutionEngine seam. That seam stands in for M6's registration and is the only simulated piece. The children are the shipped bundle dist/cli.js --acp --acp-execution-engine managed, and the Runtime worker is whatever that child launches. A scripted fake OpenAI model drives run_shell_command.
• Leftover: a group member that ignores SIGTERM and SIGHUP:

  nohup sh -c 'trap "" TERM HUP; exec perl -e "sleep 300"' &
  

  Neither the cancel path's TERM nor the PTY hangup reaches it. A plain sleep dies of the hangup on both arms, as noted in the earlier E2E comment, so it cannot tell the arms apart.
• Hosts: macOS (two full passes per scenario per arm; the ranges below cover both) and Linux arm64 (Armbian jammy, kernel 5.10, rk3588, Node 24.13; one pass per arm). Linux reproduced every macOS A/B difference in C1–C7. That covers the ⚠️ next to Linux in the PR's Tested-on table.

On head, every run wrote exactly one <incarnation>.json (244 bytes on macOS) while the Shell ran. It names the Shell leader's pid as the group and records hostPid as the Managed child, and it is on disk before the call settles. Base never writes a ledger.

Items for the author (none blocks merging)

1. Cancelling a command with a TERM-resistant member now blocks the session, even after the stop has been proven (C1).

• What happens: shellExecutionService cancels by sending TERM to the group and escalates to SIGKILL only while the leader is still alive. A member that ignores TERM and HUP therefore outlives the 10 s evidence budget. The call becomes unknown, the host blocks the session and stops the worker, and the worker stop SIGKILLs the member. The ledger is deleted at 10.14 s, before the client even receives the turn_error at 10.21 s. From then on every turn is refused with managed_runtime_outcome_unknown.
• Why it matters: this is exactly what the design says ("a group that outlives the budget makes the call's outcome unknown: the session blocks, the host stops the worker"). In product terms, though, cancelling something like a nohup'd service with a slow shutdown costs the user the whole session.
• Two options, either one enough:
  • SIGKILL the recorded group once the TERM grace has passed, while its identity is still fresh, so the cancel settles cancelled with proof.
  • Keep unknown, but lift the block once the close-time sweep proves the group gone.
• What works as intended: the budget itself. In C1b, a member with a 3 s graceful shutdown gets an honest cancelled at 3.16 s. Base reports cancelled at 0.16 s while that member is still running.

2. An unreadable ledger quarantines new Managed sessions until someone deletes it by hand, and the client is told something unrelated (C6).

• What happens: a 0-byte or truncated *.json under <projectTempDir>/managed-runtime/ makes every new session return 409 at 2 s, 10 s and 30 s. Each reaper tick re-reads the file and throws again. Deleting the file lifts the quarantine within 1.6–3.7 s. This is the end state triage finding pre-release: fix ci #1 worried about. It is real, though it arises by a different route (see below).
• Operator-facing gaps:
  • For a new session, the client sees 409 {"error":"This session cannot be resumed with the current execution engine.", "errorKind":"session_execution_engine_unavailable"}.
  • The actual reason and the file path appear only in the Managed child's stderr: -32024 The Managed engine is quarantined: … deadbeef-empty.json cannot be read; nothing it held can be proven.
• Suggestion: the design already leaves Bridge/daemon propagation to M6. It would still help to record in the M5c section that an unreadable ledger holds the quarantine until it is removed, and how to remove it, and to have M6's refusal name the ledger.

3. Four mutants survive the PR's suites, and each has a real-stack consequence. I ran 33 single-line mutants against the suites that own the code; 26 were killed. For W8, E3 and A2 I measured the consequence by mutating a copy of the shipped bundle and rerunning the same rig. Candidate tests for W8, E3 and L7 are below; each passes on head and is the only failure under its mutant.

The other three survivors:

• C1 (derived Config.reportManagedEngineQuarantine no longer delegates): the core test reports on the root Config and only clears through a derived one. Today's product passes the root Config, so this is low risk; calling deriveConfig(config).reportManagedEngineQuarantine(reason) in the test would pin it.
• L12 (write without temp + rename): atomicity only shows under a crash mid-write, so this survivor is expected.
• W9 (own ledgers not added to the skip set): masked by holdsForLiveHost, since the live child is the ledger's host. Effectively equivalent.

On the specific check triage stage 2 asked for: removing the waitForGroupExit block (E1) is killed, but only by the doubled "outlives the evidence budget" test. The named real-process test settles a cancel only once the whole process group is gone still passes under E1, because its single-process group dies together with its leader. C1b is the real-process discriminator.

diff --git a/packages/cli/src/serve/managed-runtime-session-worker.test.ts b/packages/cli/src/serve/managed-runtime-session-worker.test.ts
--- a/packages/cli/src/serve/managed-runtime-session-worker.test.ts
+++ b/packages/cli/src/serve/managed-runtime-session-worker.test.ts
@@ -1150,6 +1150,61 @@ describe.skipIf(process.platform === 'win32')(
       expect(() => process.kill(pid!, 0)).toThrow();
     });
 
+    it("quarantines the engine while an earlier child's ledger stays unproven", async () => {
+      const previous = process.env['QWEN_RUNTIME_DIR'];
+      process.env['QWEN_RUNTIME_DIR'] = path.join(root, 'runtime');
+      try {
+        const sweeperConfig = new Config({
+          sessionId: '11111111-2222-3333-4444-666666666666',
+          targetDir: root,
+          cwd: root,
+          debugMode: false,
+          model: 'test-model',
+          usageStatisticsEnabled: false,
+          telemetry: { enabled: false },
+          deferTelemetryInitialization: true,
+        });
+        const report = vi
+          .spyOn(sweeperConfig, 'reportManagedEngineQuarantine')
+          .mockImplementation(() => undefined);
+        const lift = vi
+          .spyOn(sweeperConfig, 'clearManagedEngineQuarantine')
+          .mockImplementation(() => undefined);
+        const ledgerDir = path.join(
+          sweeperConfig.storage.getProjectTempDir(),
+          'managed-runtime',
+        );
+        await mkdir(ledgerDir, { recursive: true });
+        // Debris the startup sweep can neither trust nor resolve.
+        const workFile = path.join(ledgerDir, 'unreadable.json');
+        await writeFile(workFile, '', 'utf8');
+        environment = createManagedRuntimeEnvironment(sweeperConfig, () => ({
+          command: process.execPath,
+          args: [script],
+          env: { ...process.env, FAKE_MODE: 'ok', FAKE_LOG: logFile },
+        }));
+        const deadline = Date.now() + 10_000;
+        while (report.mock.calls.length === 0) {
+          if (Date.now() > deadline) throw new Error('never quarantined');
+          await new Promise((resolve) => setTimeout(resolve, 50));
+        }
+        const reason = report.mock.calls[0]![0];
+        expect(lift).not.toHaveBeenCalled();
+
+        // Once the debris is gone the reaper proves the directory and lifts
+        // the quarantine with the reason it raised.
+        await rm(workFile, { force: true });
+        while (lift.mock.calls.length === 0) {
+          if (Date.now() > deadline) throw new Error('never lifted');
+          await new Promise((resolve) => setTimeout(resolve, 50));
+        }
+        expect(lift).toHaveBeenCalledWith(reason);
+      } finally {
+        if (previous === undefined) delete process.env['QWEN_RUNTIME_DIR'];
+        else process.env['QWEN_RUNTIME_DIR'] = previous;
+      }
+    });
+
     it('sweeps the stale worker ledgers of an earlier child when it starts', async () => {
       const previous = process.env['QWEN_RUNTIME_DIR'];
       process.env['QWEN_RUNTIME_DIR'] = path.join(root, 'runtime');
diff --git a/packages/cli/src/serve/managed-runtime-tool-executor.test.ts b/packages/cli/src/serve/managed-runtime-tool-executor.test.ts
--- a/packages/cli/src/serve/managed-runtime-tool-executor.test.ts
+++ b/packages/cli/src/serve/managed-runtime-tool-executor.test.ts
@@ -195,6 +195,32 @@ describe.skipIf(process.platform === 'win32')(
       strayGroups.delete(pgid);
     });
 
+    it('close() SIGKILLs a member that outlived the cancel and removes the ledger', async () => {
+      // The leader dies of the cancel's TERM, so no SIGKILL escalation runs;
+      // the member ignores TERM and HUP. Only close()'s own kill reaches it.
+      const exec = executor(300);
+      const member = `"${process.execPath}" -e 'process.on("SIGTERM",()=>{});process.on("SIGHUP",()=>{});setInterval(()=>{},1000)'`;
+      const input = {
+        command: `${member} & ${LONG_RUN}`,
+        description: 'leader with a TERM-ignoring member',
+      };
+      const running = exec.execute(
+        reference('call-4b', input),
+        'run_shell_command',
+        input,
+      );
+      void running.catch(() => undefined);
+      const pgid = await recordedGroup();
+      strayGroups.add(pgid);
+      // Let the member start and install its handlers.
+      await new Promise((resolve) => setTimeout(resolve, 500));
+
+      await exec.close();
+      expect(processGroupLiveness(pgid)).toBe('gone');
+      expect(existsSync(ledgerFile)).toBe(false);
+      strayGroups.delete(pgid);
+    });
+
     it('leaves a Write outside the ledger and settles without evidence', async () => {
       const exec = executor();
       const input = {
diff --git a/packages/cli/src/serve/managed-runtime-ledger.test.ts b/packages/cli/src/serve/managed-runtime-ledger.test.ts
--- a/packages/cli/src/serve/managed-runtime-ledger.test.ts
+++ b/packages/cli/src/serve/managed-runtime-ledger.test.ts
@@ -807,6 +807,46 @@ describe('Managed Runtime ledger', () => {
       expect(existsSync(workFile)).toBe(true);
     });
 
+    it('resolves a recycled worker pid that a younger Runtime worker now holds', async () => {
+      // The recorded worker started an hour ago; its pid now answers for a
+      // `managed-runtime-worker` that is seconds old: another session's
+      // live worker on a recycled id. The marker alone must not get it killed.
+      const workFile = path.join(root, 'ledger.json');
+      testInternals.writeLedgerDocument(
+        workFile,
+        {
+          pid: 101,
+          pgid: 101,
+          incarnation: 'i',
+          startedAt: Date.now() - 3_600_000,
+        },
+        [],
+      );
+      const signal = vi.fn(() => 'sent' as const);
+      await sweepWorkerLedger(workFile, {
+        proofTimeoutMs: 300,
+        sys: {
+          platform: 'linux',
+          liveness: (id) => (id === 101 ? 'alive' : 'gone'),
+          signal,
+          table: () =>
+            new Map([
+              [
+                101,
+                {
+                  pid: 101,
+                  pgid: 101,
+                  runningMs: 5_000,
+                  args: 'node dist/cli.js managed-runtime-worker',
+                },
+              ],
+            ]),
+        },
+      });
+      expect(signal).not.toHaveBeenCalled();
+      expect(existsSync(workFile)).toBe(false);
+    });
+
     it('does not hold a ledger whose named host is younger than the worker', async () => {
       // A process younger than the worker it is named as parenting is an
       // impostor on a recycled pid: the orphaned worker belongs to the sweep.

4. Minor (C3). When the Managed child dies and the group has a TERM-resistant member, the orphaned worker lingers 10.1–10.4 s before its killOutstanding, because its close() first waits out the cancel-evidence budget for the aborted call. Cleanup does complete. Signalling before waiting would make it immediate.

Triage findings re-checked

• pre-release: fix ci #1 (torn write from concurrent sweeps): the in-process mechanism does not reproduce.
  • writeLedgerDocument is writeFileSync + renameSync with no await between them, so two sweeps in one process cannot interleave inside it. In 3 × 400 witnessed sweeps of one ledger, all overlapping in their proof wait (the ledger held a root-owned group, so SIGKILL got EPERM and every sweep rewrote the file), the final file was always readable.
  • Across processes, the fixed .tmp name is not safe. With two processes writing documents of different lengths, about 3 % of concurrent reads (3.1–3.3 %) saw an unreadable file and 20–25 % of renames hit ENOENT. In 40 of 40 trials the final file still parsed, so the worst case is a transient quarantine that the next reaper tick lifts. The only product path I found with two writers is two Managed hosts on one project sweeping the same dead ledger, so a per-writer staging name is still a cheap hardening.
  • The permanent outcome (item 2) can be reached by other routes, for example a 0-byte file left by a power loss without fsync.
• Where is the config saved? #2 (witnessed sweep without an identity check): narrow, and a wedged worker does not widen it. In C7 the worker was SIGSTOPped after its Shell leader exited. The leader stayed a zombie that holds the pgid (kill(-pgid,0) returns EPERM on macOS and succeeds on Linux; either way the id is not free). So the pgid cannot be recycled behind a stale entry while the worker is wedged. A bystander kill needs the reap, a new group leader reusing the pgid and the worker's death all inside the ≤1 s prune window, and that window depends on the prune being wired up (A2 above). Whether the design sentence should be softened is the author's call.
• 如何自定义密钥文件 .env可能与其他文件冲突 #3: confirmed. The age check is load-bearing and no test covers it (L7 above).

Tests on the head tree

Not covered

Windows (unit doubles only, as the PR states); the setsid/double-fork residual; the spawn-to-pid-callback window; Bridge/daemon quarantine propagation (M6).

Review round 1 — all 34 findings addressed

Commit 4aa2aa50da implements every finding of the round-1 review (CHANGES_REQUESTED, 6 Critical + 28 Suggestion), each thread replied to individually and resolved.

Critical.

Suggestions (28). Unreadable ledgers older than the debris age retire aside instead of quarantining forever; proof budgets are per-group and concurrent; holdsForLiveHost probes a process pid (EPERM holds, never sweeps a sibling); .tmp unlink joins per-entry containment; reaper backs off to 30 s; the 1 Hz watchdog prunes by liveness (no standing ps); startup-sweep failure branch, executor/attestation/config/configParams wiring, skip-set debris, denied liveness, waitForGroupExit budget-fail, and the foreign-pid identity branch all have witnesses whose named mutants go red (five physical flips run); test leaks, dup blocks, and the two flaky assertions are repaired; Windows unwitnessed-hold contract is stated in code comments and the design, both languages; the M5c section and table now enumerate all three accepted holes.

Verification on the fix commit: ledger 44, executor 6, session-worker 70 (a 4-case byte-identical dup removed), process 7, attestation 26, env-guard 3, dispatch-error+error-response 104, config 510, acpAgent 843, core managed-session-log 53 — all pass; npm run typecheck, CLI build, and eslint on every touched file are clean.

@qwen-code /takeover

🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. Remove the autofix/takeover label (or comment @qwen-code /takeover stop) to release.

✅ AutoFix round 6 finished — view run. See this round's report below.

🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 1/100 轮）。改动内容与我反驳保留之处如下：

Address review round — M5c worker ledger (round 3: verification repair)

This round's actionable input was the deterministic verification rejection of the round-2 commit (ed94543f2f, preserved): the full packages/cli suite failed two tests. The round-2 review findings themselves were already implemented by that commit; this round adds exactly one follow-up commit (b3fdaa3ed6) that repairs the rejection.

Same-run verification repair — root cause and fix

The rejection. npm run test in packages/cli failed:

• src/commands/review/test-efficacy.integration.test.ts > refuses the reverse-apply on a filter planted after the restore screened — expect(existsSync(stamp)).toBe(true): the git PATH shim never armed.
• src/ui/utils/terminal-image-renderer.test.ts > caches a render so resize and restore do not re-spawn chafa — expected 'unavailable' to be 'ansi': the fake chafa never rendered.

Root cause (reproduced, not guessed). Both tests write an extensionless #!/usr/bin/env node executable into os.tmpdir() whose body uses require()/__dirname__. This runner's /tmp/package.json contains "type": "module", so Node loads any extensionless script under /tmp as an ES module and the shim crashes with ReferenceError: require is not defined in ES module scope before doing anything. Proven three ways:

• Direct probe of the shim outside vitest: crashes with the ESM error under /tmp, runs cleanly from a directory pinned to CommonJS.
• Unfixed tests + polluted tmpdir (/tmp): both fail — the gate's exact failures.
• Unfixed tests + clean tmpdir (/var/tmp, no up-tree package.json): both pass (71/71) — the tests are correct wherever no up-tree package.json interferes; the defect is purely environmental.

Fix. Each directory that receives a POSIX node shim now also gets a one-line {"type":"commonjs"} package.json, pinning the module interpretation the shims were written for. No assertions changed, no test weakened — the "the shim really armed" guard now genuinely exercises the plant again. Three files, +20 lines:

• test-efficacy.integration.test.ts — pin in the git shim dir.
• terminal-image-renderer.test.ts — pin in the chafa shim dir.
• mermaidImageRenderer.test.ts — same latent defect class: its createFakeMmdc/createFakeChafa helpers inline require() bodies into extensionless POSIX shims and failed 13 tests in this same environment; the pin now lives in the two helpers, covering every shim they write. Harmless no-op on a clean tmpdir (CommonJS is the default there).

Review-feedback dispositions

• Round 1 (34 findings) — all fixed in 4aa2aa50da with per-thread replies already posted; re-verified present in the tree this round. Threads listed for resolution.
• Round 2 (28 findings) — all six Criticals and 21 Suggestions were implemented by the preserved ed94543f2f (sweep verdicts proven/absent/held/retired, witnessed sweeps consulting the live table, the boot-epoch guard, addGroup deriving uptimeMs from the caller's startedAt, the liftable-quarantine activation reset, the client-safe 503 summary, waitForGroupExit dropping only the record it judged, close() skipping reaper-owned paths, the doc realignments, and the new witnesses). Spot-checked each against the source this round; threads listed for resolution.
• Still deferred: R2-13's fifth site (the exit-hook sweep catch in sweepDeadWorkerLedger) — a deterministic failure there needs a module mock this real-process test file deliberately avoids; replied on its thread (rc:4179037848) and left open.
• Maintainer verification notes (ic:5979364447, non-blocking): the W8/E3/L7 witness concerns are covered by the round-1/round-2 witnesses (startup-sweep quarantine report/lift, close-time killOutstanding survivor case, stepped-clock worker-identity case).

Mutation probes

Verification

• npx vitest run src/ui/utils/terminal-image-renderer.test.ts src/commands/review/test-efficacy.integration.test.ts (packages/cli) — 71 passed (pre-fix: 2 failed, the gate's exact failures).
• npx vitest run src/ui/utils/mermaidImageRenderer.test.ts (packages/cli) — 25 passed (pre-fix: 13 failed in this environment).
• npm run test (packages/cli, full suite, CI-like env: agent QWEN_*/SANDBOX vars scrubbed, writable HOME, the polluted /tmp left in place) — 38595 passed, 92 skipped, 0 failed (1196 files passed, 1 skipped).
• npm run build — passed.
• npm run typecheck — passed.
• npm run lint — passed (full repo).
• npx prettier --check on the three touched files — passed.
• Not run here: the Windows/macOS lanes (the pins are platform-neutral; both fixed tests are already POSIX-gated).

Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。

🧵 Resolved all 61 selected review thread(s). · 已关闭全部选中的 61 条评审线程。

Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。

🧠 Handled by Qwen Code · model/模型 kimi-k3 · CLI 0.24.7

🔀 Base updated: red check(s) [review-pr] pass on current main — merged current main via update-branch; CI will re-run.

Real-environment verification, round 2 — 6afdecc622

Follow-up to round 1 (6cb680dca3). Since then the PR has had three review rounds: 4aa2aa50da, then the autofix commits ed94543f2f/b3fdaa3ed6, then a merge of main (6afdecc622). Base for the A/B below is the new merge-base 69d060e24c.

Merge reference: the mechanism still holds, but hold the merge until the three open Criticals are fixed or explicitly deferred to a tracked pre-M6 follow-up. All three (R2-1, R3-1, R3-2) reproduce end to end in a real daemon → Managed child → Runtime worker → PTY Shell stack. Two of them end in SIGKILLs the ledger has no right to send: one hits an unrelated process, the other a live sibling session's worker.

Reproducing them also surfaced a fourth problem in the same deployment shape as R3-2. When the ACP host is PID 1 (the repo's Dockerfile ends CMD ["qwen"] with no init), any worker crash leaves the engine quarantined for good.

None of this is reachable before M6 registers the engine. The round-1 A/B results hold on both macOS and Linux, and most of my round-1 items are addressed.

The open Criticals, reproduced

All four rows ran on Linux arm64 (Armbian jammy, kernel 5.10, rk3588, Node 24.13), as root, inside a private pid + mount namespace:

• PID 1 is a 20-line C reaper.
• /bin/ps is bound to a wrapper that fails while a flag file exists.
• Recycling works by writing /proc/sys/kernel/ns_last_pid, which hands a freed pgid to an unrelated setsid process (it took 1 try every time).
• An unprovable group comes from a Shell command: perl forks a child that exits at once, then moves itself to its own group and never reaps it. The zombie keeps the Shell's group answering kill(-pgid,0), and killing that parent frees it.

Nothing outside the namespace is touched. Each row has a control arm in which the code behaves correctly.

On the new PID-1 row:

• Why it happens: the proofs treat a zombie as alive. kill(-pgid,0) succeeds on Linux and returns EPERM on macOS, so the group never reads as gone.
• Who is exposed: any host that adopts orphans without reaping them.
• Ways out (either works): count an all-zombie group as gone, using the stat column of the table you already read, or require an init and document --init/tini for the image.

A/B on the new head (macOS and Linux)

Round-1 items: status

1. Cancel blocks the session (C1): unchanged. This is by design, and it is still the trade-off I'd revisit. The worker stop proves the kill about 100 ms before the client sees the error, yet the session stays blocked.

2. Unreadable ledger (C6): improved, with one asymmetry to know. The refusal is now 503 managed_engine_quarantined with a message (for C5 it names the ledger file; for an unreadable one it says only "a ledger it could not read"). After ~60 s the file is set aside as .unreadable. What happens next depends on whether any session is open (macOS):

   • No session open (C6b): the daemon starts a fresh child per attempt, and that child finds nothing, so new sessions are admitted from ~60 s on.
   • A session open (C6c): the reaper's terminal verdict keeps the quarantine. It returned 503 at 75, 90 and 120 s with the file already set aside, and lifted only when the last session closed and the child restarted.

   The design says "stands until the child restarts", which matches C6c. An operator, though, sees no file left to explain the refusal.

3. Test gaps: E3, A2 and W8 are now pinned. L7 still survives: the autofix note says the stepped-clock case covers it, but removing isLedgerWorker's age check leaves every suite green. My round-1 candidate test still applies cleanly: it passes on this head and is the only failure under L7 (58 tests, 1 failed).

4. Orphaned worker lingers about 10 s (C3): unchanged (10.2 s).

Tests and mutation

38 of 43 mutants killed. The set is the 33 from round 1 (five re-anchored to the rewritten code) plus 10 on this round's fixes. L*, E*, A* and N6–N10 ran on macOS; W*, G*, C* and N1–N5 ran on Linux arm64, where a kill counts only through a relevant test (see the flaky test below).

• Newly pinned since round 1: E3, A2 and W8, plus every new mutant except N3. That covers retire, terminal-never-lifts, hostPid, env scrub, the boot clock, 503 mapping, backoff, close-skip and pid-callback containment.
• Survivors:
  • L7 — see item 3.
  • N3 — the per-ledger reaper's if (verdict === 'proven') return 'proven'. This is the very branch whose absence in the startup reaper is R3-1, and no test covers it. My rig could not discriminate it either: with the branch removed from the bundle, the R2-1 control still lifted, through the same in-flight proof.
  • C1 — as in round 1.
  • L12 — expected; atomicity only shows under a crash mid-write.
  • W9 — masked by holdsForLiveHost.

Suites on the head:

• macOS: every test file the PR touches is green — ledger 57, executor 10, session-worker 72, process 7, attestation 26, env-guard 3, dispatch-error 33, error-response 72, config 510, acpAgent 844, Session 1141, core 53, plus the three files autofix fixed. One combined run of all 15 files timed out 8 attestation and real-process tests under a load average near 100. Those two files re-run alone passed 33/33, twice.
• Linux arm64: serve suites 172/172 and core 52 plus 1 root-skipped, in the runs whose mutant no test sees (N3, C1). acpAgent failed only on the relevant tests in G1–G3.
• Flaky on Linux: ManagedToolExecutor physical stop › fails the call and kills the group when the ledger cannot record it asserts 'gone' the moment execute returns, while the callback's waitForGroupExit is not awaited. The killed leader can still be an unreaped zombie. It failed 1 of 10 runs alone on the unmodified head, and 6 of 8 under the parallel suites.

Not covered

• Windows.
• The setsid/double-fork residual.
• The spawn-to-pid-callback window.
• The R3-1 trigger with a table that never fails: it needs a slow or failing ps on a retry. A /bin/ps that times out under load would give the same.

Round 5 disposition — a4d72394e0

All three merge-reference items from the round-5 report are now landed and pushed.

1. R1-37's opposite hole — 1fb61b97bf. An undatable etime no longer drops the row: ProcessTableRow.runningMs is now number | undefined, and the row still counts as a member — so a group whose only live member carries the wrapped spelling judges 'unknown' (held unproven, never silently resolved 'gone', never signalled by an unwitnessed sweep), while a witnessed sweep still SIGKILLs it. An undatable worker row returns 'unknown'; an undatable host row holds. The probe the report suggested is the shipped fix (the row is kept as an unknown-age member). Witnesses: the parser keeps undatable rows with their age undefined; an unwitnessed sweep holds such a group (no signal, no unlink — discriminates against the row-drop mutation, which turns it red); a witnessed sweep signals it.

2. Review 5437392272 — 57273cda0b. [P1] A worker the sweep cannot prove stopped now keeps its ledger file to itself: the read-merge-write window is deleted entirely, so no durable addGroup can be lost behind a live writer. [P1] The directory reaper attributes names per file (namedByFile): a vanished, never-judged ledger answers only from what its own failures ever named — unreadable-at-every-read ⇒ terminal, matching the single-ledger sibling (the exact daemon timeline from the report: A deleted, B proves out beside it, 503 holds). [P2] The final re-read adopts a same-pgid replacement record (new call, new stamps), so a young replacement group is never judged 'recycled' against the stamps of the group its pgid used to name. All three threads are replied with the witness names and resolved.

3. Witnesses — 8668a5875f. The five the report asked for, each proven red under its reverting mutation and green restored:

• R1-12 — three outstanding groups cost exactly one blocking-ps consult across the fanout (plus the prune's own; a counted execFileSync mock makes it exact: 2, mutant: 4).
• R1-48 — a 10 s backward wall step inside the first proof poll leaves the 600 ms budget at ~0.7 s measured (mutant: ~10.6 s). The deadline lives on performance.now() at the same three sites.
• R1-45 — close() joins the sweep the exit hook is running over the same ledger: one recorded pass for two triggers (the overlap is scripted via a hold in the module mock, not raced).
• R1-8 — two environment creations in one window produce one directory-sweep pass (counted at the sweepStaleLedgers entry point, with an existsSync guard so a zero-pass count can't green the assertion vacuously).
• R1-5 — armed on garbage, named on a valid held-group ledger, silenced on garbage again, then the ledger vanishes and the group dies: only the accumulated name lifts (replace-instead-of-accumulate ends terminal and never lifts).

Root-runner note: the R1-36 write-fail witness no longer uses chmod(555) — it fails writes through the existing writeFileSync mock, so it fails on root exactly like everywhere else, per the file header's own note.

Still surviving mutants, parked rather than silently dropped: R1-1 (shared .tmp name + debris regex for the new names), R1-6 (provenFiles drop), the two R1-5 sawRetired sites, and R1-29's release-rethrow — which the report itself measured as unreachable dead code. Witnesses for the first three are follow-up candidates; R1-29's line can't execute, so a mutant there is unreproducible by construction.

Verification on a4d72394e0 (merge with origin/main, +14 commits, zero conflicts; symmetric marker greps show no silent loss; effective diff read in full: the same 26-file slice): npm run build and npm run typecheck clean; ledger suite 84/84; session-worker suite 97/97.

⏳ Historical-head review — head moved to a4d72394e014f43bf2caf6f73171fa2411e26b8a while this review was in flight (past the salvage threshold), so the run finished and posted against the head it reviewed: 940287f96bc7d1deda9bada751b7dceab9af36d1 (#10110). The next automatic review covers the delta from that anchor. Full log in the workflow run.

Review round R2 — disposition summary

All 7 threads of the historical-head review on 940287f96b are replied and resolved. R2-2's finding was already shipped at 1fb61b97bf (the undatable-row rule from round 5) — the review anchored the pre-fix head; the other six fixed in 74acebcfc0:

• R2-1 — a liftable quarantine refusal is translated once, at the throw site in bindAndRelease (both arms), so the restore path classifies it retryable instead of working_directory_compromised. The create path passes the already-translated error through via a narrow guard instead of rewrapping.
• R2-3 — both StandaloneSessionServiceError status ladders (acp-http dispatch, server error-response) map managed_engine_quarantined to 503, so the service surface satisfies the same 503=retry-later convention the raw child refusal already gets; retryable and Retry-After unchanged.
• R2-5 — an unreadable ledger in an aggregate no longer erases the names and counts of its siblings: concrete entries keep basename and group count, the unreadable clause is appended; unreadable-only is the clause alone, never ghost.json: 0 process group(s).
• R2-4 / R2-7 / R2-8 — the three coverage gaps now have their witnesses: the failed-launch inline sweep (launch attestation failure, group dies, file unlinks), the launchedLedgerPaths wiring (a second environment keeps the first live worker and its ledger untouched), and the unreadable wording branch of the refusal summary.

Every witness is verified red under its exact reverting mutation (raw-rethrow at the two arms, 503-tier removal in each ladder, inline-sweep deletion, empty skip set, unreadable-clause removal) and green on restore. Suites on the wave: standalone 135, both mapper files 107, session worker 99, acpAgent 850 — all green; build/typecheck/eslint clean. Census now: 61 resolved / 13 open, the 13 all being dev-bot deferrals open by design.

@qwen-code /triage

Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run

Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check.

Scripted assertions: 81 passed · 0 failed · 81 total

Flakiness gate: ⚠️ timeout — the 15-minute budget elapsed before two full rounds completed (1 done) — no flakiness signal either way

AssertionError: expected 'alive' to be 'gone'
  ❯ src/serve/managed-runtime-ledger.test.ts:2605  expect(processGroupLiveness(shell)).toBe('gone');

FAIL  … > ManagedToolExecutor physical stop > makes the call unknown when the group outlives the evidence budget
AssertionError: promise resolved "{ executionStatus: 'cancelled', …(1) }" instead of rejecting

M5 (unproven worker's ledger never written) : AssertionError: expected [ 401 ] to deeply equal [ 401, 402 ]
M6 (ps elapsed day-digit cap)               : AssertionError: expected 38109073018720000 to be undefined

if (entry.version === 3 && entry.captureSink) { return (invocation as ShellToolInvocation).execute(…, entry.captureSink); }  // :2087, no setPidCallback
if (entry.toolName === ShellTool.Name && this.options.ledger) { … ledger.addGroup({ pgid: pid, … }) … }                     // :2098

rounds=5 files=13 skipped=0
file packages/cli/src/acp-integration/acpAgent.test.ts: (cd packages/cli) npx --no-install vitest run ./src/acp-integration/acpAgent.test.ts
file packages/cli/src/acp-integration/session/Session.test.ts: (cd packages/cli) npx --no-install vitest run ./src/acp-integration/session/Session.test.ts
file packages/cli/src/config/config.test.ts: (cd packages/cli) npx --no-install vitest run ./src/config/config.test.ts
file packages/cli/src/serve/acp-http/dispatch-error.test.ts: (cd packages/cli) npx --no-install vitest run ./src/serve/acp-http/dispatch-error.test.ts
file packages/cli/src/serve/conversations/standalone-session-service.test.ts: (cd packages/cli) npx --no-install vitest run ./src/serve/conversations/standalone-session-service.test.ts
file packages/cli/src/serve/managed-runtime-attestation-worker.test.ts: (cd packages/cli) npx --no-install vitest run ./src/serve/managed-runtime-attestation-worker.test.ts
file packages/cli/src/serve/managed-runtime-ledger.test.ts: (cd packages/cli) npx --no-install vitest run ./src/serve/managed-runtime-ledger.test.ts
file packages/cli/src/serve/managed-runtime-session-worker.process.test.ts: (cd packages/cli) npx --no-install vitest run ./src/serve/managed-runtime-session-worker.process.test.ts
file packages/cli/src/serve/managed-runtime-session-worker.test.ts: (cd packages/cli) npx --no-install vitest run ./src/serve/managed-runtime-session-worker.test.ts
file packages/cli/src/serve/managed-runtime-tool-executor.test.ts: (cd packages/cli) npx --no-install vitest run ./src/serve/managed-runtime-tool-executor.test.ts
file packages/cli/src/serve/process-env-guard.test.ts: (cd packages/cli) npx --no-install vitest run ./src/serve/process-env-guard.test.ts
file packages/cli/src/serve/server/error-response.test.ts: (cd packages/cli) npx --no-install vitest run ./src/serve/server/error-response.test.ts
file packages/core/src/config/managed-session-log.test.ts: (cd packages/core) npx --no-install vitest run ./src/config/managed-session-log.test.ts


per-file results (P=pass F=fail I=infra-exit, one letter per run):
  packages/cli/src/acp-integration/acpAgent.test.ts: PP
  packages/cli/src/acp-integration/session/Session.test.ts: PP
  packages/cli/src/config/config.test.ts: PP
  packages/cli/src/serve/acp-http/dispatch-error.test.ts: PP
  packages/cli/src/serve/conversations/standalone-session-service.test.ts: PP
  packages/cli/src/serve/managed-runtime-attestation-worker.test.ts: PP
  packages/cli/src/serve/managed-runtime-ledger.test.ts: PP
  packages/cli/src/serve/managed-runtime-session-worker.process.test.ts: PP
  packages/cli/src/serve/managed-runtime-session-worker.test.ts: P
  packages/cli/src/serve/managed-runtime-tool-executor.test.ts: P
  packages/cli/src/serve/process-env-guard.test.ts: P
  packages/cli/src/serve/server/error-response.test.ts: P
  packages/core/src/config/managed-session-log.test.ts: P

verdict: timeout
summary: the 15-minute budget elapsed before two full rounds completed (1 done) — no flakiness signal either way

--- per-invocation detail (full copy in the artifact) ---
round 1 · packages/cli/src/acp-integration/acpAgent.test.ts: P (exit 0)
round 1 · packages/cli/src/acp-integration/session/Session.test.ts: P (exit 0)
round 1 · packages/cli/src/config/config.test.ts: P (exit 0)
round 1 · packages/cli/src/serve/acp-http/dispatch-error.test.ts: P (exit 0)
round 1 · packages/cli/src/serve/conversations/standalone-session-service.test.ts: P (exit 0)
round 1 · packages/cli/src/serve/managed-runtime-attestation-worker.test.ts: P (exit 0)
round 1 · packages/cli/src/serve/managed-runtime-ledger.test.ts: P (exit 0)
round 1 · packages/cli/src/serve/managed-runtime-session-worker.process.test.ts: P (exit 0)
round 1 · packages/cli/src/serve/managed-runtime-session-worker.test.ts: P (exit 0)
round 1 · packages/cli/src/serve/managed-runtime-tool-executor.test.ts: P (exit 0)
round 1 · packages/cli/src/serve/process-env-guard.test.ts: P (exit 0)
round 1 · packages/cli/src/serve/server/error-response.test.ts: P (exit 0)
round 1 · packages/core/src/config/managed-session-log.test.ts: P (exit 0)
round 2 · packages/cli/src/acp-integration/acpAgent.test.ts: P (exit 0)
round 2 · packages/cli/src/acp-integration/session/Session.test.ts: P (exit 0)
round 2 · packages/cli/src/config/config.test.ts: P (exit 0)
round 2 · packages/cli/src/serve/acp-http/dispatch-error.test.ts: P (exit 0)
round 2 · packages/cli/src/serve/conversations/standalone-session-service.test.ts: P (exit 0)
round 2 · packages/cli/src/serve/managed-runtime-attestation-worker.test.ts: P (exit 0)
round 2 · packages/cli/src/serve/managed-runtime-ledger.test.ts: P (exit 0)
round 2 · packages/cli/src/serve/managed-runtime-session-worker.process.test.ts: P (exit 0)

Evidence images

Harness scripts and raw logs are in the workflow run artifacts (7-day retention).

— Qwen Code · sandboxed verification

Code review

Static review of 74acebcf — I read the ledger module in full and every production hunk, and confirmed the claims below against the tree rather than the description. No PR code was executed (see Testing).

1. A foreground Shell admitted through the v3 capture route never reaches the ledger. This is the one finding I'd want answered before M6. In managed-runtime-tool-executor.ts's run(), the capture branch is tried first and wins:

if (entry.version === 3 && entry.captureSink) {
  return (invocation as ShellToolInvocation).execute(
    entry.controller.signal, undefined, undefined,
    undefined, undefined, undefined, entry.captureSink);   // no setPidCallback
}
if (entry.toolName === ShellTool.Name && this.options.ledger) { /* addGroup lives only here */ }

executeV3Admitted builds exactly those version: 3 + captureSink entries for foreground Shell, and that path is production-reachable: managed-runtime-tool-v3-routes.ts:231 calls executor.executeV3(...), registered by the same registerManagedRuntimeToolRoutes that the ledger is handed to. So a v3 Shell's process group is never addGroup'd — it is not SIGKILLed by killOutstanding at worker close, is invisible to both host sweeps, and its settled cancel skips the group-exit evidence entirely (that block is gated on entry.version === 2 && shellPgid !== undefined). The PR description says "Every Shell process group a session Runtime worker starts is now written to a worker-owned ledger file", and none of the new executor ledger tests drive the capture path, so the suite does not pin it either way.

If v3 is deliberately out of M5c's scope, that is fine — but then the claim needs narrowing and the residual belongs in the design doc's M5c section in both languages, next to the setsid and spawn-to-pid-callback residuals it already records honestly. If it is not deliberate, the capture branch needs the pid callback too. Worth settling now rather than at M6, when it stops being inert.

2. The same quarantine predicate is written three times. isManagedEngineQuarantineRefusal is byte-identical in acp-integration/session/Session.ts and serve/conversations/standalone-session-service.ts, and serve/acp-http/dispatch.ts open-codes the same data.errorKind === 'managed_engine_quarantined' test. The errorKind string is the contract across an ACP boundary that forbids importing the serve-side error class, so a structural check is right — but one shared guard owning that string would keep the three sites from drifting. Suggestion, not a blocker.

3. ManagedToolExecutor.forWorkspace now passes eight positional undefineds to reach the new trailing options bag. The long positional constructor predates this PR; appending a ninth slot makes the call site harder to read than it needs to be. Cosmetic.

4. launchedLedgerPaths only ever grows. Successful launches are never removed (only spawn/attest failures are), which is correct for skip-set semantics — a ledger this process launched belongs to its own exit-hook sweep, not the directory sweep. But in a long-lived daemon with worker churn the set accumulates one path per incarnation for the process lifetime. Small, and probably intentional; flagging so it is a decision rather than an accident.

5. A permanently unprovable-but-not-terminal group retries forever. startLedgerReaper backs off to a 30 s cap with no attempt limit, and the hold-unproven shape (leader dead, only late-backgrounded survivors) is exactly the one that can never resolve. That means an engine quarantined for the rest of the process lifetime plus a ps fork every 30 s. The design accepts the hold direction and I agree with it — just note that "self-healing" has no bound on this branch.

6. -32024 now carries two meanings. It was already the managed-refusal code (acpAgent.ts:1118, :1141), and this PR reuses it for managed_engine_quarantined, discriminated only by data.errorKind. I checked the mapping is unambiguous server-side — dispatch.ts tests the quarantine errorKind before the SessionExecutionEngineError branch, and error-response.ts keys on kind — so 503-vs-409 resolves correctly. A client switching on the numeric code alone still cannot tell "quarantined, retry later" from "wrong engine". Acceptable given the existing design doc already treats −32024 as the managed family; naming it so it is a choice.

What I verified rather than assumed — these are the load-bearing claims, and they hold:

• The inertness claim is true. createManagedRuntimeEnvironment has no production caller anywhere in the tree (only its own definition and a doc comment referencing it). Nothing therefore wires onManagedEngineQuarantine, the quarantine Set stays empty, and every new branch — the admission gate, the Session activation reset, the standalone-service classification, the 503 mappings — is keyed on state nothing populates yet. Even with --acp-execution-engine managed available as a flag, this slice cannot change observable behavior.
• The pid callback is wired to the right parameter. setPidCallback really is the 4th positional of ShellTool.execute (packages/core/src/tools/shell.ts:2388), and rawCapture the 7th — both call sites match.
• The lift preserves Error identity end to end (reaper closure → sink → Config → Set), so keying the quarantine Set by reason object works as designed.
• The new core methods follow the file's existing derived-config delegation — same Object.getPrototypeOf(this) as Config shape as its neighbours at config.ts:5965 and :5982.
• The ledger path is removed from the worker's env before any Shell spawns (managedRuntimeLedgerFromEnvironment), so a spawned command cannot read the name of the file accounting for it.

The ledger module itself is careful in the places that matter: unique per-write staging names so concurrent writers cannot splice each other's documents, undatable ps rows kept in the table so a group's only member is never read as absent, monotonic-clock proof budgets, ESRCH-only exit proofs so a teardown EPERM cannot end a wait, and a re-read of the final document after the worker is proven dead so a group recorded mid-sweep is not unlinked away. Comments explain genuinely non-obvious invariants rather than restating code.

sequenceDiagram
    participant P1 as Host session worker
    participant P2 as Runtime worker
    participant P3 as Ledger file
    participant P4 as Shell process group
    participant P5 as ACP engine admission
    participant P6 as Reaper
    P1->>P2: spawn with ledger path in env
    P2->>P3: write worker record (sync, atomic)
    P2->>P4: start Shell (pid callback)
    P2->>P3: addGroup before the call can settle
    P1->>P2: cancel
    P2->>P4: wait for whole group to be proven dead
    P2-->>P1: settle only on proof, else state unknown
    P1->>P3: sweep on worker exit or at close
    P3-->>P1: unproven remainder
    P1->>P5: quarantine (refuse new Managed sessions, 503)
    P1->>P6: arm chained reaper
    P6->>P3: retry until proven
    P6->>P5: lift with the same reason object

Testing

This run is on the CI path (GITHUB_EVENT_NAME=issue_comment), so per the skill's static-review rule I did not build, run, or check out any PR code. Evidence below is the PR's own CI, read through the API for the reviewed commit.

237 check-runs on 74acebcf: 31 success, 0 failure, 200 skipped, 5 cancelled, 1 in progress.

No red checks, so there is no failing-job log to quote. The review-pr in-progress run is bot orchestration (pull_request_target), not PR CI; counting only event == "pull_request" runs, nothing is pending on this commit.

One correction to the earlier review on this PR: Test (windows-latest, Node 22.x) and Test (macos-latest, Node 22.x) being skipped is repo policy, not a gap this PR caused. Both jobs carry an if: limited to merge_group, schedule, and workflow_dispatch (.github/workflows/ci.yml, test_macos and test_windows), so no PR ever runs them; they will execute in the merge queue. The 200 skipped check-runs are the same class of no-op lanes.

Not verified, and why:

• Windows behavior — not verified. queryProcessTable() returns an empty map on win32, so every Windows sweep runs without identity; the PR says so and exercises the win32 ledger and sweep shapes through sys test doubles only, with real-machine verification explicitly deferred to before M6. Combined with the repo-policy skip above, no CI run on any PR will ever exercise it.
• The v3 capture path (finding 1) — not verified either way, because no test drives it.
• The round-R2 dispositions the author summarized — not verified. The standing CHANGES_REQUESTED review on this PR was submitted against a36297bf; the head 74acebcf is the commit that claims to address it, and I did not re-check each of those threads individually.
• Real-process POSIX witnesses — not independently re-run; the green ubuntu suite is the only signal I have for them.

Sandboxed verification would settle the part static review cannot: @qwen-code /verify — that a settled cancel actually refuses to journal until the whole Shell group is proven dead, and that a crashed worker's groups are still attributable and swept. Those are behavioural claims about real process groups; the suite passing does not by itself prove the proof is load-bearing, and finding 1 is exactly the kind of gap an A/B run against the base build would expose. A verification run is already in flight from the trigger that started this triage (run 37619447558, still in_progress at review time) and will post its report to the existing qwen-triage:verify thread — worth reading alongside this, with the usual skepticism about what a report from PR-authored code says.

Real-scenario tmux testing: N/A — CI path, and there is nothing user-visible to drive: the engine registers nothing until M6, which the inertness check above confirms independently.

— Qwen Code · qwen3.8-max-2026-09-02

_{Reviewed at 74acebcfc04aeb43cd27852ef345fc9cdc9e411f · re-run with @qwen-code /triage}

Confidence: 3/5 — the mechanism is sound and the inertness claim checks out, but one question about coverage I cannot answer from the diff keeps this short of an approval.

Going back to what I would have written myself: the same thing, roughly. A durable per-worker record of every group, an identity proof that refuses to signal what it cannot date, and a visible engine state when a stop cannot be proven. I did not find a materially simpler path that still survives a crashed worker and a crashed child, which is the requirement here. So this is not a case of the PR missing an obvious shortcut.

What I did find is a gap between the claim and the code. The description says every Shell process group the worker starts is recorded; the v3 capture route starts one that is not, and nothing in the 6147 lines of new tests drives that route with a ledger. That matters less today than it will: I confirmed independently that createManagedRuntimeEnvironment has no production caller, so this slice genuinely cannot change behavior yet, and the whole quarantine chain hangs off a Set nothing populates. The cost of the gap is deferred to M6, not paid now — which is precisely why it is worth settling while it is still a one-line question instead of a hole in a live engine's stop story.

Everything else I read held up under suspicion rather than benefit of the doubt. The concurrency details are right in the places where being wrong would be silent and expensive: unique staging names per write, undatable ps rows kept as members, monotonic proof budgets, ESRCH-only exit proofs, and the re-read after a worker is proven dead so a mid-sweep record cannot be unlinked away. The comments are dense, but they encode pid-recycling and clock-domain reasoning that would otherwise take the next reader a day to reconstruct. The test-to-code ratio is what process-lifecycle code needs. I would not curse whoever maintains this.

My remaining reservations are the ones I'd want a human to weigh, not fix:

1. Finding 1 (v3 capture Shell bypasses the ledger) — needs an answer: deliberate scope, or an omission? If deliberate, narrow the claim and record the residual in both design docs.
2. Scale. 2237 production lines is over the advisory threshold, and the slice bundles four things — the ledger, cancel evidence, quarantine plus reaper, and two host sweeps. It would have been easier to review, and safer to revert, in two pieces. Landed as one, it is still coherent; I am not asking for a rewrite.
3. The standing CHANGES_REQUESTED on this PR was submitted against a36297bf, and the head 74acebcf is the commit claiming to address round R2. I did not re-verify those seven threads individually, so I cannot say whether the review is now stale. A maintainer should confirm and dismiss it if satisfied — I am not going to approve over an unresolved request-changes I did not check.

Not approving, not requesting changes. @wenshao — you own this roadmap and have admin here, so the call is yours; the one thing I could not resolve from the diff, the tests, or the description is whether the v3 capture path is meant to be inside M5c's guarantee. (The workflow's deterministic maintainer resolver needs node, which this run's permission rules block, so rather than guess a second login I am escalating to you directly and not reassigning the PR.) A sandboxed /verify run is already in flight from the trigger that started this triage — its report should be read before merge, since it is the only evidence here that bears on whether the group-exit proof is load-bearing rather than merely green.

— Qwen Code · qwen3.8-max-2026-09-02

_{Reviewed at 74acebcfc04aeb43cd27852ef345fc9cdc9e411f · re-run with @qwen-code /triage}

Triage re-run completed without a new review.

⚠️ The bot has neither a verdict nor a deferral on 74acebcfc04aeb43cd27852ef345fc9cdc9e411f — no APPROVED, CHANGES_REQUESTED, or COMMENTED review of its own. A DISMISSED one does not count: dismiss_stale_reviews voids the bot's approval on every push, which is exactly when a fresh one is needed. If this re-run was meant to review or approve, it did not, and an approval left by another account is a separate vote that does not count as the bot's own.

⚠️ 机器人在 74acebcfc04aeb43cd27852ef345fc9cdc9e411f 上既没有裁决也没有 defer —— 没有属于它自己的 APPROVED、CHANGES_REQUESTED 或 COMMENTED 评审。DISMISSED 不算：dismiss_stale_reviews 会在每次推送时作废机器人的批准，而那恰恰是需要一次新批准的时刻。如果这次重跑本应评审或批准，那么它没有做到；而其他账号留下的批准是另一张票，不能算作机器人自己的。

The stage comments above were updated with the latest result. View workflow run.

上方各阶段评论已更新为最新结果。查看工作流运行。

Walking the seven at 74acebcf so the next round spends its budget verifying rather than re-deriving — each entry gives the guard to read, the fix commit that shipped it, the witness that pins it, and the mutation that turns it red. All seven were authored and flip-probed in the waves after round 1; everything below is re-verified at this head today (marker greps and the named suites, green).

On the round label: the review served as round-1-in-budget plus this gap pass; the waves after it — 4aa2aa50da (bot R1, 34/34), 1587cbc21e (bot R2, 14 findings + R1-33 pushback), 57273cda0b (r3), 1fb61b97bf/8668a5875f (r5), 74acebcfc0 (bot round R2, 6 findings) — each posted its own summary comment with the same ledger shape as above, so the chain from "found" to "fixed, flip-probed, verified" is fully on the record.

Thread census right now: 61 resolved / 13 open, the 13 all being dev-bot deferrals that were logged as accepted deferrals in their own rounds. The standing CHANGES_REQUESTED at a36297bf23 predates every fix commit above; once your next pass confirms the seven, the branch is contention-free (MERGEABLE) and CI green (Test (ubuntu-latest), Lint & Static, integration, broker, fault-gates, Serve A/B, Live Host, the Java matrix — review-pr pending but not a gate, per your own note).

Real-environment verification, round 6 — 74acebcfc0 (merged as 718ae1e6c6)

Follow-up to round 5 (940287f96b). The new commits:

• 57273cda0b: your review 5437392272 (P1-1, P1-2, P2).
• 1fb61b97bf: R1-37 follow-up — an undatable ps row is kept as a member of unknown age.
• 8668a5875f: witnesses for R1-12, R1-48, R1-45, R1-8 and R1-5; the R1-36 witness now fails writes through the fs mock.
• 74acebcfc0: the bot's review R2 (R2-1, R2-3, R2-5, plus witnesses for R2-4, R2-7 and R2-8).
• 82978515cd (docs format) and three merges of main.

The A/B base is the new merge-base d0ddd020c8. The rigs are the same as before, plus real-process probes for P1-1 and P2.

Post-merge confirmation. This PR was merged as 718ae1e6c6 at 13:30Z while this round was running, so this is not a merge gate. The merge commit's 26 PR-touched files are byte-identical to the tested head (git diff 74acebcfc0 718ae1e6c6 -- <those files> is empty), so everything below applies to main.

The three round-5 items are resolved, and no defect turned up in the real stack:

1. R1-37: an undatable row now leaves its group unknown instead of gone. It also stops a sibling sweep from killing a live worker over an undatable host row, which the previous head did.
2. Review 5437392272: P1-1, P1-2 and P2 each reproduce on the previous head and are fixed on this one (real processes on Linux, the daemon on macOS).
3. Witnesses: R1-5, R1-45, R1-8, R1-12 and R1-48's sweep-proof deadline are now pinned. Mutation: 79 of 95 killed.

Follow-ups (tests only):

• The undatable worker and host branches (U2, U5).
• R1-48's other two deadlines.
• The unreadable-only refusal wording (T7).
• The items the author parked: R1-1, R1-6 and the sawRetired sites.

Round 5's three items, previous head vs this head

1. R1-37 follow-up. The same probe as round 5: a real live process group, a real ps table and each tree's built sweep, with only one row's etime replaced by procps's wrapped spelling 441077234-00:18:40.

Control on this head with every row datable: the own group is SIGKILLed, the newborn holder is spared on both sweeps, and the sibling is held. So the fix closes the false gone in round 5, and also a case round 5 did not test: on the previous head an undatable host row made a sibling sweep kill a live worker. One residual: on a witnessed sweep an undatable newborn holder of a recycled pgid is still SIGKILLed. That is by design ("a witnessed sweep signals it: the witness names the group the dead worker's"), and it needs both a wrapped etime, which our hosts never printed, and a pgid recycled before the witnessed sweep runs. Not blocking.

2. Your review 5437392272: all three reproduced on the previous head and fixed on this one.

• P1-1 — a live writer's record lost behind the sweep's rewrite. Real processes in a private pid + mount namespace on Linux. A fake orphan worker (argv carries the worker marker) holds a real ManagedRuntimeLedger in its own process; /bin/ps fails, so the stale sweep cannot prove the worker stopped. After every read the sweep makes of the ledger file, the worker durably publishes one more real group with addGroup. Then the worker is SIGKILLed and a second sweep runs with ps restored. 3 runs each:
  • Previous head: the sweep reads the file twice; the group published after its merge read is gone from disk after the rewrite. The second sweep returns proven, deletes the ledger, and that group is still alive, never signalled.
  • This head: one read, nothing written, nothing lost. The second sweep SIGKILLs every group; nothing is left alive.
• P1-2 — mixed failure lifts a never-judged ledger. The round-5 daemon scenario (macOS): one startup-sweep failure names an unreadable A and a B whose group is held unknown; A is deleted by hand at 4 s, B's member exits at about 25 s.
  • Previous head: 200 at 31.8 s (round 5: 35.5 s).
  • This head: 503 through 64.4 s (B's file is proven and removed at about 34 s; the reason stays terminal because A named nothing). Once the kept session closes, the next session is admitted (200 at 67.5 s), as in C6d.
• P2 — a same-pgid replacement judged against the old stamps. Same namespace rig. The worker's ledger records a group that died a minute ago on pid P. During the sweep's first ps, the live worker starts a new Shell-like group on the same pid P (ns_last_pid) and records it; the sweep then kills the worker and proves it stopped. 3 runs each:
  • Previous head: proven, ledger deleted, the new group on P alive (judged recycled against the old record).
  • This head: the new group is SIGKILLed, proven.
  • Control with a fresh pid: both heads kill it.

3. Witnesses. The five the review asked for (8668a5875f) now go red under their reverting mutants: R1-5 (R5), R1-45 (R9), R1-8 (R11), R1-12 (R12) and R1-48's sweep-proof deadline (R13). R1-48's other two deadlines (waitForGroupExit and killOutstanding; R14, R15) still survive. The code moved all three sites to performance.now(), but only the first is pinned.

The bot's review R2 fixes

• R2-5 (daemon, the P1-2 scenario's refusal text):
  • Previous head: "(a ledger it could not read held the unproven stop)".
  • This head: "(deadbeef-mixed-b.json: 1 process group(s) not proven stopped; a ledger it could not read held the unproven stop)". An unreadable ledger alone (C6) still reads as the clause alone.
• R2-3 (each tree's built toRpcError and sendBridgeError, a retryable managed_engine_quarantined service error):
  • Previous head: httpStatus 409 on both surfaces.
  • This head: 503 on both, Retry-After: 5 and retryable: true kept.
• R2-1 (restore path) needs a Conversations runtime whose child is a Managed host (an M6 composition), so it was not driven in a real stack; mutants T1–T3 below cover it.
• R2-4 / R2-7 are new witnesses only; mutants T8–T10 below.

Round 5's fixes still hold on this head (macOS daemon)

• R1-28: the first transcript read of an open session while quarantined is 200 with 8 events; new sessions are still 503.
• R1-11 / R1-44: "a sweep of its Runtime ledgers did not prove the stop", then "…; 1 more reason(s) awaiting proof".
• R1-5: lifts 6.5 s after the hand delete.
• R1-36: a Shell call with the ledger dir read-only fails with EACCES and leaves nothing running.

Regression on the earlier findings (Linux arm64 namespace rigs)

A/B on the new head (macOS and Linux)

Same as rounds 1–5. Base leaks the TERM+HUP-ignoring member in every crash shape. Head (macOS / Linux):

• C2 (worker killed): member SIGKILLed 78 / 177 ms later.
• C3 (Managed child killed): the worker lingers 10.2 / 10.3 s, then kills the member.
• C4 (double death): member killed 0.7 / 2.1 s after the next POST /session.
• C5 (undatable group): 503 until the member exits; 200 at 31.1 / 30.4 s.
• C6 (0-byte ledger): 200 2.2 / 3.7 s after the hand delete.
• C6b / C6d: as before (C6d: 503 for 46 s after the delete while a session keeps the child).
• C1 (cancel, member ignores TERM+HUP): still outcome_unknown and a blocked session; the root cause is bug: POSIX Shell cancellation leaves TERM-ignoring descendants after leader exit #13441.

The merges brought main's #13168 into the Runtime tool executor (readWorkspaceContext). It only reads files and starts no process, so it is outside what M5c stops.

Tests and mutation

79 of 95 mutants killed: round 5's 79 (re-anchored) plus 16 for this round's fixes. Verdicts exclude the executor test fails the call and kills the group when the ledger cannot record it, which is flaky on macOS and red on the Linux root runner's clean tree. N10, whose only killer is that test, was re-run alone: mutant 5/5 red (expected [] to have a length of 1), clean tree 0/5, so it is a real kill. P6 (narrow COLUMNS) only bites on Linux and ran there.

• Round 5's set: 69/79 (was 64).
  • Newly killed: R5, R9, R11, R12 and R13 by the new witnesses, and W9 by the R2-7 witness.
  • Still surviving:
    • C1, as in every round.
    • The ones the author parked: R1 and R2 (R1-1), R6 and R7 (R1-5's sawRetired sites), R8 (R1-6).
    • R14 and R15: R1-48's other two deadlines.
    • R21: unreachable.
  • Newly surviving: R22, the create path's own translation of a raw refusal. It is now redundant: the only producer in that block, bindAndRelease, translates first.
• This round's 16: 10 killed.
  • Pinned:
    • S2: P2's same-pgid adoption.
    • S3: P1-2's per-file names.
    • U1: keeping undatable rows.
    • T1 and T3: R2-1's commit arm and the create-path passthrough.
    • T4 and T5: both R2-3 ladders.
    • T6: R2-5's named entries.
    • T8: R2-4's inline sweep.
    • T10: R2-7's skip set.
    • P1-1's no-rewrite rule is pinned by the re-anchored Q2.
  • Survive:
    • U2: an undatable worker row judged by age anyway. It then reads as recycled, so a live worker would be certified gone without a signal.
    • U5: an undatable host row no longer holds. This is the case the probe above shows the previous head SIGKILLing a live sibling worker.
    • T7: the unreadable-only refusal printing "ghost.json: 0 process group(s) not proven stopped". The disposition says this wording never appears; the code is right, but no test pins it.
    • S5: names from the reaper's retries are not recorded. This fails closed: a lift that could be proven stays terminal.
    • T9: a failed launch keeps its skip-set entry (the inline sweep still runs).
    • T2: the release arm's translation, unreachable like R21.

Suites:

• The nine affected suites pass on macOS at this head: 1381 passed, 1 skipped; core 58 passed.
• On the Linux root runner, the five Managed Runtime suites pass except the executor test above.
• CI on the head: 29 checks pass.

Not covered

• R2-1's restore path and R1-29 need an M6 composition (a Conversations runtime whose child is a Managed host); covered by unit witnesses and mutants only.
• No Windows run. By reading the code: on Windows a stale sweep never has a table, so every live worker takes the unproven path that P1-1 changed; that path now never writes.
• The P1-1 and P2 probes trigger the worker's write at a chosen point (after each sweep read; during the first ps) instead of racing for it; the processes, files and sweep are real.

Post-merge close-out

Every review thread on this PR is now replied and resolved (75/75).

The 13 deferrals that were parked with recorded consent have been settled as a follow-up batch in #13630: ledger artifacts owner-only end to end ([P3] 0o700 dirs / 0o600 staging file), a close() deferring to an already-armed reaper now rejects with the arming reason instead of resolving, the structural unproven-sweep report is shared through packages/cli/src/runtime/, and the test-suite debts are paid (exit-hook sweep catch witness, foreign-errorKind terminal activation case, durability read-back + settle-based death assertion, ordered record-fail→kill-start, tolerant poll loops, 30 s ceiling for the 15 s inner-wait test, race-free debris-age staging). Two threads closed by adjudication with no new code: the mixed/unreadable quarantine wording (shipped in 74acebcfc0) and the recorder-comment overstatement (shipped in 8668a5875f), plus the retry-witness shape confirmed already-satisfactory at merge. wenshao's own deferred items remain in #13464 (autofix-owned) as before.

Each new witness went red under its reverting mutation (dropped mode bits, defer-only close, exit-hook-sweep delete) and green on restore; suites at the follow-up head: ledger 85, session worker 101, executor 70 (+1 skip), Session 1159 — all green.