Skip to content

feat(opencode): answer a question from the modal, not only reject it - #1068

Merged
Juliusolsson05 merged 4 commits into
mainfrom
feat/opencode-answer-questions
Sep 20, 2026
Merged

Juliusolsson05 merged 4 commits into
mainfrom
feat/opencode-answer-questions

Conversation

@Juliusolsson05

Copy link
Copy Markdown
Owner

Fixes #1025.

The problem

Since #878 the modal showed the agent's question but offered one action — Reject. A user could read "Do you prefer the color red or blue?" and still have no way to answer it.

The option list was never missing

foldQuestion's comment said:

opencode's question payload carries no parsed option list at this seam

That was stale. LiveStateProjector.questionFromPayload sets metadata to the whole raw payload, so questions[].options has been sitting there the entire time — the permission view already reads metadata the same way. That comment is why this shipped reject-only.

Wire contract, verified against the shipped binary

The repo's rule is to confirm against the runtime binary, not the 1.14.39 vendored checkout. Both agree here:

  • strings on out/main/runtime/opencode/darwin-arm64/opencode contains /question/:requestID/reply.
  • The schema annotates answers as "User answers in order of questions (each answer is an array of selected labels)" — positional, labels not indices.
  • The package's own conditions test already records question.replied with answers: [['Yes']].

opencode-headless already had replyQuestion; no package change was needed.

Why the view gains state — and how the boundary survives it

The rule in this subsystem is that the runtime is the single source of truth for which choices exist. A single question fits perfectly: one action per option, one click, done — unchanged.

A multi-question prompt does not. answers is positional and submitted once, so the user must choose per question and then submit, and that is selection state a runtime cannot hold.

Layer Owns
Runtime parses the payload, publishes the options, validates every submitted label against what it published
View composes the selection, renders one block per question

The view may compose a choice; it can never invent one. validateQuestionAnswers refuses an unoffered label, a label borrowed from another question, the wrong arity, and an empty selection for a question that has options — while allowing the empty answer exactly where the provider offered nothing.

Options are filtered out of the modal footer, because the same options render beside their question — unfiltered, every option appeared twice, once with a question attached and once without.

Parsing and validation are pure exported functions rather than private methods, so both are driven directly by tests instead of only when a whole session exists.

Tests

The single-question payload is the real recorded one from packages/opencode-terminal-headless/testing/fixtures/live/question-reject.json, and the first test asserts the recording really carries options so nothing below can pass vacuously. The multi-question case has no recording and is built from the wire schema — labelled as such rather than dressed up as evidence.

Mutation Result
accept any label, offered or not 2 failed
drop the arity check 1 failed
leave the footer unfiltered 3 failed
parser ignores options 1 failed
parser drops header 1 failed

npx tsc -b clean; src/providers/opencode/ green (13 files / 74 tests).

Known limitation

No multi-question stream has been captured, so that path is built from the schema. The wire allows several selected labels per question; nothing observed emits more than one, so the view picks one per question while the payload still carries the array shape.

🤖 Generated with Claude Code

Juliusolsson05 and others added 4 commits September 20, 2026 02:21
The plan records the finding that unblocked this: foldQuestion's comment says
the option list is not available at that seam, but LiveStateProjector sets
metadata to the whole raw payload, so options[] has been there all along.

Also records the wire contract as verified against the SHIPPED 1.18.31 binary
rather than the 1.14.39 vendored source: POST /question/{id}/reply with
positional per-question arrays of selected labels.

Refs #1025

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Since #878 the modal showed the agent's question but offered one action,
Reject — so a user could read "Do you prefer red or blue?" and still not
answer it.

The option list was never actually missing. `foldQuestion`'s comment said
"opencode's question payload carries no parsed option list at this seam", but
`LiveStateProjector.questionFromPayload` sets `metadata` to the WHOLE raw
payload, so `questions[].options` has been there all along. That stale comment
is why this shipped reject-only.

The wire contract is confirmed against the SHIPPED 1.18.31 binary rather than
the 1.14.39 vendored source (the two agree, but the rule is the binary):
`POST /question/{id}/reply` with `answers` — positional, one entry per question
in payload order, each an array of selected option LABELS.

