Skip to content

ask_user: suspend on a message_response gate instead of answering synchronously - #500

Merged
TheGreatAxios merged 3 commits into
mainfrom
cl-7190-ask-user-message-response-gate
Aug 30, 2026
Merged

ask_user: suspend on a message_response gate instead of answering synchronously#500
TheGreatAxios merged 3 commits into
mainfrom
cl-7190-ask-user-message-response-gate

Conversation

@TheGreatAxios

@TheGreatAxios TheGreatAxios commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

Summary

ask_user posted its question card and returned a synchronous ToolResult immediately. The only thing telling the agent to wait was a sentence of prose in the result — the director never enforced it, so WorkbenchDirector.decide fell through to DefaultDirector, which re-inferred as soon as the tool-result count reached zero. Net effect: the agent (weaker models especially) regularly answered its own question and proceeded on a guess the user never gave.

The platform already defines the right primitive — GateType enumerates message_response alongside approval — with zero implementations anywhere. This wires it up.

  • ask_user now contributes a beforeToolExtension (mirroring how the authz extension already suspends approval-gated calls) that posts the question card via the existing postQuestion, then returns { type: "suspend", gate: { type: "message_response", ... } } instead of a ToolResult. This reaches the reactor's pre-existing suspendOnGate, which registers the gate, durably persists the PendingOperation, and commits before returning to its loop — restart-safe, exactly like an approval suspension.
  • The gate's correlationId is minted from postQuestion's own questionId (not a throwaway random id) — the id that will let CL-7191 (stacked on this branch) resolve the specific gate an answer is for, once it lands.
  • timeoutMs is explicit (1 hour); a timeout clears the gate with a synthetic error result rather than hanging forever.
  • Neither the reactor nor the director special-cases ask_user by name — the fix lives entirely in @corbits/interaction-tools, via a new ToolBundle.beforeToolExtension any tool package can contribute.
  • The old "wait for it rather than guessing" prose is gone from both the tool description and result, since waiting is now structural.

Regression test: vendor/intx/inference/src/reactor.test.ts (new — this package had zero tests before) drives a real createReactor cycle, not a mocked decision shape, and proves: (1) a suspended call produces no tool.done and no re-inference, (2) a correlated reply clears the gate and drives exactly one more infer, (3) two concurrent questions answered out of order each resolve their own gate — never cross-wired.

Important caveat for reviewers

