fix(mode): prepend the assemble filter; structured_output joins the plan allowlist
Review finding with a real in-repo instance: the structured runtime's per-spawn final-assembly wrapper (prepend, post-next) re-injects structured_output OUTSIDE the mode filter, so a structured child in plan mode would see a tool the gate then denies — the soft policy and the hard gate telling different stories. The suggested fix (make the mode filter outermost) cannot beat that instance: prepend unshifts, so the per-spawn listener always registers later and wraps outer. Two-part resolution instead. Semantically, structured_output enters the shipped plan allowlist — it is a child's pure result channel, the same ask/report class as ask_user_question and exit_plan_mode, so the filter, the re-injection, and the gate now agree wherever a structured child runs in plan mode. Mechanically, the filter registers with prepend anyway: it now wraps outside every append-registered listener regardless of load order (regression test pins a pre-registered post-next mutator being filtered), narrowing the documented cosmetic residual to prepend-after-load listeners only, where the gate still covers execution. Severity note: no execution breach existed — the gate held throughout; this closes the prompt-honesty gap.
This commit is contained in:
@@ -34,9 +34,9 @@ The model-facing exit tool. Its single required argument is the plan text — a
|
||||
plan:
|
||||
section: |
|
||||
You are in plan mode: ...
|
||||
tools: [read, todo_write, web_search, web_fetch, ask_user_question, exit_plan_mode]
|
||||
tools: [read, todo_write, web_search, web_fetch, ask_user_question, structured_output, exit_plan_mode]
|
||||
```
|
||||
|
||||
Definitions are validated at load (`resolveConfig`): the built-in `plan` (read-only allowlist plus `ask_user_question`, `bash`/`subagent` excluded) merges unless overridden, `default` is rejected as a key, and allowlists may name not-yet-registered tools (registration is dynamic). An unknown name fails loudly at `set()` time.
|
||||
Definitions are validated at load (`resolveConfig`): the built-in `plan` (read-only allowlist plus the ask/report channels `ask_user_question`/`structured_output`, `bash`/`subagent` excluded) merges unless overridden, `default` is rejected as a key, and allowlists may name not-yet-registered tools (registration is dynamic). An unknown name fails loudly at `set()` time.
|
||||
|
||||
RFC: [plan mode](../../../docs/rfc/implemented/feature/2026-07-07-plan-mode.md).
|
||||
|
||||
@@ -115,7 +115,12 @@ const PLAN_SECTION
|
||||
+ 'its review fails, ask the user to switch the session out of plan mode instead '
|
||||
+ 'of retrying denied tools.'
|
||||
|
||||
const PLAN_TOOLS = ['read', 'todo_write', 'web_search', 'web_fetch', 'ask_user_question', EXIT_PLAN_MODE]
|
||||
// 'structured_output' is a structured subagent child's result channel (pure
|
||||
// reporting, the ask/exit class of read-only-safe): its runtime re-injects the
|
||||
// schema into the FINAL assembly from an outermost per-spawn listener, so
|
||||
// allowlisting is what keeps the soft filter, that re-injection, and the hard
|
||||
// gate telling one consistent story when such a child runs in plan mode.
|
||||
const PLAN_TOOLS = ['read', 'todo_write', 'web_search', 'web_fetch', 'ask_user_question', 'structured_output', EXIT_PLAN_MODE]
|
||||
|
||||
/** The review question's approve option label — the answer item is matched by it. */
|
||||
const APPROVE_LABEL = 'Approve'
|
||||
@@ -246,6 +251,12 @@ export class ModesService extends Service {
|
||||
text: context => (context.agent === undefined ? '' : this.activeDefinition(context.agent.session)?.definition.section ?? ''),
|
||||
})
|
||||
|
||||
// prepend: the filter wraps OUTSIDE every append-registered listener
|
||||
// regardless of load order, so their post-next() additions are filtered
|
||||
// too. A listener that ALSO prepends after this plugin loads (the
|
||||
// structured runtime's per-spawn wrapper) still wins the wrap — for that
|
||||
// one the allowlist carries the contract, and the hard gate covers
|
||||
// execution either way.
|
||||
ctx.on('system-prompt/assemble', async (_assembly, context, next) => {
|
||||
const result = await next()
|
||||
const agent = context.agent
|
||||
@@ -259,7 +270,7 @@ export class ModesService extends Service {
|
||||
result.tools = result.tools.filter(tool =>
|
||||
allowed.has(tool.name) && (tool.name !== EXIT_PLAN_MODE || active.name === PLAN_MODE))
|
||||
return result
|
||||
})
|
||||
}, { prepend: true })
|
||||
|
||||
ctx.on('tools/pre-execute', (exec, next): Promise<PreToolDecision> => {
|
||||
if (exec.agent === undefined) return next()
|
||||
|
||||
@@ -75,7 +75,7 @@ describe('resolveConfig', () => {
|
||||
it('merges the built-in plan definition with the read-only allowlist', () => {
|
||||
const resolved = resolveConfig({})
|
||||
const plan = resolved.definitions.get(PLAN_MODE)
|
||||
expect(plan?.tools).toEqual(['read', 'todo_write', 'web_search', 'web_fetch', 'ask_user_question', EXIT_PLAN_MODE])
|
||||
expect(plan?.tools).toEqual(['read', 'todo_write', 'web_search', 'web_fetch', 'ask_user_question', 'structured_output', EXIT_PLAN_MODE])
|
||||
expect(plan?.section).toContain('plan mode')
|
||||
})
|
||||
|
||||
@@ -327,6 +327,28 @@ describe('the soft layer', () => {
|
||||
expect(assembly.sections.find(section => section.name === 'mode:policy')?.text).toBe('reviewing')
|
||||
})
|
||||
|
||||
it('filters additions from an append-registered final-assembly mutator, regardless of load order', async () => {
|
||||
// A foreign listener that post-processes await next() and was registered
|
||||
// BEFORE dsh-mode loaded: under append ordering it would wrap OUTSIDE the
|
||||
// filter and its re-added tool would leak into the plan-mode header. The
|
||||
// filter registers with prepend, so it wraps outside every
|
||||
// append-registered listener and filters their additions too.
|
||||
const ctx = new Context()
|
||||
await ctx.plugin(SystemPrompt)
|
||||
await ctx.plugin(ToolRegistry)
|
||||
ctx.on('system-prompt/assemble', async (_assembly, _context, next) => {
|
||||
const final = await next()
|
||||
final.tools = [...final.tools, { name: 'smuggled', description: 'added after next()', parameters: {} }]
|
||||
return final
|
||||
})
|
||||
await ctx.plugin(ModesService)
|
||||
registerNamedTools(ctx, ['read'])
|
||||
const agent = agentWithSession()
|
||||
agent.session.append('mode/set', { mode: PLAN_MODE })
|
||||
const assembly = await ctx.systemPrompt.assemble({ agent })
|
||||
expect(assembly.tools.map(tool => tool.name)).toEqual(['exit_plan_mode', 'read'])
|
||||
})
|
||||
|
||||
it('treats a dropped folded definition as the default mode', async () => {
|
||||
const ctx = await setup()
|
||||
registerNamedTools(ctx, ['read', 'write'])
|
||||
|
||||
Reference in New Issue
Block a user