Skip to content

Decision card: shared Add an option - #233

Open
MaggieAppleton wants to merge 6 commits into
design/decision-card-restylefrom
design/decision-card-shared-options
Open

MaggieAppleton wants to merge 6 commits into
design/decision-card-restylefrom
design/decision-card-shared-options

Conversation

@MaggieAppleton

@MaggieAppleton MaggieAppleton commented Sep 30, 2026 •

Copy link
Copy Markdown
Collaborator

Stacked on #231 (base design/decision-card-restyle). Anyone with write access can append an option to an open decision; everyone sees it, and it is durable.

Design note

Messages (declared first in packages/protocol/question.d.ts)

  • Client to server question:option { id, question, key, label, description? }. id is the questionnaire, key a client-minted idempotency token.
  • Reply question:option: { ok: true, option, definition, repeated? } or { ok: false, reason: invalid | duplicate | full | resolved | resolving | implementation, message }.
  • Broadcast question:option-added { id, question, option, definition, by }. It carries the whole definition, so a late or duplicated event is harmless.
  • Write access is enforced by the existing socket gate (not in VIEWER_ALLOWED).

Validation (appendOption() in @chopin/question, pure and unit tested): trimmed label 1-200 chars, optional description at most 1000, question must exist, at most 10 options in total, no case-insensitive duplicate label, key must be 8-64 of [A-Za-z0-9_-]. The definition stays frozen: an append returns a new frozen definition.

Authority and ordering. The definition already lived in three places, and all three change together inside the plan's exclusive queue: the sidecar Record.definition, the open entry in the question store (persisted as openQuestions), and the Option in the plan's Questionnaire node. Order: refuse if implementation is active, the record is not open, or a submit/cancel claim is held; compute the new definition; stage the plan mutation; update record and store; Service.publish (fenced commit of the Yjs update and sidecar in one transaction); only then ack and broadcast. If the commit throws, the in-memory definition and record are restored before rethrowing (a commit failure already takes the room down via durable.fatal). I did not copy the stage() draft-removal pattern: the draft is never touched here.

Idempotency. Record.appended[key] = optionId lives in the sidecar, so a retry after a lost ack or a restart returns the same option with repeated: true and appends nothing. The option id is a server-minted ULID (the dialect requires ULIDs, and @chopin/question cannot mint them). The client reuses the same key when the same text is retried.

Open drafts. read() in the draft CRDT no longer requires a register for every option: keys must belong to the definition, a missing key reads as unselected, and a client selecting a new option creates its register (or sets choice). No draft model is rewritten, no patch is synthesised, and existing persisted models stay valid. Unknown keys are still rejected. Clients replace their definition from the ack or the broadcast, only ever forward.

Planner. It reads the plan source, which now contains the new Option; the plan revision advances because the source changed, like any other plan change. The protected-projection check compares against the live source, so it stays consistent. Nothing else needed changing.

Resolved, cancelled, resolving, implementation. Refused with resolved, resolving or implementation; nothing is mutated. An already-submitted or cancelled questionnaire is simply gone from the open store, so a late add is told it was decided.

Backward compatibility for custom answers. Nothing new can enter free-text mode (the row now adds a shared option). derive, read, answered, and resolved rendering still understand custom. An open draft already in custom mode renders its text as a selected legacy row between the options and the add row; choosing any option leaves it. Resolved cards with custom answers render exactly as before.

