From 9d3d5c085244a767cd9807948b3535f2d4092673 Mon Sep 17 00:00:00 2001 From: Bohdan Triapitsyn Date: Tue, 4 Aug 2026 11:08:09 +0300 Subject: [PATCH] fix(sessions): stop reporting a dispatched prompt that never landed `prompt_async` answers 204 as soon as OpenCode forks the run and reports every later failure only on the session event stream, so an unusable model, agent, or variant produced a session with no message while the result still claimed `promptDispatched: true`. Validate an explicitly requested model, agent, and variant against the directory's own agent and provider lists before any session, worktree, or goal is created, and confirm a new user message actually reached the session before reporting the dispatch. A failed or empty lookup never turns a valid selection into a rejection. --- .../lib/openchamber-control/DOCUMENTATION.md | 10 +- .../server/lib/openchamber-sessions/routes.js | 121 ++++++++++++++- .../lib/openchamber-sessions/routes.test.js | 145 ++++++++++++++++-- 3 files changed, 255 insertions(+), 21 deletions(-) diff --git a/packages/web/server/lib/openchamber-control/DOCUMENTATION.md b/packages/web/server/lib/openchamber-control/DOCUMENTATION.md index 3d4f1477..1d15a123 100644 --- a/packages/web/server/lib/openchamber-control/DOCUMENTATION.md +++ b/packages/web/server/lib/openchamber-control/DOCUMENTATION.md @@ -32,7 +32,15 @@ other. requires observed activity or a newly completed assistant message. - Timeout and cancellation are failures, never authoritative idle results. - Validation that protects side effects runs before session creation or - dispatch. + dispatch. An explicitly requested model, agent, or variant is checked against + the directory's own OpenCode agent and provider lists before any session, + worktree, or goal is created, because `prompt_async` accepts an unusable + selection and then fails only on the event stream. A failed or empty lookup + never turns a valid selection into a rejection. +- `promptDispatched` reports an observed dispatch, never an accepted request. + After `prompt_async` the service confirms a new user message reached the + session; when it does not, the result reports `promptDispatched: false` with + `promptError` instead of claiming success. - Send and fork dispatches without an explicit model/agent/variant reuse the target session's last user-message selection before falling back to the configured defaults; only session creation resolves defaults directly. diff --git a/packages/web/server/lib/openchamber-sessions/routes.js b/packages/web/server/lib/openchamber-sessions/routes.js index 79918817..0c82b1e0 100644 --- a/packages/web/server/lib/openchamber-sessions/routes.js +++ b/packages/web/server/lib/openchamber-sessions/routes.js @@ -272,6 +272,41 @@ const resolveRequestedDirectory = async ({ payload, readSettingsFromDiskMigrated : { ok: false, status: 400, error: validated.error || 'Invalid directory' }; }; +const PROMPT_LANDED_TIMEOUT_MS = 5_000; +const PROMPT_LANDED_POLL_MS = 150; + +const latestUserMessageID = async ({ client, sessionID, directory }) => { + let response; + try { + response = await client.session.messages({ sessionID, directory, limit: 100 }); + } catch { + return { ok: false, messageID: null }; + } + const messages = Array.isArray(response?.data) ? response.data : []; + let latest = null; + for (const message of messages) { + const info = message?.info; + if (info?.role !== 'user') continue; + if (!latest || (info.time?.created || 0) >= (latest.time?.created || 0)) latest = info; + } + return { ok: true, messageID: asNonEmptyString(latest?.id) }; +}; + +// `prompt_async` answers 204 as soon as OpenCode forks the run, and every later +// failure is reported only on the session event stream. Confirm the prompt was +// actually recorded so `promptDispatched` never claims a dispatch that vanished. +const waitForPromptLanded = async ({ client, sessionID, directory, baselineUserMessageID }) => { + const deadline = Date.now() + PROMPT_LANDED_TIMEOUT_MS; + for (;;) { + const latest = await latestUserMessageID({ client, sessionID, directory }); + // A failed lookup is not authoritative evidence that the prompt was lost. + if (!latest.ok) return true; + if (latest.messageID && latest.messageID !== baselineUserMessageID) return true; + if (Date.now() >= deadline) return false; + await new Promise((resolve) => setTimeout(resolve, PROMPT_LANDED_POLL_MS)); + } +}; + const resolveWorktreeInput = (payload) => { if (!payload?.worktree || typeof payload.worktree !== 'object') return null; const name = asNonEmptyString(payload.worktree.name); @@ -322,6 +357,48 @@ export const createOpenChamberSessionService = (dependencies) => { return null; }; + // Explicit model/agent/variant are never checked by `prompt_async`: an unknown + // agent makes the forked run fail silently, leaving a session with no message. + // Reject them before any session, worktree, or goal side effect happens. + const validateRequestedSelection = async ({ directory, requestedModel, requestedAgent, requestedVariant }) => { + if (!requestedModel && !requestedAgent && !requestedVariant) return; + const authHeaders = getOpenCodeAuthHeaders(); + const { providers, agents } = await fetchSelectionInputs({ + buildOpenCodeUrl, + authHeaders, + directory, + readSettingsFromDiskMigrated, + }); + + // An empty list means the lookup failed or returned nothing authoritative; + // it must not turn a valid selection into a rejection. + if (requestedAgent && agents.length > 0) { + const agent = agents.find((entry) => entry?.name === requestedAgent) || null; + if (!agent) { + throw new OpenChamberControlError(`Unknown agent '${requestedAgent}' for ${directory}`, 400); + } + if (!isPrimaryAgentMode(agent.mode)) { + throw new OpenChamberControlError(`Agent '${requestedAgent}' is a subagent and cannot receive a prompt directly`, 400); + } + } + + if (requestedModel && providers.length > 0) { + if (!hasProviderModel(providers, requestedModel.providerID, requestedModel.modelID)) { + throw new OpenChamberControlError( + `Unknown model '${requestedModel.providerID}/${requestedModel.modelID}' for ${directory}`, + 400, + ); + } + if (requestedVariant + && !resolveVariant(providers, requestedModel.providerID, requestedModel.modelID, requestedVariant)) { + throw new OpenChamberControlError( + `Unknown variant '${requestedVariant}' for model '${requestedModel.providerID}/${requestedModel.modelID}'`, + 400, + ); + } + } + }; + const dispatchPrompt = async ({ client, baseUrl, @@ -417,6 +494,7 @@ export const createOpenChamberSessionService = (dependencies) => { throw markGoalPartial(error); } } else { + const baseline = await latestUserMessageID({ client, sessionID, directory }); try { await runPromptAsync({ baseUrl, @@ -438,6 +516,22 @@ export const createOpenChamberSessionService = (dependencies) => { } catch (error) { throw markGoalPartial(error); } + const landed = await waitForPromptLanded({ + client, + sessionID, + directory, + baselineUserMessageID: baseline.messageID, + }); + if (!landed) { + return { + model, + agent, + variant, + promptDispatched: false, + dispatchedAsCommand: false, + promptError: 'OpenCode accepted the prompt but it never appeared in the session', + }; + } } return { model, agent, variant, promptDispatched: true, dispatchedAsCommand: Boolean(resolvedCommand) }; @@ -470,13 +564,23 @@ export const createOpenChamberSessionService = (dependencies) => { if (payload?.worktree && !worktreeInput) { throw new OpenChamberControlError('worktree.name is required when worktree is provided', 400); } + + if (typeof waitForOpenCodeReady === 'function') await waitForOpenCodeReady(10_000, 250); + + if (prompt) { + await validateRequestedSelection({ + directory: resolvedDirectory.directory, + requestedModel: model, + requestedAgent: agent, + requestedVariant: variant, + }); + } + if (worktreeInput) { worktree = await createWorktree(resolvedDirectory.directory, worktreeInput); sessionDirectory = worktree.path; } - if (typeof waitForOpenCodeReady === 'function') await waitForOpenCodeReady(10_000, 250); - const baseUrl = buildOpenCodeUrl('/', '').replace(/\/$/, ''); const authHeaders = getOpenCodeAuthHeaders(); const client = createOpencodeClient({ baseUrl, headers: authHeaders }); @@ -514,6 +618,7 @@ export const createOpenChamberSessionService = (dependencies) => { ...(prompt && dispatch.agent ? { agent: dispatch.agent } : {}), ...(prompt && dispatch.variant ? { variant: dispatch.variant } : {}), promptDispatched: dispatch.promptDispatched, + ...(dispatch.promptError ? { promptError: dispatch.promptError } : {}), dispatchedAsCommand: dispatch.dispatchedAsCommand, ...(goalInput.enabled ? { goalEnabled: true } : {}), ...(goalInput.tokenBudget ? { goalTokenBudget: goalInput.tokenBudget } : {}), @@ -566,6 +671,13 @@ export const createOpenChamberSessionService = (dependencies) => { directory = resolvedDirectory.directory; if (typeof waitForOpenCodeReady === 'function') await waitForOpenCodeReady(10_000, 250); + await validateRequestedSelection({ + directory, + requestedModel, + requestedAgent: asNonEmptyString(payload.agent), + requestedVariant: asNonEmptyString(payload.variant), + }); + const baseUrl = buildOpenCodeUrl('/', '').replace(/\/$/, ''); const authHeaders = getOpenCodeAuthHeaders(); const client = createOpencodeClient({ baseUrl, headers: authHeaders }); @@ -608,7 +720,8 @@ export const createOpenChamberSessionService = (dependencies) => { model: dispatch.model, ...(dispatch.agent ? { agent: dispatch.agent } : {}), ...(dispatch.variant ? { variant: dispatch.variant } : {}), - promptDispatched: true, + promptDispatched: dispatch.promptDispatched, + ...(dispatch.promptError ? { promptError: dispatch.promptError } : {}), dispatchedAsCommand: dispatch.dispatchedAsCommand, ...(goalInput.enabled ? { goalEnabled: true } : {}), ...(goalInput.tokenBudget ? { goalTokenBudget: goalInput.tokenBudget } : {}), @@ -624,7 +737,7 @@ export const createOpenChamberSessionService = (dependencies) => { model: dispatch.model, ...(dispatch.agent ? { agent: dispatch.agent } : {}), ...(dispatch.variant ? { variant: dispatch.variant } : {}), - promptDispatched: true, + promptDispatched: dispatch.promptDispatched, dispatchedAsCommand: dispatch.dispatchedAsCommand, ...(goalInput.enabled ? { goalEnabled: true } : {}), ...(goalInput.tokenBudget ? { goalTokenBudget: goalInput.tokenBudget } : {}), diff --git a/packages/web/server/lib/openchamber-sessions/routes.test.js b/packages/web/server/lib/openchamber-sessions/routes.test.js index e781fc62..8b964fb6 100644 --- a/packages/web/server/lib/openchamber-sessions/routes.test.js +++ b/packages/web/server/lib/openchamber-sessions/routes.test.js @@ -11,6 +11,53 @@ const createWorktreeMock = vi.fn(async () => ({ const sessionCreateMock = vi.fn(async () => ({ data: { id: 'ses_123' } })); const sessionForkMock = vi.fn(async () => ({ data: { id: 'ses_fork', title: 'Forked session' } })); const sessionMessagesMock = vi.fn(async () => ({ data: [] })); + +let existingSessionMessages = []; +let dispatchedUserMessageSeq = 0; + +// The service confirms a prompt landed by watching for a new user message, so +// the default mock behaves like OpenCode recording each dispatched prompt. +const setSessionMessages = (messages) => { + existingSessionMessages = messages; +}; + +const recordedSessionMessages = async () => { + dispatchedUserMessageSeq += 1; + return { + data: [ + ...existingSessionMessages, + { + info: { + id: `msg_dispatched_${dispatchedUserMessageSeq}`, + role: 'user', + time: { created: 1000 + dispatchedUserMessageSeq }, + }, + }, + ], + }; +}; + +// Selection inputs are fetched whenever a request names a model, agent, or +// variant, so every prompt-dispatching fetch mock must answer them. +const selectionInputResponse = (url) => { + const text = String(url); + if (text.includes('/config/providers')) { + return { + ok: true, + json: async () => ({ + providers: [ + { id: 'openai', models: [{ id: 'gpt-5.5', variants: { high: {} } }] }, + { id: 'anthropic', models: [{ id: 'claude-sonnet-5', variants: { high: {} } }] }, + ], + }), + }; + } + if (text.includes('/agent')) { + return { ok: true, json: async () => [{ name: 'build', mode: 'primary' }, { name: 'plan', mode: 'primary' }] }; + } + if (text.includes('/config')) return { ok: true, json: async () => ({}) }; + return null; +}; const sessionCommandMock = vi.fn(async () => ({ data: {} })); const commandListMock = vi.fn(async () => ({ data: [] })); globalThis.__openchamberCreateWorktreeMock = createWorktreeMock; @@ -62,8 +109,10 @@ describe('openchamber session routes', () => { createWorktreeMock.mockClear(); sessionCreateMock.mockClear(); sessionForkMock.mockClear(); + existingSessionMessages = []; + dispatchedUserMessageSeq = 0; sessionMessagesMock.mockReset(); - sessionMessagesMock.mockResolvedValue({ data: [] }); + sessionMessagesMock.mockImplementation(recordedSessionMessages); sessionCommandMock.mockReset(); sessionCommandMock.mockResolvedValue({ data: {} }); commandListMock.mockReset(); @@ -319,13 +368,11 @@ describe('openchamber session routes', () => { it('sends a goal prompt to an existing session after creating goal metadata', async () => { const originalFetch = globalThis.fetch; - const fetchMock = vi.fn(async () => ({ ok: true, text: async () => '' })); + const fetchMock = vi.fn(async (url) => selectionInputResponse(url) || { ok: true, text: async () => '' }); const createSessionGoal = vi.fn(async () => undefined); globalThis.fetch = fetchMock; try { - sessionMessagesMock.mockResolvedValue({ - data: [{ info: { id: 'msg_before', role: 'assistant', time: { created: 10, completed: 20 } } }], - }); + setSessionMessages([{ info: { id: 'msg_before', role: 'assistant', time: { created: 10, completed: 20 } } }]); const { app } = createApp({ createSessionGoal }); const response = await request(app) .post('/api/openchamber/sessions/ses_source/send') @@ -370,7 +417,7 @@ describe('openchamber session routes', () => { template: 'Take $ARGUMENTS from issue through a verified pull request. Confirm the PR covers $ARGUMENTS.', }], }); - globalThis.fetch = vi.fn(); + globalThis.fetch = vi.fn(async (url) => selectionInputResponse(url)); try { const { app } = createApp({ createSessionGoal }); const response = await request(app) @@ -393,7 +440,7 @@ describe('openchamber session routes', () => { })); expect(createSessionGoal.mock.invocationCallOrder[0]).toBeLessThan(sessionCommandMock.mock.invocationCallOrder[0]); expect(response.body).toMatchObject({ goalEnabled: true, dispatchedAsCommand: true }); - expect(globalThis.fetch).not.toHaveBeenCalled(); + expect(globalThis.fetch.mock.calls.some(([url]) => String(url).includes('/prompt_async'))).toBe(false); } finally { globalThis.fetch = originalFetch; } @@ -401,11 +448,10 @@ describe('openchamber session routes', () => { it('reuses the previous session selection when send omits model, agent, and variant', async () => { const originalFetch = globalThis.fetch; - const fetchMock = vi.fn(async () => ({ ok: true, text: async () => '' })); + const fetchMock = vi.fn(async (url) => selectionInputResponse(url) || { ok: true, text: async () => '' }); globalThis.fetch = fetchMock; try { - sessionMessagesMock.mockResolvedValue({ - data: [ + setSessionMessages([ { info: { id: 'msg_user', @@ -416,8 +462,7 @@ describe('openchamber session routes', () => { }, }, { info: { id: 'msg_before', role: 'assistant', time: { created: 10, completed: 20 } } }, - ], - }); + ]); const { app } = createApp(); const response = await request(app) .post('/api/openchamber/sessions/ses_source/send') @@ -449,7 +494,7 @@ describe('openchamber session routes', () => { it('forks from a message, dispatches the prompt, and emits the new session', async () => { const originalFetch = globalThis.fetch; const emitSessionCreatedEvent = vi.fn(); - globalThis.fetch = vi.fn(async () => ({ ok: true, text: async () => '' })); + globalThis.fetch = vi.fn(async (url) => selectionInputResponse(url) || { ok: true, text: async () => '' }); try { const { app } = createApp({ emitSessionCreatedEvent }); const response = await request(app) @@ -519,7 +564,7 @@ describe('openchamber session routes', () => { it('reports the forked session when prompt dispatch fails', async () => { const originalFetch = globalThis.fetch; - globalThis.fetch = vi.fn(async () => ({ ok: false, status: 500, text: async () => 'dispatch failed' })); + globalThis.fetch = vi.fn(async (url) => selectionInputResponse(url) || { ok: false, status: 500, text: async () => 'dispatch failed' }); try { const { app } = createApp(); const response = await request(app) @@ -584,9 +629,77 @@ describe('openchamber session routes', () => { } }); + it('rejects an unknown agent before creating a session or worktree', async () => { + const originalFetch = globalThis.fetch; + const fetchMock = vi.fn(async (url) => selectionInputResponse(url) || { ok: true, json: async () => ({ id: 'ses_123' }) }); + globalThis.fetch = fetchMock; + try { + const { app } = createApp(); + await request(app) + .post('/api/openchamber/sessions') + .send({ + directory: '/repo/app', + prompt: 'Run this', + agent: 'not-an-agent', + worktree: { name: 'side-task' }, + }) + .expect(400, { error: "Unknown agent 'not-an-agent' for /repo/app" }); + + expect(createWorktreeMock).not.toHaveBeenCalled(); + expect(fetchMock.mock.calls.some(([url]) => String(url) === 'http://opencode.test/session?directory=%2Frepo%2Fapp')).toBe(false); + expect(fetchMock.mock.calls.some(([url]) => String(url).includes('/prompt_async'))).toBe(false); + } finally { + globalThis.fetch = originalFetch; + } + }); + + it('rejects an unknown model and an unknown variant before dispatching', async () => { + const originalFetch = globalThis.fetch; + const fetchMock = vi.fn(async (url) => selectionInputResponse(url) || { ok: true, json: async () => ({ id: 'ses_123' }) }); + globalThis.fetch = fetchMock; + try { + const { app } = createApp(); + await request(app) + .post('/api/openchamber/sessions') + .send({ directory: '/repo/app', prompt: 'Run this', model: 'openai/gpt-nope' }) + .expect(400, { error: "Unknown model 'openai/gpt-nope' for /repo/app" }); + await request(app) + .post('/api/openchamber/sessions') + .send({ directory: '/repo/app', prompt: 'Run this', model: 'openai/gpt-5.5', variant: 'ultra' }) + .expect(400, { error: "Unknown variant 'ultra' for model 'openai/gpt-5.5'" }); + + expect(fetchMock.mock.calls.some(([url]) => String(url).includes('/prompt_async'))).toBe(false); + } finally { + globalThis.fetch = originalFetch; + } + }); + + it('reports promptDispatched false when the accepted prompt never reaches the session', async () => { + const originalFetch = globalThis.fetch; + const fetchMock = vi.fn(async (url) => { + if (String(url).includes('/prompt_async')) return { ok: true, text: async () => '' }; + return selectionInputResponse(url) || { ok: true, json: async () => ({ id: 'ses_123' }) }; + }); + globalThis.fetch = fetchMock; + sessionMessagesMock.mockResolvedValue({ data: [] }); + try { + const { app } = createApp(); + const response = await request(app) + .post('/api/openchamber/sessions') + .send({ directory: '/repo/app', prompt: 'Run this', model: 'openai/gpt-5.5' }) + .expect(200); + + expect(response.body.sessionId).toBe('ses_123'); + expect(response.body.promptDispatched).toBe(false); + expect(response.body.promptError).toBeTruthy(); + } finally { + globalThis.fetch = originalFetch; + } + }, 20_000); + it('does not retry a failed slash command as a normal prompt', async () => { const originalFetch = globalThis.fetch; - const fetchMock = vi.fn(); + const fetchMock = vi.fn(async (url) => selectionInputResponse(url)); commandListMock.mockResolvedValue({ data: [{ name: 'review' }] }); sessionCommandMock.mockRejectedValue(new Error('command response failed')); globalThis.fetch = fetchMock; @@ -604,7 +717,7 @@ describe('openchamber session routes', () => { .expect(500); expect(sessionCommandMock).toHaveBeenCalledTimes(1); - expect(fetchMock).not.toHaveBeenCalled(); + expect(fetchMock.mock.calls.some(([url]) => String(url).includes('/prompt_async'))).toBe(false); } finally { globalThis.fetch = originalFetch; }