fix: make @deuce's questions answerable again - #49
Merged
Conversation
Diagnoses why @Deuce's questions are unanswerable: Deuce's ask_user implementation was written against a guessed Pi RPC wire format. All three question styles fail differently, and the drawer scrolls sideways on long rows. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Pi's request type is a flat nine-arm union keyed on method with no nesting. The decoder probed for prompt/kind/params.*, which no arm carries, so every question decoded with an empty prompt -- and a string-valued options field failed to unmarshal, dropping the whole line. That dropped line is the ten-minute hang: awaiting_input never fired, so the active-work timeout was never suspended. - Pin the contract in testdata/pi-ui-protocol.json, transcribed from Pi 0.84.0's published types, so the tests fail when the product does. The previous fixtures asserted the decoder's own guess. - Derive the prompt per method, and recover the question from a bare-string options field rather than falling back to the generic title. - Route the five fire-and-forget methods to ignore, so a subagent's progress notification can no longer wedge a task in "needs your input". - Map editor to the existing input kind; the frontend union has no editor. - Decode and log extension_error at warn from DecodeStream, and name the event type on a dropped line. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Pi's extension_ui_response is a three-arm union -- value for select/input/ editor, confirmed for confirm, cancelled for any. Deuce sent a single "response" key, which Pi's dispatcher accepts, correlates, and then resolves to its parser fallback: undefined for value dialogs and false for confirm. Every answer was discarded, and every yes/no question was answered "no" regardless of what the user clicked. - Express the three arms in ExtensionUIResponse, one arm per message. - Track the dialog's method alongside its request id so the answer path can choose the arm; an awaiting task with no tracked dialog falls through to steer rather than emitting an armless response. - Map yes/no by leading token, case-insensitively, so "yes, go ahead" from the drawer composer is affirmative. Anything matching neither set logs at warn and defaults to false, so an unparsed reply can never read as approval. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Pi's select takes (title, options, opts). The extension passed (title, question, options), so the question landed in the options slot and Pi emitted options as a string -- the line Go could not unmarshal and dropped, which is the ten-minute hang. input passed the question as a placeholder, and confirm buried it behind a constant title. - Put the question in title for select, confirm, and input, so all three styles render as the bare question. - Drop the ui.select/ui.confirm capability probes. hasUI is true in rpc mode and all four dialogs exist there, so both probes were always true and their fallback branches were dead. - Own the no-answer deadline in the extension. Pi resolves a timed-out confirm to false, which is what a real "No" resolves to, so the resolved value cannot distinguish them -- an internal abort flag selects the explicit no-answer result instead. - Pass a 35-minute dialog timeout, above the runtime's 30-minute awaiting ceiling so Deuce's ceiling always fires first, commented at both ends of the cross-language invariant. - Give the extension a type-check and a behavioral suite. It previously sat outside every tsconfig project, so nothing type-checked the file and nothing asserted what its dialog calls emit. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The thread body is overflow-y:auto, which computes overflow-x to auto, so any child wider than the panel scrolls it sideways. The action row's text lived in an unclassed span, and the ellipsis declarations sat on an inline .arg -- where overflow and text-overflow do not apply, so they were inert. - Move truncation onto the flex item itself via a new q-act-txt class, applied to all three action-row branches, and drop the inert declarations from .arg. - Add min-width:0 to the task card's live row, whose .arg is a direct flex child and so does honor its existing ellipsis once it can shrink. - Carry the full text in a title attribute. The store clears pendingQuestion when a task completes, leaving this row as the only record of what was asked. - Wrap, don't truncate, the pending-question block and choice buttons -- a question the user must read to answer cannot be clipped. overflow-wrap: anywhere also reduces the buttons' min-content contribution so they shrink instead of widening the row. Also corrects a stale test-scenario line in the plan that contradicted its own step 2 on where a yes/no question carries its text. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Applying code-review findings. The first is a hole in a guard added earlier on this branch. - Recover a stale extension's free-text question from the placeholder field. The rollout guard covered select, and confirm survived incidentally, but input had no recovery: the pre-fix extension called input(title, question), so Pi put the question in placeholder and the decoder never read it. During the rollout window that rendered the boilerplate title with the question discarded -- the exact failure R2 rejects, reproduced by the guard meant to prevent it. Found independently by three reviewers. - Broaden the yes/no token sets. "yep", "okay", "go ahead" and "do it" were all delivered to the agent as refusals. The fail-to-negative default for genuinely unrecognized replies is deliberate and unchanged. - Tie the cross-language timeout invariant to the contract fixture. It was enforced only by paired comments plus a hand-copied constant in the TS test, so a one-sided edit would silently invert the ordering KTD7 exists to guarantee. A Go test now asserts the ceiling against the fixture and the TS suite reads the same field. - Correct a doc comment claiming RequestKind can be "editor"; the decoder always folds editor to input. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
@deucecan ask you a question again, and your answer reaches it. That path was broken end to end: the agent would ask, the task would sit at "working" until a ten-minute timeout killed it, and there was no way to answer. Yes/no questions were worse than unanswerable — they were delivered to the agent as "no" regardless of which button you clicked.The cause was that Deuce's
ask_userimplementation was written against a guessed Pi RPC wire format, and the tests then asserted that same guess. The suite stayed green while all three question styles were broken in production. The decoder's own source admitted it: "The exact extension_ui_request shape is pinned when the ask-user extension lands; decode best-effort." This branch reads Pi 0.84.0's published types and corrects the extension, the decoding, and the answer shape against them.What was actually broken
Three independent defects, one per layer:
select(title, question, options)against Pi's realselect(title, options, opts)optionsas a string; Go's unmarshal failed and dropped the whole line, soawaiting_inputnever fired and the active-work timeout was never suspended — the ten-minute hangprompt/params.*, keys no Pi arm carriesresponsekeyundefinedfor text,falsefor confirm. Every answer discardedBeyond the reported bug
npm:pi-subagentsis installed in every workspace, so any progress notification it emitted would have wedged that task in "needs your input" until the ceiling killed it. Five of Pi's nine UI methods are fire-and-forget and now decode to ignore.selectandinput.Design decisions worth review
overflow/text-overflowdo not apply to non-replaced inline boxes, so the existing declarations on the argument span were inert — addingmin-width: 0alone would have let the text spill and keep scrolling the thread. Prose the user must read to answer (the pending-question block, the choice buttons) wraps instead of truncating.confirmtofalse, which is exactly what a real "No" produces, so the resolved value can never distinguish them.Known accepted gap
Answering stays in the agent thread drawer. An
@deucemessage sent while a question is pending is enqueued behind the blocked task — the running-task lookup countsawaiting_inputas busy — so it cannot run until the question resolves. A user who answers in the main chat therefore still hits the original symptom. This is accepted here because the fix belongs to the chat surface, not the protocol; a session-notice mitigation is recorded as follow-up.Validation
Go suite (all 13 packages), 97 frontend tests,
tsc -b --force, and lint all pass. Each unit was implemented against a red test observed first — including the exactcannot unmarshal string into Go struct field .options of type []stringfailure that is the reported hang — and the yes/no mapping was mutation-tested to confirm a whole-string match fails the "yes, go ahead" case.Live verification against a real Pi is still outstanding and a green suite does not substitute for it: the suite proves Deuce agrees with the fixture, not that Deuce agrees with Pi. Rebuild the workspace (not restart — the extension is baked into the prebuild image), then confirm each question style end to end and that clicking No is received as a negative.
Residual review findings, deferred items, and the full verification checklist are recorded in
docs/residual-review-findings/fix-ask-user-question-protocol.md.Session-settled decisions carried from planning: repair the hand-rolled ask-user extension rather than adopting
npm:pi-ask-user(user-approved, over adopting the published package).New concepts
Counterparty-derived contract fixtures. When you integrate with a process you don't own over an informally-typed wire (JSONL over stdio, a CLI, an exec channel), the tempting move is to hand-write a decoder and then write fixtures beside it asserting what you expect to see. Those fixtures can only ever confirm the decoder's own assumptions — they cannot catch them. That is precisely how this bug shipped green for all three question styles.
The fix is to derive the fixture from the other side's published contract and check it in.
server/internal/agent/pirun/testdata/pi-ui-protocol.jsontranscribes all nine arms of Pi's request union and all three arms of its response union from@earendil-works/pi-coding-agent's own.d.tsdeclarations, records the version it came from, and is read by both the Go decoder tests and the TypeScript extension suite. A completeness test fails if the fixture grows an arm nothing asserts.Why here rather than the obvious alternative: a recorded golden stream (which this repo already has for the general event flow) only captures shapes that actually occurred during capture —
extension_ui_requestnever appeared in it, which is why the gap went unnoticed. Published types cover arms you have not exercised yet.When not to use it: if the counterparty publishes no types, docs, or readable source, a transcribed fixture is just a differently-located guess — record a real captured stream instead and say so. And a transcribed fixture is diffed against itself, not against a live counterparty, so it needs re-verifying on their upgrades.