Files

5.4 KiB

Filbert, row 64 round 1: approve (Doc-only, follow-up below)

Candidate manifest agents/sage/work/queue-64/candidate-manifest.sha256, sha256 7689cd9b0c54e2b9786ce03144c8fec09d5c43b3007b3c2bf60a194cb0659946, candidate commit c6d7d538f on 087d202bc.

What I ran. This was a static review, so I took no test slot. In my own worktree, detached at c6d7d538f:

  • sha256sum -c on the manifest had 0 non-OK lines.
  • The 28 manifest paths equal git diff --name-only 087d202bc c6d7d538f, and git show c6d7d538f matches candidate.diff.
  • Since 087d202bc, refactor has changed only docs/SESSIONS.md, docs/plans/QUEUE.md and docs/plans/queue.json, so none of the manifest paths has drifted.
  • The commit message has no attribution line.

The gate (15 package suites, 2384 pass, 0 fail, all 10 script suites) and the 13 of 13 mutant kills are Sage's receipts. I did not rerun them.

Brief (2026-10-11_managed-session-config.md, "Managed sessions get their role's skills on both harnesses").

  • role.mjs adds an optional skills key: distinct names matching [A-Za-z0-9._-]+, with "." and ".." refused, and v1 roles get []. role.test.mjs:164 covers duplicates, ".", "..", path names, non-strings and the v1 refusal.
  • resolve.mjs carries the list inside the digest.
  • roles/coder, reviewer, cto and pm.json add only the skills line, with the lists exactly as the brief gives them.
  • packages/plugins is unchanged and no source outside the tests sets MOSAIC_SKILLS, so the harness modules and MOSAIC_SKILLS stay as they were.
  • The brief's four tests, each on both harnesses:
    • Exactly the listed skills: skills.test.mjs:45 checks four roles on two harnesses, and compares each marked file with the bundle copy, the source and the digest.
    • Not enabled refuses before any spawn: skills.test.mjs:109, launcher.test.mjs:730 (pi) and :750 (Claude Code). Both launcher tests assert no session and no run.
    • Enabled but not listed stays absent: skills.test.mjs:92.
    • No list: skills.test.mjs:79, and :170 for the chief on both adapters.

Mos's checks (mos-0857).

  1. The rendered prompt.md matches the bundle copies byte for byte. skills.test.mjs:45 compares each prompt section with the copy and the source. :170 runs the real adapters against a stand-in CLI that records argv, and checks that the single --system-prompt argument equals prompt.md (less the trailing newlines $(cat) drops) and that no skill path reaches argv any other way.
  2. Discovery stays off. adapters/pi/adapter.sh:55 keeps --no-skills, and adapters/claude/adapter.sh:85 and :92 keep --restricted and --disable-slash-commands. skills.test.mjs:170 asserts all three, plus no --skill.

BUILD.md "For the reviewers".

  • The 64 KiB cap: SKILL_BYTES applies to all of a role's skills together, and the size is checked before decode. I measured the four lists at 16853 bytes (coder), 16252 (reviewer), 35888 (cto) and 29080 (pm). With the contract and the markers, each is well under the 128 KiB single-argument limit. skills.test.mjs:151 covers crossing the cap.
  • The launcher precheck order: readSkills runs in check() at launcher.mjs:252-258, after the route and contract checks, before the harness-on check, and before newRun at :357. A refusal leaves no run behind.
  • The non-sandbox test files (cli helpers, channel-fixture, git-socket, instances and thread-launch, and the three webui tests) are fixture-only. fixture() now makes the data root at 0700 and copies the repository's skills into skills-enabled, and the per-test mkdirSync(f.dataRoot) lines go.

Doc-only, follow-up (decision 137: a .md file that no test reads; no test under packages//tests or scripts/test-.sh reads packages/harness/README.md):

  • F1, packages/harness/README.md:55: the README says a symlinked skill, a bad file name, non-UTF-8, the end marker and an over-cap list each refuse skill-not-enabled ("So does a symlinked skill..."). They don't. bundle.mjs gives them the default bundle-refused code, and the launcher maps that to skill-refused at launcher.mjs:256. That is what launcher.test.mjs:730 expects for a symlink, and the readSkills comment at bundle.mjs:141-142 says so too. Only a missing skill, or a skill directory with no SKILL.md, is skill-not-enabled. The fix is "...refuses skill-not-enabled; a symlinked skill ... refuses bundle-refused (skill-refused at launch)".

Notes, none blocking:

  • A leading UTF-8 BOM: the fatal TextDecoder strips it, so for such a file the bundle copy and the prompt text would be 3 bytes shorter than the source, and the digest would cover the shorter text. No repository skill file has a BOM.
  • An fs error other than ENOENT (EACCES, for example) in found(), readdirSync or readFileSync is rethrown raw, not as a BundleError. The launcher rethrows it rather than refusing skill-refused. That still fails closed.
  • The precheck and buildBundle each read the skills. If a skill is deactivated between the two, buildBundle (launcher.mjs:371) throws after newRun, outside the later launch-failed catch. This is the same path every other buildBundle refusal already takes, so it isn't new to this row, and it fails closed.
  • Landing order: once the live stack is re-pinned to this code, every coder, reviewer, cto and pm launch refuses skill-not-enabled until each listed skill is in the live skills-enabled. Activation has to come before or with the re-pin. BUILD.md:117 already activates first.