fix(stdio): /mode is reserved even while a question prompt is active
Review finding at the seam of two surfaces this branch added to the same stdin: with an ask_user_question (or plan-review) prompt active, the line handler dispatched every line as the answer first, so '/mode plan' typed mid-question was recorded as free-text feedback — model-visible in the tool result — and the mode never changed. Command handling now runs before answer dispatch: the command executes, the question stays pending and still owns the next non-command line. A literal '/mode…' free-text answer is the trade-off deliberately spent — a swallowed command that becomes review feedback costs far more than that contrived answer shape.
This commit is contained in:
@@ -50,7 +50,7 @@ Mode definitions are validated plugin Config — per repo convention, changeable
|
|||||||
|
|
||||||
### In the terminal
|
### In the terminal
|
||||||
|
|
||||||
The stdio app gains `/mode` (print the current and available modes) and `/mode <name>` (switch + banner — a command line, never sent to the model). The exit review prompts right in the terminal with no new machinery: it is an ordinary user-interaction question, so it rides the stdio provider's one-prompt-owns-stdin queue that `ask_user_question` already uses.
|
The stdio app gains `/mode` (print the current and available modes) and `/mode <name>` (switch + banner — a command line, never sent to the model, and reserved even while a question prompt is active: a command is never recorded as an answer). The exit review prompts right in the terminal with no new machinery: it is an ordinary user-interaction question, so it rides the stdio provider's one-prompt-owns-stdin queue that `ask_user_question` already uses.
|
||||||
|
|
||||||
### Over ACP
|
### Over ACP
|
||||||
|
|
||||||
|
|||||||
@@ -349,21 +349,21 @@ export function createStdioChat(ctx: Context, config: Config, runtime: StdioRunt
|
|||||||
})
|
})
|
||||||
|
|
||||||
reader.on('line', (line) => {
|
reader.on('line', (line) => {
|
||||||
if (activeQuestion !== undefined) {
|
|
||||||
answerQuestion(line)
|
|
||||||
return
|
|
||||||
}
|
|
||||||
const text = line.trim()
|
const text = line.trim()
|
||||||
if (!text) return
|
|
||||||
const agent = ctx.agents.get(agentId)
|
|
||||||
if (!agent) {
|
|
||||||
ctx.logger.error('ui-stdio: agent "%s" is not running', agentId)
|
|
||||||
return
|
|
||||||
}
|
|
||||||
if (text === '/mode' || text.startsWith('/mode ')) {
|
if (text === '/mode' || text.startsWith('/mode ')) {
|
||||||
// A command line, never sent to the model: print or switch the session
|
// A command line, never sent to the model — and reserved even while a
|
||||||
// mode. The switch is a pending intent the mode service flushes at the
|
// question prompt is active: a command swallowed as a free-text answer
|
||||||
// next turn boundary (dsh-mode's turn-enclosure contract).
|
// would land in the tool result as model-visible feedback (the plan
|
||||||
|
// review is exactly such a prompt), so command handling runs before
|
||||||
|
// answer dispatch. The switch is a pending intent the mode service
|
||||||
|
// flushes at the next turn boundary (dsh-mode's turn-enclosure
|
||||||
|
// contract); an active question stays pending and still owns the
|
||||||
|
// next non-command line.
|
||||||
|
const agent = ctx.agents.get(agentId)
|
||||||
|
if (!agent) {
|
||||||
|
ctx.logger.error('ui-stdio: agent "%s" is not running', agentId)
|
||||||
|
return
|
||||||
|
}
|
||||||
const modes = ctx.get('modes')
|
const modes = ctx.get('modes')
|
||||||
if (modes === undefined) {
|
if (modes === undefined) {
|
||||||
output.write('session modes are not composed in this deployment\n> ')
|
output.write('session modes are not composed in this deployment\n> ')
|
||||||
@@ -385,6 +385,16 @@ export function createStdioChat(ctx: Context, config: Config, runtime: StdioRunt
|
|||||||
}
|
}
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
if (activeQuestion !== undefined) {
|
||||||
|
answerQuestion(line)
|
||||||
|
return
|
||||||
|
}
|
||||||
|
if (!text) return
|
||||||
|
const agent = ctx.agents.get(agentId)
|
||||||
|
if (!agent) {
|
||||||
|
ctx.logger.error('ui-stdio: agent "%s" is not running', agentId)
|
||||||
|
return
|
||||||
|
}
|
||||||
submittedWork = true
|
submittedWork = true
|
||||||
if (agent.status === 'running') {
|
if (agent.status === 'running') {
|
||||||
agent.steer([{ type: 'text', text }])
|
agent.steer([{ type: 'text', text }])
|
||||||
|
|||||||
@@ -871,3 +871,49 @@ describe('createStdioChat /mode command', () => {
|
|||||||
expect(ctx.modes.get(agent)).toEqual({ current: 'default' })
|
expect(ctx.modes.get(agent)).toEqual({ current: 'default' })
|
||||||
})
|
})
|
||||||
})
|
})
|
||||||
|
|
||||||
|
describe('createStdioChat /mode during an active question', () => {
|
||||||
|
it('reserves /mode while a question is pending — the command is never recorded as the answer', async () => {
|
||||||
|
const bundle = await setup()
|
||||||
|
await bundle.ctx.plugin(SystemPrompt)
|
||||||
|
await bundle.ctx.plugin(ToolRegistry)
|
||||||
|
await bundle.ctx.plugin(ModesService)
|
||||||
|
const agent = {
|
||||||
|
id: 'main' as Agent['id'],
|
||||||
|
status: 'idle',
|
||||||
|
options: {},
|
||||||
|
session: new RealSession(SessionId('main-session')),
|
||||||
|
send: () => {},
|
||||||
|
steer: () => {},
|
||||||
|
} as never as Agent
|
||||||
|
bundle.ctx.agents.register(agent)
|
||||||
|
|
||||||
|
const answer = bundle.ctx.userInteraction.ask({
|
||||||
|
questions: [{
|
||||||
|
id: 'plan-review',
|
||||||
|
header: 'Plan review',
|
||||||
|
question: 'Approve this plan and leave plan mode?',
|
||||||
|
options: [{ label: 'Approve' }, { label: 'Keep planning' }],
|
||||||
|
}],
|
||||||
|
})
|
||||||
|
await new Promise(r => setImmediate(r))
|
||||||
|
|
||||||
|
// The command runs as a command: the mode switches, the question stays
|
||||||
|
// pending (it still owns the next non-command line).
|
||||||
|
bundle.input.feed('/mode plan')
|
||||||
|
await new Promise(r => setImmediate(r))
|
||||||
|
expect(bundle.out.text()).toContain('mode → plan (applies from the next turn)')
|
||||||
|
expect(bundle.ctx.modes.get(agent)).toEqual({ current: 'default', pending: PLAN_MODE })
|
||||||
|
|
||||||
|
bundle.input.feed('1')
|
||||||
|
await expect(answer).resolves.toEqual({ answers: [{ id: 'plan-review', selected: ['Approve'] }] })
|
||||||
|
})
|
||||||
|
|
||||||
|
it('logs and drops /mode when the target agent is not running', async () => {
|
||||||
|
const { ctx, input } = await setup()
|
||||||
|
const spy = vi.spyOn(ctx.logger, 'error').mockImplementation(() => {})
|
||||||
|
input.feed('/mode plan')
|
||||||
|
await new Promise(r => setImmediate(r))
|
||||||
|
expect(spy).toHaveBeenCalledWith('ui-stdio: agent "%s" is not running', 'main')
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|||||||
Reference in New Issue
Block a user