stack/agents/filbert/work/queue-167/r1/body.md at dev-console

0 Fork 1

Files

T

jason.woltje b9ea9304fd lead: commit filbert's untracked evidence (273 files: queue-101 queue-102 queue-105 queue-107 queue-108 queue-109 queue-110 queue-112 queue-113 queue-114 queue-115 queue-116 queue-119 queue-120 queue-122 queue-123 queue-125 queue-126 queue-128 queue-129 queue-130 queue-131 queue-132 queue-133 queue-134 queue-135 queue-136 queue-137 queue-139 queue-140 queue-141 queue-142 queue-145 queue-146 queue-147 queue-148 queue-149 queue-150 queue-151 queue-152 queue-153 queue-154 queue-155 queue-156 queue-162 queue-163 queue-165 queue-166 queue-167 queue-169 queue-170 queue-175 queue-177 queue-181 queue-182 queue-184 queue-185 queue-186 queue-187 queue-188 queue-49 queue-64 queue-66 queue-70 queue-71 queue-80 queue-84 queue-85 queue-87 queue-88)

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.

2026-11-02 15:05:14 -06:00

6.7 KiB

Raw Permalink Blame History

Filbert review, row 167 round 1: approve

Candidate manifest dba383306a9e7d71f03af8e1da45b4fbb7542e22555fb156aa0c064d8adf59cc (4 paths), build.patch f022d1bca934fc9b2c4d889b24c29d97dab6542e846a1477357b5b40a67c9cfc. I applied it with --index to 81f26551b. All 4 manifest lines matched, git diff --cached --check is clean, and the diff is +128 -13 over 4 files. I read it against the brief docs/plans/2026-10-18_settings-reads-after-q169.md and BUILD.md.

The decision 123 digest (mos-0836)

The cached public/ diff is byte-equal to public.diff (sha256 1110e2e8...b556). Its +/- digest (without the +++/--- headers) is 82a8a810686282942833b667c18f6ecb758aa485ec4a386c13684c35950ce984, which is the digest Mos's go names. It touches only packages/webui/src/public/bus.js, +15 -11.

(a) The names read

settingsView (bus.js:733) now has the same catch as usage (:732) and instances (:735): a 403 or signed-out is thrown on, and any other failure stays local. The test (settings-form.test.mjs:55) checks both halves:

a 403 on names gives a page innerHTML-equal to the 403 on usage, with neither form on it;
a 503 on names keeps the Assistant section with "The current name could not be read.", an empty name field and the launch form.

That is the brief's test.

(b) One snapshot

assistantSnapshot is gone (no occurrence left in bus.js). The single settingsSnapshot is used as follows:

Reads. Both renders read through takeSnapshot (:565). It replaces the snapshot only when it is empty or no form has a draft, where a draft is settingsInputs or any assistantDrafts key (:564).
Saves and the reset. The launch save (:591), the assistant save (:670) and the reset (:613) set it from their answer. The reset no longer depends on !settingsInputs.
Reload. The reload button (:588) clears it.
The assistant payload. assistantPayload builds from it (:646, :657).

Setting the snapshot from a reset or an assistant answer is safe for the Assistant section because the answer is the whole view. launchSettings.set returns { ...view(id), transaction } (business/src/launch-settings.mjs), and view carries assistants (:93-101). A missing instance is Mos's F1, P19 370, which is out of scope here.

assistantSection now snapshots before its empty-assistants check. That is harmless, because launchForm takes the same s.launch value (:741).

The three brief tests are:

Brief path	Test	Second post
assistant save with a limits draft open	`settings-form.test.mjs:96`	revision `b`, typed total 7 kept
launch save with an assistant draft open	`settings-form.test.mjs:109`	revision `b`, tracker only
reset with an assistant draft open	`limits-browser.test.mjs:66`	revision `b`, tracker kept

The :109 test also covers reload: after a foreign write to d, the reload with the draft open makes the next submit carry d. The REQ-LIMIT :54 flip from a to b follows directly from "every successful save or reset sets it from the answer", and the comment at :42 says so.

(F1) Null bodies, and your question about one test or two

assistant.test.mjs:71 posts null to /api/name and /api/assistant-stop in turn. Each gets 400 invalid-request with sent:false, makes no bus call, and is followed by an answered valid request that makes its one call. The final count is 2.

My ruling: one test is fine, so don't split it. The brief's requirement is coverage per route, and each route gets the full set of assertions on its own lines, labelled by path. One small gap: the s.calls.at(-1) assertion at :79 has no path message. Not blocking.

Other checks

Shell pins. At base and on the candidate, bus.js has 6 method:. The POST fetch('...',{method:'POST'...}) expressions and the assistantPost('...' first arguments are identical, so shell.test.mjs needs no change. That matches BUILD.md. I compared the strings directly; I didn't run the test.
Hash pins. No 64-hex pin in packages/*/tests or scripts/test-*.sh names any of the 4 manifest paths. No test names settingsSnapshot or assistantSnapshot.
Base drift. From 81f26551b to HEAD 0434ecfef, no manifest path changed. build.patch passes git apply --cached --check on HEAD in a temporary index.
The commit message. commit-msg.txt has no attribution line.

Notes (not blocking)

A submit during a reload read throws in the receipt. The reload click sets settingsSnapshot=null and starts render(true). render keeps the old DOM until its reads finish (:1003 on), so the assistant form stays enabled in that window. A submit then hits settingsSnapshot.assistants in assistantPayload (:646). The try at :669 catches it and shows the raw TypeError text as the receipt. Nothing is posted and the draft stays.
- The launch form already had this exposure at base: settingPayload reads settingsSnapshot.models after the same clear.
- This row extends it to the assistant form, because that form used to keep its own snapshot.
- It is the same class as P19 370 (a raw TypeError from assistantPayload), so it could fold into that item.
The reload wait in the :109 test isn't tied to the reread. At settings-form.test.mjs:121 it waits for the launch receipt to be '', and the click clears that receipt synchronously. The submit after it relies on the reread having finished first. If the reread lags, the submit hits note 1, nothing is posted, and posts.length===3 times out. So the test can flake but can't false-pass. Waiting on something only the reread produces would fix it.
Running counts freeze too. BUILD.md's tradeoff (both sections render from the frozen snapshot while any draft is open) also covers the launch form's "N running" counts, which come from v.usage. They now freeze while an assistant draft is open; before, only a launch draft froze them. I'm stating it for completeness, not asking for a change.

Ran by me

Nothing that needed the slot. I applied the patch and ran sha256sum -c (4/4). I hashed the public diff and compared it with the packet, read the bus.js diff, the tests and launch-settings.mjs, compared the shell-pin strings at base and on the candidate, ran greps, and ran the HEAD apply check. I ran no test and no mutant.

Not mine

Darkwing's runs, as BUILD.md records them:

The three touched files, singly. The candidate gives pass 13, fail 0. Base with the candidate's tests gives pass 8, fail 5, each an intended mismatch, with F1 passing on base as the brief expects.
All 38 webui files, singly. Pass 231, fail 0, skipped 1 (the esbuild 0.28.2 pin).
Mutants. 8 of 8 killed, twice.

Their logs are under ~/darkwing-scratch, which I don't read. The full gate is Sage's at landing.

6.7 KiB Raw Permalink Blame History