fix(ui): carry in-progress ask answers across a stream push

[mattwilkinsonn]

Stacked on compass#31 (compass-server-port-ask), which puts Ask.answered on the wire. This is the consumer half: the UI fix that could not land without it.

The bug

adoptComms replaces comms state wholesale on every SubscribeComms push. The ask wire is atomic — one RespondToAsk per ask, issued only on the click that completes it — so on a multi-question ask every choice but the last lives only in client state. A snapshot or tail event landing mid-ask discarded the user's clicks silently, and the server could not send them back: it was never told.

preserveLocalAsks carries an unsubmitted, partially-answered ask across a push where the server has no answer of its own, keeping a server value is authoritative exactly true.

Why this was held, and what changed

The first commit's predicate approximated "the server has no value" by scanning every question for empty chosenOptionIds. Two answer shapes the server accepts and records defeat that scan, both leaving a closed ask with no chosen ids anywhere:

1. A deliberate skip. An answer entry with no chosen ids and empty custom_text is an accepted skip that satisfies the wire's coverage-of-every-question contract.
2. A custom_text-only answer to a free-text question, which carries no options to choose.

Against either, the scan reported "no authoritative value" for an ask the server had already closed, restored stale local picks over it, and left the UI offering a completing click the server refuses with ErrConflict — trading one silent-loss bug for a narrower one.

Ask.answered is the authority. The server flips it exactly once, in the same path that records the chosen ids:

go/internal/store/messages.go:435   q.ChosenOptionIDs = a.ChosenOptionIDs
go/internal/store/messages.go:438   ask.Answered = true

Same code path, so a chosen id cannot exist without the flag — which is what makes the flag strictly stronger than any scan of the questions.

One non-obvious detail: only one of three call sites uses the flag

The old predicate had three callers, and routing them all through answered would have been a bug. The client never sets answered on an ask it builds, so it is false on every local record — a server-flag test there would skip every ask and disable the carry entirely.

preserveLocalAsks, pushed ask   -> !ask.answered          server authority
preserveLocalAsks, local prev   -> chosen-ids scan        "is there an unshipped edit"
submitAsk, "nothing staged"     -> chosen-ids scan        "is anything staged to send"

The two scans are the right question at their sites — they ask about the local record, not about server authority — and each carries a comment saying so.

Verification

moon run compass-ui:test    549 pass / 0 fail / 32 files   (546 before, +3)
moon run :ci                35 completed, 0 failed
tsc --noEmit -p apps/ui     clean

Mutation-checked rather than asserted. Reverting only the predicate to the old chosen-ids scan, keeping the new tests:

(fail) a fully-skipped answered ask beats an unsubmitted local one
(fail) a custom-text-only answered ask beats an unsubmitted local one
547 pass, 2 fail

Exactly the two defeating shapes go red and every pre-existing test stays green. The third new case (answered: false, genuinely pending) stays green under both predicates — correctly: it guards the original bug, a case the old scan got right, so it is a regression guard rather than a test of this change. Stating that explicitly because a new test that passes under the old predicate is not evidence for the new one.

Fixture fidelity

wireAskMessage now defaults answered to whether chosen recorded anything. Swapping the predicate initially reddened a pre-existing test that pushed a recorded chosenOptionIds while leaving answered false — a state the real server cannot hold, per messages.go:435-438 above. The fixture was the unfaithful party, so the invariant is now encoded once in the builder instead of in each test, and no existing test's body changed.

Not fixed here

submittedAskIds is a createSignal<Set<string>> — pure client memory. After a reload it is empty, so an already-answered ask renders answerable and the click fires a respond the server refuses. Same defect class, different surface, and answered is what fixes it — but it is a separate change and this PR does not make it.

Refs SEA-1533

Co-Authored-By: seal noreply@sealedsecurity.com

---

Update: option-revision authority + accepted-then-lost-reply guard

Two follow-up commits extend the same authority principle to the block-update path and the rollback site:

• sameQuestions now compares the offered option ids, in order, not just question ids. The block-update RPC (go/internal/store/messages.go:151, :163-167) rewrites a message's whole block set requiring only that ask_id survive, so an option id can appear, vanish, or be replaced under a stable question id. Comparing question ids alone would leave the UI rendering a withdrawn option and shipping an id the server no longer offers (validateQuestionAnswer → ErrInvalidArgument). This path is designed-for, not yet wired — nothing calls the block-update RPC today.
• The .catch rollback in sendAsk now guards on !current.answered, so an accepted-then-lost-reply race (the server accepted our respond, then the reply was lost and the promise rejected) cannot roll a server-closed ask back open.

Two new mutation-checked tests in store.ask-race.test.ts (a refusal after the server accepted does not reopen the closed ask; a pushed ask that revised its options beats the local pick) — each reddens on the reverted predicate.