WHY the view gains selection state, when the rule is that the runtime owns the
choices: a multi-question prompt is answered as ONE positional set, so the user
must choose per question and submit once, and that is selection state a runtime
cannot hold. The split keeps the boundary intact — the runtime parses and
publishes the options, the view composes a choice, and the runtime VALIDATES
every submitted label against the options it actually published before
posting. The view may compose a choice; it can never invent one.

A single question keeps the old shape exactly: the runtime builds one action
per option, one click answers, and the view stays dumb for it. Those actions
are filtered out of the modal footer, because the same options render beside
their question — unfiltered, every option appeared twice, once with a question
attached and once without.

Parsing and validation are pure exported functions rather than private
methods, so both are driven by tests directly instead of only when a whole
session exists.

Mutation-checked: accepting an unoffered label fails 2, dropping the arity
check fails 1, an unfiltered footer fails 3, a parser that ignores options or
drops the header each fail 1.

Fixes #1025

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… empty label

Self-review of #1068, found by running the view's own payload through the
runtime validator.

A question OpenCode offers no options for contributed `['']` to the submitted
set. An empty string is a label OpenCode never offered, so the validator
refused the WHOLE set: Submit enabled, the click did nothing, and a
multi-question prompt containing such a question was neither answerable nor —
since the user has no reason to reach for Reject — obviously broken.

It now contributes `[]`, which is what the validator accepts and the only
truthful answer when nothing was offered to choose.

The completeness gate and the payload are both indexed by POSITION rather than
derived by filtering and re-indexing. `answers` is positional, so a list that
loses the original index is exactly how an answer ends up attached to the
wrong question.

Three tests now drive the REAL view's payload through the REAL validator,
because the two halves are written independently and nothing else proves the
modal is not offering a button whose answer is silently refused.

Mutation-checked: restoring `['']` fails 2, indexing the filtered list fails 2.

Refs #1025

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review findings on #1068. Three merge blockers, all of them the same shape:
a question can be REPLACED while the user is mid-answer, and every half of
this flow assumed it could not.

1. THE VIEW KEPT THE PREVIOUS QUESTION'S SELECTION.
   `ConditionOutlet` keys the view on `condition.kind`, and `foldQuestion`
   overwrites the question record in place, so a new question re-renders the
   SAME component instance with the old `picked` map intact. Indexed by
   position, those stale labels line up with the new questions: the submit
   gate read as satisfied and one click answered "Delete the production
   database?" with a Yes the user had given to "Add a changelog entry?".

   The trust boundary structurally cannot catch this — `Yes` IS a label the
   new question offered, so the validator is right to accept it. Only the
   view knows the selection is stale. The selection is now stamped with the
   question it belongs to and read through that stamp. Keying the outlet on
   the question id would also work, but that is shared machinery every
   provider runs through; stamping keeps the blast radius on the one view.

2. THE RUNTIME VALIDATED ONE QUESTION AND REPLIED ABOUT ANOTHER.
   `offered` came from `liveConditions` at resolve time; `questionID` came
   from the action's payload, captured when the action was built. Nothing
   checked they were the same question, and the modal was then deleted
   unconditionally. So an in-flight click across a swap sent the NEW
   question's validated answer to the OLD question's id and tore down the
   new question's modal — the user watched a prompt vanish unanswered while
   a dead question received a reply they never gave it, and the agent
   waiting on the live one was blocked with no UI left to unblock it.

   Both arms now go through `liveQuestion`, which returns the record only
   when the ids match (returning the record, not a boolean, so a caller
   cannot validate against one question and reply about another), and
   `clearResolvedQuestion`, which re-checks after the await. Reject had the
   same unconditional delete before this PR; the guard is shared, so it is
   fixed too.

3. THE RUNTIME HALF HAD NO TESTS.
   Four mutations that deleted it outright left all 77 tests green:
   publishing no per-option actions, skipping the validator entirely,
   replying with a hardcoded wrong label, and never clearing the modal. The
   cause was that the renderer test hand-wrote its own copy of the actions
   `foldQuestion` publishes and asserted against the copy — the "copy that
   drifts" this PR's own comments warn about.

   `opencodeSession.questions.test.ts` drives the REAL session with the REAL
   recorded payload through the real screen event, and derives its
   expectations from the recording rather than restating a literal the
   implementation also holds.

