From 994eaedc02aadb1ab00deadfce474d16cd10423a Mon Sep 17 00:00:00 2001 From: kingwl Date: Mon, 20 Jul 2026 14:01:53 +0800 Subject: [PATCH] review pass: purge stale enforcement language after the axis decoupling Self-review findings, all prose/metadata (no behavior change): the package description still claimed hard (execution) enforcement; the RFC Problem framing still defined plan mode as a read-only tool policy; integration/mode/snapshot-suite comments still referenced the removed sandbox clamp and cap; the header-fallback explanation said 'front-of-list insertion' where the actual mechanism is a non-tail insertion the append-only tools delta cannot express (exit_plan_mode sits mid-list alphabetically); AGENTS.md's layout line and the RFC's 'policy state/listener' phrasing move to collaboration-state wording. --- AGENTS.md | 2 +- .../feature/2026-07-07-plan-mode.md | 10 ++++---- examples/plan-acp-agent/tests/acp.snapshot.ts | 23 +++++++++---------- packages/mode/mode/README.md | 2 +- packages/mode/mode/package.json | 2 +- packages/mode/mode/tests/integration.spec.ts | 12 ++++++---- packages/mode/mode/tests/mode.spec.ts | 12 +++++----- 7 files changed, 32 insertions(+), 31 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 286b08212b..cca6f73cdf 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -23,7 +23,7 @@ packages/ @deepseek-ai/dsh- workspaces at packages/// subagent/ subagent seam + spawn/fork/ACP backends + delegation tool workflow/ workflow seam + worker-thread engine + the workflow tool todo/ the todo_write tool - mode/ session modes: plan mode as logged per-agent policy state + mode/ session modes: plan mode as logged per-agent collaboration state guard/ loop-hygiene plugins cordis/ self-referential toolset: the agent inspects/mounts plugins in its own runtime hooks/ Claude Code / Codex hook bridges + shared wire-protocol library 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 ed8bd74b33..9ba86a8c7f 100644 --- a/docs/rfc/implemented/feature/2026-07-07-plan-mode.md +++ b/docs/rfc/implemented/feature/2026-07-07-plan-mode.md @@ -4,13 +4,13 @@ Status: implemented ## Problem -The harness has no way to put an agent into a reduced-authority working state. The canonical feature that needs one is plan mode — the agent explores and designs under a read-only tool policy, produces a reviewable plan, and crosses back into full authority only through an explicit approval. [The extension cookbook](../../../cookbook/extension-cookbook.md) already reserves the row ("Plan mode — `tools/pre-execute` (deny writes) + a mode prompt section"), and [the ACP feature matrix](../../../../packages/ui/acp/acp-feature-support.md) records session modes as a known gap both reference adapters ship. Neither says where the mode STATE lives, how it survives resume and fork, or how its model-visible consequences stay honest with the session log. +The harness has no way to put an agent into a distinct working stance. The canonical feature that needs one is plan mode — the agent explores and designs under a planning stance, produces a reviewable plan, and crosses back through an explicit approval. [The extension cookbook](../../../cookbook/extension-cookbook.md) already reserves the row ("Plan mode — `tools/pre-execute` (deny writes) + a mode prompt section"), and [the ACP feature matrix](../../../../packages/ui/acp/acp-feature-support.md) records session modes as a known gap both reference adapters ship. Neither says where the mode STATE lives, how it survives resume and fork, or how its model-visible consequences stay honest with the session log. -Every shipped plan mode decomposes into the same five parts — a low-authority tool policy, a plan artifact, an approval moment, an execution-state switch, and durable state ([Prior art](#prior-art) carries the survey). Four of the five already exist here as gated infrastructure: what the model is TOLD it can do is shaped per step at [`system-prompt/assemble`](../../../../packages/core/system-prompt/README.md) and whatever ships is logged as `request/header*` events ([reconstructability](../../implemented/architecture/2026-07-05-reconstructable-requests.md)); what can RUN is gated at `tools/pre-execute` with typed decisions ([interception seams](../../implemented/feature/2026-06-30-interception-seams.md)); the approval moment is a human answer over the user-interaction seam ([`ctx.userInteraction`](../../../../packages/ui/user-interaction/README.md), the [ask-user precedent](../../implemented/feature/2026-06-25-ask-user-question.md)); durable per-agent facts are `SessionEventMap` members ([the `todo/write` precedent](../../implemented/feature/2026-06-29-todo-write-tool.md)). The missing fifth is the mode itself: a named, durable, per-agent policy state the policy listeners can read. +Every shipped plan mode decomposes into the same five parts — a low-authority tool policy, a plan artifact, an approval moment, an execution-state switch, and durable state ([Prior art](#prior-art) carries the survey). Four of the five already exist here as gated infrastructure: what the model is TOLD it can do is shaped per step at [`system-prompt/assemble`](../../../../packages/core/system-prompt/README.md) and whatever ships is logged as `request/header*` events ([reconstructability](../../implemented/architecture/2026-07-05-reconstructable-requests.md)); what can RUN is gated at `tools/pre-execute` with typed decisions ([interception seams](../../implemented/feature/2026-06-30-interception-seams.md)); the approval moment is a human answer over the user-interaction seam ([`ctx.userInteraction`](../../../../packages/ui/user-interaction/README.md), the [ask-user precedent](../../implemented/feature/2026-06-25-ask-user-question.md)); durable per-agent facts are `SessionEventMap` members ([the `todo/write` precedent](../../implemented/feature/2026-06-29-todo-write-tool.md)). The missing fifth is the mode itself: a named, durable, per-agent state its listeners can read. ## Decision -The deliverable is **plan mode**. It ships as the first **session mode** — a named, logged, per-agent COLLABORATION state: a mode definition is the guidance section the model sees, deployment config; the mode IN FORCE for an agent is session state, folded from its log. Modes are one axis and the enforcement knobs — the sandbox mode, the approval policy — are others: they never read or write each other, matching how Codex keeps its Plan/Default collaboration presets separate from its sandbox and approval settings. One new product package, `@deepseek-ai/dsh-mode` at `packages/mode/mode/` (a new top-level group, the `packages/approval/` shape), owns the event vocabulary, a thin `ctx.modes` service, and every policy listener; the loop does not change. `plan` is the only shipped definition — the mode-shaped vocabulary exists so a second mode never renames durable event types, not because more modes ship now. +The deliverable is **plan mode**. It ships as the first **session mode** — a named, logged, per-agent COLLABORATION state: a mode definition is the guidance section the model sees, deployment config; the mode IN FORCE for an agent is session state, folded from its log. Modes are one axis and the enforcement knobs — the sandbox mode, the approval policy — are others: they never read or write each other, matching how Codex keeps its Plan/Default collaboration presets separate from its sandbox and approval settings. One new product package, `@deepseek-ai/dsh-mode` at `packages/mode/mode/` (a new top-level group, the `packages/approval/` shape), owns the event vocabulary, a thin `ctx.modes` service, and every listener; the loop does not change. `plan` is the only shipped definition — the mode-shaped vocabulary exists so a second mode never renames durable event types, not because more modes ship now. The state is one `SessionEventMap` member: **`mode/set`**, a log-only, non-surface event carrying `{ mode: string }` with whole-value-replace semantics, plus a pure `foldMode(events)` that returns the mode in force — the last `mode/set`, or the default mode when none exists. Because [the log is the fact channel](../../implemented/architecture/2026-06-30-event-domain-semantics.md), resume, fork, and compaction restore the mode with no extra machinery, and UIs read flips off `session/event`. The default mode is the absence of policy — no section, no filtering, no gate — so an agent that never sees a `mode/set` behaves byte-identically to a deployment that never loads `dsh-mode`, which keeps every existing snapshot golden stable and makes the plugin safe to compose unconditionally. @@ -90,7 +90,7 @@ Contained listeners on the loop's interception seams ([defensive patterns](../.. ### The soft layer: a computed section and a narrow post-`next()` filter -A `system-prompt/assemble` waterfall listener reads the calling agent's mode (the `AssembleContext` carries `agent`) and, in a non-default mode, 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` is a front-of-list insertion with no delta form; removing it on exit is a pure removal and DOES delta) — 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. +A `system-prompt/assemble` waterfall listener reads the calling agent's mode (the `AssembleContext` carries `agent`) and, in a non-default mode, 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` at its canonical alphabetical position is a non-tail insertion the append-only tools delta cannot express; removing it on exit is a pure removal and DOES delta) — 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 same listener carries the ONE tool rule this plugin has, deliberately narrow (`prepend: true`, so it wraps outside every append-registered listener's post-`next()` mutation): `exit_plan_mode` is visible IFF the agent's folded mode is `plan` — which keeps a default-mode assembly byte-identical to a no-`dsh-mode` deployment even though the tool is always registered. Everything else passes through untouched: which tools a mode admits is not this plugin's decision. Code Mode's soft surface is the `tools:sdk` section rather than the wire schemas, and section text resolves in assemble's base, so the same wrapper re-renders that section under the same visibility rule; the default mode re-renders it too, hiding only the exit binding, keeping a default-mode Code Mode assembly byte-identical to a deployment that never loaded `dsh-mode`. There is NO `tools/pre-execute` listener: a mode gates nothing (the exit tool's own folded-mode recheck rejects an out-of-plan call), and the registry's `ask` vocabulary stays free for genuine permission gating — the exit review is a question with options and feedback, not a permission, so it lives inside the tool's own execution over the user-interaction seam. @@ -179,7 +179,7 @@ What holds now, pinned by the unit, protocol, and snapshot tiers: - The mode in force is a pure function of the session log: resume and fork restore it with no extra machinery, and a `mode/set` is followed by the matching `request/header` event (delta or fallback snapshot) on the next step with the dev invariant green throughout. - A user-driven flip narrates exactly once at the next boundary and a net-zero flip sequence narrates nothing; a tool-driven exit narrates only through its tool result. - In the default mode the plugin is invisible: assemblies are byte-identical with and without `dsh-mode` loaded, and every pre-existing snapshot golden is unchanged. -- In plan mode the guidance section and the `exit_plan_mode` tool reach both the wire request and the logged header; entering plan logs the fallback header snapshot (front-of-list insertion), the approved exit logs the pure-removal header-delta. +- In plan mode the guidance section and the `exit_plan_mode` tool reach both the wire request and the logged header; entering plan logs the fallback header snapshot (a non-tail tool insertion), the approved exit logs the pure-removal header-delta. - Plan mode changes nothing on the enforcement axes: the toolset, the sandbox mode, escalation, and the approval policy behave identically in plan and default — pairing the mode with the independent sandbox/approval knobs is how a deployment hardens planning. - Mode definitions (section text) are changeable from `cordis.yml` with no code edit; an unknown mode name fails validation loudly at `set()` time, an unknown definition key at load. - `exit_plan_mode`'s approve path flips the mode and drops the plan surface 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. diff --git a/examples/plan-acp-agent/tests/acp.snapshot.ts b/examples/plan-acp-agent/tests/acp.snapshot.ts index 2cb7ec6abe..79aa3adcdb 100644 --- a/examples/plan-acp-agent/tests/acp.snapshot.ts +++ b/examples/plan-acp-agent/tests/acp.snapshot.ts @@ -8,17 +8,16 @@ import { defineAcpSnapshotSuite, type Scenario, type SnapshotSuiteOptions } from * `DSH_SNAPSHOT=replay`). * * The recorded scenarios re-execute the fs tools AND the sandboxed bash - * executor for real on replay (the replay overlay swaps only the model), so - * the plan-mode arc doubles as a live check of plan's read-only `access` - * cap: the recorded `cat` runs under the host's actual runner (Seatbelt on - * macOS, bwrap on Linux CI). The recorded commands stay `cat`-shaped — + * executor for real on replay (the replay overlay swaps only the model): the + * recorded `cat` runs under the host's actual runner (Seatbelt on macOS, + * bwrap on Linux CI), under the sandbox's own configured mode — plan mode + * does not change it. The recorded commands stay `cat`-shaped — * byte-identical across those backends and across GNU/BSD userlands — and * the transcripts carry NO sandbox denial (a denied command's stderr is the * backend's dialect; the denial→marker path is pinned at dsh-tool-bash's - * unit tier, and the cap's clamp at dsh-mode's). Prompts pin the model to - * RELATIVE paths, because a recorded absolute temp path would neither replay - * on another host nor normalize (the normalizers scrub the RUN's own cwd, - * not the recording's). + * unit tier). Prompts pin the model to RELATIVE paths, because a recorded + * absolute temp path would neither replay on another host nor normalize + * (the normalizers scrub the RUN's own cwd, not the recording's). */ function snapshotModeFromEnv(value: string | undefined): SnapshotSuiteOptions['mode'] { @@ -50,11 +49,11 @@ const SCENARIOS: Scenario[] = [ // initial) — the full toolset plus exit_plan_mode and the mode section — // and the approved exit narrows it back by exactly that tool and section, // a pure removal the delta encoding CAN express (one header-delta; the - // ENTERING flip's front-of-list insertion has no delta form and falls back - // to a snapshot, pinned at the unit tier). The arc: setMode(plan) → the - // model runs a real `cat` through the clamped read-only sandbox and + // ENTERING flip is a non-tail insertion the append-only tools delta cannot + // express, so it falls back to a snapshot — pinned at the unit tier). The + // arc: setMode(plan) → the model runs a real `cat` inside plan and // presents the plan via exit_plan_mode → the scripted elicitation approves - // → the very next step already runs unclamped and edits for real, mid-turn. + // → the very next step already edits for real, mid-turn. { name: 'plan-mode', hasModelTurn: true, recorded: true, pinsHeader: true, headerClass: 'plan', expectedHeaderDeltas: 1 }, // The keep-planning branch: one presentation, the scripted review answers // with free-text feedback (no approval), and the corrective isError carries diff --git a/packages/mode/mode/README.md b/packages/mode/mode/README.md index a8ba4fcfda..10d923b039 100644 --- a/packages/mode/mode/README.md +++ b/packages/mode/mode/README.md @@ -10,7 +10,7 @@ The `default` mode is the absence of policy: no section, no extra tool. An agent ## What a mode carries -**The guidance section.** A `system-prompt/assemble` listener renders the mode's `section` text as the `mode:policy` section (order 50) while the mode is in force, and shows the `exit_plan_mode` tool IFF the folded mode is `plan` — on the wire and, under the registry's Code Mode, in the `tools:sdk` section alike. Every transition therefore surfaces as an attributable `request/header` event on the next step (entering plan adds the exit tool, a front-of-list insertion with no delta form → the full fallback snapshot; the approved exit removes exactly that tool and the section, a pure removal → one `request/header-delta`). +**The guidance section.** A `system-prompt/assemble` listener renders the mode's `section` text as the `mode:policy` section (order 50) while the mode is in force, and shows the `exit_plan_mode` tool IFF the folded mode is `plan` — on the wire and, under the registry's Code Mode, in the `tools:sdk` section alike. Every transition therefore surfaces as an attributable `request/header` event on the next step (entering plan adds the exit tool at its canonical alphabetical position — a non-tail insertion the append-only tools delta cannot express → the full fallback snapshot; the approved exit removes exactly that tool and the section, a pure removal → one `request/header-delta`). **Deliberately absent: enforcement.** A mode never gates execution, filters the toolset, or touches the sandbox/approval knobs — those are independent axes the user switches separately (a deployment that wants a hard read-only floor while planning flips the sandbox-mode option beside the mode picker, in either order; neither disturbs the other). A per-mode tool allow/deny list is likewise out: which tools a mode admits is an effects question — a per-tool read-only/mutating classification the harness does not yet have — parked until tool definitions declare their effects (the plan-mode RFC's deferred item). The config vocabulary is exactly `{ section }`, and an unknown key (a `tools` list or an `access` cap included) fails loud at load. diff --git a/packages/mode/mode/package.json b/packages/mode/mode/package.json index ed76d7e0cc..7582c137dc 100644 --- a/packages/mode/mode/package.json +++ b/packages/mode/mode/package.json @@ -1,6 +1,6 @@ { "name": "@deepseek-ai/dsh-mode", - "description": "Session modes for the DeepSeek Harness: plan mode as logged per-agent policy state with soft (prompt) and hard (execution) enforcement", + "description": "Session modes for the DeepSeek Harness: plan mode as a logged per-agent collaboration state with a guidance section and a user-reviewed exit", "version": "0.0.1", "private": true, "type": "module", diff --git a/packages/mode/mode/tests/integration.spec.ts b/packages/mode/mode/tests/integration.spec.ts index f0e3d28ad7..07f35500c0 100644 --- a/packages/mode/mode/tests/integration.spec.ts +++ b/packages/mode/mode/tests/integration.spec.ts @@ -82,9 +82,9 @@ describe('plan mode through the agent loop', () => { expect(header.data.header.tools?.map(tool => tool.name)).toEqual(['exit_plan_mode', 'read', 'write']) expect(header.data.header.system).toContain('plan mode') - // No general tool gate: the write RUNS — plan's non-shell restraint is - // the section's guidance (the shell restraint is the sandbox clamp, - // pinned in mode.spec). The mode itself stays plan throughout. + // No tool gate: the write RUNS — plan restrains by the section's + // guidance alone (enforcement lives on the independent sandbox/approval + // axes). The mode itself stays plan throughout. const result = findEvent(log, 'tool/result') expect(result.data.isError).toBe(false) expect(foldMode(log)).toBe(PLAN_MODE) @@ -115,8 +115,10 @@ describe('plan mode through the agent loop', () => { { type: 'text', text: 'The user switched this session to plan mode.' }, ]) // The header change is logged as a FULL fallback snapshot, not a delta: - // adding exit_plan_mode reorders the canonical tool list (it sorts - // first), and a pure reordering is inexpressible in the delta encoding. + // the tools delta can only APPEND additions, while the canonical + // (alphabetical) order places exit_plan_mode before read/write — a + // non-tail insertion the writer's round-trip guard rejects, so it + // falls back. const second = findEvent(log, 'request/header', 'last') expect(second.data.reason).toBe('fallback') expect(second.data.header.tools?.map(tool => tool.name)).toEqual(['exit_plan_mode', 'read', 'write']) diff --git a/packages/mode/mode/tests/mode.spec.ts b/packages/mode/mode/tests/mode.spec.ts index 0dcb18ccff..91d0143f9b 100644 --- a/packages/mode/mode/tests/mode.spec.ts +++ b/packages/mode/mode/tests/mode.spec.ts @@ -350,10 +350,10 @@ describe('the soft layer', () => { it('leaves foreign post-next() additions alone in plan mode (no general tool filtering)', async () => { // A foreign listener that post-processes await next(): the mode filter - // wraps outside it (prepend) but hides only the exit tool outside plan - // and the bash trio under an unhonorable cap — a foreign addition - // survives, because which tools a mode admits is deliberately not this - // plugin's decision (the effects question stays parked; module doc). + // wraps outside it (prepend) but hides only the exit tool outside plan — + // a foreign addition survives, because which tools a mode admits is + // deliberately not this plugin's decision (the effects question stays + // parked; module doc). const ctx = new Context() await ctx.plugin(SystemPrompt) await ctx.plugin(ToolRegistry) @@ -388,8 +388,8 @@ describe('the soft layer', () => { agent.session.append('mode/set', { mode: PLAN_MODE }) const assembly = await ctx.systemPrompt.assemble({ agent }) expect(assembly.tools.map(tool => tool.name)).toEqual(['run_code']) - // The SDK documents the full binding set plus the exit — plan mode no - // longer prunes capabilities; its restraint is the section + the sandbox. + // The SDK documents the full binding set plus the exit — a mode never + // prunes capabilities; it restrains by the section's guidance alone. const sdk = assembly.sections.find(section => section.name === 'tools:sdk')?.text ?? '' expect(sdk).toContain('read(args:') expect(sdk).toContain('write(args:')