Decision card: shared Add an option - #233
Open
MaggieAppleton wants to merge 6 commits into
Open
MaggieAppleton wants to merge 6 commits into
MaggieAppleton wants to merge 6 commits into
Conversation
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>
Collaborator
Author
|
Independent review (security/correctness lens). Fixed (c1c13ae)
Verified, no change needed
Leftovers (pre-existing pattern, not changed)
Checks: bun test 1755 pass/0 fail; types, ci, fix clean; Postgres 59 pass; e2e fixtures (sidecar, responsive-decisions, responsive-content) 52 pass. |
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.
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)question:option{ id, question, key, label, description? }.idis the questionnaire,keya client-minted idempotency token.question:option:{ ok: true, option, definition, repeated? }or{ ok: false, reason: invalid | duplicate | full | resolved | resolving | implementation, message }.question:option-added{ id, question, option, definition, by }. It carries the whole definition, so a late or duplicated event is harmless.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 asopenQuestions), and theOptionin 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 viadurable.fatal). I did not copy thestage()draft-removal pattern: the draft is never touched here.Idempotency.
Record.appended[key] = optionIdlives in the sidecar, so a retry after a lost ack or a restart returns the same option withrepeated: trueand appends nothing. The option id is a server-minted ULID (the dialect requires ULIDs, and@chopin/questioncannot 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 setschoice). 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,resolvingorimplementation; 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 understandcustom. 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
Calloutin the question view, used by both "Couldn't save" and "Couldn't add option" (same research-card styles).<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.AGENTS.mdand the dialect node comment, which described definitions as immutable.question-view.tsxandquestionnaire.tsxhashes; the callout's dynamic class exception moved toCallout > <div>.Before / after (2x)
Checks
bun run fix,bun run ci,bun run types: pass.bun test: 1752 pass, 2 skip, 0 fail.bun run test:postgresequivalent on a disposable container: 59 pass, 0 fail.--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.appendOptionand 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