Also from the review:

5. A multi-question set where NO question had options was unanswerable: the
   gate demanded at least one answerable question, so the button read
   "Choose an option for each question" when there was nothing to choose.
   The validator accepts all-empty answers; the gate now does too.

6. The action-name constants claimed "the two halves can never drift", which
   #1025 falsified — the view hardcoded the reply name in two places. They
   moved to `questionAnswers.ts`, the one question module both processes
   import, so the comment is true again.

7. The view header's three stale claims (reject-only questions; a richer
   answerer touching only the runtime; the view rendering whatever arrives)
   are replaced by an honest account of why the multi-question case is the
   documented exception.

Finding 4 (a refused reply is invisible, because `dispatch` discards the
result) is left for a follow-up: it is pre-existing shared conditions-core
behaviour affecting every provider, and with 1 and 2 fixed the silence no
longer hides a wrong answer.

Mutation-checked, full opencode suite (89 tests) per mutation:

  M1 no per-option actions          1 failed    (was green)
  M2 skip the validator             1 failed    (was green)
  M3 reply with a wrong label       2 failed    (was green)
  M4 never clear the modal          2 failed    (was green)
  M5 no id guard on the live q      2 failed
  M6 clear unconditionally          1 failed
  M7 stale selection survives       1 failed
  M8 gate demands an answerable q   1 failed
  restored                          89 passed

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@Juliusolsson05

Copy link
Copy Markdown
Owner Author

Review round addressed in fd5afd7f. All three blockers fixed, plus 5, 6 and 7.

1 and 2 were the same bug seen from two sides — a question can be replaced while the user is mid-answer, and every half of this flow assumed it could not. The view kept the previous question's picked map (the outlet keys on condition.kind, foldQuestion overwrites in place, so it is the same component instance); the runtime validated against the live question and replied to the payload's, then deleted the modal unconditionally. The selection is now stamped with its question and read through the stamp; both runtime arms go through a shared liveQuestion guard, and clearResolvedQuestion re-checks after the await so an in-flight reply cannot tear down a question that arrived meanwhile. Reject had the same unconditional delete before this PR and is fixed by the shared guard.

3 was the finding that mattered most. You were right about the cause: optionActions() hand-wrote a second copy of what foldQuestion publishes, so the renderer suite asserted against the copy. opencodeSession.questions.test.ts now drives the real session with the real recorded payload through the real screen event, and derives its expectations from the recording instead of restating a literal.

Your four green mutations are all red now:

mutation before after
no per-option actions at all 77/77 pass 1 failed
skip validateQuestionAnswers 77/77 pass 1 failed
reply with a hardcoded [['WRONG']] 77/77 pass 2 failed
never clear the modal 77/77 pass 2 failed

And the new guards are covered too: dropping the id comparison fails 2, clearing unconditionally fails 1, letting the stale selection survive fails 1, re-adding the some(needsAnswer) gate fails 1. Restored: 89 passed.

4 I did not fix here — it is pre-existing shared conditions-core behaviour affecting every provider, and with 1 and 2 fixed the silence no longer hides a wrong answer. Filed as #1070 with your analysis.

Thanks — 1 and 2 were genuinely invisible from where I was standing.

@Juliusolsson05
Juliusolsson05 merged commit 4b2fdb7 into main Sep 20, 2026
2 checks passed
@Juliusolsson05
Juliusolsson05 deleted the feat/opencode-answer-questions branch September 20, 2026 10:21
Juliusolsson05 added a commit that referenced this pull request Sep 20, 2026
Four PRs merged (#1068, #1069, #1071, #1072), closing #1025, #952 and #930.
#1073 and #1074 are open with their review findings fixed. #993 stays open
deliberately: the mechanism does not reproduce the reported snapshot.

Records the two reviewer instructions that produced every blocking finding —
mutate the implementation and report the survivors, and count the real corpus
rather than reasoning about it — because three of the four blockers were
regressions this loop had just introduced.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.

feat(opencode): answer OpenCode questions from the modal, not only reject them

1 participant