Decision card: shared Add an option #233
- MA
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](https://claude.com/claude-code)
- MA
Independent review (security/correctness lens).
**Fixed (c1c13aea)** - 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.
MaggieAppletonautomatic-base-change-succeededlast month on GitHub
MaggieAppletonhead-ref-force-pushed 2802d2ffdd11last month on GitHub
MaggieAppletonhead-ref-force-pushed ca6f68adb300last month on GitHub
MaggieAppletonmerged c93fd6f75229last month on GitHub
MaggieAppletonhead-ref-deletedlast month on GitHub