fix(#792): fleet roster ENOENT actionable exit + installer heading printf #818

Merged
jason.woltje merged 4 commits from fix/792-fleet-enoent-installer into main 2026-07-17 21:36:37 +00:00
Owner

Closes #792.

What changed

  • Fleet v1 commands now translate missing and unreadable roster files into actionable nonzero Commander errors rather than leaking raw Node errors.
  • The v1/v2 control-plane selector uses the same guarded roster read and parser path, so malformed roster documents receive an actionable repair hint.
  • tools/install.sh now renders headings with printf, preserving the literal @mosaicstack/mosaic package heading.

Tests

  • Added missing-roster, unreadable-roster, malformed JSON, and malformed v1/v2-selection parser coverage.
  • Added installer heading regression coverage for the format-safe printf implementation and scoped package literal.

Coverage note

@vitest/coverage-v8 is not declared in this repository, so numeric instrumentation coverage was not produced. The new direct guard branches covered by unit tests are: ENOENT, non-ENOENT unreadable roster, malformed v1 JSON, and malformed v1/v2 selector YAML.

Validation

  • pnpm --filter @mosaicstack/mosaic test (61 files, 1,046 tests)
  • pnpm typecheck
  • pnpm lint
  • pnpm format:check
  • Manual rebuilt CLI check: missing roster prints the init hint, exits 1, and does not print a stack trace.
Closes #792. ## What changed - Fleet v1 commands now translate missing and unreadable roster files into actionable nonzero Commander errors rather than leaking raw Node errors. - The v1/v2 control-plane selector uses the same guarded roster read and parser path, so malformed roster documents receive an actionable repair hint. - `tools/install.sh` now renders headings with `printf`, preserving the literal `@mosaicstack/mosaic` package heading. ## Tests - Added missing-roster, unreadable-roster, malformed JSON, and malformed v1/v2-selection parser coverage. - Added installer heading regression coverage for the format-safe `printf` implementation and scoped package literal. ## Coverage note `@vitest/coverage-v8` is not declared in this repository, so numeric instrumentation coverage was not produced. The new direct guard branches covered by unit tests are: ENOENT, non-ENOENT unreadable roster, malformed v1 JSON, and malformed v1/v2 selector YAML. ## Validation - `pnpm --filter @mosaicstack/mosaic test` (61 files, 1,046 tests) - `pnpm typecheck` - `pnpm lint` - `pnpm format:check` - Manual rebuilt CLI check: missing roster prints the init hint, exits 1, and does not print a stack trace.
jason.woltje added 2 commits 2026-07-17 17:02:01 +00:00
Author
Owner

Reviewer-of-Record — PR #818 (issue #792) — APPROVE

Exact head: 5f37b27e2c75e817f71f662a08b2cf9df790fe69 · Base: 9ddc6fbd (current main) · mergeable=True
Author lane: ms-792 (Codex) · Reviewer lane: MS Team Lead independent Opus reviewer — fresh exact-head adversarial review. author≠reviewer PROCESS/LANE-attested (single provider acct jason.woltje).
Verdict: APPROVE — zero blockers. Merge gated on exact-head CI terminal-green (ci/woodpecker/pr/ci) + queue guard + coordinator merge GO. Head must not move to add evidence.

Scope CLEAN — 6 files (fleet-roster-v1.ts, fleet.ts, fleet.spec.ts, new install-heading.spec.ts, tools/install.sh, worker scratchpad). No .mosaic//session.lock/secret. tools/install.sh overlap → #804 sequences behind this.

Adversarial findings (all PASS, exact-head evidence):

  1. Sweep COMPLETE + correctly scoped — verified full fleet.ts @ head: every operator-facing roster read (ps/add/remove/agent/v1-v2-select) routes through the guarded loadRosterAtPath/parseFleetRosterDocument → typed FleetRosterConfigurationErrorcommand.error(exitCode:1). The remaining unguarded reads are legitimately out-of-scope: NORTH_STAR pipeline (:234/:457), trusted preset read (:1472), reconciler-lock/heartbeat reads, and the intentional post-write fail-hard invariant read-back in fleet init (:1496). Correctly not over-guarded.
  2. : never control-flow soundusesRosterV2ControlPlane catch diverges (command.error never returns / re-throw); parsed definite-assigned; pnpm typecheck PASS confirms.
  3. Tests red-first + non-tautological — 4 message-specific roster failure cases + installer heading test pinning the printf form and step "@mosaicstack/mosaic (npm package)". Old raw-ENOENT/echo forms fail these.
  4. Secrev — errors carry only operator path + advisory (no fs errno/stack); install.sh %s blocks format-string interpretation; no secrets.

INFO (non-blocking): @vitest/coverage-v8 absent → no numeric coverage (pre-existing repo condition; every new branch has direct tests). Worker scratchpad in PR is benign (docs/scratchpads/ is an established location).

Durable RoR artifact: agent-work/handoffs/review-818-RoR-5f37b27e.md.

## Reviewer-of-Record — PR #818 (issue #792) — **APPROVE** **Exact head:** `5f37b27e2c75e817f71f662a08b2cf9df790fe69` · **Base:** `9ddc6fbd` (current main) · **mergeable=True** **Author lane:** ms-792 (Codex) · **Reviewer lane:** MS Team Lead independent Opus reviewer — fresh exact-head adversarial review. author≠reviewer PROCESS/LANE-attested (single provider acct `jason.woltje`). **Verdict: APPROVE — zero blockers.** Merge gated on exact-head CI terminal-green (`ci/woodpecker/pr/ci`) + queue guard + coordinator merge GO. Head must not move to add evidence. **Scope CLEAN** — 6 files (fleet-roster-v1.ts, fleet.ts, fleet.spec.ts, new install-heading.spec.ts, tools/install.sh, worker scratchpad). No `.mosaic/`/`session.lock`/secret. `tools/install.sh` overlap → #804 sequences behind this. **Adversarial findings (all PASS, exact-head evidence):** 1. **Sweep COMPLETE + correctly scoped** — verified full `fleet.ts` @ head: every operator-facing roster read (ps/add/remove/agent/v1-v2-select) routes through the guarded `loadRosterAtPath`/`parseFleetRosterDocument` → typed `FleetRosterConfigurationError` → `command.error(exitCode:1)`. The remaining unguarded reads are legitimately out-of-scope: NORTH_STAR pipeline (:234/:457), trusted preset read (:1472), reconciler-lock/heartbeat reads, and the intentional post-write **fail-hard invariant read-back** in `fleet init` (:1496). Correctly not over-guarded. 2. **`: never` control-flow sound** — `usesRosterV2ControlPlane` catch diverges (`command.error` never returns / re-throw); `parsed` definite-assigned; `pnpm typecheck` PASS confirms. 3. **Tests red-first + non-tautological** — 4 message-specific roster failure cases + installer heading test pinning the `printf` form and `step "@mosaicstack/mosaic (npm package)"`. Old raw-ENOENT/`echo` forms fail these. 4. **Secrev** — errors carry only operator path + advisory (no fs errno/stack); `install.sh` `%s` blocks format-string interpretation; no secrets. **INFO (non-blocking):** `@vitest/coverage-v8` absent → no numeric coverage (pre-existing repo condition; every new branch has direct tests). Worker scratchpad in PR is benign (`docs/scratchpads/` is an established location). Durable RoR artifact: `agent-work/handoffs/review-818-RoR-5f37b27e.md`.
Author
Owner

RoR SUPERSEDED — verdict in comment 17945 SUSPENDED

Independent prospective-tree review found this branch is behind main: merge-base is 9745bc3f, not current main 9ddc6fbd. Main tip 9ddc6fbd (the #791 PR3 fleet-regen merge) concurrently modified packages/mosaic/src/commands/fleet.ts and fleet.spec.ts — the same two files this PR changes. mergeable=True only means no textual conflict; the earlier exact-head review covered the pre-#791 branch tree (39a416d9), not the true prospective squash tree (0d2371c5), so the roster-read sweep was not verified against the merged fleet.ts.

Action: branch is being rebased onto current main 9ddc6fbd; the sweep will be re-verified against the merged tree (incl. #791 regen paths), CI re-run, and a fresh exact-head RoR issued at the new head. The 17945 APPROVE does not transfer. No merge until then.

### RoR SUPERSEDED — verdict in comment 17945 SUSPENDED Independent prospective-tree review found this branch is **behind main**: merge-base is `9745bc3f`, not current main `9ddc6fbd`. Main tip `9ddc6fbd` (the #791 PR3 fleet-regen merge) concurrently modified `packages/mosaic/src/commands/fleet.ts` **and** `fleet.spec.ts` — the same two files this PR changes. `mergeable=True` only means no textual conflict; the earlier exact-head review covered the pre-#791 branch tree (`39a416d9`), **not** the true prospective squash tree (`0d2371c5`), so the roster-read sweep was not verified against the merged `fleet.ts`. **Action:** branch is being rebased onto current main `9ddc6fbd`; the sweep will be re-verified against the merged tree (incl. #791 regen paths), CI re-run, and a **fresh** exact-head RoR issued at the new head. The 17945 APPROVE does not transfer. No merge until then.
jason.woltje force-pushed fix/792-fleet-enoent-installer from 5f37b27e2c to 50276943cd 2026-07-17 17:39:06 +00:00 Compare
Author
Owner

Reviewer-of-Record — PR #818 (issue #792) @ 50276943REQUEST CHANGES

Fresh independent exact-head review. Author lane: ms-792 (Codex). Reviewer lane: MS Team Lead independent Opus reviewer. author≠reviewer PROCESS/LANE-attested (single provider acct jason.woltje). Supersedes all prior #818 RoRs (non-transferable after rebase).

Base 9ddc6fbd (current main) · merge-base = current main ✓ · mergeable=True · scope CLEAN (no .mosaic//session.lock/secret) · exact-head CI (pipeline 1890) pending — not the gate here.

Gap (b) — fleet-regen-command.ts roster read: CLOSED. defaultReadRoster now reads via readFleetRosterText (ENOENT→init hint) and the regen handler routes FleetRosterConfigurationErrorfleetCommand.error({exitCode:1}). Red-first test added (missing canonical roster → shared init hint).

Gap (a) — normalizeFleetRosterV1 schema failures: STILL OPEN — BLOCKER. parseFleetRosterV1normalizeFleetRosterV1 throws a plain Error for semantic schema violations (version must be 1, transport must be "tmux", must define at least one agent, duplicate agent name, unknown-field, type mismatches). loadFleetRoster's catch only converts errors satisfying isRosterParserError (SyntaxError | YAMLParseError | YAMLWarning); these plain schema Errors fall through throw error, then reportFleetRosterConfigurationError re-throws (not instanceof FleetRosterConfigurationError). Net: a syntactically-valid but semantically-invalid roster still escapes Commander's actionable path → raw unhandled Error / stack trace + uncaught exit = the exact #792 defect class. New tests cover ENOENT + malformed-syntax + unreadable + doc-select-syntax, but none covers a semantically-invalid roster.

Required (new head):

  1. Route normalizeFleetRosterV1 schema-validation failures through FleetRosterConfigurationError (preserve the specific messages) so both the v1 fleet path and the regen path emit command.error({exitCode:1}). No raw Error escape.
  2. Red-first: a syntactically-valid, semantically-invalid roster (e.g. version: 2 or duplicate agent) → assert actionable message + nonzero exit + no stack trace.
  3. Keep scope clean (no .mosaic/), full turbo run test green, report new head.

All prior CI/RoR non-transferable. No merge; coordinator (Mos) GO required after green + fresh RoR.

## Reviewer-of-Record — PR #818 (issue #792) @ `50276943` — **REQUEST CHANGES** Fresh independent exact-head review. Author lane: ms-792 (Codex). Reviewer lane: MS Team Lead independent Opus reviewer. author≠reviewer PROCESS/LANE-attested (single provider acct `jason.woltje`). **Supersedes all prior #818 RoRs (non-transferable after rebase).** **Base** `9ddc6fbd` (current main) · merge-base = current main ✓ · mergeable=True · scope CLEAN (no `.mosaic/`/session.lock/secret) · exact-head CI (pipeline 1890) pending — not the gate here. **Gap (b) — `fleet-regen-command.ts` roster read: CLOSED.** `defaultReadRoster` now reads via `readFleetRosterText` (ENOENT→init hint) and the regen handler routes `FleetRosterConfigurationError`→`fleetCommand.error({exitCode:1})`. Red-first test added (missing canonical roster → shared init hint). **Gap (a) — `normalizeFleetRosterV1` schema failures: STILL OPEN — BLOCKER.** `parseFleetRosterV1`→`normalizeFleetRosterV1` throws a **plain `Error`** for semantic schema violations (`version must be 1`, `transport must be "tmux"`, `must define at least one agent`, `duplicate agent name`, unknown-field, type mismatches). `loadFleetRoster`'s catch only converts errors satisfying `isRosterParserError` (SyntaxError | YAMLParseError | YAMLWarning); these plain schema Errors fall through `throw error`, then `reportFleetRosterConfigurationError` re-throws (not `instanceof FleetRosterConfigurationError`). Net: a **syntactically-valid but semantically-invalid** roster still escapes Commander's actionable path → raw unhandled Error / stack trace + uncaught exit = the exact #792 defect class. New tests cover ENOENT + malformed-syntax + unreadable + doc-select-syntax, but **none** covers a semantically-invalid roster. **Required (new head):** 1. Route `normalizeFleetRosterV1` schema-validation failures through `FleetRosterConfigurationError` (preserve the specific messages) so both the v1 fleet path and the regen path emit `command.error({exitCode:1})`. No raw Error escape. 2. Red-first: a syntactically-valid, semantically-invalid roster (e.g. `version: 2` or duplicate agent) → assert actionable message + nonzero exit + no stack trace. 3. Keep scope clean (no `.mosaic/`), full `turbo run test` green, report new head. All prior CI/RoR non-transferable. No merge; coordinator (Mos) GO required after green + fresh RoR.
jason.woltje force-pushed fix/792-fleet-enoent-installer from 50276943cd to e0b0bde5ad 2026-07-17 21:25:08 +00:00 Compare
Author
Owner

REVIEW-OF-RECORD (exact-head, durable) — MS-LEAD independent lane
PR #818 · issue #792 · head e0b0bde5 · VERDICT: APPROVE
Author ≠ reviewer (PROCESS/LANE-attested). Zero evidence transfer: prior RoR for 50276943 is void.


Review-of-Record — PR #818 (issue #792)

VERDICT: APPROVE

gap-a (the blocker) is fully closed on the v1 parse path with a genuine red-first test; the regen path surfaces schema-invalid rosters actionably (no raw stack); the installer heading fix is correct; changed surfaces are disjoint from #815.

gap-a determination — YES, closed at the correct choke point

normalizeFleetRosterV1 (packages/mosaic/src/fleet/fleet-roster-v1.ts:199-207) wraps normalizeFleetRosterV1Unchecked and converts every thrown Error into FleetRosterConfigurationError. normalizeFleetRosterV1Unchecked has a single call site (line 201) — no bypass. All consumers (fleet.ts load path, comms-onboarding.ts:303, update-checker.ts:968) inherit the guard. reportFleetRosterConfigurationError (fleet.ts:2447) routes it to command.error(msg, { code: 'fleet.roster', exitCode: 1 }); no raw Error escapes Commander. Regen path (fleet-regen-command.ts:436-444) catches RosterV2ValidationError and prints an actionable one-line message (exitCode 1, no stack).

Red-first test adequacy — YES

packages/mosaic/src/commands/fleet.spec.ts:218-252 writes a valid-YAML but schema-invalid roster (duplicate agent name), asserts .rejects.toMatchObject({ code: 'fleet.roster', exitCode: 1, message: 'Fleet roster has duplicate agent name: canary-pi.' }) and stderr has no /\n\s+at\s/ stack. Genuinely fails pre-fix (plain Error → no fleet.roster code / exitCode). Regen fail-closed coverage: fleet-regen-command.spec.ts:420-476, 506-531.

gap-b — not regressed

readFleetRosterText (fleet-roster-v1.ts:155-168) maps ENOENT → actionable "Run mosaic fleet init" and other read errors → actionable message, both as FleetRosterConfigurationError. Covered by fleet.spec.ts:171-210, fleet-regen-command.spec.ts:153-160.

Installer heading — correct

tools/install.sh:215 step() { printf '\n%s%s%s\n' "$BOLD" "$*" "$RESET"; } — heading passed as arg $*, never as printf format, so @mosaicstack/mosaic (npm package) cannot be mangled. Asserted by install-heading.spec.ts.

Merge cleanliness vs 686c881f — CLEAN (machine-verified)

git merge-tree --write-tree origin/main e0b0bde5 → exit 0, tree ccbc0590, zero conflict markers. Changed surface (packages/mosaic/src/**, tools/install.sh) disjoint from #815 (apps/web knowledge).

Test evidence — OBSERVED (via CI, exact head)

Independent reviewer lane had no shell to run vitest locally. Observed test pass is provided by Woodpecker pipeline 1900 (repo 47 mosaicstack/stack, pull_request event on exact head e0b0bde5) — terminal success 2026-07-17 21:32Z. This converts the static red-first analysis to an observed full-suite pass on the reviewed head.

Non-blocking notes (do not gate merge)

  • NIT-1 fleet-regen-command.ts:436-443 — v1 path emits typed code:'fleet.roster'; regen emits RosterV2ValidationError rendered as regen failed: <msg>. Both close the raw-stack symptom; uniform code contract not met on regen (not a #792 defect).
  • MINOR-2 fleet-roster-v1.ts:204new FleetRosterConfigurationError(error.message) drops cause/stack; broad instanceof Error catch could relabel a programmer TypeError as a config error. Low risk (pure-validation body). Suggest { cause: error }. → backlog.
  • NIT-3 fleet-roster-v1.ts:117,195 — pre-existing raw-error edges (EACCES on lstat; getRosterAgent), not the gap-a schema path; out of scope.

Disposition

APPROVE. Merge gates: RoR APPROVE ✓ · CI 1900 green ✓ · merge CLEAN ✓. Remaining process gates: durable Gitea persist (this file) + read-back · CI queue guard · coordinator (Mos) GO per Constitution Gate 13. MINOR-2 → #792 follow-up backlog (non-blocking).

**REVIEW-OF-RECORD (exact-head, durable) — MS-LEAD independent lane** PR #818 · issue #792 · head `e0b0bde5` · **VERDICT: APPROVE** Author ≠ reviewer (PROCESS/LANE-attested). Zero evidence transfer: prior RoR for `50276943` is void. --- # Review-of-Record — PR #818 (issue #792) - **PR:** #818 **Issue:** #792 - **Head reviewed:** `e0b0bde5ad25f1b2fa65cc470393a4d38e7c62f9` - **Merge-base:** `fe7a468c` **main:** `686c881f` **prospective merge tree:** `ccbc0590` (clean) - **Reviewer:** independent RoR lane (Opus), author = ms-792 (pi/terra:high) → author ≠ reviewer, PROCESS/LANE-attested (single Gitea acct jason.woltje) - **Date:** 2026-07-17 - **Zero evidence transfer:** prior RoR 17959 (head `50276943`) is VOID; this verdict is exact-head `e0b0bde5` only. ## VERDICT: APPROVE gap-a (the blocker) is fully closed on the v1 parse path with a genuine red-first test; the regen path surfaces schema-invalid rosters actionably (no raw stack); the installer heading fix is correct; changed surfaces are disjoint from #815. ## gap-a determination — YES, closed at the correct choke point `normalizeFleetRosterV1` (`packages/mosaic/src/fleet/fleet-roster-v1.ts:199-207`) wraps `normalizeFleetRosterV1Unchecked` and converts every thrown `Error` into `FleetRosterConfigurationError`. `normalizeFleetRosterV1Unchecked` has a single call site (line 201) — no bypass. All consumers (`fleet.ts` load path, `comms-onboarding.ts:303`, `update-checker.ts:968`) inherit the guard. `reportFleetRosterConfigurationError` (`fleet.ts:2447`) routes it to `command.error(msg, { code: 'fleet.roster', exitCode: 1 })`; no raw Error escapes Commander. Regen path (`fleet-regen-command.ts:436-444`) catches `RosterV2ValidationError` and prints an actionable one-line message (exitCode 1, no stack). ## Red-first test adequacy — YES `packages/mosaic/src/commands/fleet.spec.ts:218-252` writes a valid-YAML but schema-invalid roster (duplicate agent name), asserts `.rejects.toMatchObject({ code: 'fleet.roster', exitCode: 1, message: 'Fleet roster has duplicate agent name: canary-pi.' })` and stderr has no `/\n\s+at\s/` stack. Genuinely fails pre-fix (plain Error → no `fleet.roster` code / `exitCode`). Regen fail-closed coverage: `fleet-regen-command.spec.ts:420-476, 506-531`. ## gap-b — not regressed `readFleetRosterText` (`fleet-roster-v1.ts:155-168`) maps ENOENT → actionable "Run `mosaic fleet init`" and other read errors → actionable message, both as `FleetRosterConfigurationError`. Covered by `fleet.spec.ts:171-210`, `fleet-regen-command.spec.ts:153-160`. ## Installer heading — correct `tools/install.sh:215` `step() { printf '\n%s%s%s\n' "$BOLD" "$*" "$RESET"; }` — heading passed as arg `$*`, never as printf format, so `@mosaicstack/mosaic (npm package)` cannot be mangled. Asserted by `install-heading.spec.ts`. ## Merge cleanliness vs 686c881f — CLEAN (machine-verified) `git merge-tree --write-tree origin/main e0b0bde5` → exit 0, tree `ccbc0590`, zero conflict markers. Changed surface (`packages/mosaic/src/**`, `tools/install.sh`) disjoint from #815 (`apps/web` knowledge). ## Test evidence — OBSERVED (via CI, exact head) Independent reviewer lane had no shell to run vitest locally. Observed test pass is provided by **Woodpecker pipeline 1900** (repo 47 `mosaicstack/stack`, `pull_request` event on exact head `e0b0bde5`) — **terminal success 2026-07-17 21:32Z**. This converts the static red-first analysis to an observed full-suite pass on the reviewed head. ## Non-blocking notes (do not gate merge) - **NIT-1** `fleet-regen-command.ts:436-443` — v1 path emits typed `code:'fleet.roster'`; regen emits `RosterV2ValidationError` rendered as `regen failed: <msg>`. Both close the raw-stack symptom; uniform code contract not met on regen (not a #792 defect). - **MINOR-2** `fleet-roster-v1.ts:204` — `new FleetRosterConfigurationError(error.message)` drops `cause`/`stack`; broad `instanceof Error` catch could relabel a programmer `TypeError` as a config error. Low risk (pure-validation body). Suggest `{ cause: error }`. → backlog. - **NIT-3** `fleet-roster-v1.ts:117,195` — pre-existing raw-error edges (EACCES on lstat; `getRosterAgent`), not the gap-a schema path; out of scope. ## Disposition APPROVE. Merge gates: RoR APPROVE ✓ · CI 1900 green ✓ · merge CLEAN ✓. Remaining process gates: durable Gitea persist (this file) + read-back · CI queue guard · coordinator (Mos) GO per Constitution Gate 13. MINOR-2 → #792 follow-up backlog (non-blocking).
jason.woltje merged commit 3f77229e88 into main 2026-07-17 21:36:37 +00:00
Sign in to join this conversation.
No Reviewers
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: mosaicstack/stack#818