fix(*): close the gaps between what ask_user promises and what it does

[LivXue]

Summary

ask_user advertised things it did not do, and a batch cost the user more than
one wait. This closes both, and gives the prompt the fields it needs to render a
batch honestly.

• Two schema fields nothing read. multiple and custom were declared per
  question and never passed to the broker, so a model asking for a multi-select
  got a single-select and never learned why. Both are removed. Multi-select is
  deliberately not implemented here: it would need a new answer syntax on the
  chat-channel surface, which has no dialog to hold a multi-select, and that is
  a larger decision than removing a field that never worked.
• One deadline per call, not per question. A batch was N round-trips with N
  independent timeouts, so a three-question call could hold a lane for three
  times the surface's wait. The call now shares one budget, and a spent budget
  stops the batch instead of opening a fresh wait on every question left.
• The budget is configuration. tools.ask_user.timeout (600s, must be
  positive) reaches the tool through AgentLoop, which is where the config
  already is. It deliberately does not read config at the transport: the TUI
  builds its broker before any config is loaded, and reading it there put a
  schema-validation failure in the path of the RPC server coming up.
• Calls that would waste the user's time are rejected before rendering, with
  a message that steers the retry: more than four questions, a duplicate
  question text, or a question with exactly one option, which is not a decision.
  A duplicate option label is a typo with one obvious reading, so it is deduped
  rather than rejected. Zero options remains a free-form question.
• header and recommended. A short chip label, and a 0-based index into
  options naming the option the agent would pick. When nobody answers, the
  text the model reads now names that option; before, the only signal was that
  no answer arrived.
• A recommendation is resolved against the options as submitted. The index
  counts what the caller sent, but the list was deduplicated before the index was
  resolved against it. With ["a","a","b","c"] and recommended: 2 the caller
  means "b" while the broker was handed "c", so the surface marked the wrong
  row and a timeout told the model the wrong intended fallback; an index past the
  deduped length (3 on that list) failed the range check and the recommendation
  was dropped without a word. The label is now resolved before dedup narrows the
  list. Dedup keeps every distinct label, so the resolved label is always still
  one of the choices the surface can mark.
• An undeliverable question is reported, not dropped. A question for a
  conversation with no live source was dropped with a log line the broker could
  not see, so the round-trip waited out its whole budget on a question nobody
  would ever see. The channel adapter now raises QuestionUndeliverableError
  and the broker fails safe at once.
• The prompt renders the batch. Position in the call, the questions still to
  come, the recommended option marked, and the remaining budget counting down.
  Tab attaches a note to a selection, so an answer no longer has to be either a
  choice or free text. On a chat channel the position rides in the message text.
• The countdown renders whole seconds. timeout_s is a remaining-time
  subtraction, so a 90-second budget arrives as 89.99999912502244 and the
  prompt decrements that same fractional value once a second. The formatter
  passed it straight through, so the line read 1m 29.99999912502244s on every
  tick and widened the prompt that the padding exists to keep steady. The value
  is now ceiled before the branch is chosen, since ceiling inside the branches
  renders a ceiled 59.7 as 60s rather than 1m 00s.
• The Other row answers to its own number. This one predates the rest of
  the branch. The options list draws Other as one more numbered row, but the
  quick-pick handler bounded itself at the number of real choices, so the last
  row was numbered and unreachable -- and worse than inert: the keystrokes that
  followed were swallowed by the options handler, so the next Enter submitted
  whichever option was still highlighted. Someone who thought they were typing a
  free-form answer sent a selection they never made. Its number now opens the
  text input, which is what Enter on that row already did, and the hint names
  the range it can take.

The wire payload gains header, recommended, timeout_s, index, total
and batch. clarify.request is a notification and is not part of
rpc-schema/openrpc.json, so no generated artifact changes; npm run lint:rpc
confirms generated.ts is still in sync. deep_research shares the same broker
and passes none of the new fields, so its prompt is unchanged except that it now
shows the countdown, which was always the real deadline.

timeout_s stays fractional on the wire deliberately: it is also what the model
is told about the budget, and rounding it at the source would change that value
to serve a display concern. The formatter is the layer with a width to protect.

Type

