From 976deda91b5b970add41c9deb208ae2f6cdf8312 Mon Sep 17 00:00:00 2001 From: kingwl Date: Fri, 10 Jul 2026 20:26:59 +0800 Subject: [PATCH] =?UTF-8?q?fix(mode):=20run=5Fcode=20passes=20both=20layer?= =?UTF-8?q?s=20as=20a=20transport=20=E2=80=94=20Code=20Mode=20composes=20w?= =?UTF-8?q?ith=20plan=20mode?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review finding, valid: under the registry's Code Mode the assembly's only wire tool is run_code, which the plan allowlist filtered out — leaving the model with NO tools at all, the exit review included. The composition exists today (the acp-agent example ships a code-mode overlay), so plan mode bricked it outright. run_code is a transport, not a capability: every bridged sub-call is serialized back through ToolRegistry.execute() carrying the same agent, so tools/pre-execute judges each capability individually — exactly like native calls. Both layers now exempt it by name: the filter keeps it visible (tests pin plan-mode Code Mode assembly = ['run_code']) and the gate passes the wrapper while the same run's write sub-call still denies with the plan-mode reason. Documented residual, same class as the prepend-after-load one: the SDK section renders from the registry's store, so a plan-mode program may be offered bindings whose dispatch the gate then denies — nothing runs that a native call could not. --- .../feature/2026-07-07-plan-mode.md | 6 ++- packages/mode/mode/README.md | 2 +- packages/mode/mode/package.json | 1 + packages/mode/mode/src/index.ts | 15 ++++++- packages/mode/mode/tests/mode.spec.ts | 42 +++++++++++++++++++ pnpm-lock.yaml | 3 ++ 6 files changed, 64 insertions(+), 5 deletions(-) diff --git a/docs/rfc/implemented/feature/2026-07-07-plan-mode.md b/docs/rfc/implemented/feature/2026-07-07-plan-mode.md index 316e41af40..25c4da2a74 100644 --- a/docs/rfc/implemented/feature/2026-07-07-plan-mode.md +++ b/docs/rfc/implemented/feature/2026-07-07-plan-mode.md @@ -94,7 +94,7 @@ A contained `session/event` listener ([defensive patterns](../../../defensive-pa A `system-prompt/assemble` waterfall listener reads the calling agent's mode (the `AssembleContext` carries `agent`) and, in a non-default mode, filters `assembly.tools` down to the mode's allowlist and appends the mode's guidance section. The loop already renders per step and logs the result: entering or leaving a mode surfaces on the next step as a `request/header-delta` — or as the full `request/header` fallback snapshot when the change is inexpressible in the delta encoding (adding `exit_plan_mode` resorts the canonical tool list, and a pure reordering has no delta form) — so every mode transition is an attributable log fact. The section is static per mode and the plan itself stays in the conversation (messages and tool args, already in context), so a mode does not add per-step prompt churn — re-injecting plan state into every request ([Prior art](#prior-art)'s compaction-survival hack) is unnecessary and would only burn prefix cache. -The guidance section is an ordinary registered section, `{ name: 'mode:policy', order: 50, text: context => … }` — order 50 sits after the persona (0) and before tool guidance (100–199); it resolves to the folded mode's configured text and to `''` (dropped at render) for the default mode or an agent-less assembly. The tool filter wraps with `prepend: true`: it awaits `next()` and filters the RETURNED assembly's `tools`, so additions made anywhere inside its wrap — including every append-registered listener's post-`next()` mutation, regardless of load order — are covered. The filter enforces one rule in every mode: `exit_plan_mode` is visible IFF the agent's folded mode is `plan` — which is also what keeps a default-mode assembly byte-identical to a no-`dsh-mode` deployment even though the tool is always registered. In a non-default mode it additionally intersects with the mode's allowlist. +The guidance section is an ordinary registered section, `{ name: 'mode:policy', order: 50, text: context => … }` — order 50 sits after the persona (0) and before tool guidance (100–199); it resolves to the folded mode's configured text and to `''` (dropped at render) for the default mode or an agent-less assembly. The tool filter wraps with `prepend: true`: it awaits `next()` and filters the RETURNED assembly's `tools`, so additions made anywhere inside its wrap — including every append-registered listener's post-`next()` mutation, regardless of load order — are covered. `run_code` survives the filter in every mode: under the registry's Code Mode it is the only wire tool (filtering it would strip the model of everything, the exit included), and it is a transport, not a capability — each bridged sub-call re-enters the hard gate individually. The filter enforces one rule in every mode: `exit_plan_mode` is visible IFF the agent's folded mode is `plan` — which is also what keeps a default-mode assembly byte-identical to a no-`dsh-mode` deployment even though the tool is always registered. In a non-default mode it additionally intersects with the mode's allowlist. ### The hard layer: the gate @@ -103,6 +103,8 @@ The gate denies, with a mode-naming reason that steers the model back to plannin ```text tools/pre-execute: no exec.agent → next() // agent-less calls have no session to fold folded mode = default → next() + exec.name = run_code → next() // transport: every bridged sub-call re-enters + // this gate with the same agent allowlisted → next() // plan's list includes exit_plan_mode otherwise → deny // reason names the mode and points at exit_plan_mode ``` @@ -193,4 +195,4 @@ What holds now, pinned by the unit, protocol, and snapshot tiers: - `exit_plan_mode`'s approve path flips the mode and restores the full toolset on the next step; the keep-planning path returns the corrective `isError` carrying the user's feedback and stays in plan mode; the ACP `session/set_mode` round-trip updates `current_mode_update`, and the exit review prompts through each surface's user-interaction provider. - The docs tail shipped with the landing: READMEs, regenerated catalogs (persistence log, config, cordis services, tools), the packages map and architecture rows, and the cookbook row. -The accepted costs: a pending user flip set while idle is lost if the process dies before the next turn (the UI re-applies; the idle-record primitive is the escape hatch if this bites in practice). Every mode transition is a logged header change and therefore a prefix-cache reset at the provider — inherent, visible in per-step usage, and an argument against mode-flapping UIs, not against the design. The mode filter prepends, so only a listener that ALSO prepends after `dsh-mode` loads can wrap outside it and re-widen filtered schemas — the one shipped instance is the structured runtime's per-spawn final-assembly wrapper, whose `structured_output` is on the plan allowlist precisely so the filter, that wrapper, and the gate agree; for any future such listener the hard gate keeps a re-widened tool non-executable, and the residual cost is cosmetic (the model sees a tool it cannot use), accepted rather than mechanized. Plan mode's shipped allowlist excludes `bash` and `subagent`, which costs real exploration power until the sandbox family and mode inheritance land — a deployment that accepts the risk can widen its own config today. Two in-flight stacks touch the ACP mode surface (this one and the sandbox branch's config options, whose feature-matrix stance records session modes as deliberately unmodeled): the picker-to-modes / knobs-to-config-options division pinned in the [FAQ](#faq) is the contract, and the sandbox branch owes its matrix rows an amendment on merge-down. The ACP spec's draft v2 direction reportedly slates session modes for removal in favor of config options; if that lands, the picker migrates to a config-option select mechanically — the mode state and both enforcement layers are wire-agnostic — accepted. +The accepted costs: a pending user flip set while idle is lost if the process dies before the next turn (the UI re-applies; the idle-record primitive is the escape hatch if this bites in practice). Every mode transition is a logged header change and therefore a prefix-cache reset at the provider — inherent, visible in per-step usage, and an argument against mode-flapping UIs, not against the design. The mode filter prepends, so only a listener that ALSO prepends after `dsh-mode` loads can wrap outside it and re-widen filtered schemas — the one shipped instance is the structured runtime's per-spawn final-assembly wrapper, whose `structured_output` is on the plan allowlist precisely so the filter, that wrapper, and the gate agree; for any future such listener the hard gate keeps a re-widened tool non-executable, and the residual cost is cosmetic (the model sees a tool it cannot use), accepted rather than mechanized. Code Mode's SDK section is the same residual class: it renders from the registry's store, so in plan mode a program may be offered bindings whose dispatch the gate then denies — each sub-call is judged individually, so nothing runs that a native call could not. Plan mode's shipped allowlist excludes `bash` and `subagent`, which costs real exploration power until the sandbox family and mode inheritance land — a deployment that accepts the risk can widen its own config today. Two in-flight stacks touch the ACP mode surface (this one and the sandbox branch's config options, whose feature-matrix stance records session modes as deliberately unmodeled): the picker-to-modes / knobs-to-config-options division pinned in the [FAQ](#faq) is the contract, and the sandbox branch owes its matrix rows an amendment on merge-down. The ACP spec's draft v2 direction reportedly slates session modes for removal in favor of config options; if that lands, the picker migrates to a config-option select mechanically — the mode state and both enforcement layers are wire-agnostic — accepted. diff --git a/packages/mode/mode/README.md b/packages/mode/mode/README.md index ec0c263583..7a6652ed9c 100644 --- a/packages/mode/mode/README.md +++ b/packages/mode/mode/README.md @@ -12,7 +12,7 @@ The `default` mode is the absence of policy: no section, no filtering, no gate. **Soft — what the model sees.** A `system-prompt/assemble` listener filters the returned assembly's tools down to the mode's allowlist and the `mode:policy` section (order 50) renders the mode's guidance text. Every transition therefore surfaces as an attributable `request/header` event on the next step (a delta when expressible; adding `exit_plan_mode` resorts the canonical tool list, which the delta encoding cannot express, so entering plan mode logs the full fallback snapshot). The `exit_plan_mode` tool is visible IFF the folded mode is `plan`. -**Hard — what can run.** A `tools/pre-execute` listener denies, deny-by-default against the same allowlist, any call the mode does not permit — a hallucinated call to a still-registered (or freshly re-widened) tool cannot run. Agent-less executions and the default mode pass through; the gate judges by the LOGGED mode only, never a pending intent. +**Hard — what can run.** A `tools/pre-execute` listener denies, deny-by-default against the same allowlist, any call the mode does not permit — a hallucinated call to a still-registered (or freshly re-widened) tool cannot run. Agent-less executions and the default mode pass through; the gate judges by the LOGGED mode only, never a pending intent. `run_code` passes both layers as a TRANSPORT: under the registry's Code Mode it is the only wire tool, and every bridged sub-call re-enters this gate with the same agent, so the allowlist governs each capability individually. ## `ctx.modes` diff --git a/packages/mode/mode/package.json b/packages/mode/mode/package.json index 7d0052f130..ed76d7e0cc 100644 --- a/packages/mode/mode/package.json +++ b/packages/mode/mode/package.json @@ -32,6 +32,7 @@ "devDependencies": { "@deepseek-ai/dsh-agent": "workspace:^", "@deepseek-ai/dsh-agent-loop": "workspace:^", + "@deepseek-ai/dsh-code-runtime": "workspace:^", "@deepseek-ai/dsh-llm": "workspace:^", "@deepseek-ai/dsh-session": "workspace:^", "@deepseek-ai/dsh-system-prompt": "workspace:^", diff --git a/packages/mode/mode/src/index.ts b/packages/mode/mode/src/index.ts index f4671ed9a1..b590a6deda 100644 --- a/packages/mode/mode/src/index.ts +++ b/packages/mode/mode/src/index.ts @@ -26,7 +26,7 @@ import { Context, Service } from 'cordis' import type { Agent } from '@deepseek-ai/dsh-agent' import type { Session, SessionEvent } from '@deepseek-ai/dsh-session' -import { defineTool } from '@deepseek-ai/dsh-tools' +import { defineTool, RUN_CODE_NAME } from '@deepseek-ai/dsh-tools' import type { PreToolDecision } from '@deepseek-ai/dsh-tools' import type {} from '@deepseek-ai/dsh-system-prompt' import type {} from '@deepseek-ai/dsh-user-interaction' @@ -267,8 +267,14 @@ export class ModesService extends Service { return result } const allowed = new Set(active.definition.tools) + // run_code is a TRANSPORT, not a capability: under the registry's Code + // Mode it is the only wire tool (filtering it would leave the model + // with nothing, not even the exit), and every bridged sub-call + // re-enters tools/pre-execute with the same agent, where the allowlist + // governs each capability individually. result.tools = result.tools.filter(tool => - allowed.has(tool.name) && (tool.name !== EXIT_PLAN_MODE || active.name === PLAN_MODE)) + (allowed.has(tool.name) || tool.name === RUN_CODE_NAME) + && (tool.name !== EXIT_PLAN_MODE || active.name === PLAN_MODE)) return result }, { prepend: true }) @@ -276,6 +282,11 @@ export class ModesService extends Service { if (exec.agent === undefined) return next() const active = this.activeDefinition(exec.agent.session) if (active === undefined) return next() + // Transport pass-through: a run_code program's every tool call is + // serialized back through ToolRegistry.execute() with the same agent, + // so each sub-call is judged here individually — gating the wrapper + // would only remove the vehicle, not widen or narrow any capability. + if (exec.name === RUN_CODE_NAME) return next() if (active.definition.tools.includes(exec.name)) return next() const reason = active.name === PLAN_MODE ? `tool "${exec.name}" is not available in plan mode; continue planning and present your plan with ${EXIT_PLAN_MODE} when ready` diff --git a/packages/mode/mode/tests/mode.spec.ts b/packages/mode/mode/tests/mode.spec.ts index b9429de265..1db525fc4e 100644 --- a/packages/mode/mode/tests/mode.spec.ts +++ b/packages/mode/mode/tests/mode.spec.ts @@ -7,6 +7,7 @@ import { Session, SessionId } from '@deepseek-ai/dsh-session' import type { SessionEvent } from '@deepseek-ai/dsh-session' import { AgentId, type Agent } from '@deepseek-ai/dsh-agent' import UserInteractionService, { type AskUserQuestionRequest } from '@deepseek-ai/dsh-user-interaction' +import { CodeRuntime, type CodeRunRequest, type CodeRunResult } from '@deepseek-ai/dsh-code-runtime' import ModesService, { DEFAULT_MODE, EXIT_PLAN_MODE, PLAN_MODE, foldMode, resolveConfig } from '../src/index.ts' import type { ModeConfig } from '../src/index.ts' @@ -349,6 +350,28 @@ describe('the soft layer', () => { expect(assembly.tools.map(tool => tool.name)).toEqual(['exit_plan_mode', 'read']) }) + it('keeps run_code visible in plan mode under the registry Code Mode (transport, not capability)', async () => { + // Minimal scriptable runtime: the SDK section resolves ctx.codeRuntime at + // assembly time (the code-mode.spec fake's shape). + class FakeRuntime extends CodeRuntime { + readonly language = 'typescript' + readonly isolation = 'fake' + run(_request: CodeRunRequest): Promise { return Promise.resolve({ logs: [] }) } + } + const ctx = new Context() + await ctx.plugin(SystemPrompt) + await ctx.plugin(ToolRegistry, { mode: 'code' }) + await ctx.plugin(FakeRuntime) + await ctx.plugin(ModesService) + registerNamedTools(ctx, ['read', 'write']) + const agent = agentWithSession() + agent.session.append('mode/set', { mode: PLAN_MODE }) + const assembly = await ctx.systemPrompt.assemble({ agent }) + // Code Mode's only wire tool survives the filter — without it the model + // would have NO tools at all, not even a path to the exit review. + expect(assembly.tools.map(tool => tool.name)).toEqual(['run_code']) + }) + it('treats a dropped folded definition as the default mode', async () => { const ctx = await setup() registerNamedTools(ctx, ['read', 'write']) @@ -395,6 +418,25 @@ describe('the hard layer', () => { expect(denied.content).toEqual([{ type: 'text', text: 'Error: tool "write" is not available in "review" mode' }]) }) + it('passes run_code through the gate; bridged sub-calls are judged individually', async () => { + const ctx = await setup() + // Native mode here, so a stand-in run_code can register without clashing + // with the registry's own (Code Mode) instance; the gate exempts by name. + registerNamedTools(ctx, ['run_code', 'write']) + const agent = agentWithSession() + agent.session.append('mode/set', { mode: PLAN_MODE }) + const wrapper = await execute(ctx, 'run_code', agent) + expect(wrapper.isError).toBe(false) + // A sub-dispatch re-enters execute() with the same agent — the capability + // is what the allowlist judges, exactly like a native call. + const sub = await execute(ctx, 'write', agent) + expect(sub.isError).toBe(true) + expect(sub.content).toEqual([{ + type: 'text', + text: 'Error: tool "write" is not available in plan mode; continue planning and present your plan with exit_plan_mode when ready', + }]) + }) + it('judges by the logged mode only — a pending intent does not gate', async () => { const ctx = await setup() registerNamedTools(ctx, ['write']) diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 685b9ac738..74d31079f6 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -613,6 +613,9 @@ importers: '@deepseek-ai/dsh-agent-loop': specifier: workspace:^ version: link:../../core/agent-loop + '@deepseek-ai/dsh-code-runtime': + specifier: workspace:^ + version: link:../../code-runtime/code-runtime '@deepseek-ai/dsh-llm': specifier: workspace:^ version: link:../../llm/llm