Repository navigation
Sales: an abandoned attempt's success hijacks the dialog session that replaced it #477
Description
Follow-up to #474 / PR #476, which merged while the third review round was still open. Two findings arrived against that PR's head and are not in main:
Part 1 shipped in PR #478 (
d157a97b):SalesPagenow keeps the page's failures and the open dialog's in separate state, so neither can overwrite the other. This issue stays open for part 2, which is untouched. Applying the same split to the other dialog screens is #479.
Part 1, for the record (fixed)
1. A background read wipes the open dialog's own message (codex P2)
SalesPage keeps one error slot shared by every action. A confirmed order's listOrderPayments read does not set busy, so the user can open New order and submit while it is still pending. If createOrder rejects first, its actionable 422 shows in the dialog — and then the later payment-read rejection overwrites the same slot, so the form's own explanation disappears from the still-open modal, replaced by a message about a different order.
Fix (already written, see the PR that closes this): split the state in two. error is the page's, untagged; dialogError is whichever dialog is open (they are modal, so at most one ever is, and each clears it on open). run() routes by a DIALOG_SCOPES list. Neither slot can overwrite the other, and each attempt clears only its own — so trying something new never silently drops a failure the user has not dealt with.
This also removes code: shownInDialog goes, the page copy is unconditional again, the scope tag on the page error goes, and the dismissal no longer has to clear a message.
2. An abandoned attempt's SUCCESS hijacks the session that replaced it (still open)
Neither dialog trigger is gated on busy, so this sequence is reachable:
- Open New order, pick customer A, submit → the POST is in flight.
- Cancel. The dialog closes; the request is still out.
- Open New order again, pick customer B, start filling in. (Submit is disabled — still
busy— but the form is live.) - Step 1's POST succeeds. Its success path runs unconditionally:
setActive(await getOrder(created.id))swaps the order panel to customer A's new order, andsetCreatingOrder(false)force-closes the dialog customer B's data was being typed into, discarding it.
record-payment has the milder version: a stray "Payment recorded" message and a force-closed dialog.
The abandoned-attempt marker added for #474 covers only the failure path — it suppresses a stale error, not a stale success's side effects. Fixing this properly needs a per-session generation (an attempt id captured when the dialog opens, compared before the success path touches shared state), which is more than the error-rendering fix should carry.
Pre-existing on main; not introduced by #476. Found by an internal reviewer during that PR's third round.
Activity
docs(web): say in Help where a failure message appears (#477)
docs(web): narrow the Help line to what the whole app actually does (#…
fix(web): give the sales dialogs their own error slot (#477) (#478)
[-]Sales: one error slot lets a background read wipe an open dialog's message, and an abandoned attempt's success hijacks the next one[/-][+]Sales: an abandoned attempt's success hijacks the dialog session that replaced it[/+]mforce commentedon Aug 9, 2026
Part 1 shipped in #478, merged as
d157a97b. The state is now split:erroris the page's,dialogErroris whichever dialog is open, andrun()routes a failure by scope, so a background read can no longer replace the message a user is reading in an open form.Part 2 — an abandoned attempt's success force-closing and re-navigating a session the user reopened — is untouched and keeps this issue open. It needs a per-session generation (an attempt id captured when the dialog opens, compared before the success path touches
activeor closes the dialog), which is why it was not folded into an error-rendering fix.feat(web): a per-place error store, with Sales as its first consumer (#…
docs(web): widen the Help line #478 narrowed, now that it is true (#479…
fix(web): release the order key on write success, and keep the paymen…
mforce commentedon Sep 6, 2026
Part 2 fixed and merged in #702 — squash commit
522c699e6e424055c93f69b468c9bff00722f680.What shipped.
useDialogSession— a per-scope monotonic generation. A session begins when a dialog opens and when it is dismissed; an attempt claims the current one before its firstawait; anything writing state the on-screen session owns is gated on that claim still being current. The ruleusePagedListalready applies to reads, applied to dialog sessions.What a superseded success must STILL do turned out to be the hard half, and two of the five defects found in review were there rather than in the gating:
clearKeyis not gated.keysholds one entry per scope until cleared, and it now rotates the moment the write lands — the rule the payment path already stated (F21: Customer payments + balances — order-attached, outstanding per order/customer, admin-only (#89) #90). Releasing it after the follow-up read instead stranded a spent key whenever that read failed, and the next order replayed the first, so the customer the user actually chose never got one.Acceptance criteria deliberately NOT met, so nobody assumes otherwise:
runcallers (add-item, update, remove, confirm, void) are permanently "current" by design, so the same stale-success shape survives there — e.g. start Add line, close the panel, let the request land, and the panel reopens.useDialogSessionfails open on a mistyped scope.begin("create")withclaim("create-order")never invalidates. No current path makes that mistake; it is a deliberate default so a screen that has not adoptedbeginbehaves exactly as before, rather than silently gating every success off. It is a real sharp edge if the hook is rolled out more widely.All three are recorded here rather than filed, by owner decision.
Verification: 2528 → 2540 tests. Every new guard mutation-checked — nine mutants, each reddening only the test it names, plus a green control. Three independent review seats; five defects found, all in the fix rather than the original bug.
refactor(web): extract SalesPage's dialog-write wrapper into a shared…