• Fix
• Feature
• Docs
• CI / tooling
• Refactor
• Other

Mostly fixes; the prompt affordances and the config field are additive.

Verification

Every new test was watched failing before the code that makes it pass.

Rebased onto 22b71765, which collapsed the earlier Merge branch 'main' commit
into a linear six-commit history. The base gained #361 and #362; the only file
this branch and those commits both touch is raven/agent/loop/main.py, and the
replay was clean. git merge-tree --write-tree needs git 2.38 and this box has
2.34.1, so the pre-push check was that file-overlap table plus a full re-run at
the new head rather than a dry-run merge. Everything below is from the new head,
not carried forward.

python -m pytest tests/ -q
  82 failed, 6576 passed, 47 skipped, 13 deselected

# same suite on the new base 22b71765, in a separate worktree:
  82 failed, 6545 passed, 47 skipped, 13 deselected

# full failing-ID lists from both runs, sorted and diffed:
diff branch_fail.txt base_fail.txt   -> identical (0 introduced, 0 fixed)

The 82 failures are inherited: the base fails the same 82 IDs on this box, and the
sets are identical rather than merely the same size. They are
test_tui_rpc_session.py (58), test_cli_cron_commands.py (7),
test_cli_import_commands.py (5), test_tui_commands_error_codes.py (3),
test_cli_onboard_commands.py (3), and 6 more spread across 5 files. The +31
passing tests are the ones added here.

Suites that exercise the changed code directly:

python -m pytest tests/test_ask_user_tool.py tests/test_question_broker.py \
                 tests/test_cli_gateway_commands.py tests/test_config_schema.py -q
  81 passed

npm run test -- --run src/__tests__/clarifyPrompt.test.tsx
  11 passed

npm run type-check   -> clean
npm run lint:rpc     -> OK: generated.ts in sync

python -m ruff check raven tests           -> All checks passed!
python -m ruff format --check raven tests   -> 849 files already formatted
python -m scripts.check_commit_messages github/main..HEAD -> exit 0
scripts/check_large_files.py                -> exit 0

Beyond the suites, the tool was exercised over the real path -- a real
ToolRegistry (so cast_params and the schema validator run), a real
QuestionBroker, and answers delivered through the real clarify.respond
handler on a real Dispatcher:

• the three rejections emit zero prompts, so a malformed call costs the user
  nothing, and the steer text reaches the model intact through the registry;
• a batch answered by a responder taking 0.25s per question is handed
  10.0000s, 9.7495s, 9.4991s -- each question inherits what the last one
  left;
• a batch whose budget is spent emits one prompt, not one per question;
• an undeliverable question returns in 0.001s against a 30s budget.

test_registry_dispatch_and_the_real_clarify_respond_route was added from that
exercise, because no committed test drove the registry before. It was proved
non-vacuous by renaming the recommended keyword the tool passes the broker and
watching it go red.

The two review fixes were each proved the same way:

• The recommendation cases were run against the deduped resolution and failed
  with assert 'c' == 'b' (wrong label) and assert '' == 'c' (recommendation
  dropped), then passed once the resolution moved ahead of dedup.
  test_out_of_range_recommended_index_is_ignored passes unchanged -- it has no
  duplicates, so it never encoded this.
• The countdown cases were run against the pass-through formatter and failed on
  expected ... to match /1m 30s/ and /1m 00s/, covering both the fractional
  payload and the 59.7 boundary that ceiling creates.

prompts.tsx and clarifyPrompt.test.tsx are reported by Prettier, and were
before this branch touched them: the hunks it wants are at lines this branch does
not change (a JSX ternary and a renderSync call), so they are left alone rather
than reformatted into this diff.

• Relevant tests pass locally
• Relevant lint / type checks pass locally
• User-facing docs or screenshots are updated when needed

docs/Proactivity-Implementation.md documented the two removed fields and the
old narrow payload, so that paragraph is rewritten. ui-tui/dist/entry.js is
gitignored, so testing the prompt by hand needs
npm run build --prefix ui-tui first.

Risk

User-visible changes:

• A model sending five questions, a duplicate question, or a one-option question
  now gets an error instead of prompting the user. The error text tells it what
  to do instead.
