fix(kap-server): accept question ids containing colons on resolve - #2585
Conversation
Some OpenAI-compatible providers emit tool_call ids like `AskUserQuestion:0`, which the question service adopts as the question id. The action-suffix parse then rejected the bare resolve POST as an unsupported action (40001), so clients could never submit answers. When the suffix parse fails, fall back to matching the full tail against the pending question list before emitting 40001. Also add maxRetries to the test home cleanup to absorb the async query-store shard flush (ENOTEMPTY on macOS), matching fs.test.ts.
🦋 Changeset detectedLatest commit: b35eb4d The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 71b5bef101
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (interaction.listPending('question').some((i) => i.id === tail)) { | ||
| questionId = tail; | ||
| action = 'resolve'; | ||
| } else { | ||
| reply.send(errEnvelope(ErrorCode.VALIDATION_FAILED, parsed.reason, req.id)); | ||
| return; |
There was a problem hiding this comment.
When a colon-bearing question is resolved successfully, a retry of the same bare resolve URL (for example after a dropped response) now hits this parsed.kind === 'invalid' branch again, finds no pending question, and returns 40001 before the existing isRecentlyResolved(questionId) idempotency check can run. That makes the fixed provider shape lose the route's documented duplicate-resolve behavior (40902 within the recently-resolved window); check interaction.isRecentlyResolved(tail) in this fallback before emitting the validation error.
Useful? React with 👍 / 👎.
commit: |
…tions A retried bare resolve of a colon-bearing question id re-entered the invalid-suffix fallback after the question settled, found no pending match, and returned 40001 — bypassing the recently-resolved idempotency window. Accept the tail in the fallback when it is recently resolved so the shared duplicate-resolve path emits 40902 as documented.
1 participant
Related Issue
No linked issue — the problem is explained below.
Problem
When using certain OpenAI-compatible providers/gateways, submitting an answer to an interactive question prompt (
AskUserQuestion) fails with40001 unsupported action, and the user can never get the answer through. Dismissing the same prompt works fine.Root cause: the question id is derived from the LLM tool_call id, and some providers emit ids containing a colon (e.g.
AskUserQuestion:0). The REST routePOST /sessions/{sid}/questions/{qid}parses the path tail with the shared:actionsuffix convention (split on the last colon), so the bare resolve URL.../questions/AskUserQuestion:0is misread as idAskUserQuestion+ action0, which is not a supported action. Dismiss works because.../AskUserQuestion:0:dismisssplits off a valid trailing:dismiss. Official providers never hit this because their tool_call ids contain no colons.What changed
invalid, the full unparsed tail is first matched against the session's pending question list: a hit proceeds as a normal resolve; a miss keeps the original40001behavior. The change is confined to this one handler — the dismiss path and all other:actionroutes are untouched.40001.maxRetriesto the test home cleanup to absorb the async query-store shard flush (ENOTEMPTYon macOS), matching the existing pattern infs.test.ts.This approach fits Kimi Code because it is a server-side compatibility fallback: no wire-contract change, no client changes, and it also covers in-flight sessions whose pending questions already carry colon-bearing ids.
Checklist
gen-changesetsskill, or this PR needs no changeset.gen-docsskill, or this PR needs no doc update.