jason.woltje
158205c5cc
Unclaimed files left in the canonical checkout. Inventory: canonical-cleanup-inventory.md (commit-as-evidence groups). detect-secrets scan clean on 2026-11-02: hits are sha256 pins, git object ids, test sentinels already in tracked tests and unit-file lines.
160 lines
8.8 KiB
Markdown
160 lines
8.8 KiB
Markdown
# Row 167 (#1664): Settings page reads and snapshots after Q169
|
|
|
|
Owner: Darkwing. Reviewer: Filbert. Gate owner: Sage.
|
|
Brief: `docs/plans/2026-10-18_settings-reads-after-q169.md`.
|
|
Base: 81f26551b. origin/refactor is 0434ecfef, and `git diff 81f26551b 0434ecfef` touches only
|
|
`docs/SESSIONS.md`.
|
|
Worktree: `~/darkwing-scratch/r167/wt`.
|
|
|
|
## Files
|
|
|
|
| File | Change |
|
|
|---|---|
|
|
| `packages/webui/src/public/bus.js` | items (a) and (b), +15/-11 |
|
|
| `packages/webui/tests/settings-form.test.mjs` | three tests: (a) and two of (b) |
|
|
| `packages/webui/tests/limits-browser.test.mjs` | the :54 flip, its comment, and the (b) reset test |
|
|
| `packages/webui/tests/assistant.test.mjs` | the F1 null-body test |
|
|
| this packet | |
|
|
|
|
`public.diff` is the `public/` part alone, for Mos's decision 123 go. File sha256
|
|
1110e2e80cd4880c2fd7b683035f270fec48bf278a8982140c38fa6478adb556. The +/- digest is
|
|
`grep '^[+-]' public.diff | grep -vE '^(\+\+\+|---) ' | sha256sum`, which gives
|
|
82a8a810686282942833b667c18f6ecb758aa485ec4a386c13684c35950ce984 over 26 lines.
|
|
|
|
No server file changes, and no route, write or wording changes. TOOLS.md stays as it is.
|
|
|
|
## Items
|
|
|
|
**(a) The names read rethrows.** `settingsView` (:733) now catches the `names` read the way it
|
|
catches `usage`, `limits` and `instances`. A 403 or `signed-out` is thrown on, so the page
|
|
gets the refused view ("Bus refused the read", `forget()` clears the kept reads). Any other
|
|
failure stays local, and the Assistant section shows the empty name field with "The current
|
|
name could not be read."
|
|
|
|
**(b) One snapshot.** `assistantSnapshot` is gone. `settingsSnapshot` holds the launch settings
|
|
for the launch form, the token-limit reset and the Assistant section.
|
|
|
|
- `takeSnapshot(value)` (:565) replaces the snapshot only when it is empty or no form has a
|
|
draft. A draft is `settingsInputs` set or any key in `assistantDrafts` (`launchDrafts`,
|
|
:564). The launch form and the Assistant section both read through it.
|
|
- The launch save (:591), the assistant save (:670) and the reset (:613) set the snapshot from
|
|
their answer, drafts or not. The reset used to do that only with no launch draft open.
|
|
- The reload button (:588) clears the snapshot, so the next read takes the current settings.
|
|
- `assistantPayload` (:646, :657) builds from `settingsSnapshot`.
|
|
|
|
Drafts stay per form, so typed values survive a new snapshot. The launch payload sends the
|
|
snapshot's models plus its own inputs, and an assistant or reset answer carries the launch
|
|
fields unchanged, so a launch draft's full send doesn't overwrite another form's write.
|
|
|
|
Tradeoff: while any draft is open, both sections render from the frozen snapshot, not from a
|
|
newer read. A write from elsewhere (another tab, the CLI) still ends in `revision-conflict`,
|
|
which keeps the draft and asks for a reread. That is row 155's behavior for foreign writes.
|
|
|
|
**(F1) Null bodies.** One test in `assistant.test.mjs` posts `null` to `/api/name` and to
|
|
`/api/assistant-stop` in turn. Each gets 400 `invalid-request` with `sent:false`, makes no bus
|
|
call, and the valid request that follows is answered and makes its one call. Every assertion
|
|
carries the route as its message, so a failure names the route. The brief asks for one test
|
|
per route; this is one test that covers both, each route on its own lines. If Filbert wants
|
|
two `test()` calls I'll split it. It passes on row 155's code, as the brief expects, since it
|
|
pins `body()`'s check.
|
|
|
|
## For Dewey and row 126: limits-browser.test.mjs
|
|
|
|
Row 167 changes this file in two places, and row 126 changes it too. Whichever row lands second
|
|
rebases.
|
|
|
|
1. The REQ-LIMIT-3/4 test. The comment at :41-42 now reads "A draft typed before the reset
|
|
stays, and its save carries the revision the reset returned (row 167)." At :54,
|
|
`assert.equal(save.revision,'a'.repeat(64))` becomes `'b'.repeat(64)`. The brief says every
|
|
successful reset sets the snapshot from its answer, so a launch draft open across a reset now
|
|
saves with the reset's revision `b`. Row 155 kept `a`.
|
|
2. A new test at the end of the file (:64-85, after the REQ-LIMIT test's closing `});`):
|
|
'Row 167 with an assistant draft open, a token limit reset gives the assistant submit its
|
|
revision'. Nothing else in the file changes.
|
|
|
|
## Shell pins
|
|
|
|
`shell.test.mjs` pins the POST `fetch(...)` expressions, the count of `method:` (6) and the
|
|
first arguments of `assistantPost(`. Row 167 changes code on the two lines that hold the
|
|
settings `fetch` calls, but not the expressions. I ran the test's three regexes over row 155's
|
|
`bus.js` (931a32d7f0d1) and the candidate's (c2a0dffc54df), and the output is byte-identical,
|
|
so shell.test needs no change. That check evaluates the regexes directly. The test itself runs
|
|
on Sage's gate. No test pins a hash of `public/bus.js` or names the snapshot variables.
|
|
|
|
## Out of scope: P19 369
|
|
|
|
The instance form from row 165 keeps its own revision. A settings save while an instance draft
|
|
is open, or the reverse, still ends in `revision-conflict`. Sage made that P19 369 for a later
|
|
row, which will need its own decision 123 go (sage-0968).
|
|
|
|
## Runs
|
|
|
|
All on Sage's slot (sage-0968), the three touched files run singly with the spec reporter,
|
|
TMPDIR `~/darkwing-scratch/r167/tmp`. Totals come from the awk sum over the logs.
|
|
|
|
| Run | Tree | assistant | settings-form | limits-browser | Total |
|
|
|---|---|---|---|---|---|
|
|
| cand-20261019T002107Z | candidate | 6/6 | 5/5 | 2/2 | pass 13, fail 0 |
|
|
| base-20261019T002118Z | 81f26551b with the candidate's tests | 6/6 | 2 pass, 3 fail | 0 pass, 2 fail | pass 8, fail 5 |
|
|
|
|
Each base failure is the intended mismatch. The names test waits for "Bus refused the read" and
|
|
times out. Both settings-form snapshot tests and the new reset test post `a` where `b` is
|
|
expected. The REQ-LIMIT test fails the :54 flip (`a` for `b`). The F1 test passes on base, as
|
|
the brief says it will.
|
|
|
|
## Mutants
|
|
|
|
`~/darkwing-scratch/r167/mutants.py`, out `mutants-20261019T002338Z`. Each mutant is one exact
|
|
edit to `bus.js`, run against all three test files. `bus.js` was restored and sha-checked after
|
|
each mutant (c2a0dffc54df at the end), and the sweep found none of my processes left.
|
|
|
|
| Mutant | Edit | Killed by |
|
|
|---|---|---|
|
|
| M1 | the names catch swallows a 403 and `signed-out` | settings-form: the names test |
|
|
| M2 | `takeSnapshot` always replaces | limits-browser: REQ-LIMIT (:54) and the reset test |
|
|
| M3 | the reset sets the snapshot only with no launch draft | limits-browser: REQ-LIMIT (:54) |
|
|
| M4 | the assistant save doesn't set the snapshot | settings-form: limits draft, assistant save |
|
|
| M5 | the launch save doesn't set the snapshot | settings-form: assistant draft, launch save |
|
|
| M6 | `launchDrafts` ignores `assistantDrafts` | limits-browser: the reset test |
|
|
| M7 | the `!settingsSnapshot` clause is dropped | settings-form: assistant draft, launch save (reload step) |
|
|
| M8 | reload keeps the snapshot | settings-form: assistant draft, launch save (reload step) |
|
|
|
|
8 of 8 killed.
|
|
|
|
## Candidate manifest
|
|
|
|
`candidate-manifest.sha256`:
|
|
|
|
```
|
|
c2a0dffc54dfe98b9b1ac1315879fc370e1c15135b931fc15791b07e17d854e7 packages/webui/src/public/bus.js
|
|
3223914f9419e1c256cc706a07f64223b7502c455c53e376f6e6c6ac72b287f0 packages/webui/tests/assistant.test.mjs
|
|
52c8763573dce256ac38c18310ca05ab9304ee9e4e830e2cfb4e139f7b4a005a packages/webui/tests/limits-browser.test.mjs
|
|
cf7fa330d979b9ee253ac5099f3a2532bac79604bf389a1c68fcc454a94cc3bf packages/webui/tests/settings-form.test.mjs
|
|
```
|
|
|
|
`build.patch` sha256 f022d1bca934fc9b2c4d889b24c29d97dab6542e846a1477357b5b40a67c9cfc.
|
|
|
|
### Full webui singles (mos-0836 condition 1)
|
|
|
|
On the slot Dewey handed over (dewey-0661), `~/darkwing-scratch/r167/singles-all.sh` ran all
|
|
38 webui test files on the candidate, each one alone. Out dir `all-20261019T002730Z`. The awk
|
|
sum gives pass 231, fail 0, skipped 1. The skip is editor-provenance's rebuild test, which
|
|
needs esbuild 0.28.2 where the canonical `node_modules` has 0.28.1. The skip comes from the
|
|
environment and predates this row. `shell.test` passed 5/5. The mutants ran again on the same
|
|
slot (`mutants-20261019T003056Z`) and all 8 were killed. Afterwards the candidate files matched
|
|
the manifest.
|
|
|
|
## Status
|
|
|
|
Mos gave the decision 123 go on `public.diff` 1110e2e8 with +/- digest 82a8a810 (mos-0836,
|
|
relayed in sage-0976). The go holds on three conditions: the full webui singles and the
|
|
mutants green (done, above), reviewer approvals, and the digest unchanged at landing. Any later
|
|
`public/` change goes back to Mos with its delta. Mos's F1 is that `assistantPayload` throws a
|
|
raw TypeError when a save answer lacks the open form's instance. Row 155 had the same
|
|
exposure. It is P19 370 and no part of this row.
|
|
|
|
Filbert is the only reviewer. sage-0979 corrected sage-0976, which had named Dewey too. Round 1
|
|
went in-review at rev 1915. The request is comment 29060, the candidate manifest digest is
|
|
dba383306a9e7d71f03af8e1da45b4fbb7542e22555fb156aa0c064d8adf59cc, and comment 29061 cites the
|
|
mos-0836 go.
|