Verification (current head): moon run compass-ui:test → 557 pass / 0 fail / 32 files; typecheck + biome clean; pre-push moon ci green (31 tasks).

Open questions (parked for Matt — overnight)

The sole-review pass returned 0 high / 0 medium; three lows, all on the unwired block-update path, all failing in the safe direction (never renders a withdrawn option, never ships an invalid id). They pose one design decision, deferred rather than decided by proxy:

1. preserveLocalAsks merge strategy on a block-update push. When sameQuestions matches, the function currently carries the whole local ask forward (return { kind: "ask", ask: mine }). That preserves the in-progress pick but discards any server revision to a non-id field (question text, option labels, allowMultiple) under stable ids. Two options for when the path is wired live:
   • (a) Overlay — map the pushed question objects and copy chosenOptionIds from the matching local question by questionId. Adopts every server-revised field while keeping the pick. Reviewer's recommendation; my lean too, since it keeps "a server value is authoritative" true for non-id fields as well.
   • (b) Tighten the compare — add question text + allowMultiple to sameQuestions so any such revision is treated as a shape change (adopt-pushed, drop the local pick). Simpler, but drops the pick on a purely cosmetic edit.
2. Option equality: index-order vs set-membership. Options compare by index today, so a pure reorder of an unchanged set is judged a shape change and drops the pick. Set-membership would preserve the pick across a benign reorder; index-order is defensible if position is a meaningful identity contract. Coupled to (1).
3. Fixture gap. comms-fake.ts wireAskMessage derives each option label from its id, so a relabeled-same-id option is unrepresentable — the one wire shape that would exercise the label-revision case in (1) is untestable by construction. Add an optionLabels override + a label-revision test alongside the block-update wiring.

None of these is a live bug on the current wire; the doc-comment now states the boundary honestly (commit 64b062c). Filed here so the decision lands with the block-update wiring work, not silently in preserveLocalAsks.

[mattwilkinsonn]

• fix(ui): carry in-progress ask answers across a stream push #34 👈 (View in Graphite)
• feat(compass): SEA-1519 make ask answer state server-owned on the wire #31
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[linear-code]

SEA-1533

[greptile-apps]

Greptile Summary

This PR preserves locally staged ask choices across stream updates while respecting the server-owned answered state.

• Adds the answered field to the UI ask model and wire adaptation.
• Prevents store and UI interactions with server-closed asks.
• Protects accepted responses from rollback when the RPC reply is lost.
• Adds race, reconciliation, and closed-ask interaction coverage.

Confidence Score: 4/5

The PR appears safe to merge, with only the previously reported metadata-preservation limitation remaining on an currently unwired update path.

The server-answered gates now resolve the previously reported closed-ask editing failure, while the existing preservation logic still retains complete local ask metadata when only non-ID fields change; the diff documents that the update path capable of producing that state is not currently wired.

Files Needing Attention: apps/ui/src/store.ts

Important Files Changed

Filename	Overview
apps/ui/src/store.ts	Adds ask-state preservation, server-closure write guards, shape-aware reconciliation, and rollback protection.
apps/ui/src/components/ChannelView.tsx	Locks controls and removes the submit affordance when an ask is locally submitted or server-answered.
apps/ui/src/live/adapt.ts	Maps the server-owned answered flag into the UI ask model.
apps/ui/src/store.ask-race.test.ts	Covers stream-push preservation, authoritative closure, shape changes, and accepted-response rollback races.
apps/ui/src/live/comms-fake.ts	Extends ask fixtures to model answered, free-text, and revised-option wire states faithfully.

_{Reviews (4): Last reviewed commit: "docs(ui): make the sameQuestions comment..." | Re-trigger Greptile}

[greptile-apps]

Ask metadata remains stale

When a message update retains the question and option IDs but changes question text, allowMultiple, or an option label while a local answer is staged, sameQuestions treats the asks as equivalent and preservation restores the complete old ask. The UI consequently shows stale text or labels or applies obsolete single/multi-select behavior.

Suggested change

	const sameQuestions = (a: Ask, b: Ask) =>
	a.questions.length === b.questions.length &&
	a.questions.every((q, i) => {
	const other = b.questions[i];
	return (
	other !== undefined &&
	q.questionId === other.questionId &&
	q.options.length === other.options.length &&
	q.options.every((o, j) => o.id === other.options[j]?.id)
	);
	});
	const sameQuestions = (a: Ask, b: Ask) =>
	a.questions.length === b.questions.length &&
	a.questions.every((q, i) => {
	const other = b.questions[i];
	return (
	other !== undefined &&
	q.questionId === other.questionId &&
	q.question === other.question &&
	q.allowMultiple === other.allowMultiple &&
	q.options.length === other.options.length &&
	q.options.every(
	(o, j) =>
	o.id === other.options[j]?.id &&
	o.label === other.options[j]?.label,
	)
	);
	});