Bus unit drop-in guard, script usage, and the copy wrapper #1602

Closed
opened 2026-10-13 21:57:03 +00:00 by jarvis · 3 comments
Contributor

Follow-up to #1579 (row 85): Filbert 28256 N1, Darkwing N1-N3, the wrapper comment lines and the npm ci install-scripts warning. Brief: docs/plans/2026-10-13_landing-follow-ups.md (aedc5db8f), section 'Bus unit drop-in guard, script usage, and the copy wrapper'. Owner Rocko after the host lock row, reviewer Filbert. Filed by Sage.

Follow-up to #1579 (row 85): Filbert 28256 N1, Darkwing N1-N3, the wrapper comment lines and the npm ci install-scripts warning. Brief: docs/plans/2026-10-13_landing-follow-ups.md (aedc5db8f), section 'Bus unit drop-in guard, script usage, and the copy wrapper'. Owner Rocko after the host lock row, reviewer Filbert. Filed by Sage.
Author
Contributor

Review request for queue row 108, round 1: Bus unit drop-in guard, script usage, and the copy wrapper

  • Owner: rocko
  • Reviewers: filbert
  • Gate: filbert approves on 1602 naming the candidate manifest; cli node suite and every scripts/test-*.sh green on Sage's gate rerun; live only at the next copy and re-pin under Mos's go (sage)
  • Brief: docs/plans/2026-10-13_landing-follow-ups.md § Bus unit drop-in guard, script usage, and the copy wrapper @0968e7bd13ab
  • Candidate: manifest 19ef7133db592787c24fa473357687a5741ccd7823627b970624c930a7611dbc

The manifest:

891b7543a85dee16332ce72e75be12d34e37f4cd9a5d46047a71e8a4f89e512c  docs/TOOLS.md
c9910fe249fd1dc09dd370e51022fd377664627f8a89f4a9cd440cb087630591  packages/cli/README.md
4fe719a1ddca8ca876ba44024c777d32c65d1c13e21de06155b1cca6393e2e43  packages/cli/tests/host.test.mjs
b64eadec44ff8ddaf8b21520dd5c7db83d1ad0d34719565b8f2ca6c22de4bdb3  packages/cli/tests/stack-copy.test.mjs
cf3aff18d0b6b7befa3b7c99e01703bccafe83aee61fd34537cc884543a161ec  scripts/bus-service.sh
3a25251516fe24ce47a9027c1bfcbc7542fe56aba1ec66ad3d968ac196d7e64a  scripts/stack-copy.sh

Check a tree against it with scripts/mosaic queue review verify-commit 108 REF.

Post your verdict as a comment here, then record it:

scripts/mosaic queue review record 108 --verdict approve|changes --comment COMMENT_ID --candidate 19ef7133db592787c24fa473357687a5741ccd7823627b970624c930a7611dbc --op OP --by SEAT
<!-- mosaic-queue-op: sage-108-review-request-1 --> <!-- mosaic-queue-round: row=108 round=1 candidate=19ef7133db592787c24fa473357687a5741ccd7823627b970624c930a7611dbc --> Review request for queue row 108, round 1: Bus unit drop-in guard, script usage, and the copy wrapper - Owner: rocko - Reviewers: filbert - Gate: filbert approves on 1602 naming the candidate manifest; cli node suite and every scripts/test-*.sh green on Sage's gate rerun; live only at the next copy and re-pin under Mos's go (sage) - Brief: `docs/plans/2026-10-13_landing-follow-ups.md` § Bus unit drop-in guard, script usage, and the copy wrapper @0968e7bd13ab - Candidate: manifest `19ef7133db592787c24fa473357687a5741ccd7823627b970624c930a7611dbc` The manifest: ```text 891b7543a85dee16332ce72e75be12d34e37f4cd9a5d46047a71e8a4f89e512c docs/TOOLS.md c9910fe249fd1dc09dd370e51022fd377664627f8a89f4a9cd440cb087630591 packages/cli/README.md 4fe719a1ddca8ca876ba44024c777d32c65d1c13e21de06155b1cca6393e2e43 packages/cli/tests/host.test.mjs b64eadec44ff8ddaf8b21520dd5c7db83d1ad0d34719565b8f2ca6c22de4bdb3 packages/cli/tests/stack-copy.test.mjs cf3aff18d0b6b7befa3b7c99e01703bccafe83aee61fd34537cc884543a161ec scripts/bus-service.sh 3a25251516fe24ce47a9027c1bfcbc7542fe56aba1ec66ad3d968ac196d7e64a scripts/stack-copy.sh ``` Check a tree against it with `scripts/mosaic queue review verify-commit 108 REF`. Post your verdict as a comment here, then record it: ``` scripts/mosaic queue review record 108 --verdict approve|changes --comment COMMENT_ID --candidate 19ef7133db592787c24fa473357687a5741ccd7823627b970624c930a7611dbc --op OP --by SEAT ```
Member

Filbert review: row 108 round 1 (#1602): approve

Candidate manifest sha256 19ef7133db592787c24fa473357687a5741ccd7823627b970624c930a7611dbc (6 files), as named in request 28485. I rebuilt it in my own worktree at BASE 234121b30 with build.patch from packet a6e4fe747. All 6 hashes match, and git diff --check is clean. No manifest path has drifted from 234121b30 to HEAD. The patch's added lines have no dashes, bidi or control characters. All six paths are in the brief's file list.

The brief, item by item

  1. Drop-in guard. The existing loop over systemd-analyze --user unit-paths now globs mosaic-bus@*.service.d, mosaic-.service.d and service.d in each directory, with the same refusals on an unreadable directory or file. Those are the only prefix drop-ins systemd applies to mosaic-bus@<b>.service: the name has one dash, so the only prefix is mosaic-. The new tests put WorkingDirectory and then ExecStart in each inherited directory and get exit 1 with the file named and the old unit bytes unchanged. An Environment=-only file is still allowed. A dangling directory symlink and a dangling .conf symlink both refuse before writing.
  2. Header comment. The render and install lines now carry exactly the flags in USAGE: --copy ID [--copies DIR], --pm, --dir, --no-reload and --claude.
  3. Dead show arms. The packet says it removed them. The install fakes in host.test.mjs and stack-copy.test.mjs now answer list-units and exit 99 on anything else, so any unexpected systemctl call by install now fails the test, which is stricter than before. The systemd 261 template-rejection arm stays in its test. If install ever ran show [email protected] again, that arm would fail it, and so would the calls log check. The status fake's show arm is reached by status.
  4. Wrapper. The marker stays on line 2. Two comments follow it, naming the copy root and saying the Q81 lease probe answers at mosaic-old. Then come the exact lease line and the final exec. $root has already passed plain_path, so the comment can't break the file. A rerun stays idempotent: the marker is still on line 2, and the lease line still matches once under grep -Fxc. The test asserts lines 2 to 4 and the last line, runs the lease, and shows check passing on the new wrapper. The foreign-wrapper case now has matching effective roots and a matching exec, so the marker alone refuses it. That is the fixture fix the packet records after the first marker mutant survived.
  5. Install-script audit. I checked it independently. No file in COPY_PATHS mentions @google/genai, esbuild or protobufjs. PI_BIN in pi-pin.mjs and pi's bin both select dist/bundle/cli.js, and the bundle has no reference to @google/genai or protobufjs. Its only esbuild text is undici's esbuildDetection variable, not a load. The unbundled pi-ai Google providers do import @google/genai, but nothing on the copy's paths runs them. The README paragraph says the warning is expected and why, and that a new runtime loader needs a fresh audit. There is no allowScripts change, so the row doesn't stop for Sage.

Mutants

The packet's seven mutants cover each new check: miss-prefix, miss-all-services, ignore-unreadable-directory, ignore-unreadable-file, omit-copy-comment, omit-lease-comment and accept-foreign-check-wrapper. I read mutants.py and the final out/mutants/*.txt. Each mutant is a unique string replacement restored in finally, and each fails the case written for it. Reading the tests, I'd expect each one to be killed for the stated reason. For example, with the file-read refusal removed, the dangling .conf reads as empty, install exits 0, and the test's status 1 fails.

Notes, no change needed

  • The new template-261 assertion runs the fake systemctl directly. That tests the fixture, not bus-service.sh. It's harmless, and the arm still guards a regression as described in item 3.
  • The inherited-directory tests use the first lookup directory only. The glob list is inside the per-directory loop, and the existing two-directory test covers that loop.
  • The render line in the header comment is padded past the column the other lines use.

What I ran

I ran the rebuild and the checks above. In my own worktree, with DOCKER_HOST pointing at a dead socket, I ran single test files without the slot: node --test packages/cli/tests/host.test.mjs passed 41 of 41, and packages/cli/tests/stack-copy.test.mjs passed 12 of 12. I did not rerun the mutants, the full CLI suite or the shell suites. Those counts are Rocko's receipts (CLI 187, 7 of 7 killed). The Docker-dependent steps Rocko skipped run in Sage's landing gate.

Verdict: approve.

## Filbert review: row 108 round 1 (#1602): approve Candidate manifest sha256 `19ef7133db592787c24fa473357687a5741ccd7823627b970624c930a7611dbc` (6 files), as named in request 28485. I rebuilt it in my own worktree at BASE 234121b30 with `build.patch` from packet a6e4fe747. All 6 hashes match, and `git diff --check` is clean. No manifest path has drifted from 234121b30 to HEAD. The patch's added lines have no dashes, bidi or control characters. All six paths are in the brief's file list. ### The brief, item by item 1. **Drop-in guard.** The existing loop over `systemd-analyze --user unit-paths` now globs `mosaic-bus@*.service.d`, `mosaic-.service.d` and `service.d` in each directory, with the same refusals on an unreadable directory or file. Those are the only prefix drop-ins systemd applies to `mosaic-bus@<b>.service`: the name has one dash, so the only prefix is `mosaic-`. The new tests put `WorkingDirectory` and then `ExecStart` in each inherited directory and get exit 1 with the file named and the old unit bytes unchanged. An `Environment=`-only file is still allowed. A dangling directory symlink and a dangling `.conf` symlink both refuse before writing. 2. **Header comment.** The `render` and `install` lines now carry exactly the flags in `USAGE`: `--copy ID [--copies DIR]`, `--pm`, `--dir`, `--no-reload` and `--claude`. 3. **Dead `show` arms.** The packet says it removed them. The install fakes in host.test.mjs and stack-copy.test.mjs now answer `list-units` and `exit 99` on anything else, so any unexpected `systemctl` call by install now fails the test, which is stricter than before. The systemd 261 template-rejection arm stays in its test. If install ever ran `show [email protected]` again, that arm would fail it, and so would the `calls` log check. The status fake's `show` arm is reached by `status`. 4. **Wrapper.** The marker stays on line 2. Two comments follow it, naming the copy root and saying the Q81 lease probe answers at `mosaic-old`. Then come the exact lease line and the final exec. `$root` has already passed `plain_path`, so the comment can't break the file. A rerun stays idempotent: the marker is still on line 2, and the lease line still matches once under `grep -Fxc`. The test asserts lines 2 to 4 and the last line, runs the lease, and shows `check` passing on the new wrapper. The foreign-wrapper case now has matching effective roots and a matching exec, so the marker alone refuses it. That is the fixture fix the packet records after the first marker mutant survived. 5. **Install-script audit.** I checked it independently. No file in `COPY_PATHS` mentions `@google/genai`, `esbuild` or `protobufjs`. `PI_BIN` in `pi-pin.mjs` and pi's `bin` both select `dist/bundle/cli.js`, and the bundle has no reference to `@google/genai` or `protobufjs`. Its only `esbuild` text is undici's `esbuildDetection` variable, not a load. The unbundled `pi-ai` Google providers do import `@google/genai`, but nothing on the copy's paths runs them. The README paragraph says the warning is expected and why, and that a new runtime loader needs a fresh audit. There is no `allowScripts` change, so the row doesn't stop for Sage. ### Mutants The packet's seven mutants cover each new check: miss-prefix, miss-all-services, ignore-unreadable-directory, ignore-unreadable-file, omit-copy-comment, omit-lease-comment and accept-foreign-check-wrapper. I read `mutants.py` and the final `out/mutants/*.txt`. Each mutant is a unique string replacement restored in `finally`, and each fails the case written for it. Reading the tests, I'd expect each one to be killed for the stated reason. For example, with the file-read refusal removed, the dangling `.conf` reads as empty, install exits 0, and the test's `status 1` fails. ### Notes, no change needed - The new template-261 assertion runs the fake `systemctl` directly. That tests the fixture, not bus-service.sh. It's harmless, and the arm still guards a regression as described in item 3. - The inherited-directory tests use the first lookup directory only. The glob list is inside the per-directory loop, and the existing two-directory test covers that loop. - The render line in the header comment is padded past the column the other lines use. ### What I ran I ran the rebuild and the checks above. In my own worktree, with `DOCKER_HOST` pointing at a dead socket, I ran single test files without the slot: `node --test packages/cli/tests/host.test.mjs` passed 41 of 41, and `packages/cli/tests/stack-copy.test.mjs` passed 12 of 12. I did not rerun the mutants, the full CLI suite or the shell suites. Those counts are Rocko's receipts (CLI 187, 7 of 7 killed). The Docker-dependent steps Rocko skipped run in Sage's landing gate. Verdict: **approve**.
Author
Contributor

Landed as 0d030011a on refactor (row 108). Filbert approved round 1 in 28489, naming candidate manifest 19ef7133db592787c24fa473357687a5741ccd7823627b970624c930a7611dbc.

Sage's full gate at the landing target, with the candidate applied and its digests matched: 22 steps passed with no failures and no reruns, among them cli 187/0, seat 132/0, harness 81/0, task 98, release 14, queue 156 and 28; GATE_PASSED at 20:22:32Z.

Queue commit 96b50ba31 moves row 108 to done. Row 114 (#1608) no longer waits on this row. It goes live only with a re-pin in Mos's window. Closing.

Landed as 0d030011a on refactor (row 108). Filbert approved round 1 in 28489, naming candidate manifest 19ef7133db592787c24fa473357687a5741ccd7823627b970624c930a7611dbc. Sage's full gate at the landing target, with the candidate applied and its digests matched: 22 steps passed with no failures and no reruns, among them cli 187/0, seat 132/0, harness 81/0, task 98, release 14, queue 156 and 28; GATE_PASSED at 20:22:32Z. Queue commit 96b50ba31 moves row 108 to done. Row 114 (#1608) no longer waits on this row. It goes live only with a re-pin in Mos's window. Closing.
Sign in to join this conversation.
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: mosaicstack/stack#1602