stack/agents/filbert/work/queue-125/r1/body.md

Filbert review, row 125 round 1: changes

Candidate manifest e688651be8070e48c9ede5e239149342145804cf803dccad84dffb91beac93da (36 paths), base 19fe85c3e. I reviewed and ran on 19fe85c3e with Dewey's build.patch applied (81e47c43...). The manifest matched before and after every run. Following sage-0810, I reviewed on the 19fe85c3e base, not the row 129 landing.

Two changes are requested. The first is a regression I confirmed with a probe on base and candidate. The second is documentation in a file a test reads. The rest of the design matches BUILD.md's rulings and holds up, including the fail-closed paths, the end-of-turn ordering and the loader-only reach.

Change 1: any object with a limit key is taken as a token-limit leaf, so valid saves are refused

launch-settings.mjs:13 defines isLeaf = value => own(value, "limit"). stamp() (:19) walks all of launch.limits (:91), not just tokens, and gives every such object a since. Concurrency provider and model names are free strings (launch.mjs:5, name, at most 200 characters), so a provider or model named limit makes its map look like a leaf. The map then gets a since, and the schema refuses the save.

My probe (launchSettings(...).set on a fixture business, limits replaced as shown):

launch.limits sent	base 19fe85c3e	candidate
{providers:{anthropic:3}} (control)	saved	saved
{providers:{limit:3}}	saved	refused: launch.limits.providers.since must be a whole number of 0 or more
{models:{limit:{m:2}}}	saved	refused: launch.limits must be a JSON object
{models:{anthropic:{limit:2}}}	saved	refused: launch.limits.models.anthropic.since must be a whole number of 0 or more
{tokens:{providers:{limit:{limit:5}}}}	refused (tokens unknown on base)	refused: launch.limits.tokens.providers.limit.since must be a canonical timestamp

The first three rows are caused by row 125: base saves them and the candidate doesn't. The Settings form always sends limits, so such a business can't save anything from Settings. The last row means a token limit for a provider, model, instance or key named limit can't be set either, although checkTokenLimits accepts the name.

reset() (:34) and the bus core's limitLeaves (packages/bus/src/limits.mjs:30) use the same rule. A hand-edited file with a token provider named limit validates, but the core sees tokens.providers itself as the leaf. The token-limit check's hit then fails validHit, so every launch and turn end fails closed as malformed-answer. That is safe, but wrong.

Requested: stamp and reset only the token leaves at their schema depth (tokens.instances.X, tokens.providers.X, tokens.keys.X, tokens.models.P.M), so the concurrency limits are never touched. Also make limitLeaves agree, or refuse the reserved names explicitly; I'll leave the choice to you and Sage. Add a test that the four base-valid shapes above still save, and that a token limit named limit is either set and counted or refused with a clear message.

Change 2: the CLI docs don't describe token limits, reset or the new plain output

packages/cli/README.md:786 ("Agent limits and models") and docs/TOOLS.md:609 ("Agent limits") still describe the old surface:

• mosaic limits reset PATH... (one word per level) is missing;
• token limits, since, and "not enforced" are missing;
• the plain mosaic limits output is missing. Without --json it now prints the JSON without tokens, then text lines (limits.mjs:79), so it can no longer be parsed as JSON. Nothing in the repository parses the plain output; limits.test.mjs:25 uses --json.

LIMITS_USAGE names reset. TOOLS.md alone would be doc-only under decision 137, but packages/cli/tests/host.test.mjs:826 reads packages/cli/README.md, so this is a change rather than a follow-up.

Checked and correct

• Each subject kind. token-limit.test.mjs:25 and :46 cover instance, provider, model and key, refused at the limit and allowed under it. :98 covers a key limit that is not enforced while no record names an account (decision 150 Q1).
• Refusal.
  • launcher.mjs asks mayLaunch after the concurrency check. An op failure, a hit or any other answer refuses (launch-limit-checks.test.mjs).
  • :32 pins the instance, provider and model sent.
  • The launcher imports no check, and the bus core imports only the loader and usage.mjs (limit-checks.test.mjs, the import list grep).