This PR alone does not let a live answer resolve the gate early. The mail path that would stamp headers.interchangeCorrelationId on the answer (so the reactor's pre-existing tryCorrelate can match it) is CL-7191's stacked PR, not this one — it was flagged during review that landing correlation-plumbing here would blur the two tickets' scopes. Until CL-7191 merges, a parked question is answered by whichever of its own two paths fires first: the user's actual answer (which the current, unfixed packages/chat route still relays as an ordinary message with no correlation, so it won't resolve the gate) or the 1-hour timeout with a synthetic error. That's still a strict improvement over today's silent self-answer — CL-7190's own acceptance criteria explicitly call for this timeout fallback — but it means these two PRs should be reviewed and merged together, not independently.

What changed

  • packages/interaction-tools/src/tool.tsbeforeAskUser (the new beforeToolExtension); run() now fails loud if ever reached for ask_user (naming the exact wiring defect: ResolvedTools.beforeToolExtensions not composed).
  • vendor/intx/agent/src/tool.ts, agent.tsToolBundle.beforeToolExtension (optional), composed into ResolvedTools.beforeToolExtensions alongside authz.
  • vendor/intx/inference/src/reactor.ts — a message_response branch in resumePendingOperation, and timeoutMessageFor so a timed-out question gets its own wording instead of reusing "approval timed out".
  • vendor/intx/types/src/signals.ts"message_response" added to SignalKind.
  • vendor/intx/hub-sessions/src/hub-session-lookups.tsregisterSignalCorrelation now fails loud for any signal kind other than "approval" (traced: ask_user's suspend is fully local via suspendOnGate/gates.register and never reaches this RPC — it's used exclusively by the workflow-deploy/supervisor "ask" capability's park-notify path — so this is inert hardening, not a live path change).
  • VENDORED.md / VENDORED-FROM / scripts/checks/kill-dates.txt — deltas and tree-hash updates for the four touched vendor packages.
  • DESIGN.md — swapped a disclosure example that referenced ask_user's now-removed prose for request_connection's equivalent, still-live one.

Interchange notes

No upstream Interchange defect found or filed. Every mechanism this PR wires into (BeforeToolExtension, PendingOperation, GateType.message_response, the reactor's dispatch loop honoring allow/block/suspend) was already fully present in the vendored vendor/intx/* trees, untouched — confirmed via git show HEAD diffs before making any change. This PR only adds the composition (ToolBundle.beforeToolExtensionResolvedTools.beforeToolExtensions) and the message_response resume branch that nothing had wired up yet.

A separate, narrower architectural note was filed as a follow-up (not a blocker for either PR): CL-7248 — a crash between postQuestion's external POST and suspendOnGate's durable commit can orphan a posted question card on restart. Flagged during review, out of scope for CL-7190/CL-7191.

Test plan

  • cd packages/interaction-tools && bun run typecheck && bun test — 11/11 pass
  • cd vendor/intx/inference && bun run typecheck && bun test — 3/3 pass (new suite)
  • cd vendor/intx/hub-sessions && bun run typecheck && bun test — 44/44 pass
  • cd vendor/intx/agent && bun run typecheck — clean
  • cd vendor/intx/types && bun run typecheck — clean
  • bun run check:killdates, check:tool-package-pins, check:tool-package-freshness, check:licenses — all pass
  • bunx prettier --check on every touched file
  • CI (workspace-wide) — not run locally per instructions; polled centrally

DO NOT MERGE — stacked with CL-7191 (branching from this branch), both intended to land together.

Closes CL-7190.

Re-pin disclosure

vendor/intx/inference/src/reactor.ts carries a real local delta here: git diff --stat origin/main reports 72 insertions(+), 40 deletions(-) (112 changed lines total). At the next vendor/intx/inference re-pin, this will need to be re-applied by hand — concretely:

  • timeoutMessageFor(kind: SignalKind) (new helper) — exhaustive over SignalKind so a timed-out gate gets kind-appropriate wording ("approval timed out" vs "question timed out with no answer") instead of a hardcoded approval-only string.
  • ResumeDispatch's new "answer_result" variant, and resumePendingOperation's restructuring: the single un-conditional ApprovalDecision JSON-parse that used to run before the switch moved inside the "approval" case, with a sibling "message_response" case added that takes the correlated reply's body verbatim (no parse) as the parked call's tool result.
  • The error_result/answer_result cases merged in the dispatch handler that clears the gate silently and lets the director re-infer once.

Verified before writing this: I confirmed there is exactly one call site of timeoutMessageFor (the previous hardcoded "approval timed out" string) and confirmed the outer switch (op.kind) is exhaustive via assertNever, so a future SignalKind addition fails to compile here until it's handled — a re-pin conflict here is a compile error, not a silent gap.

Upstream-worthy? Plausibly yes, in whole or in part — the pattern here (a SignalKind-keyed resume rail, with approval's JSON-decision parse as one case among several rather than baked into the shared path) is generic and not workbench-specific; nothing in this delta depends on anything else in this repo. I'm not filing this upstream myself per instructions — flagging it here for the owner to fold into Interchange's own backlog if they agree it's worth upstreaming.

Covers the suspend/resume mechanism CL-7190 introduces: interaction-tools'
own beforeToolExtension decision shape, and a new reactor-level regression
suite (vendor/intx/inference had zero tests before this) driving a real
createReactor cycle to prove a suspended tool call produces no tool.done
and no re-inference, that a correlated reply clears the gate and drives
exactly one more infer, and that two concurrent questions answered out of
order each resolve their own gate.
…chronously

The director re-inferred as soon as ask_user's tool call resolved, so the
agent regularly answered its own question — the only thing telling it to
wait was prose in the tool result. ask_user now contributes a
beforeToolExtension that posts the question card, then returns a suspend
decision on a message_response gate, reaching the reactor's existing
suspendOnGate — the same mechanism approvals already use. The turn parks
with no tool.done and no re-inference until a correlated reply arrives (or
the gate's explicit timeout fires with a synthetic error result); neither
the reactor nor the director special-cases ask_user by name.

ToolBundle gains an optional beforeToolExtension a bundle can contribute
independent of the tool's name, composed into the reactor's extension list
alongside the authz extension. signals.ts adds "message_response" to
SignalKind. registerSignalCorrelation now fails loud for any signal kind
other than "approval" rather than silently mis-persisting one it has no
co-write for (ask_user's suspend never reaches this RPC — it's fully local
via suspendOnGate/gates.register).
…xample

Records the new local deltas this introduces in vendor/intx/agent,
vendor/intx/inference, vendor/intx/types, and vendor/intx/hub-sessions in
both VENDORED.md and each package's VENDORED-FROM file, with matching
tree-hash updates in scripts/checks/kill-dates.txt. DESIGN.md's disclosure
example referenced ask_user's now-removed "do not repeat this in prose"
instruction; swapped for request_connection's equivalent, still-live one.
@TheGreatAxios
TheGreatAxios merged commit 3d153cd into main Aug 30, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant