mirror of
https://github.com/stablyai/orca.git
synced 2026-10-08 08:02:32 +00:00
fix(opencode2): only treat the question tool's form as a pane blocker (#22399)
* fix(opencode2): only treat the question tool's form as a pane blocker OpenCode 2 has one form primitive and several producers, and Orca's setup bridge mapped every `form.created` to `question.asked` — the un-evictable "the pane owner must answer this" blocker. Against opencode v2.0.12 only `metadata.kind === "question"` is the agent's question tool; `websearch.provider` is a provider picker and `mcp-elicitation` is an MCP server prompt raised on sessionID "global", which is not a session and so can never be retired by that session going idle. Admit only the question kind, remember the admitted form ids, and drop `form.replied`/`form.cancelled` for forms that were never admitted so an ignored form's resolution cannot retire a live blocker. Evidence (live v2.0.12 capture, real TUI in a PTY against `opencode serve`) in docs/bug-reproductions/opencode2-form-created-kinds. That capture also shows the reported Subagents/Shell/Terminals dock and the agent picker emit no server event at all, so they were never the `form.created` source. Refs #22371 * refactor(opencode2): drop the unreachable form-resolution guard Review was right that the admitted-form-id set defended against nothing. `clearAttentionForResolution` builds the exact key [factoryID, "AskUserQuestion", form.id, sourceSessionID] and returns null on a miss, with no session-wide fallback, and form ids are unique — so a resolution for a form Orca ignored already matches no live blocker. The guard's comment claimed a collision the key structure rules out, which is worse than no comment. Removes the set, its FIFO eviction helper, and the claim; the kind check on form.created is the whole fix. The end-to-end test stays: it pins the behavior that an MCP form raised and cancelled leaves a live question blocker standing, which is the property worth holding regardless of how it is achieved.
This commit is contained in:
@@ -201,6 +201,7 @@ describe.each(['opencode', 'opencode2'] as const)('%s plugin on OpenCode 2', (ag
|
||||
id: 'form-1',
|
||||
sessionID: 'ses_root',
|
||||
title: 'Pick',
|
||||
metadata: { kind: 'question', tool: { messageID: 'msg-0', id: 'tool-0' } },
|
||||
fields: [
|
||||
{
|
||||
title: 'Color',
|
||||
@@ -255,6 +256,119 @@ describe.each(['opencode', 'opencode2'] as const)('%s plugin on OpenCode 2', (ag
|
||||
await cleanup?.()
|
||||
})
|
||||
|
||||
// Why: OpenCode 2 raises the same form primitive for its pickers and for MCP
|
||||
// elicitations; only the question tool stamps metadata.kind "question" (v2.0.12
|
||||
// capture in docs/bug-reproductions/opencode2-form-created-kinds).
|
||||
async function runSetupBridge(
|
||||
events: { type: string; data: Record<string, unknown> }[]
|
||||
): Promise<{ names: string[]; cleanup?: () => Promise<void> }> {
|
||||
process.env.ORCA_PANE_KEY = 'tab-1:leaf-1'
|
||||
const names: string[] = []
|
||||
globalThis.fetch = vi.fn(async (_input, init) => {
|
||||
names.push(String(payload(record(JSON.parse(String(init?.body))) ?? {}).hook_event_name))
|
||||
return new Response('{}', { status: 200 })
|
||||
})
|
||||
const module = await loadPluginModule(
|
||||
agent === 'opencode2'
|
||||
? _internals.getOpenCode2PluginSource()
|
||||
: _internals.getOpenCodePluginSource()
|
||||
)
|
||||
const cleanup = await module.default?.setup?.({
|
||||
session: {
|
||||
get: async ({ sessionID }: { sessionID: string }) => ({ data: { id: sessionID } }),
|
||||
hook: async () => ({ dispose: vi.fn() })
|
||||
},
|
||||
event: {
|
||||
subscribe: async function* () {
|
||||
for (const event of events) {
|
||||
yield event
|
||||
}
|
||||
}
|
||||
}
|
||||
})
|
||||
return { names, cleanup }
|
||||
}
|
||||
|
||||
function questionForm(id: string): Record<string, unknown> {
|
||||
return {
|
||||
id,
|
||||
sessionID: 'ses_root',
|
||||
title: 'Questions',
|
||||
metadata: { kind: 'question', tool: { messageID: 'msg-0', id: 'tool-0' } },
|
||||
fields: [{ key: 'q0', title: 'Proceed?', type: 'string', options: [] }]
|
||||
}
|
||||
}
|
||||
|
||||
it.each([
|
||||
['websearch.provider', 'ses_root'],
|
||||
['mcp-elicitation', 'global']
|
||||
])('ignores a %s form instead of blocking the pane', async (kind, sessionID) => {
|
||||
const { names, cleanup } = await runSetupBridge([
|
||||
{ type: 'session.execution.started', data: { sessionID: 'ses_root' } },
|
||||
{
|
||||
type: 'form.created',
|
||||
data: {
|
||||
form: {
|
||||
id: 'form-picker',
|
||||
sessionID,
|
||||
title: 'Choose a web search provider',
|
||||
metadata: { kind },
|
||||
fields: [{ key: 'provider', title: 'Provider', type: 'string', options: [] }]
|
||||
}
|
||||
}
|
||||
},
|
||||
{ type: 'form.cancelled', data: { id: 'form-picker', sessionID } },
|
||||
{ type: 'session.execution.succeeded', data: { sessionID: 'ses_root' } }
|
||||
])
|
||||
await vi.waitFor(() => {
|
||||
expect(names.at(-1)).toBe('SessionIdle')
|
||||
})
|
||||
expect(names).not.toContain('AskUserQuestion')
|
||||
await cleanup?.()
|
||||
})
|
||||
|
||||
it('still blocks on a real question form and retires it on reply', async () => {
|
||||
const { names, cleanup } = await runSetupBridge([
|
||||
{ type: 'session.execution.started', data: { sessionID: 'ses_root' } },
|
||||
{ type: 'form.created', data: { form: questionForm('form-q') } },
|
||||
{
|
||||
type: 'form.replied',
|
||||
data: { id: 'form-q', sessionID: 'ses_root', answer: { q0: 'Yes' } }
|
||||
},
|
||||
{ type: 'session.execution.succeeded', data: { sessionID: 'ses_root' } }
|
||||
])
|
||||
await vi.waitFor(() => {
|
||||
expect(names).toContain('AskUserQuestion')
|
||||
expect(names.at(-1)).toBe('SessionIdle')
|
||||
})
|
||||
await cleanup?.()
|
||||
})
|
||||
|
||||
it('keeps a live question blocker while an ignored form is raised and resolved', async () => {
|
||||
const { names, cleanup } = await runSetupBridge([
|
||||
{ type: 'session.execution.started', data: { sessionID: 'ses_root' } },
|
||||
{ type: 'form.created', data: { form: questionForm('form-q') } },
|
||||
{
|
||||
type: 'form.created',
|
||||
data: {
|
||||
form: {
|
||||
id: 'form-mcp',
|
||||
sessionID: 'global',
|
||||
title: 'server is requesting input',
|
||||
metadata: { kind: 'mcp-elicitation', server: 'server' },
|
||||
fields: [{ key: 'elicitation', title: 'Input', type: 'string', options: [] }]
|
||||
}
|
||||
}
|
||||
},
|
||||
{ type: 'form.cancelled', data: { id: 'form-mcp', sessionID: 'global' } }
|
||||
])
|
||||
await vi.waitFor(() => {
|
||||
expect(names).toContain('AskUserQuestion')
|
||||
})
|
||||
expect(names.at(-1)).toBe('AskUserQuestion')
|
||||
await cleanup?.()
|
||||
})
|
||||
|
||||
it.each(['waiting', 'idle', 'disposed'])(
|
||||
'drops an admitted prompt overtaken by %s',
|
||||
async (transition) => {
|
||||
|
||||
Reference in New Issue
Block a user