• A batch that used to get a fresh timeout per question now shares one. A slow
  user answering question 3 of 4 past the budget gets the rest recorded as
  unanswered rather than being asked.
• The prompt shows a countdown where it showed none, including for
  deep_research, and that countdown now reads in whole seconds.
• A recommendation sent as an index past the deduped option count used to be
  dropped silently and now resolves. That is what the schema promises, but it is
  a real change for a caller that had been sending such an index.
• Typing the Other row's number now opens the text input instead of doing
  nothing.

Rollback is the six commits; nothing is persisted and no format changes on disk.
The config field defaults to the previous 600s, so an untouched config behaves
as before.

Checked and deliberately not fixed, to keep this diff to one subject:

• QuestionBroker.await_question still returns its default on
  asyncio.CancelledError, which swallows an outer cancellation. It sits beside
  the handler added here, but changing it means auditing how the agent loop
  reacts to a cancelled turn.

• The gateway's inbound gate calls pending_req(cid) and then reply(...). If
  the question times out between the two, the message answers nothing and starts
  no turn. reply already returns a bool, so using it as the condition would
  close this.

• The registry's schema validator does not implement maxItems, so the cap in
  the schema is advisory to the model and _prepare is what enforces it. That
  is deliberate -- the model gets the steer text rather than a generic
  validation error -- and a test pins the two statements of the cap together so
  they cannot drift.

• ApprovalPrompt in the same file hardcodes 1-2 quick pick, which is correct
  for its two fixed options and has no Other row, so it is not the same bug.

• approvalRemainingSeconds() feeds the approval countdown in the same file and
  is already Math.max(0, Math.ceil(...)), so it needs nothing here. It is also
  where the ceil convention comes from: the clarify countdown now rounds the
  same direction as the approval one rather than inventing a second rule. The two
  are still not made to share a formatter -- one takes an absolute deadline and
  the other a remaining duration.

• Security impact considered

• Backward compatibility considered

• Rollback path is clear for risky changes

No new secret, no new surface, and no asset moved onto an unauthenticated one.
The new config field is a timeout.

Related Issues

N/A

[gloryfromca]

Blocking: two user-visible contract errors remain in recommendation normalization and countdown rendering; see the inline notes.

Reviewed the current head against github/main. I covered the repository rules and domain glossary, the complete diff, callers through the tool registry/broker/TUI and gateway answer paths, the relevant commit history and stated PR intent, backward compatibility of the schema and wire additions, and the test changes for weakening or missing branches. The tests were not weakened, but the new tests miss both failing combinations below.

Local verification:

• uv run pytest tests/test_ask_user_tool.py tests/test_question_broker.py tests/test_cli_gateway_commands.py tests/test_config_schema.py -q -> 79 passed
• npm run test -- --run src/__tests__/clarifyPrompt.test.tsx -> 9 passed
• npm run type-check -> passed
• git diff --check github/main...HEAD -> passed

The focused suites pass, and the live GitHub checks are green, but direct production-shaped probes reproduce both inline failures.

[gloryfromca]

recommended is defined as an index into the submitted options, but this deduplicates the list before line 80 resolves that index. I reproduced options=["a","a","b","c"], recommended=2: the caller selected "b", while the broker receives choices ["a","b","c"] with recommended="c". The TUI then marks the wrong choice, and on timeout the model is told the wrong intended fallback. Resolve the selected label against the original list before deduplication (or preserve an index mapping), and add a test combining duplicates with a recommendation.

[LivXue]

Confirmed, and it had a second failure mode the note did not reach: an index that is valid against the submitted list but past the deduped length was dropped entirely rather than mis-resolved.

options=["a","a","b","c"], recommended=2  ->  broker got "c"  (caller meant "b")
options=["a","a","b","c"], recommended=3  ->  broker got ""   (caller meant "c")

Fixed in 883914e: _prepare keeps the submitted list and resolves the index against it, letting dedup narrow only choices. Dedup preserves every distinct label, so the resolved label is always still one of the choices the surface can mark -- that invariant is what makes resolving before dedup safe rather than merely different, so it is stated in the comment.