Decisions made

  • The adder's own draft does not select the new option. The draft is one shared decision. Auto-selecting would overwrite a choice someone else had just made in a single-choice question, and "adding a choice" and "choosing it" are different acts. The option appears as the next lettered row, unselected, for everyone.
  • Cap of 10 total options (spec), enforced on append only. A question the Planner created with 11-20 options keeps them but cannot grow. The row is hidden at the cap.
  • Blur behaviour: an empty field collapses on blur; typed text is kept (adding is visible to everyone, so it should be deliberate; the jig's add-on-blur would publish accidental text).
  • Client-visible pending state: Enter shows the new lettered row dimmed (with an sr-only "Adding option" status) in place of the field; on reject the field reopens with the text intact and a "Couldn't add option" callout. If the broadcast beats the ack, the real row is shown and the pending one is dropped.
  • Callout: extracted one Callout in the question view, used by both "Couldn't save" and "Couldn't add option" (same research-card styles).
  • Add row is now a real <button> ("Add an option"), and the field is a single-line input named "New option" (the textarea and the radio/checkbox trick are gone). Focus returns to the row after Enter or Escape.
  • Viewers without write access see the row disabled, not hidden, so the card keeps the same shape.
  • Description is not offered in the UI (the design has none); the protocol and validation accept it.
  • Persistence is the sidecar JSON, so no schema or storage-adapter change. I still ran the PostgreSQL suite; there is no new contract test because no storage behaviour changed.
  • Looser draft validation is the trade for not rewriting drafts: a patch that removes an option register is now valid (it reads as unselected). Unknown keys and wrong types are still rejected.
  • Updated AGENTS.md and the dialect node comment, which described definitions as immutable.
  • Design-contract: renewed question-view.tsx and questionnaire.tsx hashes; the callout's dynamic class exception moved to Callout > <div>.

Before / after (2x)

Before (#231) After
Add row
Field open (free-text custom answer, selected for the adder only)
Pending n/a
Seen by a second member n/a
Refused (duplicate) n/a

Checks

  • bun run fix, bun run ci, bun run types: pass.
  • bun test: 1752 pass, 2 skip, 0 fail.
  • bun run test:postgres equivalent on a disposable container: 59 pass, 0 fail.
  • E2E --project=fixtures e2e/sidecar.e2e.ts e2e/responsive-decisions.e2e.ts: 42 pass, including a two-browser round trip (ana adds, bo sees it, reloads, chooses and saves it) and a refused duplicate.
  • New unit tests: appendOption and draft tolerance (question.test.ts), controller behaviour (shared-options.test.ts), view rendering, and server service tests for durability-before-publication, idempotent retry, refusals (duplicate, bounds, implementation, resolving, resolved, full) and dump/restore.

🤖 Generated with Claude Code

MaggieAppleton and others added 6 commits September 30, 2026 21:46
Declares question:option in the protocol, relaxes the frozen definition to
appending while open, and commits record, draft store and plan projection
together before acknowledging or broadcasting.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
The add row is now a button that opens a focused field: Enter adds for the
whole room, Escape cancels, the new row shows dimmed while pending and a
rejection reopens the field with an error callout. Existing custom drafts
still render as a selected row.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…cknowledgement

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
The active-implementation check ran before the exclusive queue, so a claim taken
while the append waited let the live document mutate and then fail in publish,
leaving the room ahead of durable state. Re-check inside the queue, and add
tests for concurrent appends, the queued-claim window and commit-failure rollback.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@MaggieAppleton

Copy link
Copy Markdown
Collaborator Author

Independent review (security/correctness lens).

Fixed (c1c13ae)

  • Medium: addOption checked implementationActive only before the exclusive queue. A claim taken while the append waited let the live Yjs/Lexical doc mutate, then publish threw "implementation is active", leaving the room ahead of durable state (record/store rolled back, doc not). Now re-checked inside the queue before any mutation.
  • Tests added: concurrent same-key and same-label appends (one option each), queued-claim window leaves document/record untouched, commit failure restores record and open definition and nothing is published.

Verified, no change needed

  • Persistence precedes ack/broadcast; commit failure goes through durable.fatal (room rebuilt), and in-memory record/store rollback is correct. The plan doc is not rolled back on a fatal commit failure, which is consistent with other paths since the room is discarded.
  • Authorization: the socket gate (not in VIEWER_ALLOWED) rechecks access and canEdit; MCP path untouched.
  • appended is bounded by the 10-option cap (keys only recorded on success, key regex-validated); label/description bounds and question-id existence enforced in appendOption.
  • Draft loosening: unknown keys still rejected; a patch can only mark an option unselected, which any writer could already do. No cross-user corruption path.
  • UI: tokens only, real button/named input, pending/error/read-only states OK.

Leftovers (pre-existing pattern, not changed)

  • submit/cancel have the same pre-queue implementation check.
  • Restore does not validate appended shape (trusted durable data; lookups are hasOwn-guarded).

Checks: bun test 1755 pass/0 fail; types, ci, fix clean; Postgres 59 pass; e2e fixtures (sidecar, responsive-decisions, responsive-content) 52 pass.

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