From 4376d88db76cf1124dd96b43d8e511fc80568366 Mon Sep 17 00:00:00 2001 From: Hasan TASKIN Date: Fri, 28 Aug 2026 01:26:08 +0200 Subject: [PATCH 1/2] chore: enable remaining strict tsc flags --- packages/cli/src/agent.ts | 7 +++---- packages/cli/src/review.ts | 7 +++---- tsconfig.base.json | 4 ++++ 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/packages/cli/src/agent.ts b/packages/cli/src/agent.ts index 6ca28c4..a5067a5 100644 --- a/packages/cli/src/agent.ts +++ b/packages/cli/src/agent.ts @@ -1169,11 +1169,10 @@ export function watchdogTickMs(budgets: WatchdogBudgets): number { export class AgentWatchdogError extends Error { /** Retryable in D2: what has to change is the RUN or its environment, not the work on the branch. */ readonly reasonCode: ReasonCode = 'inactivity_timeout' - constructor( - readonly watchdogCause: AgentWatchdogCause, - message: string, - ) { + readonly watchdogCause: AgentWatchdogCause + constructor(watchdogCause: AgentWatchdogCause, message: string) { super(message) + this.watchdogCause = watchdogCause this.name = 'AgentWatchdogError' } } diff --git a/packages/cli/src/review.ts b/packages/cli/src/review.ts index 1a03e7c..2b2fb1d 100644 --- a/packages/cli/src/review.ts +++ b/packages/cli/src/review.ts @@ -422,11 +422,10 @@ const INVALID_JSON_RETRY_NOTE = 'Your previous output was not a valid JSON review. Output ONLY the JSON object now: no prose, no code fences.' export class AgentOutputError extends Error { - constructor( - message: string, - readonly raw: string, - ) { + readonly raw: string + constructor(message: string, raw: string) { super(message) + this.raw = raw } } diff --git a/tsconfig.base.json b/tsconfig.base.json index be1cf77..4436962 100644 --- a/tsconfig.base.json +++ b/tsconfig.base.json @@ -15,6 +15,10 @@ "noImplicitOverride": true, "noImplicitReturns": true, "noFallthroughCasesInSwitch": true, + "noUncheckedSideEffectImports": true, + "allowUnreachableCode": false, + "allowUnusedLabels": false, + "erasableSyntaxOnly": true, "exactOptionalPropertyTypes": true, "verbatimModuleSyntax": true, "skipLibCheck": true, From bff374fe815a6c94a104647a3f71ca3ef368d0a1 Mon Sep 17 00:00:00 2001 From: Hasan TASKIN Date: Fri, 28 Aug 2026 01:45:51 +0200 Subject: [PATCH 2/2] fix: keep the egress proxy alive and surface agent dying words --- CHANGELOG.md | 7 ++ package.json | 2 +- packages/cli/package.json | 2 +- packages/cli/src/agent.test.ts | 49 ++++++++++ packages/cli/src/agent.ts | 51 +++++++++- packages/cli/src/task-isolation.test.ts | 121 +++++++++++++++++++++--- packages/cli/src/task-isolation.ts | 43 ++++++++- 7 files changed, 258 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a49cee6..f10b153 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,13 @@ All notable changes to `codesema` (the npm package in `packages/cli`) are documented here. Format: [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). Versioning: [SemVer](https://semver.org). +## [0.18.3] - 2026-08-28 + +### Fixed + +- **The task egress proxy died at boot on rootful docker, silently taking every caged turn with it.** The generated squid.conf logged to /dev/stdout; squid drops from root to its `proxy` user after start, that user cannot reopen the container stdout pipe, and squid exits FATAL seconds after `run -d` reported success. With `--rm` erasing the evidence, each turn then ran with no route to the API and died minutes later as an opaque "agent command exited with code 1". The access log now goes to squid own log directory, and ensureEgressProxy probes the container right after start: a proxy that dies is reported immediately, with the crash output. Found on the first production runner, where no caged turn had ever actually reached the API. +- **A non-zero agent exit now carries the agent last words.** The final result frame of the claude stream (or the raw output tail) is appended to the exit-code error, on the host and in the cage, instead of being captured and thrown away. + ## [0.18.2] - 2026-08-28 ### Fixed diff --git a/package.json b/package.json index 68f1016..c0a91e4 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "codesema-tools", - "version": "0.18.2", + "version": "0.18.3", "private": true, "type": "module", "workspaces": [ diff --git a/packages/cli/package.json b/packages/cli/package.json index 274f1ce..3554d39 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -1,6 +1,6 @@ { "name": "codesema", - "version": "0.18.2", + "version": "0.18.3", "description": "Local merge request review, step by step. Your AI agent reviews, codesema displays.", "license": "MIT", "author": "Hasan TASKIN", diff --git a/packages/cli/src/agent.test.ts b/packages/cli/src/agent.test.ts index ad5ea99..6343416 100644 --- a/packages/cli/src/agent.test.ts +++ b/packages/cli/src/agent.test.ts @@ -6,6 +6,8 @@ import { AGENT_SETTLE_GRACE_MS, AGENT_WATCHDOG_DEFAULTS, agentEnv, + agentExitError, + agentFailureDetail, agentReasonCode, AgentWatchdogError, boundedReadOnlyReviewCommand, @@ -2135,3 +2137,50 @@ describe('createClaudeTaskParser cost', () => { expect(tokens).toEqual([150, 430]) }) }) + +describe('agentFailureDetail', () => { + test('reads the last result frame of a claude stream', () => { + const out = [ + '{"type":"system","subtype":"init"}', + '{"type":"result","subtype":"error_during_execution","result":"Request timed out"}', + ].join('\n') + expect(agentFailureDetail(out)).toBe('error_during_execution: Request timed out') + }) + + test('a success frame with text keeps the text, without the subtype', () => { + const out = '{"type":"result","subtype":"success","result":"done"}' + expect(agentFailureDetail(out)).toBe('done') + }) + + test('falls back to the raw tail when nothing parses as a result frame', () => { + expect(agentFailureDetail('warming up\nRequest timed out')).toBe('warming up Request timed out') + }) + + test('empty output has no detail', () => { + expect(agentFailureDetail('')).toBeNull() + expect(agentFailureDetail(' \n ')).toBeNull() + }) + + test('detail is capped, never a page of stream dump', () => { + const detail = agentFailureDetail( + `{"type":"result","subtype":"success","result":"${'x'.repeat(2000)}"}`, + ) + expect(detail).toHaveLength(400) + }) +}) + +describe('agentExitError', () => { + test('carries the dying words next to the exit code', () => { + const err = agentExitError( + 1, + '{"type":"result","subtype":"error_during_execution","result":"Request timed out"}', + ) + expect(err.message).toContain('exited with code 1') + expect(err.message).toContain('Request timed out') + }) + + test('stays the bare exit message when the run said nothing', () => { + const err = agentExitError(127, '') + expect(err.message).toBe('agent command exited with code 127') + }) +}) diff --git a/packages/cli/src/agent.ts b/packages/cli/src/agent.ts index a5067a5..88831af 100644 --- a/packages/cli/src/agent.ts +++ b/packages/cli/src/agent.ts @@ -1188,6 +1188,55 @@ export function watchdogMessage(cause: AgentWatchdogCause, elapsedMs: number): s return cause === 'inactivity' ? t('agent.inactivity', { m }) : t('agent.toolBudget', { m }) } +const FAILURE_DETAIL_MAX = 400 + +/** + * What a non-zero agent exit actually said: the last result frame of a claude + * JSONL stream when there is one, the tail of raw output otherwise. Without + * this the run's dying words are captured and then thrown away, and every + * failure reads "exited with code 1". + */ +export function agentFailureDetail(out: string): string | null { + const lines = out.trimEnd().split('\n') + for (let i = lines.length - 1; i >= 0; i--) { + const line = lines[i]?.trim() + if (!line || !line.startsWith('{')) { + continue + } + let frame: unknown + try { + frame = JSON.parse(line) + } catch { + continue + } + if (typeof frame !== 'object' || frame === null) { + continue + } + const f = frame as { type?: unknown; subtype?: unknown; result?: unknown; error?: unknown } + if (f.type !== 'result') { + continue + } + const text = + typeof f.result === 'string' && f.result.length > 0 + ? f.result + : typeof f.error === 'string' + ? f.error + : '' + const subtype = typeof f.subtype === 'string' && f.subtype !== 'success' ? f.subtype : '' + const detail = [subtype, text].filter((part) => part.length > 0).join(': ') + return detail.length > 0 ? detail.slice(0, FAILURE_DETAIL_MAX) : null + } + const tail = lines.slice(-3).join(' ').trim() + return tail.length > 0 ? tail.slice(0, FAILURE_DETAIL_MAX) : null +} + +/** The exit-code message, carrying the agent's dying words when it left any. */ +export function agentExitError(code: number | null, out: string): Error { + const base = t('agent.exitCode', { code }) + const detail = agentFailureDetail(out) + return new Error(detail === null ? base : `${base}: ${detail}`) +} + // --- clocks, spawning, and the shape of a kill ------------------------------ /** Time source and timers of a run; injected so no test ever waits out a budget. */ @@ -1500,7 +1549,7 @@ export function runAgent(opts: AgentRunOptions): Promise { } else if (code === 0) { resolve(parser ? (parser.finalText() ?? out) : out) } else { - reject(new Error(t('agent.exitCode', { code }))) + reject(agentExitError(code, out)) } } diff --git a/packages/cli/src/task-isolation.test.ts b/packages/cli/src/task-isolation.test.ts index 22f766a..610d6e0 100644 --- a/packages/cli/src/task-isolation.test.ts +++ b/packages/cli/src/task-isolation.test.ts @@ -741,6 +741,15 @@ describe('buildSquidConfig', () => { ).toHaveLength(1) }) + // Squid drops from root to its 'proxy' user after boot, and that user cannot + // reopen the container's stdout pipe on rootful docker: logging to + // /dev/stdout is FATAL and kills the proxy seconds after `run -d` succeeded. + test('the access log goes to squid own log directory, never /dev/stdout', () => { + const config = buildSquidConfig(['api.anthropic.com']) + expect(config).toContain('access_log stdio:/var/log/squid/access.log') + expect(config).not.toContain('/dev/stdout') + }) + // Squid dies on startup ("Bungled squid.conf") when a domain is listed both // bare and dotted, taking the whole cage with it. The dotted form alone // matches the apex and every subdomain — verified against squid 6.13. @@ -767,13 +776,18 @@ describe('buildSquidConfig', () => { describe('ensureEgressProxy', () => { test('creates an INTERNAL network, starts squid outside it, then connects it', async () => { const { calls, exec } = fakeExec((call) => - call.args[1] === 'inspect' ? ok({ code: 1 }) : ok(), + call.args.includes('{{.State.Running}}') + ? ok({ stdout: 'true\n' }) + : call.args[1] === 'inspect' + ? ok({ code: 1 }) + : ok(), ) const proxy = await ensureEgressProxy({ runtime: 'docker', execFn: exec, allowedDomains: ['api.anthropic.com'], configDir: makeDir('codesema-proxy-conf-'), + probeDelayMs: 0, }) expect(proxy.network).toMatch(/^codesema-net-[0-9a-f]{8}$/) expect(proxy.egressNetwork).toMatch(/^codesema-egress-[0-9a-f]{8}$/) @@ -805,12 +819,26 @@ describe('ensureEgressProxy', () => { test('idempotent: a second task reuses the running proxy without touching the runtime', async () => { const { calls, exec } = fakeExec((call) => - call.args[1] === 'inspect' ? ok({ code: 1 }) : ok(), + call.args.includes('{{.State.Running}}') + ? ok({ stdout: 'true\n' }) + : call.args[1] === 'inspect' + ? ok({ code: 1 }) + : ok(), ) const configDir = makeDir('codesema-proxy-conf-') - const first = await ensureEgressProxy({ runtime: 'docker', execFn: exec, configDir }) + const first = await ensureEgressProxy({ + runtime: 'docker', + execFn: exec, + configDir, + probeDelayMs: 0, + }) const before = calls.length - const second = await ensureEgressProxy({ runtime: 'docker', execFn: exec, configDir }) + const second = await ensureEgressProxy({ + runtime: 'docker', + execFn: exec, + configDir, + probeDelayMs: 0, + }) expect(second).toEqual(first) expect(calls.length).toBe(before) }) @@ -821,29 +849,66 @@ describe('ensureEgressProxy', () => { runtime: 'podman', execFn: exec, configDir: makeDir('codesema-proxy-conf-'), + probeDelayMs: 0, }) expect(argsOf(calls, 'network', 'create')).toHaveLength(0) expect(argsOf(calls, 'run')).toHaveLength(0) }) test('a different allowlist gets its own network and proxy', async () => { - const { exec } = fakeExec((call) => (call.args[1] === 'inspect' ? ok({ code: 1 }) : ok())) + const { exec } = fakeExec((call) => + call.args.includes('{{.State.Running}}') + ? ok({ stdout: 'true\n' }) + : call.args[1] === 'inspect' + ? ok({ code: 1 }) + : ok(), + ) const configDir = makeDir('codesema-proxy-conf-') const a = await ensureEgressProxy({ runtime: 'docker', execFn: exec, allowedDomains: ['api.anthropic.com'], configDir, + probeDelayMs: 0, }) const b = await ensureEgressProxy({ runtime: 'docker', execFn: exec, allowedDomains: ['api.anthropic.com', 'registry.npmjs.org'], configDir, + probeDelayMs: 0, }) expect(b.network).not.toBe(a.network) }) + // squid can die AFTER `run -d` reported success (a directive its dropped + // 'proxy' user cannot honor), and --rm erases the evidence: the probe turns + // that into a clear failure carrying the crash output, instead of an opaque + // agent failure minutes later. + test('a proxy that dies right after start is a clear failure with the crash output', async () => { + const { exec } = fakeExec((call) => { + if (call.args.includes('{{.State.Running}}')) { + return ok({ stdout: 'false\n' }) + } + if (call.args[1] === 'inspect') { + return ok({ code: 1 }) + } + if (call.args[0] === 'run' && !call.args.includes('-d')) { + return ok({ code: 1, stderr: 'FATAL: Cannot open /dev/stdout for writing.' }) + } + return ok() + }) + await expect( + ensureEgressProxy({ + runtime: 'docker', + execFn: exec, + allowedDomains: ['probe-dead.example'], + configDir: makeDir('codesema-proxy-conf-'), + probeDelayMs: 0, + }), + ).rejects.toThrow(/egress proxy exited right after start.*FATAL/s) + }) + test('a failure to create the network is reported, never swallowed', async () => { const { exec } = fakeExec((call) => { if (call.args[1] === 'inspect') { @@ -856,18 +921,24 @@ describe('ensureEgressProxy', () => { runtime: 'docker', execFn: exec, configDir: makeDir('codesema-proxy-conf-'), + probeDelayMs: 0, }), ).rejects.toThrow(/permission denied/) }) test('teardown removes what THIS process started, and nothing else', async () => { const { calls, exec } = fakeExec((call) => - call.args[1] === 'inspect' ? ok({ code: 1 }) : ok(), + call.args.includes('{{.State.Running}}') + ? ok({ stdout: 'true\n' }) + : call.args[1] === 'inspect' + ? ok({ code: 1 }) + : ok(), ) const proxy = await ensureEgressProxy({ runtime: 'docker', execFn: exec, configDir: makeDir('codesema-proxy-conf-'), + probeDelayMs: 0, }) calls.length = 0 await teardownEgressProxy({ runtime: 'docker', execFn: exec }) @@ -896,7 +967,11 @@ describe('bootstrapAgentHome', () => { const path = join(credentials, '.credentials.json') writeFileSync(path, '{"claudeAiOauth":{"accessToken":"sk-secret"}}') const { calls, exec } = fakeExec((call) => - call.args[1] === 'inspect' ? ok({ code: 1 }) : ok(), + call.args.includes('{{.State.Running}}') + ? ok({ stdout: 'true\n' }) + : call.args[1] === 'inspect' + ? ok({ code: 1 }) + : ok(), ) const home = await bootstrapAgentHome({ runtime: 'podman', @@ -925,7 +1000,11 @@ describe('bootstrapAgentHome', () => { test('an OAuth token in the environment means nothing is copied at all', async () => { const { calls, exec } = fakeExec((call) => - call.args[1] === 'inspect' ? ok({ code: 1 }) : ok(), + call.args.includes('{{.State.Running}}') + ? ok({ stdout: 'true\n' }) + : call.args[1] === 'inspect' + ? ok({ code: 1 }) + : ok(), ) const home = await bootstrapAgentHome({ runtime: 'docker', @@ -939,7 +1018,13 @@ describe('bootstrapAgentHome', () => { }) test('no credentials on the host: honest "missing", the cage still runs', async () => { - const { exec } = fakeExec((call) => (call.args[1] === 'inspect' ? ok({ code: 1 }) : ok())) + const { exec } = fakeExec((call) => + call.args.includes('{{.State.Running}}') + ? ok({ stdout: 'true\n' }) + : call.args[1] === 'inspect' + ? ok({ code: 1 }) + : ok(), + ) const home = await bootstrapAgentHome({ runtime: 'docker', taskId, @@ -970,7 +1055,11 @@ describe('bootstrapAgentHome', () => { const path = join(dir, 'auth.json') writeFileSync(path, '{"token":"ok"}') const { calls, exec } = fakeExec((call) => - call.args[1] === 'inspect' ? ok({ code: 1 }) : ok(), + call.args.includes('{{.State.Running}}') + ? ok({ stdout: 'true\n' }) + : call.args[1] === 'inspect' + ? ok({ code: 1 }) + : ok(), ) const home = await bootstrapAgentHome({ runtime: 'docker', @@ -995,7 +1084,11 @@ describe('bootstrapAgentHome', () => { test('memoized per task: two turns bootstrap once', async () => { const { calls, exec } = fakeExec((call) => - call.args[1] === 'inspect' ? ok({ code: 1 }) : ok(), + call.args.includes('{{.State.Running}}') + ? ok({ stdout: 'true\n' }) + : call.args[1] === 'inspect' + ? ok({ code: 1 }) + : ok(), ) const opts = { runtime: 'docker', @@ -1745,7 +1838,11 @@ describe('runContainerTurn', () => { function rig(over: { spawn?: (opts: ContainerSpawnOptions) => Promise } = {}) { const { calls, exec } = fakeExec((call) => - call.args[1] === 'inspect' ? ok({ code: 1 }) : ok(), + call.args.includes('{{.State.Running}}') + ? ok({ stdout: 'true\n' }) + : call.args[1] === 'inspect' + ? ok({ code: 1 }) + : ok(), ) const spawned: ContainerSpawnOptions[] = [] const spawnFn = (opts: ContainerSpawnOptions): Promise => { diff --git a/packages/cli/src/task-isolation.ts b/packages/cli/src/task-isolation.ts index 20c2fb0..28c164e 100644 --- a/packages/cli/src/task-isolation.ts +++ b/packages/cli/src/task-isolation.ts @@ -34,6 +34,7 @@ import { AGENT_KILL_GRACE_MS, AGENT_SETTLE_GRACE_MS, AGENT_WATCHDOG_DEFAULTS, + agentExitError, AgentWatchdogError, armStreamWatchdog, flagPresent, @@ -900,7 +901,10 @@ export function buildSquidConfig(domains: readonly string[]): string { ...(clean.length > 0 ? ['http_access allow CONNECT allowed'] : []), 'http_access deny all', 'cache deny all', - 'access_log stdio:/dev/stdout', + // squid starts as root then drops to its 'proxy' user, which cannot + // reopen the container's stdout pipe: logging to /dev/stdout is FATAL on + // rootful docker and kills the proxy at boot (verified against squid 6.13). + 'access_log stdio:/var/log/squid/access.log', 'pid_filename none', '', ] @@ -929,6 +933,8 @@ export type EnsureEgressProxyOptions = { allowedDomains?: readonly string[] /** Directory the generated squid.conf is written to; defaults to a tmp dir. */ configDir?: string + /** Wait before the liveness probe; 0 in tests (no real container to settle). */ + probeDelayMs?: number } /** @@ -1006,6 +1012,39 @@ export async function ensureEgressProxy(opts: EnsureEgressProxyOptions): Promise throw new Error(t('isolation.proxyFailed', { error: buildFailure(connected) })) } startedProxies.add(container) + // A broken squid.conf kills squid AFTER `run -d` reported success, and + // --rm erases the evidence: without this probe the crash surfaces as an + // opaque agent failure minutes later, with nothing left to inspect. + await new Promise((resolveDelay) => setTimeout(resolveDelay, opts.probeDelayMs ?? 1_500)) + const alive = await exec( + opts.runtime, + ['container', 'inspect', '--format', '{{.State.Running}}', container], + { timeoutMs: 20_000 }, + ) + if (alive.code !== 0 || alive.stdout.trim() !== 'true') { + const crash = await exec( + opts.runtime, + [ + 'run', + '--rm', + '--network', + egressNetwork, + '-v', + `${configPath}:/etc/squid/squid.conf:ro`, + '--security-opt', + 'no-new-privileges', + '--memory', + '512m', + EGRESS_PROXY_IMAGE, + ], + { timeoutMs: 30_000 }, + ).catch(() => null) + const detail = + crash === null ? 'container gone before it could be inspected' : buildFailure(crash) + throw new Error( + t('isolation.proxyFailed', { error: `egress proxy exited right after start: ${detail}` }), + ) + } } return { network, @@ -1798,7 +1837,7 @@ export const spawnContainer: ContainerSpawnFn = (opts) => } else if (code === 0) { resolve(out) } else { - reject(new Error(t('agent.exitCode', { code }))) + reject(agentExitError(code, out)) } } const killClient = (signal: NodeJS.Signals): void => {