fix(ask-user): a cancelled or timed-out ask shows the partial answers

Merged
CO
2d ago
+785-64

Part of sisyphuslabs/omo-desktop-app#4042

Stacked on #3999 (head d2da60dcbe). This branch starts from #3999's head because test 4 and the LOW fix need #3999's degradeQuestion, which keeps partial answers on abort. Until #3999 merges, this diff also shows #3999's three commits; the change in this PR is the top commit only. Follow-up from the #3999 review: https://github.com/code-yeongyu/senpi/pull/3999

Problem

When an ask was cancelled or timed out after the user had already answered some of its questions, the model got only The user dismissed the question. (cancelled), or a timeout line without the questions left open. This happened on both the structured RPC question path (ConnectionQuestionBridge, whose draft answers survive the abort) and the degraded fallback (degradeQuestion, which keeps partial answers on abort since #3999). The formatter (ask-user/format.ts, case cancelled) threw the answers away.

Change

  • ask-user/format.ts: a cancelled response with answers now reads The user cancelled the question after answering part of it., then each header: answer line, then Unanswered: .... A cancel with no answers keeps today's exact text. The non-required timed_out text already listed the selected answers; it now also adds the Unanswered: line. Required questions still go through the unchanged gated path (#2949).
  • connection-question-bridge.ts (#3999 review, LOW 1): degradeQuestion checks the signal again after select returns "Other (type an answer)", so a cancel that lands while the select is open never opens ui.input.

Tests (RED first)

New test/suite/ask-user-cancelled-partial-answers.test.ts: 4 of 5 failed before the fix, 5 of 5 pass after it.

  1. Structured path: cancelled with 1 of 2 answered shows q1: A and Unanswered: q2.
  2. Same for timed_out.
  3. No answers: still The user dismissed the question. (passed before and after, as intended).
  4. Fallback aborted after q1: shows q1.
  5. Select returns Other while the signal aborts: no input opens.

ask-user-schema.test.ts: the timed-out-with-draft snapshot now includes Unanswered: Library.

Verification

  • ask-user / question / rpc-question suites (48 files): 383 passed, 2 failed. The 2 failures are in regressions/issue-2951-held-question.test.ts (worker-host Listener deadline). They fail the same way on the unchanged #3999 head d2da60dcbe, so this change did not cause them.
  • bun run check: exit 0.
  • node scripts/check-pr-changelog.mjs --base <merge-base>: PASS (CHANGELOG [Unreleased] Fixed entry; src/modes/rpc/changes.md entry).

Summary by cubic

A cancelled or timed-out ask now shows the model the answers the user gave before it settled, listing the questions left unanswered, instead of only a dismissal text. Only a cancel the user chose (the cancel button or Remove on a card, Esc or Ctrl+C in the terminal question, /answer skip) or a failing question UI delivers the partial answers; lifecycle cancels (turn abort, session close, ask-user disabled, disconnected RPC client) stay silent even when the draft held answers, and a cancel with nothing answered keeps today's dismissal text.

QuestionResponse.cancelOrigin carries the origin on the response, and showsPartialAnswers in format.ts is the single decision point read by the tool result, deliverAnswer, and /answer skip. Async questions (waitForAnswer: false) deliver partial answers once, the transcript chip marks the question (cancelled), /answer skip adds no second copy, and a blocking question whose UI failed shows the reason plus the partial answers. A timed-out ask lists several answers one per line and the questions left unanswered.

Clients without the question capability no longer get a trailing "Anything else? (optional)" prompt after every ask; the degraded fallback reports each answered question as progress so a fallback ask that times out after the first answer still delivers it, and drops a pick that lands after an abort before opening another dialog.

  • degradeQuestion checks the abort signal again after a select returns, so no pick or "Other" input opens after a cancel.
  • A control-endpoint cancel from another client (the desktop's Remove) is treated as the user's and keeps the draft reported as progress, so the terminal delivers partial answers once, like a host.

Second commit (fb67ce927a): the fallback reports progress

The real-CLI QA below showed that the fallback's partial answers never reached the model. The ask-user extension settles an abort or its idle timeout from its own draft before degradeQuestion returns, and the fallback never reported progress, so that draft was empty. degradeQuestion now calls opts.onProgress({ answers }) after each answered question, the draft shape the structured bridge reports.

New test/suite/ask-user-degraded-progress.test.ts runs the real ask-user extension (pending question + idle timer) with degradeQuestion as its question UI: q1 is answered, then (a) the tool signal aborts, (b) the 1-minute idle timer fires (fake timers). Both failed before the fix ("The user dismissed the question." / a timeout line without answers) and pass after it.

Suites at fb67ce927a: 49 files, 385 passed, 2 failed (the same issue-2951-held-question worker-host failures that also occur on the unchanged #3999 head). bun run check exit 0. Changelog gate PASS.

QA (real CLI, senpi-qa RPC channel, fake model server, no question capability)

Driver: the source CLI in --mode rpc; the fake model calls ask_user_question with two questions; the client answers q1's select with "A" and leaves q2's open. Evidence is kept locally under local-ignore/qa-evidence/20261023-senpi-4002-ask-partial-cancel/ (sha256 below).

  • Idle timeout (askUser.timeoutMinutes 1): PASS. The tool message the model receives in its next request: 
    The user did not answer within 1 minutes. (사용자가 답변을 안하고 timeout 으로 종료됨)
Before going idle the user had selected: q1: A
Unanswered: q2
Continue the work to completion on your best judgment; do not ask this question again this turn.
  • Turn abort (RPC abort): NOT met, outside this PR. The model receives Tool execution aborted. packages/agent/src/agent-loop.ts (executePreparedToolCall) races every tool against the turn's abort signal and replaces the tool's result with that error the moment the turn aborts, so no tool result, ask-user's included, reaches the model after a turn abort. The ask-user settlement itself now carries q1 (unit test (a)); the agent loop discards it. Changing that is an agent-loop contract for every tool (upstream-owned), so it is reported separately rather than changed here.
  • Structured path (set_client_info with question): not driven end to end. In this stdio run the host still opened select dialogs after set_client_info declared the capability, so the run took the fallback path; why is unverified. The structured bridge's cancel and timeout with a draft are covered by ask-user-cancelled-partial-answers.test.ts tests 1 and 2.
2e37a6ea85d140fff85060c5d476a6796a808eeca3fbc388f263cfe33bdb9551  degraded-timeout-tool-result.txt
f12aef6ce1a0c6059e274846fc7abfa3929a70261f2baa23be569751442e186a  degraded-timeout-rpc-events.jsonl
6649f7db21780cb439218bf30306ed52cdb03e64cd215911a5d49f8d06ae27e7  degraded-timeout-model-requests.json
ccd31ef387dd07db6b56eed0caab422d4b28094ecaeb72d62a3f0e50d2c3bf72  degraded-tool-result.txt
8c4c2b3949c723755e43e54b345501fc5c6a039e51d5418ca17716e1bb8f2def  degraded-rpc-events.jsonl
a9497afd297cf43d6e2793694f07ba1deb709a262cd3c6044b52cc2ca9573839  degraded-model-requests.json
59af6129d548e34915689058797d690a394d3292de2928c6bcc31ddf89fa92e2  driver.mjs

Third commit (b848952320): review folds and lead rules

A turn abort drops the ask's result like any tool's (lead ruling: no agent-loop change); partial answers are kept when a question is cancelled or times out. docs/tui.md says the same.

  • /answer skip: with nothing answered it still sends "The user dismissed the question."; after partial answers it sends them, and the transcript chip shows a leading ↳ (cancelled) row so it no longer looks like a submitted answer (review MEDIUM 1).
  • Async question (waitForAnswer: false): cancelled with nothing answered stays silent as before; cancelled after partial answers, deliverAnswer sends one message with them ("The user cancelled the question after answering part of it." + answers + Unanswered:). /answer skip on such a question does not send a second copy (review MEDIUM 2, lead rule).
  • A cancelled blocking question with a reason (for example "Question UI failed: ...") shows the reason and the partial answers instead of only the reason (review LOW 1).
  • degradeQuestion checks the abort as soon as select returns, so a label picked after the abort is not recorded, the same as "Other" (review LOW 2).

New test/suite/ask-user-cancel-partials-surfaces.test.ts (6 tests): 4 failed before the change for the intended reasons, and the 2 nothing-answered cases passed before and after, as intended. All 6 pass now.

Verification at b848952320, after bun run build: ask-user / question / rpc-question suites, 50 files, 393 of 393 pass. issue-2951-held-question now passes because the workspace is built; its earlier failure was the unbuilt-workspace prerequisite, as the review found. bun run check exits 0. Changelog gate passes; it counts 1 production path, interactive-mode.ts, which is covered in src/modes/interactive/changes.md.

Fourth commit (dffa293a0b): only a cancel the user chose shows partial answers

The rule (lead). A cancelled question shows the model the answers given before it only when the user chose the cancel (the cancel button or Remove on a card, Esc or Ctrl+C in the terminal question, /answer skip), or when its UI failed. Every lifecycle cancel stays silent as before, even with answers: a turn abort (async asks included), the session closing, ask-user being disabled, and an RPC client disconnecting. A turn abort also still replaces a blocking ask's result like any tool's. A timeout keeps its partial answers.

How. The origin travels on QuestionResponse.cancelOrigin as an explicit enum, QuestionCancelOrigin: user, ui_failure, turn_abort, session_closed, ask_user_disabled.

  • The TUI widget, /answer skip and the RPC bridge's { cancelled: true } set user. cancelAll and aborts set nothing.
  • The builtin's cancel(origin, ...) sets the rest.
  • showsPartialAnswers in format.ts is the one decision. The blocking tool result, deliverAnswer and /answer skip all read it.

Review folds.

  • MEDIUM 1: an async ask plus a turn abort sends no message and starts no turn.
  • MEDIUM 2: a session close and ask-user being disabled send nothing.
  • LOW 1: /answer skip adds no copy when the builtin delivers the answers. /answer only reaches async questions: shownQuestion reads pendingQuestions, which only showAsyncQuestion fills, and waitForAnswer: true goes to the overlay (interactive-mode.ts:3703). So no blocking skip can double them. The keyboard-model test pins the reachable path.
  • LOW 2: an async UI failure delivers its reason and the partial answers.
  • LOW 3: CHANGELOG and docs/tui.md list exactly which cancels deliver and which stay silent.
  • fb67 LOW 2: a non-user delivery (a UI failure) reads "The question was cancelled after part of it was answered."
  • fb67 LOW 3: a timeout with several answers lists them one per line. A single answer stays inline, so the #2949 pins are unchanged.

Tests. The new ask-user-cancel-origin.test.ts has 8 tests; 7 failed on b848952320 for the intended reasons and all 8 pass now. Tests from earlier rounds that encoded "any cancel shows partials" now follow the rule: an abort stays a dismissal, and a user cancel shows them.

Verification at dffa293a0b (after bun run build): the ask-user, question, rpc-question and regression suites (65 files, issue-2951 included) pass 491, with 1 skipped. bun run check exits 0. The changelog gate passes, covering 2 production paths.

Fifth commit (ddec708580): another client's cancel keeps the draft

This folds review LOW 1 at dffa293a0b. The desktop's Remove on a question that a terminal session asked arrives through the control endpoint as { cancelled: true } with no answers. That cancel now carries cancelOrigin: "user", and the builtin no longer lets a cancel with no answers wipe the draft the UI reported as progress. So the answers the user gave in the terminal reach the model once, the same as on a host. A cancel that carries answers still updates the draft; ask-user-extension's typed-draft required-question test pins this.

The new parity case in session-control-answer-parity.test.ts: q1 is answered as progress, then a cancel frame arrives. Before the fix the host delivered the partial answers and the terminal delivered []. Now both deliver the same single message.

LOW 2 (the /answer skip status line) and LOW 3 (an explicit connection_closed origin) are in the follow-up #4011, per the lead.

Verification at ddec708580, after bun run build: the ask-user, question, rpc-question, session-control and regression suites (67 files) pass 506, with 1 skipped. bun run check exits 0. The changelog gate passes.

Sixth commit (1238112ed2): proof that lifecycle cancels stay silent

accept() keeps the stored draft on every cancel that carries no answers. The new test/suite/ask-user-lifecycle-cancel-silence.test.ts proves the lifecycle cancels still stay silent with that draft kept. It covers a turn abort, the session closing, ask-user being disabled and a closed RPC connection (cancelAll), each after q1 was answered as progress.

  • Async: no sendUserMessage, so no turn starts.
  • Blocking: the tool result keeps today's text. That is "The user dismissed the question." for a turn abort and a closed connection, and the existing reason line for a session close or ask-user being disabled. None of them names the answer.
  • Both: the settled response still carries the draft.

8 of 8 pass, and they passed at ddec708580 before any other change: this commit is a proof, not a fix. The tool.ts comment and the interactive changes.md entry now say the draft-keeping applies to every answer-less cancel.

Verification at 1238112ed2, after bun run build: the ask-user, question, rpc-question, session-control and regression suites (68 files) pass 514, with 1 skipped. bun run check exits 0. The changelog gate passes.