stack/agents/darkwing/work/queue-155/followup-brief.md

Follow-up row brief: Settings page reads and snapshots after Q169 (row 155 review notes)

Draft by Darkwing for Sage (sage-0899), from Mos's two public/bus.js notes on row 155, which Mos (mos-0816) and Filbert (#1649 comment 29010) ruled non-blocking, plus a third instance I found while checking them, and Dewey's F1 (#1649 comment 29012, sage-0903; withdrawn in 29014). Line numbers are packages/webui/src/public/bus.js in row 155's candidate (manifest 949873cf…, base 7ce156259). This row starts after row 155 lands and rebases on that landing.

Suggested gate: Filbert approves the candidate manifest; Mos's decision 123 go on the public/ diff; webui suite and every scripts/test-*.sh green on Sage's gate. Suggested owner: Darkwing, reviewer Filbert, gate owner Sage.

Neither item loses data today. (a) shows an empty name field where it should show the sign-in state. (b) makes the next save fail with revision-conflict, which keeps the draft and tells the human to reread. The fix makes the page match the other reads and stop sending a revision it already knows is old.

Items

1. (a) The names read rethrows like the others. settingsView (:674) reads usage and limits with a catch that rethrows a 403 or signed-out and keeps any other failure local. The names read at :678 catches everything, so a signed-out or forbidden answer becomes "the name could not be read". Give it the same catch: rethrow 403 and signed-out, and keep the empty field and its text for any other failure. Test (webui settings-form or assistant): a fixture whose names read answers 403 gets the same page as a 403 on usage, and a fixture whose names read fails another way still renders the Assistant section with "could not be read".

2. (b) One settings snapshot. The page holds the launch settings twice: settingsSnapshot (:559) for the launch form and the token-limit reset, and assistantSnapshot (:627) for the Assistant section. Each save refreshes only its own:

   • the launch form save (:587) sets settingsSnapshot, not assistantSnapshot;
   • the assistant save (:666) sets assistantSnapshot, not settingsSnapshot;
   • row 125's reset (:609) sets settingsSnapshot when no launch draft is open, never assistantSnapshot.

   So a save in one form while the other has an open draft leaves the other with the old revision, and its next save is refused. The render paths re-snapshot only when their own form has no draft (:563, :631), which is what keeps the stale copy alive.

   Fix: one snapshot for both forms. Every successful save or reset sets it from the answer, and drafts stay per form (settingsInputs, assistantDrafts, nameDrafts), so an open draft keeps its typed values over the new snapshot. The launch form payload (:577) already builds from the snapshot plus its inputs; the assistant payload (:642-653) does the same with only the changed fields. The reload button (:584) clears the one snapshot. Test, one per path, in webui settings-form (the launch and assistant saves) and limits-browser (the reset):

   • with a limits draft open, save the assistant, then submit the limits draft: its posted revision is the one the assistant save returned, and the typed limit is still in the field;
   • with an assistant draft open, save the launch form, then submit the assistant draft: its posted revision is the one the launch save returned;
   • with an assistant draft open, reset a token limit, then submit the assistant draft: its posted revision is the one the reset returned.

   Each test fails on row 155's candidate, where the second post carries the old revision.

3. (F1) Null bodies on the Console write routes. Dewey's F1: Object.keys(value) throws on a JSON null in seatSend (packages/webui/src/serve.mjs:241, the new /api/name and /api/assistant-stop routes), settingsSend (:203) and projectFileSend (:276), and an async handler throw with no unhandledRejection handler ends the Console.

   I checked it against the candidate, and that throw can't happen there. Each handler reads its body through body() (:35, from ea00ec66d) inside a try, and body() parses the JSON and throws body must be a JSON object for null, an array or any non-object (:45-46). The catch answers 400 invalid-request before the Object.keys line runs. Two of the four already have passing tests for it: settings-write.test.mjs:17 posts null to /api/settings and expects 400, and project-file.test.mjs:76 ("a body that is JSON but not an object is refused 400, not thrown on") posts null, [], 5, "text" and true and expects 400 with "JSON object". The two seatSend routes have no such test.

   Dewey withdrew F1 in #1649 comment 29014 (rev 1856) and agreed the item is tests only, with no local check beyond body(). Test, one per seatSend route in assistant.test.mjs: posting null to /api/name and to /api/assistant-stop gets 400 invalid-request, no write call is made, and a following valid request still gets an answer. The settings and project file routes keep their existing tests. Both new tests pass on row 155's candidate; they pin body()'s check on the two routes that lack one.

The shell write-boundary test (shell.test.mjs) pins every POST fetch and every assistantPost( route in bus.js. None of the three adds or changes a write, so its list and count stay as row 155 leaves them; if the fix touches a fetch( line, that test changes with it.

Out of scope

Server-side settings handling, the routes, and any change to what a save sends beyond the revision.