Deviation from the shape you suggested: you offered "resolve the selected label against the original list (or preserve an index mapping)" and I took the first. An index map would have to stay correct at two use sites and there is only one consumer.

Tests: one case per failure mode. I reverted the resolution to the deduped list and watched them fail with assert 'c' == 'b' and assert '' == 'c', then restored. test_out_of_range_recommended_index_is_ignored passes unchanged -- it has no duplicates, so it never encoded this.

One behaviour change worth naming: the second case now produces a recommendation where it previously produced none. That is what the schema promises, but it is a real change for a caller that had been sending such an index.

[gloryfromca]

The real payload is fractional: AskUserTool passes deadline - loop.time() to the broker. With a configured 90-second budget I reproduced timeout_s=89.99999912502244; this formatter renders 1m 29.99999912502244s, not the tested 1m 30s shape. Every normal ask_user countdown can therefore expose a long decimal and widen the prompt. Round the seconds before both branches (using the desired deadline convention, likely ceil), and cover a fractional timeoutS in the component test.

[LivXue]

Confirmed, and it is not confined to the first frame. ClarifyPrompt seeds remaining from req.timeoutS and ticks it with s - 1, so the fractional residue rides the whole countdown: 89.99999912502244 -> 88.99999912502244 -> and so on. Every frame carries the decimal, not just the one on mount.

Fixed in 47774ef. The part of your note that is load-bearing is "before both branches": the comparison is secs < 60, so ceiling inside the branches renders a ceiled 59.7 as "60s" instead of "1m 00s". Ceil rather than round, so the last second stays on screen until the deadline has actually passed.

Tests: timeoutS: 89.99999912502244 asserts 1m 30s and that no decimal point appears in the frame, plus 59.7 for the boundary the rounding itself creates. Reverted the formatter and watched both fail on expected ... to match /1m 30s/ and /1m 00s/, then restored.

Deliberately not fixed: the payload stays fractional on the wire. timeout_s is also what the model is told about the budget, and rounding at the source would change that value to serve a display concern. The formatter is the layer with a width to protect.

[gloryfromca]

No blockers; this can merge as far as I am concerned.

The two prior findings are fixed on this revision. I reproduced the submitted-index cases through AskUserTool: duplicate narrowing now leaves recommended=2 mapped to b and recommended=3 mapped to c, while genuinely invalid and boolean indices remain ignored. The countdown now ceilings before choosing the seconds/minutes branch, so the real fractional payload renders in whole seconds and the 59.7-second boundary renders as 1m 00s.

The rebase range-diff shows the four previously reviewed commits are patch-identical; only the two fix commits are new. I checked the full current diff, AGENTS.md and the Runtime/TUI domain contracts, callers through the tool/broker/TUI path, the relevant history, backward compatibility, and whether tests were weakened. The only absorbed-base path overlap is raven/agent/loop/main.py; the base change is in stream finish-reason handling, away from this PR's constructor/config/tool-registration hunks. No tests were weakened.

Local verification:

• uv run pytest tests/test_ask_user_tool.py tests/test_question_broker.py tests/test_cli_gateway_commands.py tests/test_config_schema.py -q -> 81 passed
• npm run test -- --run src/__tests__/clarifyPrompt.test.tsx -> 11 passed
• npm run type-check -> passed
• git diff --check github/main...HEAD -> passed
• git merge-tree --write-tree HEAD github/main -> clean

At posting time, GitHub's unit job is still in progress and bridge is queued; all completed checks are green.

[LivXue]

Both findings are fixed, one commit each, with the reasoning in their threads.

883914ee  resolve a recommendation against the options as submitted
47774efd  render the clarify countdown in whole seconds

Both had a wider blast radius than the notes reached, and both are described in
the threads: the recommendation was not only mis-resolved but silently dropped
when the index fell past the deduped length, and the countdown carried its
decimal on every tick rather than only the first frame, because the prompt
decrements the raw payload.

The branch is rebased onto 22b7176, which collapsed the earlier merge commit
into a linear history, so the SHAs you cited are stale. The description is
rewritten at the new head: the full-suite comparison is redone against the new
base (82 failed on both sides, failing-ID lists diffed as identical rather than
merely equal in count), and the two new verification paragraphs replace the ones
written before these fixes existed.