diff --git a/.changeset/questions-colon-id-resolve.md b/.changeset/questions-colon-id-resolve.md new file mode 100644 index 000000000..6404aacb9 --- /dev/null +++ b/.changeset/questions-colon-id-resolve.md @@ -0,0 +1,6 @@ +--- +"@moonshot-ai/kap-server": patch +"@moonshot-ai/kimi-code": patch +--- + +Fix submitting answers to interactive question prompts being rejected when the model provider returns tool call IDs containing colons (some OpenAI-compatible gateways). diff --git a/packages/kap-server/src/routes/questions.ts b/packages/kap-server/src/routes/questions.ts index c642e53d4..a31908132 100644 --- a/packages/kap-server/src/routes/questions.ts +++ b/packages/kap-server/src/routes/questions.ts @@ -18,6 +18,15 @@ * Fastify cannot disambiguate `:question_id` from `:question_id:dismiss` on the * same path prefix. The POST body is therefore validated manually (the dismiss * path carries an empty body), not via the Zod preHandler. + * + * **Colon-bearing question ids**: the question id is derived from the LLM + * tool_call id, and some providers emit ids containing a colon (e.g. + * `AskUserQuestion:0`), which the action-suffix parse rejects as an unknown + * action. The resolve handler therefore falls back to matching the FULL tail + * against the pending list before emitting 40001 — a hit resolves, a miss + * keeps the validation error. (Dismiss is unaffected: `id:0:dismiss` parses + * off the final colon.) + * * Error mapping (REST.md §3.6): * - 40401 (session.not_found) — no live session matches {sid} @@ -164,12 +173,6 @@ export function registerQuestionsRoutes(app: QuestionRouteHost, core: Scope): vo defaultAction: 'resolve', resourceLabel: 'question', }); - if (parsed.kind === 'invalid') { - reply.send(errEnvelope(ErrorCode.VALIDATION_FAILED, parsed.reason, req.id)); - return; - } - const questionId = parsed.id; - const action: 'resolve' | 'dismiss' = parsed.kind === 'bare' ? 'resolve' : parsed.action; const handle = await resumeSessionById(core.accessor, session_id); if (handle === undefined) { @@ -180,6 +183,30 @@ export function registerQuestionsRoutes(app: QuestionRouteHost, core: Scope): vo } const interaction = handle.accessor.get(ISessionInteractionService); + + let questionId: string; + let action: 'resolve' | 'dismiss'; + if (parsed.kind === 'invalid') { + // Compat fallback: some providers emit tool_call ids CONTAINING a + // colon (e.g. `AskUserQuestion:0`), which the action-suffix parse + // rejects as an unknown action. Treat the full tail as the question + // id when it matches a pending (or recently-resolved, so duplicate + // resolves keep the 40902 semantics) question; otherwise keep 40001. + if ( + interaction.listPending('question').some((i) => i.id === tail) || + interaction.isRecentlyResolved(tail) + ) { + questionId = tail; + action = 'resolve'; + } else { + reply.send(errEnvelope(ErrorCode.VALIDATION_FAILED, parsed.reason, req.id)); + return; + } + } else { + questionId = parsed.id; + action = parsed.kind === 'bare' ? 'resolve' : parsed.action; + } + const pendingInteraction = interaction .listPending('question') .find((i) => i.id === questionId); diff --git a/packages/kap-server/test/questions.test.ts b/packages/kap-server/test/questions.test.ts index a69d6af46..7e7a28fbe 100644 --- a/packages/kap-server/test/questions.test.ts +++ b/packages/kap-server/test/questions.test.ts @@ -86,7 +86,9 @@ describe('server-v2 /api/v1/sessions/{sid}/questions', () => { server = undefined; } if (home !== undefined) { - await rm(home, { recursive: true, force: true }); + // maxRetries: the async query-store shard writer can still be flushing + // after close (ENOTEMPTY on macOS) — same retry pattern as fs.test.ts. + await rm(home, { recursive: true, force: true, maxRetries: 5, retryDelay: 50 }); home = undefined; } }); @@ -334,6 +336,58 @@ describe('server-v2 /api/v1/sessions/{sid}/questions', () => { expect(body.code).toBe(40405); }); + it('resolves a question whose id contains a colon (provider tool_call id)', async () => { + const sid = await createSession(); + // Real-world shape: no explicit id, so the question id falls back to the + // tool_call id — which some providers emit as `{function_name}:{index}`. + const resultPromise: Promise = questionService(sid).request({ + toolCallId: 'AskUserQuestion:0', + questions: [ + { + question: 'Pick one', + options: [{ label: 'Yes' }, { label: 'No' }], + }, + ], + }); + + const list = await getJson(`/api/v1/sessions/${sid}/questions?status=pending`); + expect(list.body.data.items[0]!.question_id).toBe('AskUserQuestion:0'); + + const { body } = await postJson( + `/api/v1/sessions/${sid}/questions/AskUserQuestion%3A0`, + { answers: { q_0: { kind: 'single', option_id: 'opt_0_0' } } }, + ); + expect(body.code).toBe(0); + expect(body.data.resolved).toBe(true); + await expect(resultPromise).resolves.toEqual({ answers: { 'Pick one': 'Yes' } }); + }); + + it('keeps 40001 for a colon tail that matches no pending question', async () => { + const sid = await createSession(); + const { body } = await postJson(`/api/v1/sessions/${sid}/questions/q-9:0`, { + answers: { q_0: { kind: 'single', option_id: 'opt_0_0' } }, + }); + expect(body.code).toBe(40001); + }); + + it('returns 40902 on a duplicate resolve of a colon-id question', async () => { + const sid = await createSession(); + questionService(sid).enqueue({ + toolCallId: 'AskUserQuestion:1', + questions: [{ question: 'Pick one', options: [{ label: 'Yes' }] }], + }); + const url = `/api/v1/sessions/${sid}/questions/AskUserQuestion%3A1`; + await postJson(url, { + answers: { q_0: { kind: 'single', option_id: 'opt_0_0' } }, + }); + + const dup = await postJson<{ resolved: false }>(url, { + answers: { q_0: { kind: 'single', option_id: 'opt_0_0' } }, + }); + expect(dup.body.code).toBe(40902); + expect(dup.body.data).toEqual({ resolved: false }); + }); + it('returns 40401 for an unknown session', async () => { const { body } = await getJson('/api/v1/sessions/nope/questions?status=pending'); expect(body.code).toBe(40401);