From 6c123ce671d559482f2215c6935d083e5d742d7d Mon Sep 17 00:00:00 2001 From: Jason Woltje Date: Thu, 5 Nov 2026 21:44:24 -0600 Subject: [PATCH] fix(harness): report Claude helper boundary failures (#797) --- BUILD-LOG.md | 9 ++++ adapters/README.md | 7 ++- adapters/claude/adapter.sh | 41 ++++++++++------ adapters/claude/stream.mjs | 9 ++-- docs/SESSIONS.md | 1 + packages/harness/tests/bundle.test.mjs | 2 +- .../harness/tests/claude-session.test.mjs | 22 ++++++--- .../harness/tests/model-activity.test.mjs | 49 ++++++++++++++++++- 8 files changed, 113 insertions(+), 27 deletions(-) diff --git a/BUILD-LOG.md b/BUILD-LOG.md index 7a1eee0ef..27346833f 100644 --- a/BUILD-LOG.md +++ b/BUILD-LOG.md @@ -6211,3 +6211,12 @@ Before: candidate 1bd0077391f1925db5e66796b98540e48f13bfc2 discarded complete er After: the private stream reader renders every complete final result string independently of success/failure, retains CLI nonzero exit codes, fails error-typed zero-exit turns, and withholds result text on stream-integrity/helper marker failures. The adapter's existing success-only session persistence remains unchanged. The Claude plugin declares adapter-owned activity in its existing bundle environment; the runner reads module metadata, ignores ambient declarations and keeps stdout fallback for Pi/worker/mock paths. No core harness-name conditional, watchdog timing/default, broker, roles, contracts, policy or live state change. Bundle tests pin the helper SHA-256 to 9c2cc28b7e719c1d49913401fffc952759d8633e50ae39ca36bfc50729b9415b. Verification: before product edits, whole-file negative controls on 1bd007739 passed 68 and failed 7; the real installed Claude CLI HTTP 400 test failed on empty streaming stdout versus 38 text-mode bytes. It now passes with identical stdout bytes and exit 1, no activity marker and no new session ID, using only localhost, isolated HOME/configuration and a dummy key. Added subtype/is_error/nonzero controls, corrupt-after-result/oversized/truncated/helper marker-failure cases and real runner fallback controls. Final frozen gate: 414 passed, 0 failed/skipped/cancelled/todo across 31 whole files, one node --test invocation per file, at most two concurrently. Node v24.22.1 on the requested PATH; set -o pipefail and fresh scratch TMPDIR. The initial broad gate's sole architecture failure (412/413) is retained honestly; moved the declaration into the plugin and reran all 31 files green. Whitespace, source hashes and append-only prefix checks pass. Report: /mnt/storage/scratch/mosaic-lead/mvp3/coder-stall-fix-r2-report.md; per-file TAPs, exact commands, negative controls, source/pin scans and hashes: coder-stall-fix-r2-evidence/. Local commit follows verification; independent exact-commit review remains with the lead. No push, stash, forge write, real model call, contact with 10.1.1.53, deployment or live recovery. + + +## 2026-11-05 — MVP 3 coder-stall review F2 correction (#797), local implementation handoff + +Before: 8094901bce2d46f952786eea6469643d8dac8868 propagated helper death/load errors without the fixed stream diagnostic; Node exit 1 or SIGKILL status 137 could also be ordinary complete CLI error codes. Explicit correction charter authorizes one new local commit on fix/mvp3-coder-stall-797. Queue next coder refused this linked worktree; no workaround or forge/queue mutation. + +After: the adapter captures a private fd-3 completion handshake independently of answer stdout, CLI stderr and exit status. The helper's child does not inherit the completion descriptor. Complete CLI results retain their bytes and nonzero codes; invalid/absent completion emits exactly the fixed diagnostic and cannot save a new session. Helper stacks and shell crash text stay private. No completion file or writable mount, no broker/role/contract changes, and no watchdog timing/default changes. Helper SHA-256 and bundle pin: ce66fb2581dd789163f227030b71a26af04ce0d165fc5da6da6818eeac044ed0. + +Verification: added adjacent SIGKILL, missing-helper, static-import and runtime-exception regressions plus complete CLI exit-1/137 byte-parity controls. Before product edits, whole-file baseline repeat had 20 pass/4 intended fail; the first run's additional short-watchdog assertion failure is retained honestly. Focused fixed file passes 24/24. Final requested matrix: 420 pass, 0 fail/skip/cancel/todo across 31 whole files, one sequential node --test invocation per file. Node v24.22.1, requested PATH first, set -o pipefail and fresh TMPDIR from mktemp -d /mnt/storage/scratch/tmp/XXXXXX. Real CLI localhost HTTP 400 retains identical 38 stdout bytes, exit 1 and empty stderr, no activity/session. Adapted direct and 13 managed hostile/fallback probes pass; managed helper death/missing-helper stderr is exact, with no activity/session and recorded descendants reaped. Five real bubblewrap invocations preserve read-only stream.mjs/session.json and distinguish helper SIGKILL from handled CLI errors at 1/137. Source/pin scans, whitespace and append-only prefix checks pass. Evidence and report: /mnt/storage/scratch/mosaic-lead/mvp3/coder-stall-fix-r3-report.md and coder-stall-fix-r3-evidence/. Local review candidate only; independent exact-commit review remains with the lead. No push, stash, amend/rewrite, forge write, real model call, contact with 10.1.1.53, deployment or live operation. diff --git a/adapters/README.md b/adapters/README.md index e60a3597f..4faacb40d 100644 --- a/adapters/README.md +++ b/adapters/README.md @@ -95,7 +95,12 @@ activity; its final stdout may be a synthetic error. The runner retains stdout activity marking for Pi and mock adapters. Malformed or incomplete streams emit only the fixed diagnostic and fail the turn; a failed or interrupted first turn does not persist a reusable session ID. The helper is mounted read-only and -its digest is recorded in the launch bundle. Pi retains print mode and signals +its digest is recorded in the launch bundle. A private completion pipe separates +handled CLI exits from helper launch/runtime failures, even if their exit codes +collide. The adapter emits the fixed diagnostic if completion is absent or the +stream is invalid, suppressing helper stacks and shell crash text. CLI stderr +keeps its existing channel; the CLI does not inherit the completion pipe. No extra +writable mount or completion file is needed. Pi retains print mode and signals activity through its existing extension. Mock remains a test-only echo. ### A role's skills (row 64) diff --git a/adapters/claude/adapter.sh b/adapters/claude/adapter.sh index 963792ea9..6d15960ec 100644 --- a/adapters/claude/adapter.sh +++ b/adapters/claude/adapter.sh @@ -87,18 +87,31 @@ fi # text in the system prompt (row 64) ALLOWED=mcp__mosaic [ -z "${MOSAIC_TOOLS:-}" ] || ALLOWED="$MOSAIC_TOOLS,mcp__mosaic" -node "$STREAM" "$CLAUDE" -p "$MOSAIC_REQUEST" \ - --output-format stream-json --verbose --include-partial-messages \ - --system-prompt-file "$PROMPT_FILE" \ - --model "$MOSAIC_MODEL" \ - --effort "$MOSAIC_THINKING" \ - --restricted \ - --tools "${MOSAIC_TOOLS:-}" \ - --allowedTools "$ALLOWED" \ - --permission-mode dontAsk \ - --settings "$MOSAIC_CLAUDE_SETTINGS" \ - --strict-mcp-config \ - --mcp-config "$MOSAIC_CLAUDE_MCP_CONFIG" \ - --disable-slash-commands \ - "$SESSION_FLAG" "$SESSION_ID" || exit $? +# Completion travels on fd 3, independently of stdout and the CLI exit code. +# fd 4 preserves answer stdout; fd 5 preserves CLI stderr while helper/shell +# crashes stay private. The CLI never inherits the completion descriptor. +HELPER_STATUS=0 +{ + HELPER_COMPLETION=$( + { node "$STREAM" "$CLAUDE" -p "$MOSAIC_REQUEST" \ + --output-format stream-json --verbose --include-partial-messages \ + --system-prompt-file "$PROMPT_FILE" \ + --model "$MOSAIC_MODEL" \ + --effort "$MOSAIC_THINKING" \ + --restricted \ + --tools "${MOSAIC_TOOLS:-}" \ + --allowedTools "$ALLOWED" \ + --permission-mode dontAsk \ + --settings "$MOSAIC_CLAUDE_SETTINGS" \ + --strict-mcp-config \ + --mcp-config "$MOSAIC_CLAUDE_MCP_CONFIG" \ + --disable-slash-commands \ + "$SESSION_FLAG" "$SESSION_ID" 3>&1 1>&4 4>&-; } 2>/dev/null + ) || HELPER_STATUS=$? +} 4>&1 5>&2 +if [ "$HELPER_COMPLETION" != complete ]; then + printf 'claude adapter: invalid or incomplete model stream\n' >&2 + [ "$HELPER_STATUS" -ne 0 ] || HELPER_STATUS=2 +fi +[ "$HELPER_STATUS" -eq 0 ] || exit "$HELPER_STATUS" [ "$SESSION_FLAG" = --resume ] || printf '%s\n' "$SESSION_ID" > "$ID_FILE" diff --git a/adapters/claude/stream.mjs b/adapters/claude/stream.mjs index f19d3623d..29af9905f 100644 --- a/adapters/claude/stream.mjs +++ b/adapters/claude/stream.mjs @@ -1,18 +1,17 @@ // Consume Claude Code's SDK event stream privately. A complete final result // reaches stdout, including errors, with the same newline as text mode. import { spawn } from 'node:child_process'; +import { writeSync } from 'node:fs'; import { firstOutputMarker } from '../../packages/harness/src/first-output.mjs'; const mark = process.env.MOSAIC_MODEL_ACTIVITY_DIR ? firstOutputMarker(process.env.MOSAIC_MODEL_ACTIVITY_DIR) : () => {}; const [binary, ...args] = process.argv.slice(2); -const child = spawn(binary, args, { stdio: ['ignore', 'pipe', 'inherit'] }); +const child = spawn(binary, args, { stdio: ['ignore', 'pipe', 5] }); let pending = '', answer, resultSeen = false, resultFailed = false, failed = false, killTimer; const fail = () => { if (failed) return; failed = true; - // Never put event bodies, provider errors or credentials in diagnostics. - process.stderr.write('claude adapter: invalid or incomplete model stream\n'); child.kill('SIGTERM'); killTimer = setTimeout(() => child.kill('SIGKILL'), 5000).unref(); }; @@ -68,4 +67,8 @@ child.on('close', (code, signal) => { process.stdout.write(answer.endsWith('\n') ? answer : answer + '\n'); } process.exitCode = code || (failed || resultFailed ? 2 : 0); + // Only a fully consumed stream can acknowledge completion. The adapter + // owns the fixed diagnostic, including when this helper never loads/dies. + // spawn's explicit stdio keeps fd 3 out of the CLI and its descendants. + writeSync(3, failed ? 'invalid' : 'complete'); }); diff --git a/docs/SESSIONS.md b/docs/SESSIONS.md index c9dc78df7..9e4c0219c 100644 --- a/docs/SESSIONS.md +++ b/docs/SESSIONS.md @@ -2067,3 +2067,4 @@ are never rewritten or removed; corrections are new entries. 2026-11-05 | Codex implementation child | MVP 3 #797 D2 coder-stall fix on fix/mvp3-coder-stall-797 from 05fdfef9e | Claude private streaming activity marker + answer-only stdout; Pi model-event coverage; silent/interrupted controls preserved. Final Node 24.22.1 full 28-file gate: 376 pass, 0 fail/skip/cancel. Queue refused linked worktree; no workaround. Local commit only, no push/stash/live actions. Report: /mnt/storage/scratch/mosaic-lead/mvp3/coder-stall-fix-r1-report.md; independent review remains with lead. 2026-11-05 | Implementation child | MVP 3 #797 coder-stall review F1 on fix/mvp3-coder-stall-797 from 1bd007739 | Preserve complete failed Claude result stdout and exit status; module-declared model activity disables synthetic stdout marking while Pi/worker/mock fallback remains. Real localhost HTTP 400 regression fails baseline and passes fix; final Node 24.22.1 gate 414/414 across 31 whole files, no skips. One local commit only; no push/stash/forge/live actions. Queue refused linked worktree without workaround. Report: /mnt/storage/scratch/mosaic-lead/mvp3/coder-stall-fix-r2-report.md; independent review remains with lead. +2026-11-05 | Implementation child | MVP 3 #797 coder-stall review F2 on fix/mvp3-coder-stall-797 from 8094901bc | Private helper completion pipe supplies exact fixed diagnostics for SIGKILL/load/runtime failures while complete CLI errors preserve text bytes/status, with no synthetic activity or new session. Baseline 20 pass/4 intended fail; final Node 24.22.1 sequential 31-file matrix 420/420, no skips. Direct, managed and read-only bubblewrap controls pass; helper digest pin updated. One local commit only; queue refused linked worktree without workaround; no push/stash/forge/live actions. Report: /mnt/storage/scratch/mosaic-lead/mvp3/coder-stall-fix-r3-report.md; independent review remains with lead. diff --git a/packages/harness/tests/bundle.test.mjs b/packages/harness/tests/bundle.test.mjs index 7cdf1455c..5b0dec391 100644 --- a/packages/harness/tests/bundle.test.mjs +++ b/packages/harness/tests/bundle.test.mjs @@ -152,7 +152,7 @@ test("a claude-code bundle adds the wrapped gate hook and the MCP config", (t) = assert.ok(m.typed.every((n) => n.startsWith("mcp__mosaic__"))); assert.equal(m.sources.claudeStream.path, join(REPO, "adapters/claude/stream.mjs")); assert.equal(m.sources.claudeStream.sha256, sha(m.sources.claudeStream.path)); - assert.equal(m.sources.claudeStream.sha256, "9c2cc28b7e719c1d49913401fffc952759d8633e50ae39ca36bfc50729b9415b"); + assert.equal(m.sources.claudeStream.sha256, "ce66fb2581dd789163f227030b71a26af04ce0d165fc5da6da6818eeac044ed0"); assert.deepEqual(Object.keys(m.sources).sort(), ["claudeGate", "claudeStream", "gate", "mcpServer", "tools", "unslopCheck", "unslopHook", "unslopLists"]); // Row 66 (#1559): the unslop hooks run after a reply and before the next prompt, and never block. for (const [event, which] of [["Stop", "stop"], ["UserPromptSubmit", "prompt"]]) { diff --git a/packages/harness/tests/claude-session.test.mjs b/packages/harness/tests/claude-session.test.mjs index a8c8faac9..cdb88f4e2 100644 --- a/packages/harness/tests/claude-session.test.mjs +++ b/packages/harness/tests/claude-session.test.mjs @@ -20,6 +20,18 @@ const which = spawnSync("sh", ["-c", "command -v claude"], { encoding: "utf8" }) const CLAUDE = which.status === 0 ? which.stdout.trim() : null; const skip = CLAUDE ? false : "claude is not on PATH"; +// Reconstruct the pre-stream text invocation, including its success-only +// session persistence, without requiring the private helper handshake. +function textAdapterSource() { + const source = readFileSync(ADAPTER, 'utf8'); + const args = source.match(/node "\$STREAM" "\$CLAUDE"([\s\S]*?)"\$SESSION_FLAG" "\$SESSION_ID"/); + assert.ok(args, 'stream CLI invocation found'); + return source.slice(0, source.indexOf('# Completion travels')) + + '"$CLAUDE"' + args[1].replace('--output-format stream-json --verbose --include-partial-messages', '--output-format text') + + '"$SESSION_FLAG" "$SESSION_ID" || exit $?\n' + + "[ \"$SESSION_FLAG\" = --resume ] || printf '%s\\n' \"$SESSION_ID\" > \"$ID_FILE\"\n"; +} + async function session(t, script, tools = ["read", "bash"], done = (r) => `ANSWER ${JSON.stringify(r.map((x) => [x.isError, x.content.slice(0, 600)]))}`) { const dir = scratch(t); const workspace = join(dir, "ws"); @@ -148,9 +160,7 @@ for (const answer of ['Answer', ' Answer é 🦆\nsecond line\n', 'Answer\n\n'] test(`claude: streaming preserves the real CLI text answer byte for byte (${JSON.stringify(answer)})`, { skip }, async t => { const { dir, workspace, env } = await session(t, [], [], () => answer); const textAdapter = join(dir, 'text-adapter.sh'); - writeFileSync(textAdapter, readFileSync(ADAPTER, 'utf8') - .replace('node "$STREAM" "$CLAUDE"', '"$CLAUDE"') - .replace('--output-format stream-json --verbose --include-partial-messages', '--output-format text')); + writeFileSync(textAdapter, textAdapterSource()); const text = await turn({ ...env, MOSAIC_SESSION_DIR: join(dir, 'text-session') }, 'answer', workspace, textAdapter); const stream = await turn(env, 'answer', workspace); assert.equal(text.code, 0, text.stderr); @@ -178,9 +188,7 @@ test("claude: HTTP 400 keeps real text-mode stdout and exit status without model await new Promise(resolve => api.listen(0, '127.0.0.1', resolve)); t.after(() => api.close()); const textAdapter = join(dir, 'text-adapter.sh'); - writeFileSync(textAdapter, readFileSync(ADAPTER, 'utf8') - .replace('node "$STREAM" "$CLAUDE"', '"$CLAUDE"') - .replace('--output-format stream-json --verbose --include-partial-messages', '--output-format text')); + writeFileSync(textAdapter, textAdapterSource()); const results = []; for (const [name, adapter] of [['text', textAdapter], ['stream', ADAPTER]]) { const runDir = join(dir, name); @@ -200,6 +208,8 @@ test("claude: HTTP 400 keeps real text-mode stdout and exit status without model assert.deepEqual(text.stdoutBytes, Buffer.from('API Error: 400 review-local-api-error\n')); assert.deepEqual(stream.stdoutBytes, text.stdoutBytes); assert.equal(stream.code, text.code); + assert.equal(text.stderr, ''); + assert.equal(stream.stderr, ''); }); // --restricted is what keeps CLAUDE.md files and auto-memory out of the diff --git a/packages/harness/tests/model-activity.test.mjs b/packages/harness/tests/model-activity.test.mjs index 23a5fcb5d..7bfad564c 100644 --- a/packages/harness/tests/model-activity.test.mjs +++ b/packages/harness/tests/model-activity.test.mjs @@ -2,7 +2,7 @@ import test from 'node:test'; import assert from 'node:assert/strict'; import { spawn, spawnSync } from 'node:child_process'; -import { existsSync, mkdirSync, readFileSync, statSync, unlinkSync, writeFileSync } from 'node:fs'; +import { existsSync, mkdirSync, readFileSync, statSync, symlinkSync, unlinkSync, writeFileSync } from 'node:fs'; import { dirname, join } from 'node:path'; import { watchModelOutput } from '../../cli/src/model-stall.mjs'; import { firstTurnMarker } from '../src/first-output.mjs'; @@ -31,7 +31,7 @@ function fixture(t, mode = 'tool') { return { runDir, workspace, env, cli }; } function start(t, f) { - const child = spawn('/bin/sh', [ADAPTER], { env: f.env, stdio: ['ignore', 'pipe', 'pipe'], detached: true }); + const child = spawn('/bin/sh', [f.adapter ?? ADAPTER], { env: f.env, stdio: ['ignore', 'pipe', 'pipe'], detached: true }); const kill = () => { try { process.kill(-child.pid, 'SIGTERM'); } catch {} }; let stdout = Buffer.alloc(0), stderr = ''; child.stdout.on('data', b => { stdout = Buffer.concat([stdout, b]); }); @@ -142,6 +142,51 @@ test('Claude helper marker failure keeps the fixed diagnostic and emits no resul assert.equal(existsSync(join(f.runDir, 'proxy-first-output')), false); }); +// These failures happen outside the helper's internal fail() handler. +for (const mode of ['sigkill', 'missing', 'static-import', 'runtime']) { + test(`Claude helper ${mode} failure reports only the fixed diagnostic without activity or a session`, { timeout: 10_000 }, async t => { + const f = fixture(t); + if (mode === 'sigkill') { + writeFileSync(f.cli, `#!/bin/sh\nexec '${process.execPath}' -e 'process.kill(process.ppid, "SIGKILL")'\n`); + } else { + f.adapter = join(f.runDir, 'adapter.sh'); + symlinkSync(ADAPTER, f.adapter); + // The adapter resolves the helper beside its invocation path. Do not + // modify repository source to inject a missing/broken module. + if (mode !== 'missing') writeFileSync(join(f.runDir, 'stream.mjs'), mode === 'static-import' + ? "import './credential-canary-missing.mjs';\n" + : "throw Error('credential-canary-runtime');\n"); + } + const result = await start(t, f).done; + assert.notEqual(result.code, 0); + assert.equal(result.stdout.length, 0); + assert.equal(result.stderr, 'claude adapter: invalid or incomplete model stream\n'); + assert.equal(existsSync(join(f.runDir, 'session/claude-session-id')), false); + assert.equal(existsSync(join(f.runDir, 'proxy-first-output')), false); + }); +} + +// A handled CLI result can return the same codes as a dead/unloaded helper. +for (const code of [1, 137]) { + test(`Claude complete CLI error exit ${code} preserves text bytes without a helper diagnostic`, async t => { + const f = fixture(t); + const script = join(f.runDir, 'cli.mjs'); + writeFileSync(script, `const answer = 'API Error: fixture é 🦆\\n\\n'; + process.stdout.write(process.argv.includes('text') ? answer + : JSON.stringify({ type: 'result', subtype: 'success', is_error: true, result: answer }) + '\\n'); + process.exitCode = ${code};`); + writeFileSync(f.cli, `#!/bin/sh\nexec '${process.execPath}' '${script}' "$@"\n`); + const result = await start(t, f).done; + const text = spawnSync(f.cli, ['--output-format', 'text'], { env: f.env }); + assert.equal(text.status, code); + assert.equal(result.code, text.status); + assert.deepEqual(result.stdout, text.stdout); + assert.equal(result.stderr, ''); + assert.equal(existsSync(join(f.runDir, 'session/claude-session-id')), false); + assert.equal(existsSync(join(f.runDir, 'proxy-first-output')), false); + }); +} + for (const mode of ['failed', 'failed-success-subtype', 'failed-zero-exit', 'nonzero-success']) { test(`Claude ${mode} renders the final result independently of failure without a session or synthetic activity`, async t => { const f = fixture(t, mode), result = await start(t, f).done;