Coder/deepseek-harness
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(mode): an approved exit leaves plan mode at the step boundary, not mid-batch

Browse Source 
Review finding: the exit tool's direct mode/set append flipped the
folded mode while the loop could still execute further tool calls from
the SAME assistant response — a same-batch exit_plan_mode + write pair
would sail past tools/pre-execute under 'default' even though the
request was assembled under the plan-shaped header. That broke the
design's own invariant (a step's executions run under the mode its
assembly folded), which the pending-intent flush was built to hold for
user flips.

The tool now records the switch as a pending intent like every other
writer, flushed at this step's end (still in-turn); pending intents
carry a narrate flag so the exit's flush stays silent — the tool result
is its narration — while user flips keep the coalesced boundary notice.
The gate, folding the logged mode only, now provably covers the whole
batch: regression test pins approve-then-write-in-the-same-batch as
denied, and the widened toolset still arrives on the next step.

The recorded scenarios are re-recorded: the fixture now shows mode/set
landing after step/end, before the widened fallback header.
This commit is contained in:
kingwl
2026-07-10 10:35:43 +08:00
parent 4e09fdeafb
commit 14f7976e3d
11 changed files with 1122 additions and 838 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
packages/mode/mode/README.md
View File

@@ -22,7 +22,7 @@ The `default` mode is the absence of policy: no section, no filtering, no gate.
## `exit_plan_mode`
The model-facing exit tool. Its single required argument is the plan text — a durable, replayable log artifact riding the ordinary `tool/call` event. `execute` re-checks the folded mode, then conducts the review over the user-interaction seam (`ctx.get('userInteraction')`, opportunistic): one single-select question — Approve, or Keep planning — with the free-text channel open. Approve appends `mode/set { mode: 'default' }` in-turn and the next step's assembly restores the full toolset; every other outcome (keep-planning with the user's feedback verbatim, an aborted question, no provider) returns the corrective `isError` and the mode stays `plan`. `presentCall` renders a `generic` card titled by the plan's first heading with the plan markdown as content; over ACP the review rides the same elicitation flow as `ask_user_question`, in the terminal the stdio provider's prompt queue.
The model-facing exit tool. Its single required argument is the plan text — a durable, replayable log artifact riding the ordinary `tool/call` event. `execute` re-checks the folded mode, then conducts the review over the user-interaction seam (`ctx.get('userInteraction')`, opportunistic): one single-select question — Approve, or Keep planning — with the free-text channel open. Approve records the switch back to `default` as a silent boundary-applied pending intent (flushed at this step's end — the gate stays plan-mode for any remaining call of the same assistant response) and the next step's assembly restores the full toolset; every other outcome (keep-planning with the user's feedback verbatim, an aborted question, no provider) returns the corrective `isError` and the mode stays `plan`. `presentCall` renders a `generic` card titled by the plan's first heading with the plan markdown as content; over ACP the review rides the same elicitation flow as `ask_user_question`, in the terminal the stdio provider's prompt queue.
## Config

30
packages/mode/mode/src/index.ts
View File

@@ -205,8 +205,13 @@ export class ModesService extends Service {
/** Validated definitions (built-in `plan` merged unless overridden). */
readonly resolved: ResolvedModes
/** The latest user-selected mode per session, awaiting its turn-boundary flush. */
private readonly pendingIntents = new WeakMap<Session, string>()
/**
* The latest selected mode per session, awaiting its turn-boundary flush.
* `narrate` is true for user selections (the flush appends the coalesced
* notice when the header disagrees) and false for the exit tool's own
* switch, which narrates through its tool result instead.
*/
private readonly pendingIntents = new WeakMap<Session, { mode: string; narrate: boolean }>()
/** The unknown folded-mode name already narrated per session (once per name). */
private readonly droppedNoticed = new WeakMap<Session, string>()
@@ -302,7 +307,14 @@ export class ModesService extends Service {
? 'The user chose to keep planning; revise the plan and present it again.'
: `The user chose to keep planning; their feedback: ${feedback}`)
}
agent.session.append('mode/set', { mode: DEFAULT_MODE })
// A boundary-applied switch, NOT a direct append: the loop may still
// execute further tool calls from the SAME assistant response after
// this one, and they were requested under the plan-shaped header — a
// same-batch exit_plan_mode + write pair must not smuggle the write
// past the gate. The flush at this step's end appends the mode/set
// (still in-turn), so the next step's assembly widens; narrate: false —
// this result IS the narration.
this.pendingIntents.set(agent.session, { mode: DEFAULT_MODE, narrate: false })
const note = item.custom === undefined || item.custom === '' ? '' : ` User note: ${item.custom}`
return [{ type: 'text', text: `Plan approved — plan mode exited; the full toolset returns on your next step.${note}` }]
},
@@ -341,7 +353,7 @@ export class ModesService extends Service {
get(agent: Agent): { current: string; pending?: string } {
const current = this.activeDefinition(agent.session)?.name ?? DEFAULT_MODE
const pending = this.pendingIntents.get(agent.session)
return pending === undefined ? { current } : { current, pending }
return pending === undefined ? { current } : { current, pending: pending.mode }
}
/**
@@ -358,9 +370,9 @@ export class ModesService extends Service {
throw new Error(`unknown mode "${mode}" — available modes: ${this.list().join(', ')}`)
}
const session = agent.session
const target = this.pendingIntents.get(session) ?? this.get(agent).current
const target = this.pendingIntents.get(session)?.mode ?? this.get(agent).current
if (mode === target) return
this.pendingIntents.set(session, mode)
this.pendingIntents.set(session, { mode, narrate: true })
}
/** The folded mode's definition, or `undefined` for the default mode and for a folded name the config no longer defines. */
@@ -381,11 +393,13 @@ export class ModesService extends Service {
*/
private onBoundary(session: Session, turnStart: boolean): void {
if (turnStart) this.noticeDroppedDefinition(session)
const target = this.pendingIntents.get(session)
if (target === undefined) return
const pending = this.pendingIntents.get(session)
if (pending === undefined) return
this.pendingIntents.delete(session)
const target = pending.mode
if (target === foldMode(session.events)) return
session.append('mode/set', { mode: target })
if (!pending.narrate) return
const told = modeAtLastHeader(session.events)
if (told === undefined || told === target) return
const text = target === DEFAULT_MODE

34
packages/mode/mode/tests/mode.spec.ts
View File

@@ -452,17 +452,49 @@ describe('exit_plan_mode', () => {
expect(foldMode(agent.session.events)).toBe(PLAN_MODE)
})
it('approve: appends mode/set default in-turn and confirms', async () => {
it('approve: records the boundary-applied switch and confirms (the fold flips at the flush)', async () => {
const { ctx, agent, asked } = await setupWithReview({ selected: ['Approve'] })
const result = await callExit(ctx, agent)
expect(result.isError).toBe(false)
expect(result.content).toEqual([{ type: 'text', text: 'Plan approved — plan mode exited; the full toolset returns on your next step.' }])
// Boundary-applied, not a direct append: the fold stays plan until the
// step's end, so the gate covers any remaining call of the SAME batch.
expect(foldMode(agent.session.events)).toBe(PLAN_MODE)
expect(ctx.modes.get(agent)).toEqual({ current: PLAN_MODE, pending: DEFAULT_MODE })
boundary(ctx, agent.session, 'step/end')
expect(foldMode(agent.session.events)).toBe(DEFAULT_MODE)
expect(asked).toHaveLength(1)
expect(asked[0]?.agent).toBe(agent)
expect(asked[0]?.questions[0]?.options?.map(option => option.label)).toEqual(['Approve', 'Keep planning'])
})
it('an approved exit cannot smuggle a same-batch call past the gate', async () => {
const { ctx, agent } = await setupWithReview({ selected: ['Approve'] })
registerNamedTools(ctx, ['write'])
const approved = await callExit(ctx, agent)
expect(approved.isError).toBe(false)
// The next call of the SAME assistant response (no boundary between):
// requested under the plan-shaped header, so the gate must still deny it.
const smuggled = await execute(ctx, 'write', agent)
expect(smuggled.isError).toBe(true)
expect(smuggled.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',
}])
boundary(ctx, agent.session, 'step/end')
const next = await execute(ctx, 'write', agent)
expect(next.isError).toBe(false)
})
it('the exit flush narrates nothing — the tool result is the narration', async () => {
const { ctx, agent } = await setupWithReview({ selected: ['Approve'] })
header(agent.session)
await callExit(ctx, agent)
boundary(ctx, agent.session, 'step/end')
expect(foldMode(agent.session.events)).toBe(DEFAULT_MODE)
expect(noticeTexts(agent.session)).toEqual([])
})
it('approve with a note carries the note into the confirmation', async () => {
const { ctx, agent } = await setupWithReview({ selected: ['Approve'], custom: 'ship it small' })
const result = await callExit(ctx, agent)