• End-of-turn stop.
  • The broker asks mustStop in the finally after the usage end row, and keeps the stop until the run's next message.receive or state.
  • The turn's RESULT and read receipt land first. limit-stop.test.mjs:51 and runner.test.mjs (exit 22, limit-check revoke, reason with limit and count) show it, and my f6 below is killed by both.
  • A thread run is released, not revoked, and its end record carries limit (thread-launch.test.mjs:384; the two new tests ran, 10/10, 0 skipped).
• Reset. token-limit-settings.test.mjs:61 checks the actor (chief and jason) and the old and new since in the change log. It also checks two paths at once and the refusal of every non-leaf path, with the file bytes unchanged. The CLI, Settings route, page and organization.set tool reach the same writer. tools.test.mjs holds the three key lists equal.
• Known records only. usage.mjs turnRecords keeps one record per (run, turn, source), with the end row winning. Only an end row with status = 'known' counts. My f1 (end row doesn't win) is killed by usage.test.mjs and limit-checks.test.mjs:256.
• No usage source. limit-checks.test.mjs:256 uses @none unknown rows, the source name usage.mjs:71 writes when no source is switched on. They add 0, count as unknown turns, and never refuse on their own. Row 124's usage.test.mjs drives the real no-source collect. See the note below.
• Not enforced. limits-runtime.test.mjs:45 (no check switched on), limits-tokens.test.mjs:18 (CLI) and limits-browser.test.mjs (Settings) show it. The route returns only the token rows, never the document (limits-route.test.mjs).
• The Settings save keeps token limits as read (bus.js, settingPayload). Without that, a form save would have dropped them. Dewey's W06 pins it.

Non-blocking notes

• No usage source, end to end. limits-runtime.test.mjs:52 drives usage.end with no usage source, but its limit is 0, so it stops either way. With limit 1, asserting count 0, one unknown turn and no stop would test BUILD.md's "nothing stops" through the real path.
• My survivors, all on lines with no brief acceptance item:
  • f2: remove the background limits.refresh after plugins.set (runtime.mjs:97). It survived limits-runtime.test.mjs. Without the refresh, mosaic limits would show not-loaded for a newly switched-on check until the next launch or turn end refreshes.
  • f3: pass args to the check without structuredClone (limits.mjs:85). It survived limit-checks.test.mjs, so nothing shows a check can't change the business's limits or records for the next check.
  • f4: on a reset with a dirty Settings draft, replace the snapshot anyway (bus.js:609). It survived limits-browser.test.mjs. Either behaviour is safe: with the guard, the later draft save gets a revision conflict and keeps the draft.
• Cost. Every mayLaunch and every turn end reads all of the business's usage_records and re-reads the organization file. That's fine now, but it grows with history.
• Base drift. Row 129 has landed on bus.js, serve.mjs and shell.test.mjs. Dewey reports the patch applies to fdb1d4b34 with --3way; I didn't run it there. Sage's landing gate covers this.

Ran by me

Slot from sage-0814. The busy check was CLEAR before each step, the manifest was checked before and after, TMPDIR was in my own scratch, and DOCKER_HOST was a dead socket.

• Singles, 18:11:42Z to 18:12:26Z, one node --test per file:

  • bus: limit-checks 14/14, limit-stop 5/5, limits-runtime 3/3, process 8/8, usage 14/14;
  • business: token-limit-settings 3/3, launch-settings 8/8;
  • cli: launch-limit-checks 4/4, launch-limits 1/1, limits-tokens 4/4, limits 4/4, thread-launch 10/10;
  • harness: runner 26/26, tools 20/20;
  • plugins: kinds 5/5, loader 17/17, token-limit 10/10;
  • webui: limits-browser 1/1, limits-route 4/4, shell 5/5.

  The awk sum is 166/166 over 20 files, 0 skipped.

• Mutants, 18:12:31Z to 18:13:07Z. All six controls passed first.

  • Killed: f1 (end row doesn't win), f5 (checks used without the setting match, limit-checks.test.mjs 8-10), f6 (ask on every usage phase, limit-stop.test.mjs:51 and the runner test).
  • Survived: f2, f3, f4, as noted.

• Probe, 18:13:07Z to 18:13:08Z, on both trees, as in the table.

Not mine

Dewey's gate on 19fe85c3e (BUILD.md): 16 packages 2236/2236, 17 singles 140/140, 10 scripts rc=0, 74 of 74 mutants killed. I read mutants.mjs and did not rerun it.