From 604369f8864bcb837f99a36e76e89fd8abe4e40b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=85=B3=E5=A4=A9=E8=B1=AA?= Date: Thu, 3 Sep 2026 14:26:29 +0800 Subject: [PATCH 1/3] fix(cli): drop the unreachable sandbox recovery state Review of the args-correlation approach showed that `recovered` is unreachable on every path `maka run` can take: - The live projector sets `args: undefined` on every tool_start (session-continuity carries only argsPreview), so serialized-args equality can never hold for a live observation. - Non-interactive runs deny every widening request (both deny sites answer decision deny), so the boundary cannot move mid-Turn and a blocked call cannot later succeed against the same target. `sandboxBoundary` therefore collapses to none|unresolved: a sandbox failure stays unresolved for the rest of the Turn on both the live and durable paths, stableArgsJson and the args plumbing go away, and run-command-core / activation-command lose their recovered clauses. Exit codes are unchanged on every reachable path. Named follow-ups: the sandbox_boundary_request branch in run-command-core (~:404) has no production caller now that nothing clears the failure set, and an unresolved exit still swallows the Turn's final message - the text the runtime asks the model to produce after a denial - which matters more now that unresolved is the pin outcome. Generated-by: GLM-5.3-Flash (ZCode) --- .../runtime-host-run-command.test.ts | 142 +++++++++++++++--- packages/cli/src/activation-command.ts | 5 +- packages/cli/src/run-command-core.ts | 7 +- packages/cli/src/runtime-host-run-command.ts | 34 +---- 4 files changed, 134 insertions(+), 54 deletions(-) diff --git a/packages/cli/src/__tests__/runtime-host-run-command.test.ts b/packages/cli/src/__tests__/runtime-host-run-command.test.ts index c0173f2f6e..b293cc952c 100644 --- a/packages/cli/src/__tests__/runtime-host-run-command.test.ts +++ b/packages/cli/src/__tests__/runtime-host-run-command.test.ts @@ -313,6 +313,82 @@ describe('Runtime Host maka run adapter', () => { ); }); + test('returns exit code 1 when a same-named tool retries a different target (live)', async () => { + const fixture = runFixture({ + turnEvents: sandboxBoundaryEvents( + 'turn-1', + 'step-1', + 'step-2', + 'Partial answer', + 'Read', + { path: '/workspace/README.md' }, + { path: '/outside/secret.txt' }, + ), + }); + + const exitCode = await runFixtureCommand(fixture, ['accept same-name different target']); + + assert.equal(exitCode, 1); + }); + + test('returns exit code 1 when a same-named tool retries a different target (durable)', async () => { + const fixture = runFixture({ + graph: true, + finalMessages: sandboxBoundaryMessages( + 'step-1', + 'step-2', + 'Read', + { path: '/workspace/README.md' }, + { path: '/outside/secret.txt' }, + ), + }); + + const exitCode = await runFixtureCommand(fixture, [ + 'accept durable same-name different target', + '--graph', + ]); + + assert.equal(exitCode, 1); + }); + + test('keeps a same-target retry unresolved because the boundary cannot move (live)', async () => { + const fixture = runFixture({ + turnEvents: sandboxBoundaryEvents( + 'turn-1', + 'step-1', + 'step-2', + 'Recovered answer', + 'Read', + { path: '/workspace/README.md' }, + { path: '/workspace/README.md' }, + ), + }); + + const exitCode = await runFixtureCommand(fixture, ['accept same-target retry']); + + assert.equal(exitCode, 1); + }); + + test('returns exit code 1 when a denied widening precedes any later tool success', async () => { + const stderr: string[] = []; + const fixture = runFixture({ + turnEvents: deniedWideningEvents('turn-1'), + }); + + const exitCode = await runFixtureCommand( + fixture, + ['accept post-denial success'], + () => {}, + (text) => stderr.push(text), + ); + + assert.equal(exitCode, 1); + assert.equal( + stderr.join(''), + 'maka run: sandbox boundary expansion is unavailable in non-interactive mode\n', + ); + }); + test('returns exit code 1 when reconnect restores a missed sandbox failure', async () => { let publishReplacement = () => {}; const fixture = runFixture({ @@ -351,18 +427,26 @@ describe('Runtime Host maka run adapter', () => { assert.equal(stdout.join(''), 'Final graph answer\n'); }); - test('returns exit code 0 when a root Graph boundary failure recovers', async () => { + test('keeps a root Graph boundary failure unresolved even when a later same-named call succeeds', async () => { const stdout: string[] = []; + const stderr: string[] = []; const fixture = runFixture({ graph: true, turnEvents: sandboxBoundaryEvents('turn-1', 'step-1', 'step-2', 'Recovered answer'), }); - const exitCode = await runFixtureCommand(fixture, ['recover once', '--graph'], (text) => - stdout.push(text), + const exitCode = await runFixtureCommand( + fixture, + ['recover once', '--graph'], + (text) => stdout.push(text), + (text) => stderr.push(text), ); - assert.equal(exitCode, 0); - assert.equal(stdout.join(''), 'Final graph answer\n'); + assert.equal(exitCode, 1); + assert.equal(stdout.join(''), ''); + assert.equal( + stderr.join(''), + 'maka run: sandbox boundary expansion is unavailable in non-interactive mode\n', + ); }); test('returns exit code 1 when a same-step Graph sibling succeeds after a sandbox failure', async () => { @@ -465,7 +549,7 @@ describe('Runtime Host maka run adapter', () => { assert.equal(observed.at(-1)?.finalOutput, 'Final graph answer'); }); - test('reports a recovered sandbox boundary from live and durable Turns', async () => { + test('keeps the sandbox boundary unresolved across live and durable Turns', async () => { const live = await observeFixtureOutcome({ turnEvents: sandboxBoundaryEvents('turn-1', 'step-1', 'step-2', 'Recovered answer'), }); @@ -474,8 +558,8 @@ describe('Runtime Host maka run adapter', () => { finalMessages: sandboxBoundaryMessages('step-1', 'step-2'), }); - assert.equal(live.sandboxBoundary, 'recovered'); - assert.equal(durable.sandboxBoundary, 'recovered'); + assert.equal(live.sandboxBoundary, 'unresolved'); + assert.equal(durable.sandboxBoundary, 'unresolved'); }); test('leaves sandbox failures unresolved when their provider steps are unavailable', async () => { @@ -491,7 +575,7 @@ describe('Runtime Host maka run adapter', () => { assert.equal(durable.sandboxBoundary, 'unresolved'); }); - test('returns a recovered boundary to unresolved after a later sandbox failure', async () => { + test('keeps the boundary unresolved across interleaved successes and a later sandbox failure', async () => { const outcome = await observeFixtureOutcome({ turnEvents: sandboxFailureAfterRecoveryEvents('turn-1'), }); @@ -1413,16 +1497,22 @@ function sandboxBoundaryMessages( failureStepId: string | undefined, successStepId: string | undefined, successToolName = 'Read', + successArgs: unknown = {}, + failureArgs: unknown = {}, ): StoredMessage[] { const sameStep = failureStepId !== undefined && failureStepId === successStepId; return [ ...graphMessages(false), - ...(failureStepId === undefined ? [] : [storedToolCall('turn-2', 'tool-1', failureStepId, 5)]), - ...(sameStep ? [storedToolCall('turn-2', 'tool-2', successStepId, 6, successToolName)] : []), + ...(failureStepId === undefined + ? [] + : [storedToolCall('turn-2', 'tool-1', failureStepId, 5, 'Read', failureArgs)]), + ...(sameStep + ? [storedToolCall('turn-2', 'tool-2', successStepId, 6, successToolName, successArgs)] + : []), sandboxFailureToolResult('turn-2', 7), ...(successStepId === undefined || sameStep ? [] - : [storedToolCall('turn-2', 'tool-2', successStepId, 8, successToolName)]), + : [storedToolCall('turn-2', 'tool-2', successStepId, 8, successToolName, successArgs)]), successfulToolResult('turn-2', 9), { type: 'turn_state', @@ -1560,18 +1650,34 @@ async function* sandboxBoundaryEvents( successStepId: string | undefined, text: string, successToolName = 'Read', + successArgs: unknown = {}, + failureArgs: unknown = {}, ): AsyncIterable { const sameStep = failureStepId !== undefined && failureStepId === successStepId; - if (failureStepId !== undefined) yield toolStart(turnId, 'tool-1', failureStepId, 1); - if (sameStep) yield toolStart(turnId, 'tool-2', successStepId, 2, successToolName); + if (failureStepId !== undefined) { + yield toolStart(turnId, 'tool-1', failureStepId, 1, 'Read', failureArgs); + } + if (sameStep) { + yield toolStart(turnId, 'tool-2', successStepId, 2, successToolName, successArgs); + } yield sandboxFailureToolResult(turnId, 3); if (successStepId !== undefined && !sameStep) { - yield toolStart(turnId, 'tool-2', successStepId, 4, successToolName); + yield toolStart(turnId, 'tool-2', successStepId, 4, successToolName, successArgs); } yield successfulToolResult(turnId, 5); yield* eventsFor(turnId, text, 6); } +async function* deniedWideningEvents(turnId: string): AsyncIterable { + yield toolStart(turnId, 'tool-1', 'step-1', 1, 'Read', { path: '/outside/secret.txt' }); + yield sandboxFailureToolResult(turnId, 2); + yield toolStart(turnId, 'tool-2', 'step-2', 3, 'request_sandbox_boundary'); + yield successfulToolResult(turnId, 4, 'tool-2'); + yield toolStart(turnId, 'tool-3', 'step-3', 5, 'Read', { path: '/workspace/README.md' }); + yield successfulToolResult(turnId, 6, 'tool-3'); + yield* eventsFor(turnId, 'Recovered answer', 7); +} + async function* projectedSameStepSandboxFailureEvents(turnId: string): AsyncIterable { yield toolStart(turnId, 'tool-1', 'step-1', 1); yield toolStart(turnId, 'tool-2', 'step-1', 2); @@ -1715,6 +1821,7 @@ function toolStart( stepId: string, ts: number, toolName = 'Read', + args: unknown = {}, ): Extract { return { type: 'tool_start', @@ -1723,7 +1830,7 @@ function toolStart( ts, toolUseId, toolName, - args: {}, + args, stepId, }; } @@ -1734,6 +1841,7 @@ function storedToolCall( stepId: string, ts: number, toolName = 'Read', + args: unknown = {}, ): Extract { return { type: 'tool_call', @@ -1741,7 +1849,7 @@ function storedToolCall( turnId, ts, toolName, - args: {}, + args, stepId, }; } diff --git a/packages/cli/src/activation-command.ts b/packages/cli/src/activation-command.ts index aa0fce3f51..17511f233b 100644 --- a/packages/cli/src/activation-command.ts +++ b/packages/cli/src/activation-command.ts @@ -588,10 +588,7 @@ export async function runMakaActivationCli( if (invocation?.failure?.class === 'permission_denied') { return finish('blocked', 'permission_denied', undefined, 'grant_permission'); } - if ( - (streamBoundaryFailure && invocation?.sandboxBoundary !== 'recovered') || - invocation?.sandboxBoundary === 'unresolved' - ) { + if (streamBoundaryFailure || invocation?.sandboxBoundary === 'unresolved') { return finish('blocked', 'permission_required', undefined, 'grant_permission'); } if (!invocation) return finish('fatal_failure', 'missing_invocation'); diff --git a/packages/cli/src/run-command-core.ts b/packages/cli/src/run-command-core.ts index 2fba37b35e..2590b7a2a1 100644 --- a/packages/cli/src/run-command-core.ts +++ b/packages/cli/src/run-command-core.ts @@ -82,7 +82,7 @@ export interface MakaRunOutcome { status: 'completed' | 'failed'; finalOutput?: string; failure?: { class: string; message?: string }; - sandboxBoundary: 'none' | 'unresolved' | 'recovered'; + sandboxBoundary: 'none' | 'unresolved'; } export interface MakaRunContextInput { @@ -311,10 +311,7 @@ export async function runMakaTextCliCore( ...(parsed.options.hostProfileId ? { hostProfileId: parsed.options.hostProfileId } : {}), ...(parsed.options.projectId ? { projectId: parsed.options.projectId } : {}), runOutcomeObserver: (result) => { - if (result.sandboxBoundary === 'recovered') { - boundaryFailureInvocationIds.delete(result.outcomeId); - unclassifiedBoundaryFailure = false; - } else if (result.sandboxBoundary === 'unresolved') { + if (result.sandboxBoundary === 'unresolved') { boundaryFailureInvocationIds.add(result.outcomeId); unclassifiedBoundaryFailure = false; } diff --git a/packages/cli/src/runtime-host-run-command.ts b/packages/cli/src/runtime-host-run-command.ts index daa99e57b4..742d7d24bd 100644 --- a/packages/cli/src/runtime-host-run-command.ts +++ b/packages/cli/src/runtime-host-run-command.ts @@ -596,14 +596,10 @@ class TurnOutcomeClassifier { >(); readonly #unresolvedSandboxFailures = new Map< string, - { - readonly failedStepId: string | undefined; - readonly failedToolName: string | undefined; - } + { readonly failedStepId: string | undefined; readonly failedToolName: string | undefined } >(); #finalOutput: string | undefined; #terminal: TerminalOutcomeObservation | undefined; - #sandboxBoundaryRecovered = false; constructor(outcomeId: string) { this.#outcomeId = outcomeId; @@ -628,29 +624,16 @@ class TurnOutcomeClassifier { }); return; case 'tool_result': { - const call = this.#callByToolUseId.get(observation.toolUseId); if (observation.outcome === 'sandbox_failure') { + const call = this.#callByToolUseId.get(observation.toolUseId); this.#unresolvedSandboxFailures.set(observation.toolUseId, { failedStepId: call?.stepId, failedToolName: call?.toolName, }); - return; - } - const unresolved = [...this.#unresolvedSandboxFailures.values()]; - // The wire has no retry identity. A later success can only prove recovery - // when there is exactly one unresolved candidate. - if ( - observation.outcome === 'success' && - call?.toolName !== 'request_sandbox_boundary' && - unresolved.length === 1 && - call?.stepId !== undefined && - unresolved[0]?.failedStepId !== undefined && - call.stepId !== unresolved[0].failedStepId && - call.toolName === unresolved[0].failedToolName - ) { - this.#unresolvedSandboxFailures.clear(); - this.#sandboxBoundaryRecovered = true; } + // No clearing path: `maka run` denies every widening request, so the + // boundary cannot move mid-Turn and a later success cannot prove that + // a blocked call recovered. The failure stays unresolved to the end. return; } } @@ -662,12 +645,7 @@ class TurnOutcomeClassifier { const terminal = this.#terminal; if (!terminal && incomplete === 'pending') return undefined; const completed = terminal?.status === 'completed'; - const sandboxBoundary = - this.#unresolvedSandboxFailures.size > 0 - ? 'unresolved' - : this.#sandboxBoundaryRecovered - ? 'recovered' - : 'none'; + const sandboxBoundary = this.#unresolvedSandboxFailures.size > 0 ? 'unresolved' : 'none'; const failure = terminal?.status === 'failed' ? terminal.failure From 77a641e79bf39c2fd489cd7a36f4d2a5a1215950 Mon Sep 17 00:00:00 2001 From: ggbdpq Date: Thu, 3 Sep 2026 21:42:35 +0800 Subject: [PATCH 2/3] refactor(cli): collapse the sandbox failure ledger to the ids that gate the outcome Per review on #4506: with the recovery heuristic gone, the classifier's per-failure step/tool bookkeeping participated in no decision while reading as though it did. The unresolved set is now just the blocked tool-use ids, and the tool_call observation variant with its step/name projections is deleted. Also pins the activation transition this deletion creates: an activation whose stream carried a sandbox boundary failure while the invocation completed now blocks with permission_required and exits 3 instead of completing with exit 0, and trims the same-target fixture that traversed identical code to the different-target case. --- .../src/__tests__/activation-command.test.ts | 51 +++++++++++++++++++ .../runtime-host-run-command.test.ts | 44 +++------------- packages/cli/src/runtime-host-run-command.ts | 43 +--------------- 3 files changed, 60 insertions(+), 78 deletions(-) diff --git a/packages/cli/src/__tests__/activation-command.test.ts b/packages/cli/src/__tests__/activation-command.test.ts index 6a6978daa8..36d5756f21 100644 --- a/packages/cli/src/__tests__/activation-command.test.ts +++ b/packages/cli/src/__tests__/activation-command.test.ts @@ -461,6 +461,57 @@ describe('maka activate JSONL protocol', () => { }); }); + test('blocks a completed invocation whose stream carried a boundary failure', async () => { + // Before #4506 the classifier cleared `sandboxBoundary` when a later tool + // succeeded, so this activation completed with exit 0. The stream event is + // the hard fact; the outcome shape here (`completed`, boundary `none`) is + // exactly what that clearing produced. + const lines: string[] = []; + const boundaryFailure = { + kind: 'text', + text: 'Write requires an approved session sandbox boundary expansion.', + sandboxFailure: { + reason: 'sandbox_boundary_required', + requiredExpansion: { + filesystem: { + entries: [{ path: '/tmp/output', access: 'write', scope: 'subtree' }], + }, + }, + }, + } as const; + const result = await runMakaActivationCli( + [ + '--state-root', + ROOTS.stateRoot, + '--workspace-root', + ROOTS.workspaceRoot, + '--config-root', + ROOTS.configRoot, + ], + { + ...fakeDeps({ + result: completedResult(), + events: [ + { + type: 'tool_result', + id: 'event-boundary-result', + turnId: 'turn-1', + ts: 1, + toolUseId: 'tool-boundary', + isError: true, + content: boundaryFailure, + }, + ], + }), + writeStdout: (text) => lines.push(text.trim()), + }, + ); + + assert.equal(result, 3); + assert.equal(JSON.parse(lines.at(-1)!).status, 'blocked'); + assert.equal(JSON.parse(lines.at(-1)!).reason, 'permission_required'); + }); + test('retries non-permission blocked sessions instead of requesting permission', async () => { for (const blockedReason of ['auth', 'tool_failed'] as const) { const lines: string[] = []; diff --git a/packages/cli/src/__tests__/runtime-host-run-command.test.ts b/packages/cli/src/__tests__/runtime-host-run-command.test.ts index b293cc952c..051ba675fc 100644 --- a/packages/cli/src/__tests__/runtime-host-run-command.test.ts +++ b/packages/cli/src/__tests__/runtime-host-run-command.test.ts @@ -321,8 +321,6 @@ describe('Runtime Host maka run adapter', () => { 'step-2', 'Partial answer', 'Read', - { path: '/workspace/README.md' }, - { path: '/outside/secret.txt' }, ), }); @@ -334,13 +332,7 @@ describe('Runtime Host maka run adapter', () => { test('returns exit code 1 when a same-named tool retries a different target (durable)', async () => { const fixture = runFixture({ graph: true, - finalMessages: sandboxBoundaryMessages( - 'step-1', - 'step-2', - 'Read', - { path: '/workspace/README.md' }, - { path: '/outside/secret.txt' }, - ), + finalMessages: sandboxBoundaryMessages('step-1', 'step-2', 'Read'), }); const exitCode = await runFixtureCommand(fixture, [ @@ -351,24 +343,6 @@ describe('Runtime Host maka run adapter', () => { assert.equal(exitCode, 1); }); - test('keeps a same-target retry unresolved because the boundary cannot move (live)', async () => { - const fixture = runFixture({ - turnEvents: sandboxBoundaryEvents( - 'turn-1', - 'step-1', - 'step-2', - 'Recovered answer', - 'Read', - { path: '/workspace/README.md' }, - { path: '/workspace/README.md' }, - ), - }); - - const exitCode = await runFixtureCommand(fixture, ['accept same-target retry']); - - assert.equal(exitCode, 1); - }); - test('returns exit code 1 when a denied widening precedes any later tool success', async () => { const stderr: string[] = []; const fixture = runFixture({ @@ -1497,22 +1471,20 @@ function sandboxBoundaryMessages( failureStepId: string | undefined, successStepId: string | undefined, successToolName = 'Read', - successArgs: unknown = {}, - failureArgs: unknown = {}, ): StoredMessage[] { const sameStep = failureStepId !== undefined && failureStepId === successStepId; return [ ...graphMessages(false), ...(failureStepId === undefined ? [] - : [storedToolCall('turn-2', 'tool-1', failureStepId, 5, 'Read', failureArgs)]), + : [storedToolCall('turn-2', 'tool-1', failureStepId, 5, 'Read')]), ...(sameStep - ? [storedToolCall('turn-2', 'tool-2', successStepId, 6, successToolName, successArgs)] + ? [storedToolCall('turn-2', 'tool-2', successStepId, 6, successToolName)] : []), sandboxFailureToolResult('turn-2', 7), ...(successStepId === undefined || sameStep ? [] - : [storedToolCall('turn-2', 'tool-2', successStepId, 8, successToolName, successArgs)]), + : [storedToolCall('turn-2', 'tool-2', successStepId, 8, successToolName)]), successfulToolResult('turn-2', 9), { type: 'turn_state', @@ -1650,19 +1622,17 @@ async function* sandboxBoundaryEvents( successStepId: string | undefined, text: string, successToolName = 'Read', - successArgs: unknown = {}, - failureArgs: unknown = {}, ): AsyncIterable { const sameStep = failureStepId !== undefined && failureStepId === successStepId; if (failureStepId !== undefined) { - yield toolStart(turnId, 'tool-1', failureStepId, 1, 'Read', failureArgs); + yield toolStart(turnId, 'tool-1', failureStepId, 1, 'Read'); } if (sameStep) { - yield toolStart(turnId, 'tool-2', successStepId, 2, successToolName, successArgs); + yield toolStart(turnId, 'tool-2', successStepId, 2, successToolName); } yield sandboxFailureToolResult(turnId, 3); if (successStepId !== undefined && !sameStep) { - yield toolStart(turnId, 'tool-2', successStepId, 4, successToolName, successArgs); + yield toolStart(turnId, 'tool-2', successStepId, 4, successToolName); } yield successfulToolResult(turnId, 5); yield* eventsFor(turnId, text, 6); diff --git a/packages/cli/src/runtime-host-run-command.ts b/packages/cli/src/runtime-host-run-command.ts index 742d7d24bd..9bb0da7e93 100644 --- a/packages/cli/src/runtime-host-run-command.ts +++ b/packages/cli/src/runtime-host-run-command.ts @@ -574,12 +574,6 @@ type TurnOutcomeObservation = readonly status: 'failed'; readonly failure: NonNullable; } - | { - readonly kind: 'tool_call'; - readonly toolUseId: string; - readonly stepId: string | undefined; - readonly toolName: string; - } | { readonly kind: 'tool_result'; readonly toolUseId: string; @@ -590,14 +584,7 @@ type TerminalOutcomeObservation = Extract(); - readonly #unresolvedSandboxFailures = new Map< - string, - { readonly failedStepId: string | undefined; readonly failedToolName: string | undefined } - >(); + readonly #unresolvedSandboxFailures = new Set(); #finalOutput: string | undefined; #terminal: TerminalOutcomeObservation | undefined; @@ -617,19 +604,9 @@ class TurnOutcomeClassifier { this.#terminal = observation; } return; - case 'tool_call': - this.#callByToolUseId.set(observation.toolUseId, { - stepId: observation.stepId, - toolName: observation.toolName, - }); - return; case 'tool_result': { if (observation.outcome === 'sandbox_failure') { - const call = this.#callByToolUseId.get(observation.toolUseId); - this.#unresolvedSandboxFailures.set(observation.toolUseId, { - failedStepId: call?.stepId, - failedToolName: call?.toolName, - }); + this.#unresolvedSandboxFailures.add(observation.toolUseId); } // No clearing path: `maka run` denies every widening request, so the // boundary cannot move mid-Turn and a later success cannot prove that @@ -686,14 +663,6 @@ function observationFromSessionEvent(event: SessionEvent): TurnOutcomeObservatio if (event.type === 'complete') { return observationFromCompleteEvent(event); } - if (event.type === 'tool_start') { - return { - kind: 'tool_call', - toolUseId: event.toolUseId, - stepId: event.stepId, - toolName: event.toolName, - }; - } return event.type === 'tool_result' ? observationFromToolResult(event) : undefined; } @@ -723,14 +692,6 @@ function observationFromStoredMessage(message: StoredMessage): TurnOutcomeObserv }, }; } - if (message.type === 'tool_call') { - return { - kind: 'tool_call', - toolUseId: message.id, - stepId: message.stepId, - toolName: message.toolName, - }; - } return message.type === 'tool_result' ? observationFromToolResult(message) : undefined; } From 0796f1112e65383b787e429c6fd80561f17a6ee1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=85=B3=E5=A4=A9=E8=B1=AA?= Date: Fri, 4 Sep 2026 16:31:28 +0800 Subject: [PATCH 3/3] refactor(cli): collapse boundary tracking to a single boolean Astro-Han's P2/P3 review items: - P2 (activation test comment): the fixture guards the current contract rather than pinning the transition, since `recovered` no longer exists in the outcome type and an injected `MakaRunOutcome` cannot express the old shape. PR body states plainly that the `maka activate` transition is not regression-coverable after the deletion. - P3 (run-command-core): `boundaryFailureInvocationIds` and `unclassifiedBoundaryFailure` collapse into a single `boundaryFailure` boolean. The set stored ids whose only reader was `.size > 0`, and the dead assignment at `:316` (setting `false` immediately after adding an id) existed to pair with the `recovered` branch this PR deleted. Five lines removed, no assertion changes. - P3 (test fixture names): "retries a different target" renamed to "keeps the boundary unresolved when a later same-named call succeeds" since the fixtures pass no args and the classifier reads no args after the deletion. - P3 (test helper params): reverted the `args` parameter additions on `toolStart` and `storedToolCall` (four call sites: two in `deniedWideningEvents`, one in `sandboxBoundaryEvents`, one in `sandboxBoundaryMessages`). The classifier no longer reads args, so the path strings were decoration. Verification: runtime-host-run-command 39/39, activation-command 12/12, format:check clean, asf-headers clean. Generated-by: GLM-5.3-Flash (ZCode) --- .../src/__tests__/activation-command.test.ts | 10 +++--- .../runtime-host-run-command.test.ts | 32 ++++++------------- packages/cli/src/run-command-core.ts | 16 +++------- 3 files changed, 21 insertions(+), 37 deletions(-) diff --git a/packages/cli/src/__tests__/activation-command.test.ts b/packages/cli/src/__tests__/activation-command.test.ts index 36d5756f21..e3b125cab1 100644 --- a/packages/cli/src/__tests__/activation-command.test.ts +++ b/packages/cli/src/__tests__/activation-command.test.ts @@ -462,10 +462,12 @@ describe('maka activate JSONL protocol', () => { }); test('blocks a completed invocation whose stream carried a boundary failure', async () => { - // Before #4506 the classifier cleared `sandboxBoundary` when a later tool - // succeeded, so this activation completed with exit 0. The stream event is - // the hard fact; the outcome shape here (`completed`, boundary `none`) is - // exactly what that clearing produced. + // Guards the current contract: a completed invocation whose stream carried + // a boundary failure reports `blocked` / `permission_required` with exit 3. + // The `maka activate` transition itself (main completed with exit 0 when + // the classifier cleared `recovered`) is not regression-coverable after + // the deletion: `recovered` no longer exists in the outcome type, so an + // injected `MakaRunOutcome` cannot express the old shape. const lines: string[] = []; const boundaryFailure = { kind: 'text', diff --git a/packages/cli/src/__tests__/runtime-host-run-command.test.ts b/packages/cli/src/__tests__/runtime-host-run-command.test.ts index 051ba675fc..fc13e5b2eb 100644 --- a/packages/cli/src/__tests__/runtime-host-run-command.test.ts +++ b/packages/cli/src/__tests__/runtime-host-run-command.test.ts @@ -313,15 +313,9 @@ describe('Runtime Host maka run adapter', () => { ); }); - test('returns exit code 1 when a same-named tool retries a different target (live)', async () => { + test('keeps the boundary unresolved when a later same-named call succeeds (live)', async () => { const fixture = runFixture({ - turnEvents: sandboxBoundaryEvents( - 'turn-1', - 'step-1', - 'step-2', - 'Partial answer', - 'Read', - ), + turnEvents: sandboxBoundaryEvents('turn-1', 'step-1', 'step-2', 'Partial answer', 'Read'), }); const exitCode = await runFixtureCommand(fixture, ['accept same-name different target']); @@ -329,7 +323,7 @@ describe('Runtime Host maka run adapter', () => { assert.equal(exitCode, 1); }); - test('returns exit code 1 when a same-named tool retries a different target (durable)', async () => { + test('keeps the boundary unresolved when a later same-named call succeeds (durable)', async () => { const fixture = runFixture({ graph: true, finalMessages: sandboxBoundaryMessages('step-1', 'step-2', 'Read'), @@ -1475,12 +1469,8 @@ function sandboxBoundaryMessages( const sameStep = failureStepId !== undefined && failureStepId === successStepId; return [ ...graphMessages(false), - ...(failureStepId === undefined - ? [] - : [storedToolCall('turn-2', 'tool-1', failureStepId, 5, 'Read')]), - ...(sameStep - ? [storedToolCall('turn-2', 'tool-2', successStepId, 6, successToolName)] - : []), + ...(failureStepId === undefined ? [] : [storedToolCall('turn-2', 'tool-1', failureStepId, 5)]), + ...(sameStep ? [storedToolCall('turn-2', 'tool-2', successStepId, 6, successToolName)] : []), sandboxFailureToolResult('turn-2', 7), ...(successStepId === undefined || sameStep ? [] @@ -1625,7 +1615,7 @@ async function* sandboxBoundaryEvents( ): AsyncIterable { const sameStep = failureStepId !== undefined && failureStepId === successStepId; if (failureStepId !== undefined) { - yield toolStart(turnId, 'tool-1', failureStepId, 1, 'Read'); + yield toolStart(turnId, 'tool-1', failureStepId, 1); } if (sameStep) { yield toolStart(turnId, 'tool-2', successStepId, 2, successToolName); @@ -1639,11 +1629,11 @@ async function* sandboxBoundaryEvents( } async function* deniedWideningEvents(turnId: string): AsyncIterable { - yield toolStart(turnId, 'tool-1', 'step-1', 1, 'Read', { path: '/outside/secret.txt' }); + yield toolStart(turnId, 'tool-1', 'step-1', 1); yield sandboxFailureToolResult(turnId, 2); yield toolStart(turnId, 'tool-2', 'step-2', 3, 'request_sandbox_boundary'); yield successfulToolResult(turnId, 4, 'tool-2'); - yield toolStart(turnId, 'tool-3', 'step-3', 5, 'Read', { path: '/workspace/README.md' }); + yield toolStart(turnId, 'tool-3', 'step-3', 5); yield successfulToolResult(turnId, 6, 'tool-3'); yield* eventsFor(turnId, 'Recovered answer', 7); } @@ -1791,7 +1781,6 @@ function toolStart( stepId: string, ts: number, toolName = 'Read', - args: unknown = {}, ): Extract { return { type: 'tool_start', @@ -1800,7 +1789,7 @@ function toolStart( ts, toolUseId, toolName, - args, + args: {}, stepId, }; } @@ -1811,7 +1800,6 @@ function storedToolCall( stepId: string, ts: number, toolName = 'Read', - args: unknown = {}, ): Extract { return { type: 'tool_call', @@ -1819,7 +1807,7 @@ function storedToolCall( turnId, ts, toolName, - args, + args: {}, stepId, }; } diff --git a/packages/cli/src/run-command-core.ts b/packages/cli/src/run-command-core.ts index 2590b7a2a1..c57897fd0d 100644 --- a/packages/cli/src/run-command-core.ts +++ b/packages/cli/src/run-command-core.ts @@ -280,8 +280,7 @@ export async function runMakaTextCliCore( } let outcome: MakaRunOutcome | undefined; - let unclassifiedBoundaryFailure = false; - const boundaryFailureInvocationIds = new Set(); + let boundaryFailure = false; let context: MakaRunContext; try { context = await deps.createContext({ @@ -311,10 +310,7 @@ export async function runMakaTextCliCore( ...(parsed.options.hostProfileId ? { hostProfileId: parsed.options.hostProfileId } : {}), ...(parsed.options.projectId ? { projectId: parsed.options.projectId } : {}), runOutcomeObserver: (result) => { - if (result.sandboxBoundary === 'unresolved') { - boundaryFailureInvocationIds.add(result.outcomeId); - unclassifiedBoundaryFailure = false; - } + if (result.sandboxBoundary === 'unresolved') boundaryFailure = true; outcome = result; }, }); @@ -400,7 +396,7 @@ export async function runMakaTextCliCore( : {}), })) { if (event.type === 'sandbox_boundary_request') { - unclassifiedBoundaryFailure = true; + boundaryFailure = true; deps.writeStderr( 'maka run: sandbox boundary expansion is unavailable in non-interactive mode\n', ); @@ -411,7 +407,7 @@ export async function runMakaTextCliCore( } const sandboxFailureReason = sessionEventSandboxBoundaryFailureReason(event); if (sandboxFailureReason) { - unclassifiedBoundaryFailure = true; + boundaryFailure = true; deps.writeStderr( sandboxFailureReason === 'requires_bypass' ? 'maka run: sandbox bypass requires an explicit --yolo\n' @@ -443,9 +439,7 @@ export async function runMakaTextCliCore( return 1; } if (streamFailed) return 1; - if (unclassifiedBoundaryFailure || boundaryFailureInvocationIds.size > 0) { - return 1; - } + if (boundaryFailure) return 1; if (!outcome) { deps.writeStderr('maka run: runtime produced no outcome\n'); return 1;