Repository navigation
fix(conditions): tell the user when an answer was refused - #1099
Conversation
Closes #1070. `dispatch` did `await resolveCustom(action)` and discarded the answer, and every view calls `void dispatch(action)`. So a resolver that refused — `{ ok: false, reason: 'option-not-found' }` — produced no toast, no log and no state change: the button did nothing, and nothing anywhere said why. A resolver that THREW was worse than silent, because `void dispatch(...)` turned it into an unhandled rejection as well. It became a path ordinary users hit when #1025 added the answer validator — correctly, since that is the trust boundary stopping a renderer from answering with a label the provider never offered. Click an option for a question the agent has just replaced and the click is refused, silently. The dispatcher now reports instead of deciding. It cannot show anything itself — it is shared by the app and the phone and must not know about either — so the caller passes a reporter and picks the surface, and `describeConditionRefusal` lives beside it so both surfaces say the same thing about the same refusal. In the app that surface is the PANE toast, which is where `sendConditionKey` already puts a key it could not write, and it appears beside the condition the user just clicked. The phone replaces its `failedAtStep ?? 'Action failed'` — an internal step name, absent for most reasons — with the same description. Three deliberate choices, each with its reason in the code: - An unrecognised answer shape is SUCCESS, not a refusal. A resolver that returns something new must not make every click look refused. - The pty arm is not reported here: `sendConditionKey` already reads main's boolean and shows a pane toast, and reporting twice would double the message for the one arm that was never silent. - `dispatch` never rejects any more, including when no resolver was wired at all — the `void dispatch(action)` callers assume exactly that. `failedAtStep` is carried into the message rather than thrown away with the rest of the answer. Mutations, all killed: discard the answer again 2 failed; a thrown resolver rejects again 2; an unrecognised answer counts as a refusal 1; failedAtStep dropped from the message 1; the outlet stops passing the reporter 1; the pty arm reports too 1. That fifth one survived until the suite had a case that renders the OUTLET rather than building the dispatcher itself — the reporter can be threaded correctly everywhere and still not be passed at the one place that matters. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…the wiring Round-one review findings on #1099. - The phone rebuilt every remote refusal as `aborted` with the internal token pasted into `failedAtStep`, so four of the five messages were unreachable there and the fifth printed the thing this fix exists to remove. The reason now travels as itself from `RemoteServer.applyPermissionReply`; older hosts that send only the flattened `error` still land on `aborted`. - The AskUserQuestion row rendered "Answer failed: option-not-found at select-option" verbatim — a log line shown to a person. It goes through the same describer the toast does, so the row and the toast now say the identical thing about the identical refusal. - A swallowed throw left no trace at all: the view form does not rethrow, so the only record of an IPC failure was a toast with no cause attached. The reporter gets the user-facing half; the console gets the error. - docs/design/conditions-system.md still said the custom arm throws when a surface passes no resolver. It does on the control plane and does not in a view, and that split is load-bearing — it is now written down. - The phone dispatch moved out of SessionView into its own module. It was reachable only by mounting the entire phone screen, so nothing tested the phone half of "a refused answer must say why". - TileLeaf's reporter was untested because the one TileLeaf test mocks the outlet away to `() => null`. A probe outlet hands the real reporter back, and the toast surface, z-index, wording and 6000ms hold are asserted against the real GlobalToast. Eight mutations measured one at a time against a clean tree: describer bypassed (4/4 fail), duration dropped (1/4), phone reason discarded (3/6), phone clear removed (3/6), pty fallback removed (1/6), row prints the token again (2/4), journal loses the cause (1/32), unrecognised returns blank (1/6). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Review round one — all eight findings addressed in 1. Control-plane refusals were swallowed ( 2. Unknown provider reasons returned 3. 4. The phone reached one of five messages and printed the token while doing it. Fixed where you said it belonged: 5. A swallowed throw left no trace. 6. Pane toast under the Radix scrim. Already moved to 7. 8. Surviving mutations M18/M19/M20 — you were right that nothing tested the wiring.
Eight mutations, each applied individually against a verified-clean tree (a batched harness reported a live mutation as dead earlier in this series, so they are measured one at a time now):
🤖 Generated with Claude Code |
…erge Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Closes #1070. Found while reviewing #1068; filed rather than fixed there because the blast radius is all of
conditions-core.What was wrong
src/shared/conditions-core/dispatch.tsdidawait resolveCustom(action)and discarded the result, and every view callsvoid dispatch(action). So{ ok: false, reason: 'option-not-found' | 'invalid-payload' | … }produced no toast, no log and no state change — the button simply did nothing.A resolver that threw was worse than silent:
void dispatch(...)made it an unhandled promise rejection on top of the invisible failure.It became a path ordinary users hit when #1025 added the answer validator — correctly, since that is the trust boundary that stops a renderer answering with a label the provider never offered. Click an option for a question the agent has just replaced, and the click is refused, silently.
The fix
The dispatcher reports instead of deciding. It cannot show anything itself — it is shared by the app and the phone and must not know about either — so the caller passes a reporter and chooses the surface, and
describeConditionRefusallives beside it so both surfaces say the same thing about the same refusal.sendConditionKeyalready puts a key it could not write, and which appears beside the condition the user just clicked.failedAtStep ?? 'Action failed — it may have expired.'— an internal step name that is absent for most reasons — with the same description.Each message says what happened and what to do: "That answer no longer matches what the agent is asking. Read the question again and answer it." Three pairs of reasons share wording on purpose, because they are the same thing to the person reading them.
Three choices worth stating, each with its reason in the code:
sendConditionKeyalready reads main's boolean and shows a pane toast; reporting twice would double the message for the one arm that was never silent.dispatchnever rejects, including when no resolver was wired at all — which is exactly what thevoid dispatch(action)callers assume.failedAtStepis carried into the message rather than thrown away with the rest.Tests
16 cases: the answer reader (including "an unrecognised shape is not a refusal"), the dispatcher's reporting and its no-reject contract, the pty carve-out, every reason mapping to something actionable, and two end-to-end cases — one through the real OpenCode question view, one through
ProviderConditionOutlet.Mutations, all killed:
failedAtStepdropped from the messageThe fifth survived until the suite had a case that renders the outlet rather than building the dispatcher itself: the reporter can be threaded correctly everywhere and still not be passed at the one place that matters.
npx tsc -bclean;src/providers+src/shared+src/renderer/src/workspace282 files / 1980 tests pass.🤖 Generated with Claude Code