From 32a0ffba13345e23a6102dc616437ec9addcd4ba Mon Sep 17 00:00:00 2001 From: "jason.woltje" Date: Thu, 16 Jul 2026 23:01:26 +0000 Subject: [PATCH] feat(mosaic): manifest-owned upgrade guard so updates never wipe operator config (#791) (#802) --- .woodpecker/ci.yml | 18 + docs/design/791-upgrade-config-protection.md | 290 +++++++++++++++ .../791-upgrade-config-protection.md | 216 +++++++++++ .../mosaic/framework/framework-manifest.txt | 85 +++++ packages/mosaic/framework/install.sh | 243 ++++++++----- .../mosaic/framework/tools/_lib/manifest.sh | 253 +++++++++++++ .../quality/scripts/test-install-migration.sh | 9 +- .../scripts/test-upgrade-manifest-guard.sh | 224 ++++++++++++ .../quality/scripts/test-upgrade-rollback.sh | 319 ++++++++++++++++ .../mosaic/src/config/file-adapter.test.ts | 29 +- packages/mosaic/src/config/file-adapter.ts | 43 +-- .../src/framework/manifest-parity.spec.ts | 343 ++++++++++++++++++ .../mosaic/src/framework/manifest.spec.ts | 327 +++++++++++++++++ packages/mosaic/src/framework/manifest.ts | 248 +++++++++++++ packages/mosaic/src/platform/file-ops.ts | 20 +- packages/mosaic/src/runtime/update-checker.ts | 10 +- .../src/stages/finalize-sync-abort.spec.ts | 136 +++++++ packages/mosaic/src/stages/finalize.ts | 22 +- 18 files changed, 2711 insertions(+), 124 deletions(-) create mode 100644 docs/design/791-upgrade-config-protection.md create mode 100644 docs/scratchpads/791-upgrade-config-protection.md create mode 100644 packages/mosaic/framework/framework-manifest.txt create mode 100644 packages/mosaic/framework/tools/_lib/manifest.sh create mode 100644 packages/mosaic/framework/tools/quality/scripts/test-upgrade-manifest-guard.sh create mode 100644 packages/mosaic/framework/tools/quality/scripts/test-upgrade-rollback.sh create mode 100644 packages/mosaic/src/framework/manifest-parity.spec.ts create mode 100644 packages/mosaic/src/framework/manifest.spec.ts create mode 100644 packages/mosaic/src/framework/manifest.ts create mode 100644 packages/mosaic/src/stages/finalize-sync-abort.spec.ts diff --git a/.woodpecker/ci.yml b/.woodpecker/ci.yml index 1789aa8..e3c1959 100644 --- a/.woodpecker/ci.yml +++ b/.woodpecker/ci.yml @@ -42,6 +42,23 @@ steps: - bash packages/mosaic/framework/tools/quality/scripts/check-resident-budget.sh --self-test - bash packages/mosaic/framework/tools/quality/scripts/check-resident-budget.sh + # Blocking gate (#791): a framework upgrade must never write or delete an + # operator-owned path. The HARD GATE proves an unanticipated operator sentinel + # survives a keep-mode reseed byte-identical (with rsync present AND absent — + # keep mode is a single cp-based path that must not depend on rsync), and that a + # corrupt/empty/missing manifest aborts fail-closed leaving operator files + # untouched (B2/B3). The rollback gate proves a mid-sync failure is rolled back + # from the pre-update snapshot (B1). The migration matrix pins the v2→v3 + # contract-file semantics. Pure bash, no node_modules — runs early alongside + # sanitization. + upgrade-guard: + image: *node_image + commands: + - apk add --no-cache bash rsync + - bash packages/mosaic/framework/tools/quality/scripts/test-upgrade-manifest-guard.sh + - bash packages/mosaic/framework/tools/quality/scripts/test-upgrade-rollback.sh + - bash packages/mosaic/framework/tools/quality/scripts/test-install-migration.sh + typecheck: image: *node_image commands: @@ -50,6 +67,7 @@ steps: depends_on: - install - sanitization + - upgrade-guard # lint, format, and test are independent — run in parallel after typecheck lint: diff --git a/docs/design/791-upgrade-config-protection.md b/docs/design/791-upgrade-config-protection.md new file mode 100644 index 0000000..2f1be6c --- /dev/null +++ b/docs/design/791-upgrade-config-protection.md @@ -0,0 +1,290 @@ +# Design — #791: Framework upgrades must not destroy operator-owned config under `~/.config/mosaic` + +- **Issue:** mosaicstack/stack#791 +- **Branch:** `feat/791-upgrade-config-protection` (off `origin/main` `9745bc3f`) +- **Author:** ms-791 worker lane +- **Status:** Phase 1 — DESIGN, awaiting MS-LEAD confirmation before implementation +- **Ratified scope (Mos-approved, not re-litigated):** deliver **(b) strict ownership separation [PRIMARY]** + **(a) transactional pre-update snapshot [safety net]** + **(d) regeneration-from-SSOT [recovery]**. **(c) periodic backup timer is DEFERRED** — noted as future work only. + +--- + +## 1. Current updater behavior + exact wipe mechanism (evidence) + +### 1.1 What runs on `mosaic update` + +`mosaic update` re-seeds the framework by invoking the **bash installer** in sync-only, keep mode: + +- `packages/mosaic/src/runtime/update-checker.ts:509` `buildReseedCommand()` returns + `bash /install.sh` with env `MOSAIC_SYNC_ONLY=1`, `MOSAIC_INSTALL_MODE=keep`, + `MOSAIC_HOME=`. +- The same `install.sh` is the direct/`tools/install.sh` upgrade path and the framework-vN migration path. + +So the destructive surface is **`packages/mosaic/framework/install.sh`**. + +### 1.2 The wipe + +`sync_framework()` (`install.sh:177`) performs, in `keep` mode: + +``` +rsync -a --delete --exclude .git --exclude .framework-version --exclude '*.pre-constitution.bak' \ + [--exclude "/$path" for each PRESERVE_PATHS entry] SOURCE_DIR/ TARGET_DIR/ +``` + +- `install.sh:199` — `rsync -a --delete`. **`--delete` prunes every path in `~/.config/mosaic` + that is NOT present in the shipped framework source**, unless excluded. +- `install.sh:47` — `PRESERVE_PATHS` is the **only** thing standing between `--delete` and operator + data. It is a _denylist of exclusions_: + ``` + PRESERVE_PATHS=("CONSTITUTION.md" "AGENTS.md" "SOUL.md" "USER.md" "TOOLS.md" "STANDARDS.md" + "memory" "sources" "credentials" "fleet/roster.yaml" "fleet/roster.json" "fleet/agents" + "fleet/run" "fleet/backlog" "fleet/roles.local") + ``` +- The cp-fallback (no rsync) is equally destructive: `install.sh:223` + `find "$TARGET_DIR" -mindepth 1 -maxdepth 1 ... -exec rm -rf {} +` then re-copies source, restoring + only PRESERVE_PATHS globs. + +**Root-cause model:** _"Everything under `~/.config/mosaic` is framework-owned and pruneable UNLESS +explicitly preserved."_ Any operator path the list forgets is destroyed on the next upgrade. + +### 1.3 The exact operator paths wiped + +Cross-referencing the issue's operator-owned list against `PRESERVE_PATHS`: + +| Operator path (issue #791) | In PRESERVE_PATHS? | Fate on `mosaic update` | +| ----------------------------------------------------------------- | --------------------------------------- | ----------------------- | +| `agents/*.conf` (per-agent runtime) | **NO** | **WIPED** | +| `policy/*.md` (operator overlays) | **NO** | **WIPED** | +| `*.local.md` (SOUL/USER/STANDARDS) | **NO** | **WIPED** | +| harvester / SOP artifacts + timers | **NO** | **WIPED** | +| `tools/_lib/credentials.json` | **NO** (`credentials/` dir ≠ this path) | **WIPED** | +| `fleet/agents/*.env` | yes (`fleet/agents`, added by #631) | survives | +| `memory/`, `fleet/roster.*`, `fleet/backlog`, `fleet/roles.local` | yes | survives | + +The `fleet/agents`, `memory`, `fleet/backlog` entries were **retro-added after prior incidents** +(#631). This whack-a-mole is the structural signature of a denylist. + +**Stale-comment evidence:** `update-checker.ts:492` claims the reseed preserves +"`SOUL/USER/*.local/credentials`" — but `PRESERVE_PATHS` contains **no `*.local` entry**. The code +documents protection it does not deliver. + +### 1.4 Second code path (TS) — already non-destructive, but drifted + +`FileConfigAdapter.syncFramework()` (`packages/mosaic/src/config/file-adapter.ts:157`) → +`syncDirectory()` (`packages/mosaic/src/platform/file-ops.ts:66`) is a **copy-overlay**: it copies +source over target and skips preserved paths, but **never deletes** target paths absent from source +(`file-ops.ts:77-109`). It is used by the wizard/init flow, not `mosaic update`. + +Two problems remain: + +1. Its `preservePaths` (`file-adapter.ts:164-185`) has **already diverged** from `install.sh` — it is + **missing `fleet/backlog` and `fleet/roles.local`**. Two hand-maintained denylists, drifted. This + is direct evidence for a single shared SSOT manifest. +2. Even non-destructive, it will happily _overwrite_ an operator file that collides with a + framework-shipped path unless that path is on its (incomplete) preserve list. + +### 1.5 Existing snapshot is inadequate for rollback + +`make_snapshot()`/`restore_snapshot()` (`install.sh:76-87`) copy `TARGET_DIR` to `mktemp -d` under +`/tmp`, restore **only on `ERR/INT/TERM` trap**, and are **deleted on success** (`cleanup_snapshot`, +`install.sh:345`). Consequences: ephemeral `/tmp`, no retention, no post-success rollback, and **no +`mosaic restore`**. It is crash-safety only, not the transactional safety net #791 requires. + +--- + +## 2. Fix (b) — Strict ownership separation [PRIMARY / root cause] + +### 2.1 Ownership model (invert to allow-list) + +Replace _"framework-owned unless preserved"_ with _"operator-owned unless framework-owned"_, resolved +**per target path** with operator carve-outs winning inside shared framework subtrees. + +Two declared lists, one SSOT data file shipped in the framework +(`framework/framework-manifest.json`), consumed by **both** bash and TS: + +- **`framework` globs** — paths the updater is entitled to create / overwrite / prune. Authored to + match exactly what the framework ships in `packages/mosaic/framework/` (e.g. `CONSTITUTION.md`, + `AGENTS.md`, `STANDARDS.md`, `TOOLS.md`, `guides/**`, `constitution/**`, `templates/**`, `tools/**`, + `skills/**`, `mcp/**`, `defaults/**`, `fleet/examples/**`, `fleet/roles/**`, `fleet/profiles/**`, + `fleet/roster.schema.json`). +- **`operatorReserved` globs** — NEVER written or pruned, even nested inside a `framework` subtree; + these **win** over `framework` (deny-wins / most-specific-wins). At minimum: + `agents/**`, `policy/**`, `memory/**`, `sources/**`, `credentials/**`, `*.local.md`, + `tools/_lib/credentials.json`, `fleet/roster.yaml`, `fleet/roster.json`, `fleet/agents/**`, + `fleet/run/**`, `fleet/backlog/**`, `fleet/roles.local/**`, plus operator harvester/SOP artifacts. + +### 2.2 Ownership resolution for a target path `P` + +1. `P` matches `operatorReserved` → **operator-owned**: updater MUST NOT write, MUST NOT delete. +2. else `P` matches `framework` → **framework-owned**: may overwrite; may prune **only if absent from + the current SOURCE** (a genuinely retired framework file). +3. else (matches neither) → **UNKNOWN ⇒ operator-owned by default (fail-safe)**: never delete. + +Rule 3 is the actual root-cause fix: an operator path the manifest authors forget is still protected, +because _unknown defaults to operator_. A denylist can never provide this guarantee. + +### 2.3 Sync mechanism change (the mechanically-critical part) + +`--delete` cannot express "prune only framework-owned" without re-enumerating every operator path +(the denylist trap). So: + +1. **Drop `--delete` from the bulk sync.** Copy `SOURCE → TARGET` non-destructively (writes/overwrites + all framework files; deletes nothing). rsync without `--delete`, or the existing overlay copy. +2. **Explicit manifest-scoped prune pass.** Iterate the **`framework` manifest** (not the whole tree); + for each framework path present in `TARGET` but **absent in `SOURCE`**, delete it — after + re-checking it does not match `operatorReserved`. Because the prune iterates only declared + framework globs, operator/unknown paths are **structurally unreachable** by deletion. + +This is implemented in both bash `sync_framework()` and TS `syncFramework()` from the shared manifest. +A pure **prune-planner** function (TS) computes the delete-set from +`(manifest, sourceListing, targetListing)` so the invariant is unit-testable in isolation. +`PRESERVE_PATHS` becomes redundant (kept as a defense-in-depth alias mapping to `operatorReserved`, or +removed) — either way the two lists stop drifting because they read one file. + +### 2.4 HARD GATE test — "upgrade touches no path outside the manifest" + +Filesystem-observation test in the existing `test-install-migration.sh` harness pattern (mktemp +`MOSAIC_HOME`, `MOSAIC_SYNC_ONLY=1`), plus TS specs: + +1. Seed a throwaway `TARGET` with a realistic operator mix — one sentinel per operator class: + `agents/x.conf`, `policy/p.md`, `SOUL.local.md`, `memory/m.md`, + `tools/_lib/credentials.json` (with a secret value), `fleet/agents/a.env`, `fleet/roster.yaml`, + `harvester/sop.md`, **and a deliberately-unanticipated `unknown-operator-dir/x`**. +2. Record hash+mtime of every sentinel. +3. Run the upgrade from a `SOURCE` containing none of those operator paths. +4. **Assert:** every sentinel exists, byte-identical, **mtime unchanged** (not even rewritten). The + `unknown-operator-dir` surviving proves the fail-safe default — a denylist could not pass this case. +5. **Positive controls:** framework files WERE updated; a retired framework file WAS pruned. +6. **Property test** (TS prune-planner): for fuzzed operator paths, `deleteSet ⊆ {matches framework ∧ +in target ∧ not in source}` and `deleteSet ∩ operatorReserved = ∅`. + +--- + +## 3. Fix (a) — Transactional pre-update snapshot [safety net] + +- **Destination:** `${XDG_STATE_HOME:-~/.local/state}/mosaic/backups/pre-update-/`. + **Outside `~/.config/mosaic`** (so no future sync can sweep it) and outside any repo. +- **Perms:** dir `0700`, files `0600` — enforced with `umask 077` around the copy **and** explicit + `chmod`. Never world-readable. +- **Scope:** the operator-owned surface (`operatorReserved` paths that exist) — bounded; does not copy + the framework tree. +- **Timing:** taken before ANY mutation in the upgrade flow. +- **Post-sync verify + selective restore:** after sync, diff the operator surface against the snapshot; + since (b) should never touch operator paths, any diff means a manifest bug — restore the affected + paths from the snapshot and warn loudly. This is precisely (a) catching a miss in (b). +- **Retention:** keep N most-recent (default 5; `MOSAIC_BACKUP_RETENTION` override); prune older. +- **`mosaic restore`:** `--list` (default, dry-run) enumerates snapshots by timestamp; + `--from ` restores that snapshot over the operator surface, confirmation-gated. Reports + counts/paths only. +- **Secret-safety:** snapshot copy and restore never emit file **contents**; only paths/counts. + Tests assert `0700/0600` and that no secret value appears in stdout/stderr. + +--- + +## 4. Fix (d) — Regeneration-from-SSOT [recovery] + +The incident's live blast radius: `fleet/agents/*.env` (systemd `EnvironmentFile` sources) gone → +`mosaic-agent@` boots **unit defaults** on restart (because `EnvironmentFile=-...` is +absent-tolerant) → **silent identity/runtime/workdir downgrade**. + +The SSOT for those `.env` files is the roster. The reconciler **already** separates a +`regenerate-projections-from-roster` projection phase from lifecycle +(`packages/mosaic/src/fleet/fleet-reconciler.ts:93,234`; env rendering in +`generated-env-boundary.ts:149-264`). + +**`mosaic fleet regen`** is therefore a **thin recovery-framed wrapper over the existing projection +phase** — it does NOT reimplement fleet logic and does NOT preempt in-flight FCM cards (M4/M5): + +- Regenerates derivable config (per-agent `*.env.generated`, unit files) from roster SSOT. +- **Preview-first:** dry-run default; `--write` to apply. Idempotent. +- **Never restarts agents** (the recovery order forbids restart-before-verify). +- Prints the runbook's next step (verify `EnvironmentFile` resolves, THEN restart). + +Alternatively documentable as `install.sh --relink` per the issue; `mosaic fleet regen` is preferred +because it reuses the merged reconciler plumbing. + +--- + +## 5. Secret-safety approach (secrev surface) + +- Snapshots/backups: `0700`/`0600`, outside any repo, never world-readable. (§3) +- No secret **value** ever emitted to logs/stdout/stderr by snapshot, restore, sync, or regen — + paths/counts only. Adversarial test: a secret value placed in `tools/_lib/credentials.json` must + never appear in installer or command output. +- `tools/_lib/credentials.json` is an explicit `operatorReserved` carve-out inside the framework-owned + `tools/**` subtree — it is never overwritten or pruned. +- The HARD GATE test doubles as a secret-safety test (asserts the credentials sentinel is untouched). + +--- + +## 6. Test plan (TDD, tests-first, ≥85% on new code, co-located `*.spec.ts`) + +1. **Manifest SSOT parity** — bash and TS resolve identical framework/operator sets from the one file; + a test fails if either path hard-codes a divergent list. +2. **Manifest completeness** — every path shipped in `framework/` is covered by a `framework` glob (so + a new shipped file cannot silently fall outside the manifest and become un-prunable/undeclared). +3. **HARD GATE** — upgrade touches nothing outside the manifest, incl. the unanticipated-path case + (§2.4). +4. **Prune-planner** unit + property tests (§2.4.6). +5. **Snapshot** — perms `0700/0600`, correct destination, retention prune, secret value absent from + output. +6. **Restore** — `--list` / `--from` round-trip restores operator surface byte-exact; confirmation + gate; no secret leakage. +7. **Regen** — roster→env projection deterministic + idempotent; dry-run makes no writes; `--write` + restores `*.env`; **never** issues a lifecycle/restart call. +8. **Cross-path regression** — TS `syncFramework` and bash `install.sh` agree on a shared fixture + (closes the current #631-style drift). + +Gates before every push: `pnpm typecheck && pnpm lint && pnpm format:check` + mosaic package tests +green. Never `--no-verify`. + +--- + +## 7. web1 recovery runbook (operator-agnostic; web1 specifics live in the issue as evidence only) + +For a currently-wiped fleet EnvironmentFile state — **do NOT service-restart while +`fleet/agents/*.env` is absent** (a restart boots unit defaults and silently downgrades identity): + +1. **Regenerate:** `mosaic fleet regen --write` — rebuild `~/.config/mosaic/fleet/agents/*.env` from + roster SSOT. +2. **Verify each unit resolves to the intended runtime/workdir** _before_ any restart: + `systemctl --user show mosaic-agent@ -p EnvironmentFile` and confirm the generated env exists + and carries the intended `MOSAIC_AGENT_*` runtime/workdir values. +3. **Only then** `systemctl --user restart mosaic-agent@`, one unit at a time. + +If config (not just fleet env) was lost, `mosaic restore --list` → `mosaic restore --from ` before +step 1. + +--- + +## 8. Proposed PR split (reviewable; DAG-ordered) + +| PR | Scope | Depends | Review focus | +| --- | -------------------------------------------------------------------------------------------------------------------------------------------------------- | ------- | -------------------------- | +| PR1 | **PRIMARY** — shared `framework-manifest.json` + ownership resolver + non-deleting sync + scoped prune (bash + TS) + **HARD GATE** + prune-planner tests | — | correctness (root fix) | +| PR2 | **Safety net** — pre-update snapshot (`~/.local/state`, 0700/0600, retention) + post-sync verify/restore + `mosaic restore` | PR1 | **secrev** (backup/secret) | +| PR3 | **Recovery** — `mosaic fleet regen` (projection-only, preview-first, no restart) + docs (upgrade-safety + recovery runbook) | PR1 | correctness + docs | + +Rationale: PR1 closes the failure class on its own; if PR2/PR3 slip, the class stays fixed. Each PR is +one reviewable unit with its own tests ≥85%. Independent review (author≠reviewer) on all; **secrev** on +PR2 (and PR1's secret-sentinel assertions). + +## 9. Deferred (noted per scope) + +**(c) periodic backup timer** — a systemd user timer snapshotting operator dirs on a cadence +(defense-in-depth for non-upgrade losses). Explicitly **out of scope now**; future phase. + +## 10. Constraints honored + +- **Framework-PR firewall:** manifest + logic are operator-agnostic; no SOUL/USER/operator specifics + in framework code; web1 details are issue evidence only. +- **Capacity-fill:** must not preempt M5-001 or #790; `fleet regen` reuses merged FCM-M3 plumbing and + does not overlap FCM-M4/M5 migration cards. +- **Delivery gates:** TDD tests-first, ≥85% new-code coverage, trunk-based squash PRs, independent + review + secrev, completion = merged PR + descendant-main green + #791 closed. + +--- + +**Requesting MS-LEAD confirmation of:** (1) the manifest allow-list + non-deleting-sync + scoped-prune +approach as the (b) root-cause fix; (2) snapshot destination/retention + `mosaic restore` UX; +(3) `mosaic fleet regen` as a projection-only wrapper; (4) the 3-PR split. Implementation begins only +on your confirmation. diff --git a/docs/scratchpads/791-upgrade-config-protection.md b/docs/scratchpads/791-upgrade-config-protection.md new file mode 100644 index 0000000..4a5f39a --- /dev/null +++ b/docs/scratchpads/791-upgrade-config-protection.md @@ -0,0 +1,216 @@ +# Scratchpad — #791 Upgrade config protection (ms-791 worker lane) + +**Lane:** web1:ms-791 → reports to MS-LEAD (web1:mosaic-100). Do NOT contact Jason/Mos directly. +**Worktree:** `/home/hermes/agent-work/stack-agents-dir-791`, branch `feat/791-upgrade-config-protection` +off `origin/main` `9745bc3f` (verified exact head). + +## Mission prompt (verbatim intent) +Protect operator-owned config under `~/.config/mosaic` from framework-upgrade wipes. Ratified +combination (Mos-approved, do NOT re-litigate): (b) strict ownership separation [PRIMARY] + (a) +transactional pre-update snapshot [safety net] + (d) regeneration-from-SSOT [recovery]. (c) periodic +timer DEFERRED. HARD GATE: unit test that an upgrade run touches NO path outside the manifest. +Design-first: write design doc, send to MS-LEAD, WAIT for confirmation before impl. + +## Session 1 (2026-07-16) — Phase 1 design + +### Evidence gathered (wipe mechanism, file/line) +- `mosaic update` → `update-checker.ts:509` `buildReseedCommand` → `bash install.sh` + (`MOSAIC_SYNC_ONLY=1`, `MOSAIC_INSTALL_MODE=keep`). +- Wipe = `packages/mosaic/framework/install.sh:199` `rsync -a --delete` + `PRESERVE_PATHS` denylist + (`install.sh:47`). cp-fallback `install.sh:223` `find ... -exec rm -rf`. +- Denylist gaps → WIPED: `agents/*.conf`, `policy/*.md`, `*.local.md`, harvester/SOP, + `tools/_lib/credentials.json`. +- Stale comment `update-checker.ts:492` claims `*.local` preserved — PRESERVE_PATHS has no such entry. +- TS path `file-adapter.ts:157` → `file-ops.ts:66` `syncDirectory` = non-destructive copy-overlay, BUT + its preserve list (`file-adapter.ts:164`) already DRIFTED from install.sh (missing `fleet/backlog`, + `fleet/roles.local`). Evidence for single shared manifest SSOT. +- Existing snapshot (`install.sh:76`) = /tmp, crash-trap only, deleted on success → inadequate; no + `mosaic restore`. +- `fleet-reconciler.ts:93,234` already has `regenerate-projections-from-roster` phase separate from + lifecycle → `mosaic fleet regen` = thin projection-only wrapper (no restart), no FCM-M4/M5 preemption. + +### Design decisions +- **(b)** Invert to allow-list: shared `framework/framework-manifest.json` (framework globs + + operatorReserved carve-outs); resolve per-path, deny-wins; **UNKNOWN ⇒ operator (fail-safe)**. + Mechanism: drop `--delete`; non-deleting bulk copy + explicit manifest-scoped prune pass (iterate + framework globs only → operator/unknown structurally unreachable). Pure prune-planner fn for tests. +- **(a)** Snapshot to `~/.local/state/mosaic/backups/pre-update-/` 0700/0600, retention N=5, + post-sync verify+restore, `mosaic restore --list/--from`. No secret values in output. +- **(d)** `mosaic fleet regen` projection-only, preview-first, never restart. +- HARD GATE test includes a deliberately-unanticipated operator path to prove fail-safe default. +- **PR split:** PR1 manifest+guard (root fix, ships alone) → PR2 snapshot/restore (secrev) → PR3 + regen+docs. PR2/PR3 depend on PR1. + +### Status +Design doc written: `docs/design/791-upgrade-config-protection.md`. Sent to MS-LEAD. + +## Session 1 (cont.) — MS-LEAD CONFIRMED → Phase 2 GO +All 4 asks approved. Binding conditions: +- TDD tests-first, red-first proof per PR; ≥85% new-code; co-located `*.spec.ts`; never `--no-verify`. +- HARD GATE test (§2.4, unanticipated sentinel survives byte-identical + mtime unchanged) = MERGE-BLOCKING for PR1. +- Manifest-completeness test (§6.2) required. +- Bash+TS read ONE shared `framework-manifest.json`; parity test (§6.1) required (closes #631 drift class). +- UNKNOWN⇒operator (rule 3) non-negotiable. Keep prune-planner PURE. +- `fleet regen`: NEVER restart; dry-run default, `--write` to apply; "never issues restart" test mandatory. +- Independent review every PR; PR2 dedicated secrev. +- One PR at a time through DAG. Report PR1 exact head + red→green evidence for review commission. + +### Now: implementing PR1 (manifest + resolver + non-deleting sync + scoped prune + guard tests). + +## Session 2 (2026-07-16) — PR1 built, tests-first, red→green proven + +Deviation noted to MS-LEAD in PR: manifest is `framework-manifest.txt` (line-oriented), NOT `.json`. +Rationale: keep the bash installer free of a python3/jq dependency. The "ONE shared file, parity- +tested" requirement is honored — `manifest-parity.spec.ts` drives the bash resolver as a subprocess +and asserts byte-identical ownership vs the TS resolver over 34 probe paths spanning every class. + +### PR1 artifacts +- SSOT: `packages/mosaic/framework/framework-manifest.txt` ([framework]/[operator], deny-wins, fail-safe). +- TS resolver: `src/framework/manifest.ts` (pure: parse/matchGlob/resolveOwnership/frameworkSubtreeRoots/ + planPrune) + `manifest.spec.ts` (18 tests incl. planPrune property test + §6.2 completeness). +- Bash resolver: `framework/tools/_lib/manifest.sh` (compiled globs → fork-free `manifest_is_framework`; + CLI `resolve|subtree-roots|classify`). Sourced by install.sh. +- HARD GATE (§2.4): `framework/tools/quality/scripts/test-upgrade-manifest-guard.sh` — keep-mode reseed, + 10 operator sentinels (incl. unanticipated `unknown-operator-dir/x`, `harvester/sop.md`, + `fleet/my-fleet.yaml`) survive byte-identical + mtime-unchanged; retired framework file pruned; + secret value absent from output. RED=31 fail (orig install.sh) → GREEN=48 pass (fixed). +- install.sh: keep mode now manifest-driven (`sync_framework_keep`, no `--delete`); overwrite unchanged. + PRESERVE_PATHS denylist deleted. +- TS sync: `file-ops.syncDirectory` gains `isOperatorOwned` guard; `file-adapter.syncFramework` derives + it from `loadManifest` — hardcoded (drifted) preservePaths deleted. Fixture uses the REAL manifest. +- Parity: `manifest-parity.spec.ts` (§6.1) — bash↔TS agree on 34 paths + subtree roots. +- Migration matrix `test-install-migration.sh`: F6 flipped — `my-fleet.yaml` now MUST survive (fail-safe). +- CI: new merge-blocking `upgrade-guard` step (`.woodpecker/ci.yml`) runs both bash suites (adds rsync). +- update-checker.ts reseed comment corrected to the manifest model. + +### Gates (all green) +- `pnpm typecheck` ✓ · `pnpm lint` ✓ · `pnpm format:check` ✓ +- Full mosaic vitest: 1062 passed (cli-smoke needs `pnpm build` first — build-artifact dep, not this change). +- HARD GATE 48/48 · migration 21/21 · parity 3/3 · manifest 18/18 · file-adapter 8/8. + +### PR opened + reported (2026-07-16) +- **PR #802** http://git.mosaicstack.dev/mosaicstack/stack/pulls/802 — base `main`@`9745bc3f`, + head `34e55d4a` (commit `feat(mosaic): manifest-owned upgrade guard…`). 15 files, +1160/-142. +- Reported PR head + red→green evidence to MS-LEAD (web1:mosaic-100); queued (lead busy). + Standing by for the independent-review commission at head `34e55d4a`. +- **TWO items flagged to MS-LEAD for decision (awaiting reply):** + 1. Deviation `.txt` vs `.json` — confirm accept (parity-tested) or convert to `.json`+jq. + 2. `pr-create -i 791` appended `Fixes #791` → would auto-close the tracking issue on PR1 merge + while PR2/PR3 remain. Recommended edit to `Part of #791`; awaiting go-ahead to patch PR body. +- DO NOT start PR2/PR3 until PR1 merges (DAG; one PR at a time). + +### MS-LEAD ruling → #797 ledger-survival sentinel folded into PR1 (2026-07-16) +MS-LEAD ruled both my decisions: (1) `.txt` format ACCEPTED (parity must be strict/merge-blocking incl. +format edge cases + negative probe); (2) trailer `Fixes #791`→`Part of #791` APPROVED (patched PR #802 +body via Gitea API — tracking issue no longer auto-closes on PR1 merge). Plus Mos-ELEVATED merge-blocker +(spec `~/agent-work/planning/epic-796/791-ledger-survival-sentinel-SPEC.md`): #797 Runtime Session Ledger +must survive upgrade. Two coupled deliverables landed in PR1: +- (i) Carve-out: `fleet/run/**` was ALREADY an explicit `[operator]` entry — glob matches the spec's + pinned `fleet/run/**` EXACTLY, so NO divergence to route back to planner-opus. Strengthened its comment + to name the ledger (`fleet/run/sessions/` events.ndjson + ledger.json) so it is unmistakably load-bearing. +- (ii) HARD-GATE sentinel: seeded populated ledger (events.ndjson 3 events + ledger.json node+edge+gen, + 0600 under 0700) into test-upgrade-manifest-guard.sh sentinels; asserts byte-identical + mtime-unchanged + + dir-perms unchanged. Negative control (retired framework file IS pruned) relabeled explicitly. + HARD GATE now 58/58 (was 48). +- Decision-1 parity hardening: format-edge fixtures (comments/blanks/whitespace, duplicate+overlapping + globs deny-wins, section/glob-ordering independence) + explicit UNKNOWN→operator negative probe, driven + through BOTH resolvers via MANIFEST_FILE override. Parity 7/7 (was 3). +- RED-FIRST honesty note: the bash ledger sentinel stays GREEN even against the pre-fix installer (the + ledger was incidentally safe from the rsync --delete bug; overall pre-fix run 30/58 as expected). The + carve-out's TRUE load-bearing value (deny-wins if framework ownership ever broadens to `fleet/**`) is + isolated by a dedicated resolver-seam red→green in manifest.spec.ts: WITHOUT `fleet/run/**` operator + entry + hypothetical `fleet/**` framework → ledger resolves framework and planPrune DELETES it (RED); + WITH the carve-out → deny-wins → operator, unprunable (GREEN). manifest.spec.ts 21/21 (was 18). +- Gates all green: typecheck ✓ lint ✓ format:check ✓ · full mosaic vitest 1069 passed · HARD GATE 58/58 + · migration 21/21. Committing FORWARD on the branch (NOT rebasing 34e55d4a out from under review). + +### MS-LEAD REQUEST CHANGES @ 0a5e703a → B1/B2/B3 fixed red-first (2026-07-16) +MS-LEAD returned REQUEST CHANGES (routed merge-blockers satisfied; 2 CRITICAL reliability defects from +the commissioned independent review). Fixed forward on the branch, red-first: +- **B1 (CRITICAL) — dead ERR trap.** install.sh had `set -euo pipefail` (no `-E`), so the + `trap restore_snapshot ERR` never fired for a failure inside sync_framework_keep() (function body) — + a mid-sync abort left a half-written target with NO rollback. Fix: `set -Eeuo pipefail` (errtrace) + + disarm the trap at the top of restore_snapshot() to prevent re-entrancy. New gate + `test-upgrade-rollback.sh`: injects a mid-sync `cp` EACCES (read-only divergent framework file); + Part A asserts the shipped installer rolls back (restore message fires AND target byte-identical to + pre-upgrade); Part B control strips `-E` and asserts the rollback message does NOT fire (dead trap) — + self-verifying red→green. 7/7. +- **B2/B3 (CRITICAL) — empty/unreadable/malformed manifest divergence.** Pre-fix: TS `parseManifest('')` + returned `{framework:[],operator:[]}` (NO throw) → silent no-op "Installation complete"; bash aborted + fragilely (the `_manifest_compile` `"${MANIFEST_OPERATOR[@]:-}"` artifact returned 1 with no message) + AND the CLI dispatch swallowed manifest_load's rc (no `|| exit`) so `resolve` exited 0 resolving + everything operator. Fix (fail-loud + identical both langs): + * TS `parseManifest`: throw on zero framework entries; `loadManifest`: wrap read error → + "Cannot read framework manifest …". + * bash `manifest_load`: explicit unreadable guard (`[[ ! -r ]]`) + zero-`[framework]` guard, both loud + stderr + return 1; `_manifest_compile` gets explicit `return 0` (kills the empty-array artifact); + CLI dispatch `manifest_load … || exit 1`. + * `finalize.ts`: wrap syncFramework → `spin.stop('Framework sync aborted …')` + rethrow (never falls + through to "Installation complete"). + Tests: manifest.spec.ts +5 fail-closed (empty/comment-only/operator-only/empty-section/missing); + manifest-parity.spec.ts +7 failure-mode parity (both reject empty/comment-only/operator-only/ + empty-section/entry-before-header/unknown-header/missing — TS throws, bash CLI exits non-zero+stderr); + HARD GATE +4 end-to-end fail-closed matrices (empty/operator-only/malformed/missing → abort non-zero, + manifest error surfaced, every operator sentinel byte-identical). RED proven by reverting + manifest.ts+manifest.sh to HEAD → 12 new tests fail; restore → 40/40 green. +- **Non-blocking addressed.** MEDIUM install.sh:222 find-empty now warns on a real failure instead of + blanket `|| true`. LOW: corrected the "both destructive paths rsync vs cp" overstatement in the HARD + GATE header + cp-fallback comment + ci.yml (keep mode is a single cp-based path; the rsync-present vs + -absent runs prove rsync-independence). `.pre-constitution.bak` triage: single-shot backup is + intentional (reconcile_framework_files backs up once), no change. +- Gates: typecheck ✓ lint ✓ format:check ✓ · full mosaic vitest 1081 passed (was 1069, +12) · HARD GATE + 118/118 (was 58) · rollback 7/7 (new) · migration 21/21. No --no-verify. Rollback test wired into + ci.yml upgrade-guard. Committing FORWARD (no rebase of 34e55d4a/0a5e703a). + +### Codex round 2 (pre-push self-review) → blockers A/B + should-fix C fixed red-first (2026-07-16) +Before committing round 1 I re-ran codex on the change set; it surfaced two fresh reliability defects +and one messaging defect on the SAME rollback/manifest path. Fixed forward, red-first: +- **Blocker-A (CRITICAL) — signal trap resumed instead of terminating.** A bash INT/TERM handler that + merely `restore_snapshot` (returns) does NOT terminate the script — execution RESUMES past the + interrupt, cleans the snapshot and reports success, leaving a partial post-interrupt update. Fix: + `trap 'restore_snapshot; exit 1' ERR INT TERM` so both the errtrace (ERR) and signal (INT/TERM) paths + exit non-zero. Rollback test Part C: a `cp` shim that `kill -TERM $PPID` mid-sync then succeeds (so + set -e never fires and only the signal path governs) → asserts abort non-zero + restore fires + does + NOT print "file phase complete"; control strips `exit 1` and asserts the buggy resume-to-success. +- **Blocker-B (CRITICAL) — degenerate `[framework]` section resolved everything operator.** A manifest + whose framework entries are all empty / bare-dot (`/`, `./`, `.`, `..`) passed the non-empty guard yet + yielded zero usable globs → nothing is framework → a keep-mode sync silently no-ops (bash resolved + `operator`, exit 0). Fix (both langs, parity): reject when no entry has a char other than `/`/`.` — + TS `isUsableFrameworkGlob` = `/[^/.]/.test(normalizeRel(glob))`, throws `ManifestError`; bash mirror + loops `[[ "$(_manifest_norm "$_g")" =~ [^/.] ]]`, loud stderr + return 1. Tests: manifest.spec.ts + `it.each(['/','./','.','..','/\n./'])` throw; parity +3 `expectBothReject` (root-slash/dot-slash/ + bare-dot). RED: reverting the guard makes `[framework]\n/` resolve `operator` exit 0. +- **Should-fix-C — misleading abort message.** finalize.ts printed one generic "may be partially + applied" for every sync failure. A `ManifestError` is a PRE-sync validation abort (manifest is + validated before any copy) → nothing was written; conflating it with a mid-copy failure misdirects + recovery. Fix: introduce `ManifestError` (exported from manifest.ts, thrown by every fail-closed + parse/load path), and classify in finalize.ts — ManifestError → "no files were changed"; any other → + "may be partially applied". New co-located `finalize-sync-abort.spec.ts` (3 tests) asserts both + branches re-throw the original error + the correct message, and that config writes are never reached. + RED proven by collapsing the classification → the ManifestError test fails. +- Gates: typecheck ✓ lint ✓ format:check ✓ · full mosaic vitest 1094 (was 1081, +3 finalize-abort; + manifest specs already counted) · HARD GATE 193/193 · rollback 14/14 · migration 21/21. + +### Codex round 3 (pre-push self-review) → blockers D1/D2 fixed red-first (2026-07-16) +Re-ran codex again; it found two more rollback-path gaps `set -E` cannot catch. Fixed forward, red-first: +- **Blocker-D1 (CRITICAL) — `find` scan failures swallowed by process substitution.** Both the overlay + copy and the scoped prune consumed `< <(find … -print0)`. Bash does NOT propagate the producer's exit + status to the `while`, so an EACCES/I/O failure mid-scan truncates the file list yet leaves the loop + exiting 0 → a partial upgrade commits and reports success; the ERR/restore trap never fires. Fix: + `_scan_or_die` runs `find … -print0 > "$tmp"` to completion, checks its status, and returns non-zero + (→ ERR trap → restore) on failure; both loops now read from the checked temp file. Rollback test + Part D: a `find` shim that fails every `-print0` scan → shipped installer aborts non-zero + restores + + emits "Could not enumerate framework files" + target byte-identical; control neuters the `# D1-GUARD` + `return 1` → find failure swallowed, upgrade wrongly reports "file phase complete", no rollback. +- **Blocker-D2 (CRITICAL) — silent `set -e` exit on a failed target reset.** restore_snapshot did a bare + `rm -rf "$TARGET_DIR"; mkdir -p "$TARGET_DIR"` (trap disarmed, under set -e). If `rm`/`mkdir` fails — + possibly after `rm` deleted part of the target — the script exits immediately, skipping the cp AND the + recovery pointer, leaving a half-removed target and an orphaned snapshot the operator can't locate. + Fix: `if ! rm -rf … || ! mkdir -p …; then fail "Snapshot restore could not reset … preserved at: + $SNAPSHOT_DIR — copy it back …"; return 1; fi` (tested like the cp -a check; snapshot NOT deleted). + Rollback test Part E: cp-poison triggers restore + an `rm` shim fails `rm -rf ` → shipped + emits the recovery pointer, the named snapshot dir survives, secret value never leaked; control deletes + the recovery line → operator gets no pointer. RED: reverting D1+D2 → 7 shipped/control assertions fail. +- Gates: typecheck ✓ lint ✓ format:check ✓ · full mosaic vitest 1094 · HARD GATE 193/193 · + rollback 28/28 (was 14, +14 for D1/D2 with controls) · migration 21/21. shellcheck clean on new lines. + No --no-verify. Committing FORWARD (no rebase of 34e55d4a/0a5e703a). diff --git a/packages/mosaic/framework/framework-manifest.txt b/packages/mosaic/framework/framework-manifest.txt new file mode 100644 index 0000000..8c328cf --- /dev/null +++ b/packages/mosaic/framework/framework-manifest.txt @@ -0,0 +1,85 @@ +# Mosaic framework path-ownership manifest — SSOT for the updater. +# +# This single file is the source of truth consumed by BOTH the bash installer +# (packages/mosaic/framework/install.sh) and the TypeScript config adapter +# (packages/mosaic/src/config/file-adapter.ts). A parity test asserts both +# paths resolve the same ownership from this file, so the two can never drift +# (the failure mode that #631 patched by hand in two places). +# +# Format: one glob per line, relative to the mosaic home (~/.config/mosaic). +# - Lines starting with '#' and blank lines are ignored. +# - '[framework]' / '[operator]' switch the active section. +# - '**' matches any depth; '*' matches within a single path segment. +# +# Ownership resolution for a path P (deny-wins / fail-safe): +# 1. P matches an [operator] glob -> operator-owned. +# 2. else P matches a [framework] glob -> framework-owned. +# 3. else (matches neither) -> OPERATOR-OWNED BY DEFAULT. +# +# Rule 3 is the root-cause fix for #791: a path the manifest authors never +# anticipated is protected because UNKNOWN defaults to operator. The updater +# may only ever create/overwrite framework-owned paths, and may only prune a +# framework-owned path that lives inside a shipped framework subtree and is +# absent from the current framework source (a genuinely retired file). +# Operator-owned and unknown paths are structurally unreachable by pruning. + +[framework] +# Top-level framework contract files (also reconciled from defaults/ on upgrade). +CONSTITUTION.md +AGENTS.md +STANDARDS.md +# Shipped framework subtrees — pruning is scoped to these roots. +adapters/** +constitution/** +CONTRIBUTING.md +defaults/** +examples/** +guides/** +install.sh +install.ps1 +LICENSE +profiles/** +runtime/** +systemd/** +templates/** +tools/** +# Fleet: only the framework-seeded fleet subtrees are framework-owned. +fleet/README.md +fleet/examples/** +fleet/profiles/** +fleet/roles/** +fleet/roster.schema.json +fleet/services/** +# The manifest itself is framework-owned. +framework-manifest.txt + +[operator] +# Identity / user-seeded contract files — generated by the wizard or seeded +# once from defaults/, then owned by the operator. Never overwritten on upgrade. +SOUL.md +USER.md +TOOLS.md +# Local overlays (tighten-only) authored by the operator. +*.local.md +# Operator-owned trees the updater must never write over or prune. +agents/** +policy/** +memory/** +sources/** +credentials/** +# Secret-bearing operator file INSIDE the framework-owned tools/ subtree. +# Listed explicitly so the deny-wins rule carves it out of tools/**. +tools/_lib/credentials.json +# Operator-owned fleet state (roster SSOT, per-agent env, heartbeats, backlog, +# persona overrides). Losing these silently downgrades a running fleet (#791). +fleet/roster.yaml +fleet/roster.json +fleet/agents/** +# Runtime state, incl. the #797 Runtime Session Ledger at fleet/run/sessions/ +# (events.ndjson journal + ledger.json projection). This carve-out is the +# mechanism that makes the ledger upgrade-safe: an upgrade that wiped it would +# defeat its reason to exist. The HARD GATE (test-upgrade-manifest-guard.sh) +# proves a populated ledger survives byte-identical + mtime-unchanged. +fleet/run/** +fleet/backlog/** +fleet/roles.local/** diff --git a/packages/mosaic/framework/install.sh b/packages/mosaic/framework/install.sh index 4e437f4..e176ff5 100755 --- a/packages/mosaic/framework/install.sh +++ b/packages/mosaic/framework/install.sh @@ -1,5 +1,10 @@ #!/usr/bin/env bash -set -euo pipefail +# -E (errtrace): the ERR trap must propagate INTO functions and command +# substitutions. Without it the `trap restore_snapshot ERR` set below is dead +# code for any failure inside sync_framework_keep() (its whole body runs in a +# function) — a mid-sync failure would abort with a half-written target and NO +# rollback (#791 B1). Keep -E first so every later function inherits the trap. +set -Eeuo pipefail # ─── Mosaic Framework Installer ────────────────────────────────────────────── # @@ -19,32 +24,19 @@ SOURCE_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" TARGET_DIR="${MOSAIC_HOME:-$HOME/.config/mosaic}" INSTALL_MODE="${MOSAIC_INSTALL_MODE:-prompt}" -# Files/dirs protected from rsync --delete during sync. NOTE: framework-owned -# entries (CONSTITUTION/AGENTS/STANDARDS) ARE re-applied afterward by -# reconcile_framework_files (overwrite + backup-once); the rest stay user-owned. -# User-created content in these paths survives rsync --delete. -# -# fleet/* — the framework SEEDS fleet/examples, fleet/roles, fleet/profiles, and -# fleet/roster.schema.json (synced normally — every fleet/roles/*.md role contract -# and fleet/profiles/*.yaml system-type profile lands automatically via this sync, -# so no per-file entry is needed; exact preserved roster paths are anchored to -# the top level only and do NOT shadow fleet/profiles/*.yaml). The user's -# own fleet files MUST -# survive `mosaic update` (which runs this sync automatically): the active -# rosters (`fleet/roster.yaml` and `fleet/roster.json`), per-agent env -# (`fleet/agents/`), heartbeat run dir (`fleet/run/`), and the Mosaic-native -# backlog-of-record store (`fleet/backlog/` — embedded PGlite data dir; see -# packages/mosaic/src/commands/fleet-backlog.ts). Without these, an update -# wipes the operator's fleet AND their backlog. Glob entries are honored by -# both the rsync path (`--exclude`) and the glob-aware cp fallback below. -# -# fleet/roles.local — the persona OVERRIDE layer (H4). Baseline personas in -# fleet/roles/ are reseeded normally on every update (delivering new baseline -# personas), so any local edit there would be clobbered. User customizations -# and user-ADDED personas instead live in fleet/roles.local/ and MUST survive -# `mosaic update` — they win over the baseline on merge (AC-NS-7; see -# packages/mosaic/src/commands/fleet-personas.ts). -PRESERVE_PATHS=("CONSTITUTION.md" "AGENTS.md" "SOUL.md" "USER.md" "TOOLS.md" "STANDARDS.md" "memory" "sources" "credentials" "fleet/roster.yaml" "fleet/roster.json" "fleet/agents" "fleet/run" "fleet/backlog" "fleet/roles.local") +# Shared framework path-ownership manifest reader (#791). Parity with +# packages/mosaic/src/framework/manifest.ts — both consume framework-manifest.txt. +# Sourcing does not run its CLI dispatch (guarded by BASH_SOURCE==$0). +# shellcheck source=tools/_lib/manifest.sh +source "$SOURCE_DIR/tools/_lib/manifest.sh" + +# Which paths a keep-mode upgrade may touch is no longer a hand-maintained +# denylist. It is derived from the shared framework-manifest.txt (#791): the +# updater only ever creates/overwrites framework-owned paths and only prunes a +# retired framework file inside a shipped framework subtree. Everything else — +# every operator file, and every path the manifest never anticipated — is +# operator-owned by default (fail-safe) and is never written or deleted. See +# sync_framework_keep() below and packages/mosaic/src/framework/manifest.ts. # Framework-owned contract files: re-copied from defaults/ on every upgrade (the # user must not edit them; a divergent copy is backed up once before overwrite). @@ -75,14 +67,45 @@ step() { echo -e "\n${BOLD}$1${RESET}"; } SNAPSHOT_DIR="" make_snapshot() { is_existing_install || return 0 + # mktemp -d creates the dir 0700 — the snapshot (which mirrors operator config, + # possibly including secrets) is never world-readable. SNAPSHOT_DIR="$(mktemp -d "${TMPDIR:-/tmp}/mosaic-snapshot-XXXXXX")" - cp -a "$TARGET_DIR/." "$SNAPSHOT_DIR/" 2>/dev/null || true + # The snapshot MUST be complete: restore rebuilds the target from it, so a + # partial capture (unreadable file, disk-full, I/O error) would silently + # discard whatever it missed. If cp -a cannot copy the whole tree, abort NOW — + # before the restore trap is armed and before anything is mutated. Fail closed + # rather than proceed with a snapshot we cannot trust (#791 blocker-2). + if ! cp -a "$TARGET_DIR/." "$SNAPSHOT_DIR/"; then + fail "Could not capture a complete pre-upgrade snapshot of $TARGET_DIR — aborting before any changes were made (fail-closed)." + rm -rf "$SNAPSHOT_DIR"; SNAPSHOT_DIR="" + exit 1 + fi } restore_snapshot() { + # Disarm the trap first: restore runs under `set -e`, and a non-zero step + # inside it must not re-enter this handler (errtrace makes ERR fire in + # functions now). One restore attempt, then let the script exit non-zero. + trap - ERR INT TERM [[ -n "$SNAPSHOT_DIR" && -d "$SNAPSHOT_DIR" ]] || return 0 fail "Install interrupted/failed — restoring previous state from snapshot" - rm -rf "$TARGET_DIR"; mkdir -p "$TARGET_DIR" - cp -a "$SNAPSHOT_DIR/." "$TARGET_DIR/" 2>/dev/null || true + # Reset the target before rebuilding from the snapshot — but CHECK it. Under + # `set -e` (trap already disarmed) a bare `rm -rf; mkdir -p` that fails would + # exit the whole script immediately, after `rm` may have deleted part of the + # target, WITHOUT ever printing the recovery pointer below — the operator would + # be left with a half-removed target and no idea the snapshot survives in /tmp. + # Test the reset explicitly (like the cp -a below), and on failure keep the + # snapshot and tell the operator where it is (#791 blocker-D2). + if ! rm -rf "$TARGET_DIR" || ! mkdir -p "$TARGET_DIR"; then + fail "Snapshot restore could not reset $TARGET_DIR. Your previous configuration is preserved at: $SNAPSHOT_DIR — copy it back into $TARGET_DIR manually." + return 1 + fi + # Surface an incomplete restore instead of swallowing it: the snapshot is the + # last good copy, so if cp cannot fully rebuild the target we must NOT delete + # the snapshot — point the operator at it for manual recovery (#791 blocker-2). + if ! cp -a "$SNAPSHOT_DIR/." "$TARGET_DIR/"; then + fail "Snapshot restore did not complete cleanly. Your previous configuration is preserved at: $SNAPSHOT_DIR — copy it back into $TARGET_DIR manually." + return 1 + fi } cleanup_snapshot() { [[ -n "$SNAPSHOT_DIR" && -d "$SNAPSHOT_DIR" ]] && rm -rf "$SNAPSHOT_DIR"; SNAPSHOT_DIR=""; } @@ -184,63 +207,105 @@ sync_framework() { return fi - if command -v rsync >/dev/null 2>&1; then - local rsync_args=(-a --delete --exclude ".git" --exclude ".framework-version" --exclude "*.pre-constitution.bak") - - if [[ "$INSTALL_MODE" == "keep" ]]; then - # Anchor to the transfer root (leading /) so we preserve the TOP-LEVEL - # ~/.config/mosaic/ without also excluding defaults/ from sync - # (reconcile_framework_files needs the freshly-synced defaults/ copies). - for path in "${PRESERVE_PATHS[@]}"; do - rsync_args+=(--exclude "/$path") - done - fi - - rsync "${rsync_args[@]}" "$SOURCE_DIR/" "$TARGET_DIR/" + if [[ "$INSTALL_MODE" == "keep" ]]; then + # The `mosaic update` path. Manifest-driven, never-deleting-outside-framework: + # operator config is structurally protected (#791). No rsync --delete here. + # The manifest is already loaded+validated in main() BEFORE the snapshot/trap + # (a fail-closed manifest must abort without ever restoring over operator + # files — see the pre-flight in main, #791 blocker-1). + sync_framework_keep return fi - # Fallback: cp-based sync. Exact top-level preserved paths mirror the - # root-anchored rsync excludes above. - local preserve_tmp="" - if [[ "$INSTALL_MODE" == "keep" ]]; then - preserve_tmp="$(mktemp -d "${TMPDIR:-/tmp}/mosaic-preserve-XXXXXX")" - local match rel - for path in "${PRESERVE_PATHS[@]}"; do - # Unquoted $path lets the glob expand against TARGET_DIR; nullglob makes a - # non-matching pattern vanish instead of staying literal. - shopt -s nullglob - for match in "$TARGET_DIR/"$path; do - [[ -e "$match" ]] || continue - rel="${match#"$TARGET_DIR/"}" - mkdir -p "$preserve_tmp/$(dirname "$rel")" - cp -R "$match" "$preserve_tmp/$rel" - done - shopt -u nullglob - done - fi + # overwrite mode — a full replace, chosen only for a fresh install or when the + # operator explicitly asks to replace everything. No operator state to protect. + sync_framework_overwrite +} - find "$TARGET_DIR" -mindepth 1 -maxdepth 1 ! -name ".git" ! -name ".framework-version" ! -name "*.pre-constitution.bak" -exec rm -rf {} + +# Enumerate a NUL-delimited file list via `find` into the temp file $1, failing +# CLOSED if find errors. We capture to a checked file instead of consuming +# `< <(find …)` directly because a process substitution discards the producer's +# exit status: an EACCES/I/O failure partway through a scan would truncate the +# list yet leave the reading `while` loop exiting 0, so a partial upgrade would +# commit and report success and the ERR/restore trap would never fire. Running +# find to completion first, then checking its status, turns that silent +# truncation into a fail-closed abort that the restore trap can act on (#791 +# blocker-D1). $1 after the shift is the scan root — named in the error. +_scan_or_die() { + local out="$1"; shift + if ! find "$@" -print0 > "$out"; then + fail "Could not enumerate framework files under '$1' — aborting before committing an incomplete sync (fail-closed)." + return 1 # D1-GUARD + fi +} + +# Keep-mode sync: create/refresh framework-owned files and prune only retired +# framework files inside shipped framework subtrees. Operator-owned and unknown +# paths (fail-safe default) are never written and never deleted — the #791 HARD +# GATE. Single code path (no rsync) so it is byte-for-byte parity-testable. +sync_framework_keep() { + local src="$SOURCE_DIR" dst="$TARGET_DIR" abs rel root list + + # 1) Overlay copy — every framework-owned source file, refreshed only when its + # bytes changed (no mtime churn on unchanged files, never on operator files). + # The source scan is captured fail-closed (#791 blocker-D1): a find failure + # aborts the sync (→ ERR trap → restore) rather than silently truncating it. + list="$(mktemp)" + _scan_or_die "$list" "$src" -type f || { rm -f "$list"; return 1; } + while IFS= read -r -d '' abs; do + rel="${abs#"$src"/}" + case "$rel" in + .git|.git/*|.framework-version|*.pre-constitution.bak) continue ;; + esac + manifest_is_framework "$rel" || continue + if [[ -f "$dst/$rel" ]] && cmp -s "$abs" "$dst/$rel"; then continue; fi + [[ "$rel" == */* ]] && mkdir -p "$dst/${rel%/*}" + cp "$abs" "$dst/$rel" + done < "$list" + rm -f "$list" + + # 2) Scoped prune — within each shipped framework subtree root, remove + # framework-owned target files the current source no longer ships. Operator + # carve-outs (e.g. tools/_lib/credentials.json) resolve to operator and are + # skipped; unknown paths resolve to operator too — both are unreachable here. + # Each subtree scan is captured fail-closed for the same reason as the copy. + while IFS= read -r root; do + [[ -n "$root" && -d "$dst/$root" ]] || continue + list="$(mktemp)" + _scan_or_die "$list" "$dst/$root" -type f || { rm -f "$list"; return 1; } + while IFS= read -r -d '' abs; do + rel="${abs#"$dst"/}" + case "$rel" in *.pre-constitution.bak) continue ;; esac + [[ -f "$src/$rel" ]] && continue # still shipped + manifest_is_framework "$rel" || continue + rm -f "$abs" + done < "$list" + rm -f "$list" + # Drop framework dirs left empty by the prune (never touches a dir that still + # holds an operator file — those are never emptied). A genuine find failure + # (unreadable dir) is surfaced as a warning rather than silently swallowed; + # the "directory not empty" races we tolerate are ignored via -delete's own + # rc, not by hiding stderr — so a real error is still visible to the operator. + if ! find "$dst/$root" -type d -empty -delete 2>/dev/null; then + warn "prune: could not fully sweep empty framework dirs under $root (left as-is)" + fi + done < <(manifest_subtree_roots) +} + +# Overwrite-mode sync: full replace. Only reached for a fresh install or an +# explicit operator "replace everything" choice, so nothing is preserved. +sync_framework_overwrite() { + if command -v rsync >/dev/null 2>&1; then + rsync -a --delete \ + --exclude ".git" --exclude ".framework-version" --exclude "*.pre-constitution.bak" \ + "$SOURCE_DIR/" "$TARGET_DIR/" + return + fi + find "$TARGET_DIR" -mindepth 1 -maxdepth 1 \ + ! -name ".git" ! -name ".framework-version" ! -name "*.pre-constitution.bak" \ + -exec rm -rf {} + cp -R "$SOURCE_DIR"/. "$TARGET_DIR"/ rm -rf "$TARGET_DIR/.git" - - if [[ -n "$preserve_tmp" ]]; then - # Restore by re-globbing the SAME patterns against preserve_tmp, so each - # preserved item is restored at its own relative path (e.g. only - # fleet/roster.yaml is replaced — the freshly-synced fleet/examples stays). - for path in "${PRESERVE_PATHS[@]}"; do - shopt -s nullglob - for match in "$preserve_tmp/"$path; do - [[ -e "$match" ]] || continue - rel="${match#"$preserve_tmp/"}" - rm -rf "$TARGET_DIR/$rel" - mkdir -p "$TARGET_DIR/$(dirname "$rel")" - cp -R "$match" "$TARGET_DIR/$rel" - done - shopt -u nullglob - done - rm -rf "$preserve_tmp" - fi } # ═══════════════════════════════════════════════════════════════════════════════ @@ -311,9 +376,23 @@ else ok "Install mode: overwrite" fi +# Pre-flight (keep mode): load + validate the framework manifest BEFORE taking a +# snapshot or arming the restore trap. A fail-closed manifest (missing / empty / +# malformed) must abort here WITHOUT deleting or restoring over operator files — +# the snapshot/restore path exists only for a genuine mid-sync mutation failure, +# not for a validation failure that has touched nothing yet (#791 blocker-1). +if [[ "$INSTALL_MODE" == "keep" ]]; then + manifest_load +fi + # Snapshot before any destructive file operation; restore on interrupt/failure. +# The trap MUST exit after restoring: a bash INT/TERM handler that merely returns +# does NOT terminate the script — execution would resume past the interrupt, +# clear the snapshot, and report success, leaving a partial post-interrupt update +# (#791 blocker-A). `restore_snapshot; exit 1` guarantees a non-zero exit for +# both the errtrace (ERR) and signal (INT/TERM) paths. make_snapshot -trap 'restore_snapshot' ERR INT TERM +trap 'restore_snapshot; exit 1' ERR INT TERM sync_framework diff --git a/packages/mosaic/framework/tools/_lib/manifest.sh b/packages/mosaic/framework/tools/_lib/manifest.sh new file mode 100644 index 0000000..20b79cc --- /dev/null +++ b/packages/mosaic/framework/tools/_lib/manifest.sh @@ -0,0 +1,253 @@ +#!/usr/bin/env bash +# Shared bash reader for framework-manifest.txt (#791). +# +# This is the bash half of the SSOT ownership resolver; the TypeScript half is +# packages/mosaic/src/framework/manifest.ts. BOTH read the same +# framework-manifest.txt and MUST resolve identical ownership for any path — the +# parity test (manifest-parity.spec.ts) invokes this file's `resolve` CLI and +# compares it against the TS resolver, so the two can never drift (the #631 +# two-copies failure class this closes). +# +# Ownership resolution (deny-wins / fail-safe): +# 1. operator glob matches -> operator +# 2. else framework glob -> framework +# 3. else -> operator (UNKNOWN defaults to operator, #791) +# +# Globs are compiled once at load into exact-prefix checks or POSIX EREs, so the +# hot resolver (manifest_is_framework) forks no subprocesses — the installer +# calls it once per file across the whole tree. +# +# Usage as a library (source it, then): +# manifest_load [manifest-file] # populates + compiles the manifest +# manifest_is_framework # rc 0 = framework-owned, rc 1 = operator +# manifest_resolve # echoes: framework | operator +# manifest_subtree_roots # echoes shipped framework `dir/**` roots +# +# Usage as a CLI (parity harness): +# bash manifest.sh resolve +# bash manifest.sh subtree-roots +# bash manifest.sh classify # reads paths on stdin -> "\t" + +MANIFEST_FRAMEWORK=() +MANIFEST_OPERATOR=() + +# Compiled forms (parallel arrays). _*_KIND[i] is "exact" or "re". +_MF_KIND=(); _MF_EXACT=(); _MF_RE=() +_MO_KIND=(); _MO_EXACT=(); _MO_RE=() +_MF_ROOTS=() + +_manifest_default_root() { cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd; } + +# Normalize a path/glob: backslashes -> slashes, strip leading ./ and /, strip +# trailing / (mirrors normalizeRel in manifest.ts). +_manifest_norm() { + local p="$1" + p="${p//\\//}" + p="${p#./}" + while [[ "$p" == /* ]]; do p="${p#/}"; done + while [[ "$p" == */ ]]; do p="${p%/}"; done + printf '%s' "$p" +} + +# Translate a normalized glob into a POSIX ERE body (mirrors globToRegExpBody). +_manifest_glob_to_ere() { + local pattern; pattern="$(_manifest_norm "$1")" + local out="" c n i len=${#pattern} trailing + for (( i = 0; i < len; i++ )); do + c="${pattern:i:1}" + if [[ "$c" == "*" ]]; then + n="${pattern:i+1:1}" + if [[ "$n" == "*" ]]; then + i=$((i + 1)) + trailing=0 + if [[ "${pattern:i+1:1}" == "/" ]]; then i=$((i + 1)); trailing=1; fi + if [[ "$out" == */ ]]; then + out="${out%/}(/.*)?" + elif [[ "$trailing" -eq 1 ]]; then + out="$out(.*/)?" + else + out="$out.*" + fi + else + out="$out[^/]*" + fi + else + case "$c" in + .|+|\?|^|\$|\{|\}|\(|\)|\||\[|\]|\\) out="$out\\$c" ;; + *) out="$out$c" ;; + esac + fi + done + printf '%s' "$out" +} + +# Compile one raw glob into (kind, exact, re) appended to the given section. +# $1 = raw glob, $2 = section letter (F|O). +_manifest_compile_one() { + local norm; norm="$(_manifest_norm "$1")" + [[ -n "$norm" ]] || return 0 + if [[ "$norm" == *"*"* ]]; then + local re="^$(_manifest_glob_to_ere "$norm")\$" + if [[ "$2" == F ]]; then + _MF_KIND+=(re); _MF_EXACT+=(""); _MF_RE+=("$re") + else + _MO_KIND+=(re); _MO_EXACT+=(""); _MO_RE+=("$re") + fi + else + if [[ "$2" == F ]]; then + _MF_KIND+=(exact); _MF_EXACT+=("$norm"); _MF_RE+=("") + else + _MO_KIND+=(exact); _MO_EXACT+=("$norm"); _MO_RE+=("") + fi + fi + [[ "$2" == F && "$norm" == */"**" ]] && _MF_ROOTS+=("${norm%/**}") + return 0 +} + +_manifest_compile() { + _MF_KIND=(); _MF_EXACT=(); _MF_RE=(); _MF_ROOTS=() + _MO_KIND=(); _MO_EXACT=(); _MO_RE=() + local g + for g in "${MANIFEST_FRAMEWORK[@]:-}"; do [[ -n "$g" ]] && _manifest_compile_one "$g" F; done + for g in "${MANIFEST_OPERATOR[@]:-}"; do [[ -n "$g" ]] && _manifest_compile_one "$g" O; done + # Explicit success: an empty operator array makes the final `[[ -n "" ]] && …` + # short-circuit to rc 1, which would otherwise become this function's (and + # manifest_load's) return code — a spurious failure (#791 B2). Never rely on + # the last loop's exit status here. + return 0 +} + +# Load + compile the manifest. Rejects a malformed file the same way +# parseManifest() does (entry before a section header / unknown header). +manifest_load() { + local file="${1:-}" + [[ -n "$file" ]] || file="$(_manifest_default_root)/framework-manifest.txt" + # Fail CLOSED on a missing/unreadable manifest. Without this, `done < "$file"` + # aborts on a raw redirection error with no explanation; downstream that reads + # as "no framework paths" and an upgrade could no-op silently (#791 B2/B3). + if [[ ! -r "$file" ]]; then + echo "manifest: cannot read manifest file: $file — refusing to sync (fail-closed)." >&2 + return 1 + fi + MANIFEST_FRAMEWORK=() + MANIFEST_OPERATOR=() + local section="" line + while IFS= read -r line || [[ -n "$line" ]]; do + line="${line#"${line%%[![:space:]]*}"}" # ltrim + line="${line%"${line##*[![:space:]]}"}" # rtrim + [[ -z "$line" || "${line:0:1}" == "#" ]] && continue + case "$line" in + "[framework]") section=framework; continue ;; + "[operator]") section=operator; continue ;; + "["*) echo "manifest: unknown section header: $line" >&2; return 1 ;; + esac + if [[ -z "$section" ]]; then + echo "manifest: entry before any [section] header: $line" >&2 + return 1 + fi + if [[ "$section" == framework ]]; then + MANIFEST_FRAMEWORK+=("$line") + else + MANIFEST_OPERATOR+=("$line") + fi + done < "$file" + # An empty or comment-only manifest defines NO framework-owned paths. Treating + # that as valid would make every path resolve operator and an upgrade prune + # nothing / write nothing — a silent no-op indistinguishable from success. + # Fail loud instead, mirroring parseManifest()'s throw in manifest.ts (#791 B2). + if [[ ${#MANIFEST_FRAMEWORK[@]} -eq 0 ]]; then + echo "manifest: no [framework] entries in $file — refusing to sync (empty or malformed manifest)." >&2 + return 1 + fi + # An entry like `/` or `./` normalizes to nothing and compiles to a glob that + # matches no path — so a manifest whose only [framework] entries are degenerate + # passes the count guard above but leaves the framework matcher empty: every + # path resolves operator, the exact silent no-op we fail closed against. Require + # at least one entry with a real (non-slash, non-dot) character. Mirrors + # parseManifest()'s `isUsableFrameworkGlob` `/[^/.]/` test in manifest.ts (#791 blocker-B). + local _g _usable=0 + for _g in "${MANIFEST_FRAMEWORK[@]:-}"; do + if [[ "$(_manifest_norm "$_g")" =~ [^/.] ]]; then _usable=1; break; fi + done + if [[ "$_usable" -eq 0 ]]; then + echo "manifest: no usable [framework] entries in $file (every entry is empty or a bare dot segment) — refusing to sync (malformed manifest)." >&2 + return 1 + fi + _manifest_compile + return 0 +} + +# Fork-free: does $1 (a mosaic-home-relative path) match an operator glob? +_mo_matches() { + local path="$1" i n=${#_MO_KIND[@]} re pat + for (( i = 0; i < n; i++ )); do + if [[ "${_MO_KIND[i]}" == exact ]]; then + pat="${_MO_EXACT[i]}" + [[ "$path" == "$pat" || "$path" == "$pat/"* ]] && return 0 + else + re="${_MO_RE[i]}" + [[ "$path" =~ $re ]] && return 0 + fi + done + return 1 +} + +# Fork-free: does $1 match a framework glob? +_mf_matches() { + local path="$1" i n=${#_MF_KIND[@]} re pat + for (( i = 0; i < n; i++ )); do + if [[ "${_MF_KIND[i]}" == exact ]]; then + pat="${_MF_EXACT[i]}" + [[ "$path" == "$pat" || "$path" == "$pat/"* ]] && return 0 + else + re="${_MF_RE[i]}" + [[ "$path" =~ $re ]] && return 0 + fi + done + return 1 +} + +# The installer hot path — no subshell. rc 0 = framework-owned, rc 1 = operator +# (deny-wins / fail-safe). Assumes an already-clean POSIX relative path. +manifest_is_framework() { + _mo_matches "$1" && return 1 + _mf_matches "$1" && return 0 + return 1 +} + +# Echo the ownership of a path: framework | operator. Normalizes first, so it is +# safe for CLI / test callers passing unnormalized input. +manifest_resolve() { + local path; path="$(_manifest_norm "$1")" + if manifest_is_framework "$path"; then echo framework; else echo operator; fi +} + +# Echo each shipped framework subtree root (a `dir/**` entry, without the /**). +manifest_subtree_roots() { + local r + for r in "${_MF_ROOTS[@]:-}"; do [[ -n "$r" ]] && printf '%s\n' "$r"; done +} + +# CLI dispatch — only when executed directly, never when sourced. +if [[ "${BASH_SOURCE[0]}" == "${0}" ]]; then + set -o pipefail + # Propagate a fail-closed manifest_load (missing/empty/malformed) as a non-zero + # exit instead of continuing to resolve against empty compiled arrays — that is + # what lets the parity test assert bash and TS reject the same bad inputs (#791 B2). + manifest_load "${MANIFEST_FILE:-}" || exit 1 + cmd="${1:-}" + case "$cmd" in + resolve) manifest_resolve "${2:?path required}" ;; + subtree-roots) manifest_subtree_roots ;; + classify) + while IFS= read -r p; do + [[ -z "$p" ]] && continue + printf '%s\t%s\n' "$(manifest_resolve "$p")" "$p" + done + ;; + *) + echo "usage: manifest.sh {resolve |subtree-roots|classify}" >&2 + exit 2 + ;; + esac +fi diff --git a/packages/mosaic/framework/tools/quality/scripts/test-install-migration.sh b/packages/mosaic/framework/tools/quality/scripts/test-install-migration.sh index ea9aa6c..c925f4d 100755 --- a/packages/mosaic/framework/tools/quality/scripts/test-install-migration.sh +++ b/packages/mosaic/framework/tools/quality/scripts/test-install-migration.sh @@ -61,8 +61,10 @@ MOSAIC_HOME="$T5" MOSAIC_INSTALL_MODE=bogus MOSAIC_SYNC_ONLY=1 bash "$INSTALL" > chk "F5 failure: invalid mode rejected (nonzero exit)" "[ $rc -ne 0 ]" chk "F5 failure: SOUL + credentials intact" "grep -q orig '$T5/SOUL.md' && grep -q keepme '$T5/credentials/c.json'" -# F6 — keep-mode re-seed (the `mosaic update` path) MUST preserve only the -# exact user-owned roster paths while refreshing framework-owned schema/examples. +# F6 — keep-mode re-seed (the `mosaic update` path) MUST preserve ALL user-owned +# fleet state — including an unanticipated file the manifest never names, which +# resolves to operator-owned by the #791 fail-safe — while refreshing the +# framework-owned schema/examples. T6=$(mktemp -d); mkdir -p "$T6/fleet/examples" "$T6/fleet/run" "$T6/fleet/agents" printf '# persona\n' > "$T6/SOUL.md" # makes it a recognized existing install (→ keep mode) printf 'version: 1\nagents:\n - name: coder0\n' > "$T6/fleet/roster.yaml" @@ -75,13 +77,14 @@ printf '{"stale":true}\n' > "$T6/fleet/roster.schema.json" E6=$(mktemp -d) cp "$T6/fleet/roster.yaml" "$E6/roster-yaml.expected" cp "$T6/fleet/roster.json" "$E6/roster-json.expected" +cp "$T6/fleet/my-fleet.yaml" "$E6/my-fleet.expected" cp "$T6/fleet/run/coder0.hb" "$E6/run.expected" cp "$T6/fleet/agents/coder0.env" "$E6/agent.expected" echo 3 > "$T6/.framework-version" run "$T6" keep chk "F6 reseed: exact roster.yaml bytes survive keep-mode sync" "cmp -s '$T6/fleet/roster.yaml' '$E6/roster-yaml.expected'" chk "F6 reseed: exact roster.json bytes survive keep-mode sync" "cmp -s '$T6/fleet/roster.json' '$E6/roster-json.expected'" -chk "F6 reseed: unrelated fleet YAML is not preserved" "[ ! -f '$T6/fleet/my-fleet.yaml' ]" +chk "F6 reseed: unanticipated operator fleet file survives (fail-safe, #791)" "cmp -s '$T6/fleet/my-fleet.yaml' '$E6/my-fleet.expected'" chk "F6 reseed: per-agent env bytes survive" "cmp -s '$T6/fleet/agents/coder0.env' '$E6/agent.expected'" chk "F6 reseed: heartbeat bytes survive" "cmp -s '$T6/fleet/run/coder0.hb' '$E6/run.expected'" chk "F6 reseed: framework examples are refreshed" "grep -q orchestrator '$T6/fleet/examples/general.yaml'" diff --git a/packages/mosaic/framework/tools/quality/scripts/test-upgrade-manifest-guard.sh b/packages/mosaic/framework/tools/quality/scripts/test-upgrade-manifest-guard.sh new file mode 100644 index 0000000..f221e68 --- /dev/null +++ b/packages/mosaic/framework/tools/quality/scripts/test-upgrade-manifest-guard.sh @@ -0,0 +1,224 @@ +#!/usr/bin/env bash +# test-upgrade-manifest-guard.sh — the #791 HARD GATE. +# +# Proves that a keep-mode framework upgrade (the `mosaic update` path: +# install.sh with MOSAIC_INSTALL_MODE=keep MOSAIC_SYNC_ONLY=1) touches NO path +# outside the framework-owned manifest. Every operator-owned sentinel — including +# a deliberately UNANTICIPATED one the manifest never names — must survive +# byte-identical with an unchanged mtime (not even rewritten). Framework files +# must still update, and a retired framework file inside a shipped subtree must +# still be pruned. No operator secret value may appear in installer output. +# +# Keep mode is a SINGLE code path (sync_framework_keep, a manifest-driven cp +# overlay + scoped prune — no rsync). The matrix still runs twice, once with +# rsync on PATH and once with it hidden, to prove the keep path is genuinely +# rsync-independent: it must obey the manifest identically whether or not rsync +# happens to be installed (rsync --delete is only ever used by overwrite mode, +# which has no operator state to protect). +# +# It also runs a fail-closed matrix (#791 B2/B3): an empty, operator-only, +# malformed, or missing manifest must ABORT the upgrade loudly and leave every +# operator path untouched — never silently no-op to "complete". +# +# Usage: bash test-upgrade-manifest-guard.sh +set -uo pipefail + +FW="$(cd "$(dirname "${BASH_SOURCE[0]}")/../../.." && pwd)" # packages/mosaic/framework +INSTALL="$FW/install.sh" + +pass=0; fail=0 +chk() { if eval "$2"; then echo " ✓ $1"; pass=$((pass + 1)); else echo " ✗ $1"; fail=$((fail + 1)); fi; } + +SECRET='SUPER-SECRET-TOKEN-do-not-log-3f9a' + +# Seed a throwaway MOSAIC_HOME with an operator sentinel per ownership class. +seed_home() { + local H="$1" + mkdir -p "$H/agents" "$H/policy" "$H/memory" "$H/tools/_lib" \ + "$H/fleet/agents" "$H/fleet/run/sessions" "$H/harvester" \ + "$H/unknown-operator-dir" "$H/guides" + printf '# persona\n' > "$H/SOUL.md" # marks a recognized existing install → keep mode + printf 'MODEL=opus\n' > "$H/agents/coder0.conf" + printf '# operator policy\n' > "$H/policy/custom.md" + printf '# soul overlay\n' > "$H/SOUL.local.md" + printf '# operator memory\n' > "$H/memory/note.md" + printf 'TOKEN=%s\n' "$SECRET" > "$H/tools/_lib/credentials.json" + printf 'MOSAIC_AGENT_NAME=coder0\n' > "$H/fleet/agents/coder0.env" + printf 'version: 2\nagents:\n - name: coder0\n' > "$H/fleet/roster.yaml" + printf '# harvester SOP\n' > "$H/harvester/sop.md" + printf 'operator data the manifest never anticipated\n' > "$H/unknown-operator-dir/x" + printf 'version: 1\nagents:\n - name: mine\n' > "$H/fleet/my-fleet.yaml" + + # #797 Runtime Session Ledger (Mos-elevated to a #791 PR1 merge-blocker): a + # populated ledger under fleet/run/sessions/ must survive the upgrade — a + # runtime ledger an upgrade rsync can wipe is worthless. Seed it exactly as + # #797 writes it: a non-empty append journal + a non-empty compacted + # projection, files 0600 under a 0700 dir. + printf '%s\n%s\n%s\n' \ + '{"seq":1,"kind":"session.spawn","node":"sess-42","generation":7}' \ + '{"seq":2,"kind":"lease.grant","node":"sess-42","lease":"web1"}' \ + '{"seq":3,"kind":"dispatch.create","from":"sess-42","to":"disp-9"}' \ + > "$H/fleet/run/sessions/events.ndjson" + printf '%s\n' \ + '{"generation":7,"nodes":[{"id":"sess-42","kind":"session"}],"edges":[{"from":"sess-42","to":"disp-9","kind":"dispatch"}]}' \ + > "$H/fleet/run/sessions/ledger.json" + chmod 0700 "$H/fleet/run" "$H/fleet/run/sessions" + chmod 0600 "$H/fleet/run/sessions/events.ndjson" "$H/fleet/run/sessions/ledger.json" + + # A retired framework file inside a shipped subtree (absent from source) — must be pruned. + printf '# retired guide\n' > "$H/guides/RETIRED-OLD-GUIDE.md" + echo 3 > "$H/.framework-version" +} + +OPERATOR_SENTINELS=( + "agents/coder0.conf" + "policy/custom.md" + "SOUL.local.md" + "memory/note.md" + "tools/_lib/credentials.json" + "fleet/agents/coder0.env" + "fleet/roster.yaml" + "harvester/sop.md" + "unknown-operator-dir/x" + "fleet/my-fleet.yaml" + # #797 Runtime Session Ledger — populated journal + projection must survive. + "fleet/run/sessions/events.ndjson" + "fleet/run/sessions/ledger.json" +) + +run_matrix() { + local label="$1"; shift # extra env / PATH override applied to the run + local H E OUT rel before_hash after_hash before_mt after_mt + H=$(mktemp -d); E=$(mktemp -d); OUT=$(mktemp) + seed_home "$H" + + # Snapshot hash + mtime of every operator sentinel before the upgrade. + for rel in "${OPERATOR_SENTINELS[@]}"; do + sha256sum "$H/$rel" | awk '{print $1}' > "$E/$(echo "$rel" | tr / _).hash" + stat -c %Y "$H/$rel" > "$E/$(echo "$rel" | tr / _).mt" + done + + # Snapshot the ledger directory permission bits (#797 assert: perms unchanged). + local before_dirperm after_dirperm + before_dirperm=$(stat -c %a "$H/fleet/run/sessions") + + # The upgrade under test (keep + sync-only = the `mosaic update` reseed path). + MOSAIC_HOME="$H" MOSAIC_INSTALL_MODE=keep MOSAIC_SYNC_ONLY=1 "$@" bash "$INSTALL" >"$OUT" 2>&1 + + # HARD GATE: every operator sentinel survives byte-identical AND mtime-unchanged. + for rel in "${OPERATOR_SENTINELS[@]}"; do + before_hash=$(cat "$E/$(echo "$rel" | tr / _).hash") + before_mt=$(cat "$E/$(echo "$rel" | tr / _).mt") + after_hash=$(sha256sum "$H/$rel" 2>/dev/null | awk '{print $1}') + after_mt=$(stat -c %Y "$H/$rel" 2>/dev/null || echo MISSING) + chk "[$label] operator sentinel survives byte-identical: $rel" \ + "[ -n '$after_hash' ] && [ '$before_hash' = '$after_hash' ]" + chk "[$label] operator sentinel not rewritten (mtime unchanged): $rel" \ + "[ '$before_mt' = '$after_mt' ]" + done + + # #797 assert 7: the ledger directory's permission bits are unchanged. + after_dirperm=$(stat -c %a "$H/fleet/run/sessions" 2>/dev/null || echo MISSING) + chk "[$label] ledger dir perms unchanged (#797): $before_dirperm" \ + "[ '$before_dirperm' = '$after_dirperm' ]" + + # Positive controls / negative controls — prove the test discriminates: the + # upgrade DOES write and prune framework-owned paths, so the operator sentinels + # (incl. the #797 ledger) survive because of the manifest, not because the + # upgrade is a no-op. + chk "[$label] positive control: framework file present after upgrade (guides synced)" \ + "[ -f '$H/guides/E2E-DELIVERY.md' ]" + chk "[$label] negative control: retired framework file inside a subtree IS pruned" \ + "[ ! -f '$H/guides/RETIRED-OLD-GUIDE.md' ]" + chk "[$label] manifest itself is installed" "[ -f '$H/framework-manifest.txt' ]" + + # Secret-safety: the operator secret value never appears in installer output. + chk "[$label] operator secret value absent from installer stdout/stderr" \ + "! grep -q '$SECRET' '$OUT'" + + rm -rf "$H" "$E" "$OUT" +} + +# Fail-closed matrix (#791 B2/B3 + blocker-1): run install.sh from a COPY of the +# framework so the shipped manifest can be corrupted. Every corruption must abort +# the upgrade non-zero with a manifest error, leaving all operator sentinels +# byte-identical AND on the SAME inode. The inode check is the load-bearing part: +# manifest validation is hoisted BEFORE make_snapshot/the restore trap, so a bad +# manifest must abort without ever snapshotting, deleting, and restoring the +# target. Were validation still armed under the ERR trap, restore_snapshot would +# rm -rf + rebuild the target — same bytes but a NEW inode (broken hard links, +# changed ctime), which a content-only hash would miss (#791 blocker-1). +run_failclosed() { + local label="$1" mutate="$2" + local SRC H E OUT rc rel before_hash after_hash before_ino after_ino key + SRC=$(mktemp -d); H=$(mktemp -d); E=$(mktemp -d); OUT=$(mktemp) + cp -a "$FW/." "$SRC/" + case "$mutate" in + empty) : > "$SRC/framework-manifest.txt" ;; + operator-only) printf '[operator]\nSOUL.md\n*.local.md\n' > "$SRC/framework-manifest.txt" ;; + malformed) printf 'stray.md\n[framework]\nguides/**\n' > "$SRC/framework-manifest.txt" ;; + degenerate) printf '[framework]\n/\n./\n[operator]\nSOUL.md\n' > "$SRC/framework-manifest.txt" ;; + missing) rm -f "$SRC/framework-manifest.txt" ;; + esac + + seed_home "$H" + for rel in "${OPERATOR_SENTINELS[@]}"; do + key=$(echo "$rel" | tr / _) + sha256sum "$H/$rel" | awk '{print $1}' > "$E/$key.hash" + stat -c '%i' "$H/$rel" > "$E/$key.ino" + done + + MOSAIC_HOME="$H" MOSAIC_INSTALL_MODE=keep MOSAIC_SYNC_ONLY=1 bash "$SRC/install.sh" >"$OUT" 2>&1 + rc=$? + + chk "[fail-closed:$label] upgrade aborts non-zero" "[ '$rc' -ne 0 ]" + chk "[fail-closed:$label] refuses loudly with a manifest error" \ + "grep -qi 'manifest' '$OUT'" + for rel in "${OPERATOR_SENTINELS[@]}"; do + key=$(echo "$rel" | tr / _) + before_hash=$(cat "$E/$key.hash") + after_hash=$(sha256sum "$H/$rel" 2>/dev/null | awk '{print $1}') + chk "[fail-closed:$label] operator sentinel untouched: $rel" \ + "[ -n '$after_hash' ] && [ '$before_hash' = '$after_hash' ]" + before_ino=$(cat "$E/$key.ino") + after_ino=$(stat -c '%i' "$H/$rel" 2>/dev/null) + chk "[fail-closed:$label] operator sentinel not deleted/recreated (inode stable): $rel" \ + "[ -n '$after_ino' ] && [ '$before_ino' = '$after_ino' ]" + done + chk "[fail-closed:$label] operator secret value absent from output" \ + "! grep -q '$SECRET' '$OUT'" + + chmod -R u+w "$SRC" "$H" 2>/dev/null || true + rm -rf "$SRC" "$H" "$E" "$OUT" +} + +echo "#791 upgrade manifest guard (HARD GATE):" + +# 1) rsync path (if available on this host). +if command -v rsync >/dev/null 2>&1; then + run_matrix "rsync" +else + echo " · rsync not installed — skipping rsync-path matrix" +fi + +# 2) rsync-absent path — hide rsync behind a scratch PATH. Keep mode never calls +# rsync, so this must resolve identically to run (1); it proves the keep path +# does not silently depend on rsync being installed. (Provide the coreutils the +# installer needs on the stripped PATH.) +FBIN=$(mktemp -d) +for t in bash cp find mktemp rm mkdir chmod cmp sed grep cat dirname basename stat sha256sum awk tr; do + p=$(command -v "$t" 2>/dev/null) && ln -s "$p" "$FBIN/$t" +done +run_matrix "rsync-absent" env "PATH=$FBIN" +rm -rf "$FBIN" + +# 3) fail-closed matrix (#791 B2/B3) — corrupt the shipped manifest four ways. +run_failclosed "empty-manifest" empty +run_failclosed "operator-only" operator-only +run_failclosed "malformed-manifest" malformed +run_failclosed "degenerate-framework" degenerate +run_failclosed "missing-manifest" missing + +echo +echo "RESULT: $pass passed, $fail failed" +[ "$fail" -eq 0 ] diff --git a/packages/mosaic/framework/tools/quality/scripts/test-upgrade-rollback.sh b/packages/mosaic/framework/tools/quality/scripts/test-upgrade-rollback.sh new file mode 100644 index 0000000..d0b91ba --- /dev/null +++ b/packages/mosaic/framework/tools/quality/scripts/test-upgrade-rollback.sh @@ -0,0 +1,319 @@ +#!/usr/bin/env bash +# test-upgrade-rollback.sh — the #791 B1 regression gate. +# +# A keep-mode upgrade takes a pre-update snapshot and installs an ERR/INT/TERM +# trap that restores it if the sync aborts midway (install.sh: make_snapshot + +# `trap restore_snapshot`). That trap is only reached if `set -E` (errtrace) is +# active — otherwise a failure INSIDE sync_framework_keep() (which runs entirely +# in a function) never fires the trap, and the upgrade aborts leaving a +# half-written target with NO rollback. This test proves: +# +# Part A (the gate): the shipped installer rolls back a mid-sync failure — +# the restore message fires, the corrupted file is put +# back, AND the whole target is byte-identical to its +# pre-upgrade state. +# Part B (the control): the SAME installer with `-E` stripped does NOT roll back +# (dead trap) — the mid-sync corruption survives, proving +# errtrace is load-bearing. If anyone removes `set -E`, +# Part A goes red. +# +# The mid-sync failure is injected with a PATH-shadowing `cp` shim rather than +# file permissions. The earlier 0400/EACCES approach was NOT portable: Woodpecker +# runs steps as root (node:24-alpine has no USER directive), and root overwrites a +# 0400 file, so the failure never fired and this gate silently passed (#791 +# blocker-3). The shim fails deterministically for one framework-owned +# destination regardless of uid, and — like a real interrupted cp (disk-full +# mid-write) — leaves a partially-written target behind, so rollback has real +# damage to undo and the control has real damage to expose. +# +# Usage: bash test-upgrade-rollback.sh +set -uo pipefail + +FW="$(cd "$(dirname "${BASH_SOURCE[0]}")/../../.." && pwd)" # packages/mosaic/framework +INSTALL="$FW/install.sh" +ORIG_PATH="$PATH" + +# The `-E`-stripped control installer must live INSIDE $FW: install.sh derives +# SOURCE_DIR from its own path and `source`s $SOURCE_DIR/tools/_lib/manifest.sh, +# so a copy anywhere else aborts at the source line before ever reaching the sync +# loop — which would make the control a false negative. A root dotfile is +# operator-owned (unknown→operator), so the sync loop skips it. Clean up on exit. +STRIPPED="$FW/.install-rollback-control.tmp.sh" +NOEXIT="$FW/.install-noexit-control.tmp.sh" +D1CTRL="$FW/.install-d1guard-control.tmp.sh" +D2CTRL="$FW/.install-d2guard-control.tmp.sh" +rm -f "$STRIPPED" "$NOEXIT" "$D1CTRL" "$D2CTRL" +trap 'rm -f "$STRIPPED" "$NOEXIT" "$D1CTRL" "$D2CTRL"' EXIT + +pass=0; fail=0 +chk() { if eval "$2"; then echo " ✓ $1"; pass=$((pass + 1)); else echo " ✗ $1"; fail=$((fail + 1)); fi; } + +SECRET='SUPER-SECRET-TOKEN-do-not-log-b1' +# A framework-owned file the shim fails the copy of. The seeded target holds GOOD +# bytes; source ships different bytes, so sync_framework_keep() attempts the copy +# and the shim intercepts it. Root-level framework files sort before guides/, so +# several framework files are already refreshed when the copy reaches this one. +POISON_REL='guides/E2E-DELIVERY.md' +GOOD='GOOD-REFERENCE-CONTENT-pre-upgrade-b1' +GARBAGE='PARTIAL-WRITE-GARBAGE-mid-sync-b1' + +# A `cp` shim: for the poisoned destination, simulate an interrupted copy — write +# partial garbage to the target, then fail — otherwise delegate to the real cp +# (resolved via the ORIGINAL PATH so make_snapshot/restore still work). +make_cp_shim() { + local dir="$1" + cat > "$dir/cp" < "\$dest" 2>/dev/null || true + exit 1 ;; +esac +exec env PATH="$ORIG_PATH" cp "\$@" +SHIM + chmod +x "$dir/cp" +} + +# A `find` shim that fails every enumeration scan (`-print0`) as if it hit an +# EACCES/I/O error partway — it emits the real (here: complete) list first, then +# exits non-zero, exactly the class of failure a `< <(find …)` process +# substitution silently swallows. All non-`-print0` finds (e.g. the -delete +# sweep) delegate to the real find on the original PATH. Used to prove #791 +# blocker-D1: the shipped installer must honor find's exit status and roll back. +make_find_fail_shim() { + local dir="$1" + cat > "$dir/find" <` (the restore's target +# reset) and delegates every other rm to the real one. Used to prove #791 +# blocker-D2: when the target reset inside restore_snapshot fails, the installer +# must emit the manual-recovery pointer (snapshot path) instead of exiting +# silently under `set -e`. FAIL_RM_TARGET is exported into the installer env. +make_rm_fail_shim() { + local dir="$1" + cat > "$dir/rm" <<'SHIM' +#!/usr/bin/env bash +last="${@: -1}" +if [ -n "${FAIL_RM_TARGET:-}" ] && [ "$last" = "$FAIL_RM_TARGET" ]; then + exit 1 +fi +exec env PATH="$ORIG_PATH_FOR_RM" rm "$@" +SHIM + chmod +x "$dir/rm" +} + +seed_home() { + local H="$1" + mkdir -p "$H/agents" "$H/tools/_lib" "$H/memory" "$H/guides" + printf '# persona\n' > "$H/SOUL.md" # recognized install → keep mode + snapshot + printf 'MODEL=opus\n' > "$H/agents/coder0.conf" + printf '# operator memory\n' > "$H/memory/note.md" + printf 'TOKEN=%s\n' "$SECRET" > "$H/tools/_lib/credentials.json" + echo 3 > "$H/.framework-version" + # Good pre-upgrade bytes; source ships different bytes, so cp is attempted. + printf '%s' "$GOOD" > "$H/$POISON_REL" +} + +# Run one keep-mode upgrade against $1=installer, seeding a fresh home and a +# byte-for-byte reference of the pre-upgrade state, with the cp shim first on +# PATH. Echoes: "\t\t\t". +run_upgrade() { + local installer="$1" shim_maker="${2:-make_cp_shim}" H REF OUT SHIM rc + H=$(mktemp -d); REF=$(mktemp -d); OUT=$(mktemp); SHIM=$(mktemp -d) + seed_home "$H" + env PATH="$ORIG_PATH" cp -a "$H/." "$REF/" # pre-upgrade reference (real cp) + "$shim_maker" "$SHIM" + set +e + PATH="$SHIM:$ORIG_PATH" \ + MOSAIC_HOME="$H" MOSAIC_INSTALL_MODE=keep MOSAIC_SYNC_ONLY=1 bash "$installer" >"$OUT" 2>&1 + rc=$? + set -e 2>/dev/null || true + rm -rf "$SHIM" + printf '%s\t%s\t%s\t%s\n' "$rc" "$OUT" "$REF" "$H" +} + +echo "#791 upgrade rollback (B1 regression gate):" + +# ── Part A: the shipped installer must roll back a mid-sync failure ─────────── +IFS=$'\t' read -r rcA OUTA REFA HA < <(run_upgrade "$INSTALL") + +chk "[shipped] upgrade aborts non-zero on the injected mid-sync failure" \ + "[ '$rcA' -ne 0 ]" +chk "[shipped] restore_snapshot fires (rollback message present)" \ + "grep -q 'restoring previous state from snapshot' '$OUTA'" +chk "[shipped] the corrupted file is restored to its pre-upgrade bytes" \ + "[ \"\$(cat '$HA/$POISON_REL')\" = '$GOOD' ]" +chk "[shipped] target rolled back byte-identical to pre-upgrade state" \ + "diff -r '$REFA' '$HA' >/dev/null 2>&1" +chk "[shipped] operator secret value absent from installer output" \ + "! grep -q '$SECRET' '$OUTA'" + +# ── Part B: control — strip `-E`, the trap is dead, no rollback happens ─────── +# Proves errtrace is what makes the trap reachable. If `set -E` is ever removed +# from install.sh, Part A's rollback assertions fail exactly like this control. +sed 's/^set -Eeuo pipefail/set -euo pipefail/' "$INSTALL" > "$STRIPPED" +chk "[control] the -E strip actually changed the installer" \ + "! cmp -s '$INSTALL' '$STRIPPED'" + +IFS=$'\t' read -r rcB OUTB REFB HB < <(run_upgrade "$STRIPPED") +chk "[control] without -E the upgrade still aborts non-zero" \ + "[ '$rcB' -ne 0 ]" +# The load-bearing, deterministic proof of B1: without errtrace the ERR trap +# never fires for a failure inside sync_framework_keep(), so no rollback runs. +chk "[control] without -E the rollback message does NOT fire (dead trap)" \ + "! grep -q 'restoring previous state from snapshot' '$OUTB'" +chk "[control] without -E the mid-sync corruption survives (no rollback)" \ + "[ \"\$(cat '$HB/$POISON_REL')\" = '$GARBAGE' ]" + +# ── Part C: an INT/TERM interrupt must terminate, not resume (blocker-A) ────── +# A bash signal trap that merely returns lets the script continue past the +# interrupt — restoring the snapshot, then resuming the sync and reporting +# success. We inject a SIGTERM mid-sync with a cp that SUCCEEDS (so set -e never +# fires and ONLY the signal path governs), and assert the shipped installer +# restores AND exits without reporting success. The control strips `exit 1` from +# the trap and shows the buggy resume-to-success. +make_term_shim() { + local dir="$1" + cat > "$dir/cp" </dev/null # signal install.sh; the copy still succeeds + exec env PATH="$ORIG_PATH" cp "\$@" ;; +esac +exec env PATH="$ORIG_PATH" cp "\$@" +SHIM + chmod +x "$dir/cp" +} + +# Run one keep-mode upgrade with the SIGTERM shim. Echoes "\t\t". +run_signal_upgrade() { + local installer="$1" H OUT SHIM rc + H=$(mktemp -d); OUT=$(mktemp); SHIM=$(mktemp -d) + seed_home "$H" + make_term_shim "$SHIM" + set +e + PATH="$SHIM:$ORIG_PATH" \ + MOSAIC_HOME="$H" MOSAIC_INSTALL_MODE=keep MOSAIC_SYNC_ONLY=1 bash "$installer" >"$OUT" 2>&1 + rc=$? + set -e 2>/dev/null || true + rm -rf "$SHIM" + printf '%s\t%s\t%s\n' "$rc" "$OUT" "$H" +} + +IFS=$'\t' read -r rcC OUTC HC < <(run_signal_upgrade "$INSTALL") +chk "[signal] SIGTERM mid-sync aborts non-zero (trap exits, does not resume)" \ + "[ '$rcC' -ne 0 ]" +chk "[signal] restore_snapshot fires on the interrupt" \ + "grep -q 'restoring previous state from snapshot' '$OUTC'" +chk "[signal] does NOT resume to report sync success after the interrupt" \ + "! grep -q 'file phase complete' '$OUTC'" + +# Control: strip `exit 1` from the signal trap → the handler returns, the script +# resumes past the interrupt and wrongly reports success. In $FW so SOURCE_DIR resolves. +sed "s/trap 'restore_snapshot; exit 1' ERR INT TERM/trap 'restore_snapshot' ERR INT TERM/" \ + "$INSTALL" > "$NOEXIT" +chk "[control] the exit-strip actually changed the installer" \ + "! cmp -s '$INSTALL' '$NOEXIT'" +IFS=$'\t' read -r _rcD OUTD HD < <(run_signal_upgrade "$NOEXIT") +chk "[control] without 'exit 1' the trap resumes and reports sync success (the bug)" \ + "grep -q 'file phase complete' '$OUTD'" + +# ── Part D: a failed source/prune `find` scan must abort + roll back (D1) ───── +# A `< <(find …)` process substitution discards find's exit status, so an +# EACCES/I/O failure mid-scan would truncate the file list yet leave the reading +# loop exiting 0 — a partial upgrade committed and reported as success, with the +# ERR/restore trap never firing. The shipped installer captures the scan into a +# checked temp file (_scan_or_die) and aborts on failure. We inject a `find` that +# fails every `-print0` scan and assert the shipped installer rolls back. +IFS=$'\t' read -r rcE OUTE REFE HE < <(run_upgrade "$INSTALL" make_find_fail_shim) +chk "[find-fail] a failing framework scan aborts the upgrade non-zero" \ + "[ '$rcE' -ne 0 ]" +chk "[find-fail] restore_snapshot fires on the aborted scan" \ + "grep -q 'restoring previous state from snapshot' '$OUTE'" +chk "[find-fail] the abort is a fail-closed enumeration error (not a silent truncation)" \ + "grep -q 'Could not enumerate framework files' '$OUTE'" +chk "[find-fail] target rolled back byte-identical to pre-upgrade state" \ + "diff -r '$REFE' '$HE' >/dev/null 2>&1" + +# Control: neuter the D1 guard (turn its `return 1` into a no-op) so a find +# failure is swallowed exactly as `< <(find …)` would — the scan appears to +# succeed and the upgrade reports completion with NO rollback. +sed 's/return 1 # D1-GUARD/: # D1-GUARD-DISABLED/' "$INSTALL" > "$D1CTRL" +chk "[control] the D1-guard strip actually changed the installer" \ + "! cmp -s '$INSTALL' '$D1CTRL'" +IFS=$'\t' read -r _rcF OUTF REFF HF < <(run_upgrade "$D1CTRL" make_find_fail_shim) +chk "[control] with the D1 guard disabled the find failure is swallowed (no rollback)" \ + "! grep -q 'restoring previous state from snapshot' '$OUTF'" +chk "[control] with the D1 guard disabled the upgrade wrongly reports success" \ + "grep -q 'file phase complete' '$OUTF'" + +# ── Part E: a failed target reset inside restore must not exit silently (D2) ── +# restore_snapshot resets the target (`rm -rf; mkdir -p`) before rebuilding from +# the snapshot. Under `set -e` (trap disarmed) a bare reset that fails would exit +# the whole script immediately — after `rm` may have deleted part of the target — +# WITHOUT printing where the snapshot lives. We trigger a rollback (cp poison) AND +# fail the target reset (rm shim), then assert the shipped installer emits the +# manual-recovery pointer and preserves the snapshot. +run_rmfail_upgrade() { + local installer="$1" H OUT SHIM rc + H=$(mktemp -d); OUT=$(mktemp); SHIM=$(mktemp -d) + seed_home "$H" + make_cp_shim "$SHIM" # poison cp → triggers the abort + restore + make_rm_fail_shim "$SHIM" # rm -rf fails → exercises the D2 reset guard + set +e + PATH="$SHIM:$ORIG_PATH" ORIG_PATH_FOR_RM="$ORIG_PATH" FAIL_RM_TARGET="$H" \ + MOSAIC_HOME="$H" MOSAIC_INSTALL_MODE=keep MOSAIC_SYNC_ONLY=1 bash "$installer" >"$OUT" 2>&1 + rc=$? + set -e 2>/dev/null || true + rm -rf "$SHIM" + printf '%s\t%s\t%s\n' "$rc" "$OUT" "$H" +} + +IFS=$'\t' read -r rcG OUTG HG < <(run_rmfail_upgrade "$INSTALL") +chk "[reset-fail] a failed target reset still aborts non-zero" \ + "[ '$rcG' -ne 0 ]" +chk "[reset-fail] the manual-recovery pointer is emitted (not a silent set -e exit)" \ + "grep -q 'Snapshot restore could not reset' '$OUTG'" +chk "[reset-fail] the recovery message points at a preserved snapshot dir" \ + "grep -q 'preserved at: .*mosaic-snapshot' '$OUTG'" +SNAP_E="$(grep -o '/[^ ]*mosaic-snapshot[^ ]*' "$OUTG" | head -1)" +chk "[reset-fail] the named snapshot directory actually survives for recovery" \ + "[ -n '$SNAP_E' ] && [ -d '$SNAP_E' ]" +chk "[reset-fail] operator secret value never appears in installer output" \ + "! grep -q '$SECRET' '$OUTG'" + +# Control: delete the D2 recovery line so a failed reset returns non-zero with NO +# operator pointer — the observable defect (half-reset target, snapshot orphaned +# in /tmp with no path told to the operator). Proves the message is load-bearing. +sed '/Snapshot restore could not reset/d' "$INSTALL" > "$D2CTRL" +chk "[control] the D2-recovery strip actually changed the installer" \ + "! cmp -s '$INSTALL' '$D2CTRL'" +IFS=$'\t' read -r _rcH OUTH HH < <(run_rmfail_upgrade "$D2CTRL") +chk "[control] without the D2 recovery line the operator gets no snapshot pointer" \ + "! grep -q 'Snapshot restore could not reset' '$OUTH'" +[ -n "${SNAP_E:-}" ] && rm -rf "$SNAP_E" +# Reap any snapshot the reset-fail runs left in /tmp (reset failed → never cleaned). +grep -o '/[^ ]*mosaic-snapshot[^ ]*' "$OUTH" 2>/dev/null | head -1 | while read -r s; do rm -rf "$s"; done + +# Cleanup ($STRIPPED / $NOEXIT / $D1CTRL / $D2CTRL are also removed by the EXIT trap). +for d in "$HA" "$REFA" "$HB" "$REFB" "$HC" "$HD" "$HE" "$REFE" "$HF" "$REFF" "$HG" "$HH"; do rm -rf "$d"; done +rm -f "$OUTA" "$OUTB" "$OUTC" "$OUTD" "$OUTE" "$OUTF" "$OUTG" "$OUTH" \ + "$STRIPPED" "$NOEXIT" "$D1CTRL" "$D2CTRL" + +echo +echo "RESULT: $pass passed, $fail failed" +[ "$fail" -eq 0 ] diff --git a/packages/mosaic/src/config/file-adapter.test.ts b/packages/mosaic/src/config/file-adapter.test.ts index 9019436..ae56ff8 100644 --- a/packages/mosaic/src/config/file-adapter.test.ts +++ b/packages/mosaic/src/config/file-adapter.test.ts @@ -1,9 +1,22 @@ import { describe, it, expect, beforeEach, afterEach } from 'vitest'; -import { mkdtempSync, mkdirSync, writeFileSync, rmSync, readFileSync, existsSync } from 'node:fs'; +import { + mkdtempSync, + mkdirSync, + writeFileSync, + rmSync, + readFileSync, + existsSync, + copyFileSync, +} from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; +import { fileURLToPath } from 'node:url'; import { FileConfigAdapter, DEFAULT_SEED_FILES } from './file-adapter.js'; +// The real shipping manifest — the fixture uses it verbatim so these tests +// exercise production ownership resolution, not a synthetic copy (#791). +const REAL_FRAMEWORK_ROOT = fileURLToPath(new URL('../../framework', import.meta.url)); + /** * Regression tests for the `FileConfigAdapter.syncFramework` seed behavior. * @@ -34,6 +47,13 @@ function makeFixture(): { sourceDir: string; mosaicHome: string; defaultsDir: st mkdirSync(defaultsDir, { recursive: true }); mkdirSync(mosaicHome, { recursive: true }); + // #791: syncFramework resolves ownership from the shared manifest under the + // source dir. Seed the real one so keep-mode syncs behave as in production. + copyFileSync( + join(REAL_FRAMEWORK_ROOT, 'framework-manifest.txt'), + join(sourceDir, 'framework-manifest.txt'), + ); + // Framework-contract defaults we expect the wizard to seed. writeFileSync(join(defaultsDir, 'CONSTITUTION.md'), '# CONSTITUTION default\n'); writeFileSync(join(defaultsDir, 'AGENTS.md'), '# AGENTS default\n'); @@ -102,9 +122,10 @@ describe('FileConfigAdapter.syncFramework — defaults seeding', () => { }); it('overwrites framework-owned files (backup-once) but preserves user-seeded files', async () => { - // Plant a root-level AGENTS.md in sourceDir so syncDirectory's preserve is exercised. - writeFileSync(join(fixture.sourceDir, 'AGENTS.md'), '# shipped AGENTS from source root\n'); - + // Contract files (CONSTITUTION/AGENTS/STANDARDS) ship only under defaults/ — + // reconcile_framework_files is their sole writer (backup-once). The bulk + // sync never sees a root-level copy, so a user's edited root file is backed + // up, not silently clobbered, on upgrade. writeFileSync(join(fixture.mosaicHome, 'TOOLS.md'), '# user-customized TOOLS\n'); writeFileSync(join(fixture.mosaicHome, 'AGENTS.md'), '# user-customized AGENTS\n'); diff --git a/packages/mosaic/src/config/file-adapter.ts b/packages/mosaic/src/config/file-adapter.ts index 1480d6a..4bd4f23 100644 --- a/packages/mosaic/src/config/file-adapter.ts +++ b/packages/mosaic/src/config/file-adapter.ts @@ -34,6 +34,7 @@ import { buildToolsTemplateVars, } from '../template/builders.js'; import { atomicWrite, backupFile, syncDirectory } from '../platform/file-ops.js'; +import { loadManifest, resolveOwnership } from '../framework/manifest.js'; /** * Parse a SoulConfig from an existing SOUL.md file. @@ -155,38 +156,22 @@ export class FileConfigAdapter implements ConfigService { } async syncFramework(action: InstallAction): Promise { - // Must match PRESERVE_PATHS in packages/mosaic/framework/install.sh so - // the bash and TS install paths have the same upgrade-preservation - // semantics. Contract files (AGENTS.md, STANDARDS.md, TOOLS.md) are - // seeded from defaults/ on first install and preserved thereafter; - // identity files (SOUL.md, USER.md) are generated by wizard stages and - // must never be touched by the framework sync. - const preservePaths = - action === 'keep' || action === 'reconfigure' - ? [ - 'CONSTITUTION.md', - 'AGENTS.md', - 'SOUL.md', - 'USER.md', - 'TOOLS.md', - 'STANDARDS.md', - 'memory', - 'sources', - 'credentials', - // User-authored fleet data MUST survive `mosaic update`'s re-seed. - // The framework seeds only fleet/examples + fleet/roles + - // fleet/roster.schema.json; the operator's roster, per-agent env, and - // heartbeat run dir stay user-owned. (Mirror of install.sh PRESERVE_PATHS.) - 'fleet/roster.yaml', - 'fleet/roster.json', - 'fleet/agents', - 'fleet/run', - ] - : []; + // #791: ownership is derived from the shared framework manifest + // (packages/mosaic/framework/framework-manifest.txt) — the SAME file the + // bash installer reads — so the TS and bash paths can never drift. On an + // upgrade (keep/reconfigure) the sync must NEVER write an operator-owned + // path: every operator file, and any path the manifest never anticipated + // (which resolves to operator by the fail-safe default), is left untouched. + // A fresh install ('overwrite'/'reconfigure' onto an empty home) seeds the + // full tree, so the guard applies only when preserving an existing home. + const guardOwnership = action === 'keep' || action === 'reconfigure'; + const manifest = guardOwnership ? loadManifest(this.sourceDir) : undefined; syncDirectory(this.sourceDir, this.mosaicHome, { - preserve: preservePaths, excludeGit: true, + isOperatorOwned: manifest + ? (relPath) => resolveOwnership(manifest, relPath) === 'operator' + : undefined, }); // Reconcile framework-contract files from framework/defaults/ into the mosaic diff --git a/packages/mosaic/src/framework/manifest-parity.spec.ts b/packages/mosaic/src/framework/manifest-parity.spec.ts new file mode 100644 index 0000000..f2915f6 --- /dev/null +++ b/packages/mosaic/src/framework/manifest-parity.spec.ts @@ -0,0 +1,343 @@ +import { afterAll, describe, it, expect } from 'vitest'; +import { execFileSync, spawnSync } from 'node:child_process'; +import { existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { + loadManifest, + parseManifest, + resolveOwnership, + frameworkSubtreeRoots, +} from './manifest.js'; + +/** + * Bash ↔ TS parity (#791, §6.1). + * + * The installer (bash) and the config adapter (TS) each resolve path ownership + * from framework-manifest.txt. If the two resolvers disagreed on a single path, + * an upgrade could protect a file on one code path and wipe it on the other — + * exactly the two-copies drift that #631 patched by hand. This test drives the + * bash resolver (`tools/_lib/manifest.sh`) as a subprocess and asserts it agrees + * with the TS resolver for a broad set of paths spanning every ownership class. + */ + +const FRAMEWORK_ROOT = fileURLToPath(new URL('../../framework', import.meta.url)); +const MANIFEST_SH = join(FRAMEWORK_ROOT, 'tools', '_lib', 'manifest.sh'); + +const hasBash = (() => { + try { + execFileSync('bash', ['-c', 'true'], { stdio: 'ignore' }); + return true; + } catch { + return false; + } +})(); + +function bashResolve(relPath: string): string { + return execFileSync('bash', [MANIFEST_SH, 'resolve', relPath], { + encoding: 'utf-8', + }).trim(); +} + +/** Drive the bash resolver against an arbitrary manifest file (MANIFEST_FILE override). */ +function bashResolveWith(manifestFile: string, relPath: string): string { + return execFileSync('bash', [MANIFEST_SH, 'resolve', relPath], { + encoding: 'utf-8', + env: { ...process.env, MANIFEST_FILE: manifestFile }, + }).trim(); +} + +function bashSubtreeRoots(): string[] { + return execFileSync('bash', [MANIFEST_SH, 'subtree-roots'], { encoding: 'utf-8' }) + .split('\n') + .map((s) => s.trim()) + .filter((s) => s.length > 0); +} + +/** + * Drive the bash resolver CLI against a manifest file and report how it exited. + * A fail-closed manifest must make the CLI exit non-zero with a message on + * stderr — never exit 0 having silently resolved everything to operator. + */ +function bashCli(manifestFile: string): { status: number; stderr: string } { + const res = spawnSync('bash', [MANIFEST_SH, 'resolve', 'CONSTITUTION.md'], { + encoding: 'utf-8', + env: { ...process.env, MANIFEST_FILE: manifestFile }, + }); + return { status: res.status ?? -1, stderr: res.stderr ?? '' }; +} + +// Paths spanning every ownership class: framework single-files, framework +// subtrees, operator declared trees, operator carve-out inside a framework +// subtree, local overlays, and deliberately UNANTICIPATED paths (fail-safe). +const PROBE_PATHS = [ + 'CONSTITUTION.md', + 'AGENTS.md', + 'STANDARDS.md', + 'install.sh', + 'framework-manifest.txt', + 'guides/E2E-DELIVERY.md', + 'tools/git/pr-create.sh', + 'tools/_lib/manifest.sh', + 'defaults/SOUL.md', + 'fleet/README.md', + 'fleet/roles/coder.md', + 'fleet/roster.schema.json', + 'fleet/examples/general.yaml', + // operator + 'SOUL.md', + 'USER.md', + 'TOOLS.md', + 'SOUL.local.md', + 'USER.local.md', + 'STANDARDS.local.md', + 'agents/coder0.conf', + 'policy/custom.md', + 'memory/note.md', + 'sources/skills/x.md', + 'credentials/c.json', + 'tools/_lib/credentials.json', + 'fleet/roster.yaml', + 'fleet/roster.json', + 'fleet/agents/coder0.env', + 'fleet/run/coder0.hb', + // #797 Runtime Session Ledger — must resolve operator on both paths. + 'fleet/run/sessions/events.ndjson', + 'fleet/run/sessions/ledger.json', + 'fleet/backlog/data.db', + 'fleet/roles.local/custom.md', + // unanticipated → operator (fail-safe) + 'harvester/sop.md', + 'unknown-operator-dir/x', + 'fleet/my-fleet.yaml', + 'random-root-file.md', + 'tools/some-new-framework-tool.sh', +]; + +describe.skipIf(!hasBash)('bash ↔ TS manifest parity (§6.1)', () => { + it('the bash resolver CLI exists and is executable', () => { + expect(existsSync(MANIFEST_SH)).toBe(true); + }); + + it('bash and TS resolve identical ownership for every probe path', () => { + const manifest = loadManifest(FRAMEWORK_ROOT); + const disagreements: Array<{ path: string; ts: string; bash: string }> = []; + for (const p of PROBE_PATHS) { + const ts = resolveOwnership(manifest, p); + const bash = bashResolve(p); + if (ts !== bash) disagreements.push({ path: p, ts, bash }); + } + expect(disagreements).toEqual([]); + }); + + it('bash and TS agree on the framework subtree roots', () => { + const manifest = loadManifest(FRAMEWORK_ROOT); + expect(bashSubtreeRoots().sort()).toEqual(frameworkSubtreeRoots(manifest).sort()); + }); +}); + +/** + * Format-safety parity (#791, Decision 1 — the `.txt` line-oriented format is + * accepted only because both resolvers agree on the format edge cases a hand- + * edited text file invites: comments, blank lines, stray whitespace, duplicate + * and overlapping globs (where deny-wins must resolve), and section ordering. + * Each fixture is driven through BOTH resolvers (bash via MANIFEST_FILE, TS via + * parseManifest) and must agree AND land on the expected ownership. Any + * divergence here means the format itself is unsafe and must be fixed/converted. + */ +describe.skipIf(!hasBash)('bash ↔ TS manifest format-edge parity (§6.1, Decision 1)', () => { + const tmp = mkdtempSync(join(tmpdir(), 'mf-parity-')); + afterAll(() => rmSync(tmp, { recursive: true, force: true })); + + let fixtureSeq = 0; + function writeFixture(text: string): string { + const file = join(tmp, `manifest-${fixtureSeq++}.txt`); + writeFileSync(file, text); + return file; + } + + // Both resolvers must agree, and on the expected value, for every probe. + function expectParity(text: string, cases: ReadonlyArray): void { + const file = writeFixture(text); + const manifest = parseManifest(text); + for (const [path, expected] of cases) { + const ts = resolveOwnership(manifest, path); + const bash = bashResolveWith(file, path); + expect(bash, `bash disagrees with TS on ${path}`).toBe(ts); + expect(ts, `ownership of ${path}`).toBe(expected); + } + } + + it('tolerates comments, blank lines, and leading/trailing whitespace identically', () => { + // Entries and headers are padded with spaces/tabs; comments and blanks are + // interleaved. Both resolvers must trim and ignore them the same way. + const text = [ + '# leading comment', + ' ', + '\t[framework] ', + ' tools/** ', + '# mid-section comment', + '', + '\tguides/**\t', + ' [operator] ', + '\ttools/_lib/credentials.json ', + '*.local.md', + '', + ].join('\n'); + expectParity(text, [ + ['tools/git/pr-create.sh', 'framework'], + ['guides/E2E-DELIVERY.md', 'framework'], + ['tools/_lib/credentials.json', 'operator'], // deny-wins carve-out inside tools/** + ['SOUL.local.md', 'operator'], + ['nowhere/unknown.md', 'operator'], // negative probe: matches NO rule → operator + ]); + }); + + it('resolves deny-wins for overlapping and duplicate globs identically', () => { + // Framework claims tools/** (twice) and the overlapping tools/git/**; + // operator carves out tools/_lib/**. Operator must win the overlap on both. + const text = [ + '[framework]', + 'tools/**', + 'tools/**', // duplicate — must not change resolution + 'tools/git/**', // overlaps tools/** + '[operator]', + 'tools/_lib/**', + ].join('\n'); + expectParity(text, [ + ['tools/git/pr-create.sh', 'framework'], + ['tools/other.sh', 'framework'], + ['tools/_lib/credentials.json', 'operator'], // deny-wins over both framework globs + ['tools/_lib/nested/deep.json', 'operator'], + ]); + }); + + it('is independent of section and glob ordering', () => { + // Same rule set, operator section first and entries reordered. Resolution + // must be identical because deny-wins checks all operator globs before any + // framework glob — order within or between sections cannot matter. + const forward = [ + '[framework]', + 'guides/**', + 'tools/**', + '[operator]', + 'tools/_lib/credentials.json', + '*.local.md', + ].join('\n'); + const reversed = [ + '[operator]', + '*.local.md', + 'tools/_lib/credentials.json', + '[framework]', + 'tools/**', + 'guides/**', + ].join('\n'); + const probes: ReadonlyArray = [ + ['tools/git/pr-create.sh', 'framework'], + ['guides/E2E-DELIVERY.md', 'framework'], + ['tools/_lib/credentials.json', 'operator'], + ['SOUL.local.md', 'operator'], + ['unanticipated/path.md', 'operator'], + ]; + expectParity(forward, probes); + expectParity(reversed, probes); + // And the two orderings agree path-for-path on both resolvers. + const fFile = writeFixture(forward); + const rFile = writeFixture(reversed); + for (const [path] of probes) { + expect(bashResolveWith(fFile, path)).toBe(bashResolveWith(rFile, path)); + } + }); + + it('defaults an unmatched path to operator on both resolvers (UNKNOWN → operator)', () => { + // A manifest that names only a narrow framework slice. Everything else — + // including paths under no rule at all — must fail safe to operator. + const text = ['[framework]', 'guides/**', '[operator]', 'agents/**'].join('\n'); + expectParity(text, [ + ['guides/x.md', 'framework'], + ['agents/coder0.conf', 'operator'], + ['totally/unlisted/file.txt', 'operator'], // negative probe + ['fleet/run/sessions/ledger.json', 'operator'], // unlisted → operator + ['README.md', 'operator'], + ]); + }); +}); + +/** + * Failure-mode parity (#791 B2/B3). A bad manifest is the dangerous case: if the + * two resolvers DISAGREED on rejection — one throwing while the other quietly + * resolved everything to operator — an upgrade could fail loud on one code path + * and no-op on the other. So for every malformed/empty/missing manifest, BOTH + * must reject: TS throws, and the bash CLI exits non-zero with a stderr message. + */ +describe.skipIf(!hasBash)('bash ↔ TS manifest failure-mode parity (§6.1, B2/B3)', () => { + const tmp = mkdtempSync(join(tmpdir(), 'mf-failmode-')); + afterAll(() => rmSync(tmp, { recursive: true, force: true })); + + let seq = 0; + function writeFixture(text: string): string { + const file = join(tmp, `bad-manifest-${seq++}.txt`); + writeFileSync(file, text); + return file; + } + + // TS throws AND bash CLI exits non-zero with a non-empty stderr — identical rejection. + function expectBothReject(label: string, manifestFile: string): void { + expect(() => parseManifestFile(manifestFile), `TS accepted ${label}`).toThrow(); + const cli = bashCli(manifestFile); + expect(cli.status, `bash did not exit non-zero for ${label}`).not.toBe(0); + expect(cli.stderr.trim().length, `bash was silent for ${label}`).toBeGreaterThan(0); + } + + // Read the file for the TS side the same way loadManifest does, so both halves + // see identical bytes (loadManifest keys off a directory, not an arbitrary file). + function parseManifestFile(file: string): void { + parseManifest(readFileSync(file, 'utf-8')); + } + + it('both reject a completely empty manifest', () => { + expectBothReject('empty', writeFixture('')); + }); + + it('both reject a comment/blank-only manifest', () => { + expectBothReject('comment-only', writeFixture('# header only\n\n \n')); + }); + + it('both reject an operator-only manifest (zero framework paths)', () => { + expectBothReject('operator-only', writeFixture('[operator]\nSOUL.md\n*.local.md\n')); + }); + + it('both reject a [framework] section with no entries', () => { + expectBothReject('empty-framework-section', writeFixture('[framework]\n[operator]\nSOUL.md\n')); + }); + + it('both reject a [framework] entry that normalizes to an empty glob (/)', () => { + expectBothReject('root-slash-framework', writeFixture('[framework]\n/\n')); + }); + + it('both reject a [framework] entry that normalizes to nothing (./)', () => { + expectBothReject('dot-slash-framework', writeFixture('[framework]\n./\n[operator]\nSOUL.md\n')); + }); + + it('both reject [framework] entries that are only bare dot segments', () => { + expectBothReject('bare-dot-framework', writeFixture('[framework]\n.\n..\n')); + }); + + it('both reject an entry that appears before any section header', () => { + expectBothReject('entry-before-header', writeFixture('stray.md\n[framework]\nguides/**\n')); + }); + + it('both reject an unknown section header', () => { + expectBothReject('unknown-header', writeFixture('[bogus]\nx\n')); + }); + + it('both reject a missing manifest file (fail-closed, not empty result)', () => { + const missing = join(tmp, 'does-not-exist.txt'); + // TS: loadManifest would throw a read error; here read-then-parse throws on read. + expect(() => parseManifestFile(missing)).toThrow(); + const cli = bashCli(missing); + expect(cli.status).not.toBe(0); + expect(cli.stderr.trim().length).toBeGreaterThan(0); + }); +}); diff --git a/packages/mosaic/src/framework/manifest.spec.ts b/packages/mosaic/src/framework/manifest.spec.ts new file mode 100644 index 0000000..bc5ab5a --- /dev/null +++ b/packages/mosaic/src/framework/manifest.spec.ts @@ -0,0 +1,327 @@ +import { describe, it, expect } from 'vitest'; +import { readdirSync, statSync } from 'node:fs'; +import { join, relative } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { + parseManifest, + loadManifest, + matchGlob, + resolveOwnership, + frameworkSubtreeRoots, + planPrune, + ManifestError, + type FrameworkManifest, +} from './manifest.js'; + +const FRAMEWORK_ROOT = fileURLToPath(new URL('../../framework', import.meta.url)); + +const SAMPLE = ` +# comment +[framework] +CONSTITUTION.md +guides/** +tools/** + +[operator] +SOUL.md +*.local.md +agents/** +tools/_lib/credentials.json +`; + +describe('parseManifest', () => { + it('splits entries into framework and operator sections, ignoring comments/blanks', () => { + const m = parseManifest(SAMPLE); + expect(m.framework).toEqual(['CONSTITUTION.md', 'guides/**', 'tools/**']); + expect(m.operator).toEqual([ + 'SOUL.md', + '*.local.md', + 'agents/**', + 'tools/_lib/credentials.json', + ]); + }); + + it('rejects an entry that appears before any section header', () => { + expect(() => parseManifest('stray.md\n[framework]\n')).toThrow(/before any \[section\]/); + }); + + it('rejects an unknown section header', () => { + expect(() => parseManifest('[bogus]\nx\n')).toThrow(/Unknown manifest section/); + }); +}); + +// Fail-closed parsing/loading (#791 B2/B3). An empty, comment-only, operator-only, +// or unreadable manifest must NOT resolve to "framework owns nothing" (which would +// make an upgrade a silent no-op). Both must throw so finalizeStage surfaces the +// abort instead of reporting "Installation complete". The bash reader rejects the +// same inputs — asserted for parity in manifest-parity.spec.ts. +describe('parseManifest / loadManifest fail closed on empty or unreadable input', () => { + it('throws on a completely empty manifest', () => { + expect(() => parseManifest('')).toThrow(/no \[framework\] paths/); + }); + + it('throws on a comment- and blank-only manifest (no entries at all)', () => { + expect(() => parseManifest('# just a header comment\n\n \n')).toThrow( + /no \[framework\] paths/, + ); + }); + + it('throws when only an [operator] section is present (zero framework paths)', () => { + expect(() => parseManifest('[operator]\nSOUL.md\n*.local.md\n')).toThrow( + /no \[framework\] paths/, + ); + }); + + it('throws on a [framework] header with no entries beneath it', () => { + expect(() => parseManifest('[framework]\n\n[operator]\nSOUL.md\n')).toThrow( + /no \[framework\] paths/, + ); + }); + + // Degenerate framework entries that pass the length check but normalize to a + // glob matching nothing — the manifest would silently protect the whole tree + // as operator (#791 blocker-B). Both `/` and `./` normalize to '' ; `.`/`..` + // are bare-dot segments. + it.each([['/'], ['./'], ['.'], ['..'], ['/\n./']])( + 'throws when the only [framework] entry (%j) normalizes to nothing usable', + (entry) => { + expect(() => parseManifest(`[framework]\n${entry}\n`)).toThrow( + /no usable \[framework\] paths/, + ); + }, + ); + + it('accepts a wildcard-only framework glob (** is usable)', () => { + expect(() => parseManifest('[framework]\n**\n')).not.toThrow(); + }); + + it('loadManifest throws a clear fail-closed error when the manifest file is missing', () => { + const missingRoot = fileURLToPath(new URL('./__no_such_framework_root__', import.meta.url)); + expect(() => loadManifest(missingRoot)).toThrow(/Cannot read framework manifest/); + }); + + // The distinct error type is what lets finalizeStage tell a pre-sync validation + // abort (nothing written) from a mid-sync filesystem failure (#791 blocker-C). + it('every fail-closed rejection is a ManifestError', () => { + expect(() => parseManifest('')).toThrow(ManifestError); + expect(() => parseManifest('[operator]\nSOUL.md\n')).toThrow(ManifestError); + expect(() => parseManifest('[framework]\n/\n')).toThrow(ManifestError); + expect(() => parseManifest('[bogus]\nx\n')).toThrow(ManifestError); + expect(() => parseManifest('stray.md\n[framework]\n')).toThrow(ManifestError); + const missingRoot = fileURLToPath(new URL('./__no_such_framework_root__', import.meta.url)); + expect(() => loadManifest(missingRoot)).toThrow(ManifestError); + }); +}); + +describe('matchGlob', () => { + it('matches an exact file', () => { + expect(matchGlob('CONSTITUTION.md', 'CONSTITUTION.md')).toBe(true); + expect(matchGlob('CONSTITUTION.md', 'AGENTS.md')).toBe(false); + }); + + it('treats a bare directory entry as covering its descendants', () => { + expect(matchGlob('memory', 'memory')).toBe(true); + expect(matchGlob('memory', 'memory/notes.md')).toBe(true); + expect(matchGlob('memory', 'memoryfoo')).toBe(false); + }); + + it('** matches any depth including the root itself', () => { + expect(matchGlob('agents/**', 'agents')).toBe(true); + expect(matchGlob('agents/**', 'agents/a.conf')).toBe(true); + expect(matchGlob('agents/**', 'agents/nested/deep.conf')).toBe(true); + expect(matchGlob('agents/**', 'agentsX')).toBe(false); + }); + + it('* stays within a single segment', () => { + expect(matchGlob('*.local.md', 'SOUL.local.md')).toBe(true); + expect(matchGlob('*.local.md', 'a/SOUL.local.md')).toBe(false); + }); +}); + +describe('resolveOwnership (deny-wins + fail-safe)', () => { + const m = parseManifest(SAMPLE); + + it('operator globs win over framework globs (carve-out inside a framework subtree)', () => { + expect(resolveOwnership(m, 'tools/_lib/credentials.json')).toBe('operator'); + expect(resolveOwnership(m, 'tools/git/pr-create.sh')).toBe('framework'); + }); + + it('framework-declared paths resolve to framework', () => { + expect(resolveOwnership(m, 'guides/E2E-DELIVERY.md')).toBe('framework'); + expect(resolveOwnership(m, 'CONSTITUTION.md')).toBe('framework'); + }); + + it('UNKNOWN paths default to operator (the #791 root-cause guarantee)', () => { + expect(resolveOwnership(m, 'agents/coder0.conf')).toBe('operator'); // declared + expect(resolveOwnership(m, 'harvester/sop.md')).toBe('operator'); // undeclared → fail-safe + expect(resolveOwnership(m, 'totally-unknown-dir/x')).toBe('operator'); + expect(resolveOwnership(m, 'random-root-file.md')).toBe('operator'); + }); +}); + +describe('planPrune (pure prune planner)', () => { + const m = parseManifest(SAMPLE); + + it('prunes a retired framework file inside a shipped subtree', () => { + const del = planPrune({ + manifest: m, + targetPaths: ['guides/OLD.md', 'guides/KEEP.md'], + sourcePaths: ['guides/KEEP.md'], + }); + expect(del).toEqual(['guides/OLD.md']); + }); + + it('never prunes operator-reserved paths even when absent from source', () => { + const del = planPrune({ + manifest: m, + targetPaths: ['agents/coder0.conf', 'tools/_lib/credentials.json', 'SOUL.local.md'], + sourcePaths: [], + }); + expect(del).toEqual([]); + }); + + it('never prunes UNKNOWN paths outside every framework subtree (fail-safe)', () => { + const del = planPrune({ + manifest: m, + targetPaths: ['harvester/sop.md', 'my-fleet.yaml', 'unknown-dir/deep/x'], + sourcePaths: [], + }); + expect(del).toEqual([]); + }); + + it('never prunes single-file framework entries (reconcile-managed, not in subtree)', () => { + const del = planPrune({ manifest: m, targetPaths: ['CONSTITUTION.md'], sourcePaths: [] }); + expect(del).toEqual([]); + }); + + it('property: delete-set ⊆ {framework-owned ∧ in-target ∧ not-in-source} and ∩ operator = ∅', () => { + const operatorish = [ + 'agents/a.conf', + 'policy/p.md', + 'SOUL.local.md', + 'memory/m.md', + 'tools/_lib/credentials.json', + 'harvester/sop.md', + 'unknown-top/x', + 'another-unknown/deep/y.txt', + ]; + const frameworkish = ['guides/A.md', 'guides/sub/B.md', 'tools/git/x.sh']; + const targetPaths = [...operatorish, ...frameworkish]; + const del = planPrune({ manifest: m, targetPaths, sourcePaths: [] }); + + for (const p of del) { + expect(resolveOwnership(m, p)).toBe('framework'); + expect(targetPaths).toContain(p); + } + // No operator/unknown path ever appears in the delete-set. + for (const p of operatorish) expect(del).not.toContain(p); + }); +}); + +// The #797 Runtime Session Ledger lives at fleet/run/sessions/. Today it is safe +// twice over: it matches the explicit `fleet/run/**` operator carve-out AND, even +// without it, the UNKNOWN→operator fail-safe. This test isolates the CARVE-OUT's +// load-bearing value by simulating a future framework author who broadens fleet +// ownership to `fleet/**`: without the operator carve-out the ledger would resolve +// framework and be pruned; deny-wins is what keeps it protected. If deleting the +// `fleet/run/**` line ever stops turning this test red, the carve-out has silently +// stopped mattering — which is exactly the #797 regression we are gating against. +describe('fleet/run/** carve-out is load-bearing for the #797 ledger (deny-wins)', () => { + const LEDGER = ['fleet/run/sessions/events.ndjson', 'fleet/run/sessions/ledger.json']; + // A framework that (hypothetically) ships all of fleet/** as a subtree. + const withoutCarveOut: FrameworkManifest = { + framework: ['fleet/**'], + operator: [], + }; + const withCarveOut: FrameworkManifest = { + framework: ['fleet/**'], + operator: ['fleet/run/**'], + }; + + it('RED without the carve-out: the ledger resolves framework and is pruned', () => { + for (const p of LEDGER) expect(resolveOwnership(withoutCarveOut, p)).toBe('framework'); + const del = planPrune({ manifest: withoutCarveOut, targetPaths: LEDGER, sourcePaths: [] }); + expect(del.sort()).toEqual([...LEDGER].sort()); + }); + + it('GREEN with the carve-out: deny-wins makes the ledger operator and unprunable', () => { + for (const p of LEDGER) expect(resolveOwnership(withCarveOut, p)).toBe('operator'); + const del = planPrune({ manifest: withCarveOut, targetPaths: LEDGER, sourcePaths: [] }); + expect(del).toEqual([]); + }); + + it('the SHIPPED manifest reserves fleet/run/** so the ledger is operator-owned', () => { + const shipped = loadManifest(FRAMEWORK_ROOT); + for (const p of LEDGER) expect(resolveOwnership(shipped, p)).toBe('operator'); + // And it is structurally unreachable by pruning even if it were in a subtree. + expect(planPrune({ manifest: shipped, targetPaths: LEDGER, sourcePaths: [] })).toEqual([]); + }); +}); + +describe('frameworkSubtreeRoots', () => { + it('returns only the /** subtree roots, not single-file entries', () => { + const m = parseManifest(SAMPLE); + expect(frameworkSubtreeRoots(m)).toEqual(['guides', 'tools']); + }); +}); + +// ── SSOT manifest: shipped-file completeness (§6.2) ────────────────────────── +// A newly-shipped framework file must not silently fall outside the manifest — +// if it did, the updater could neither guarantee it as framework-owned nor +// prune it when retired. Every file the framework actually ships must resolve +// to `framework` (except the defaults/{SOUL,USER}.md identity seeds, which are +// operator-owned by design). +describe('manifest completeness against shipped framework tree', () => { + const manifest = loadManifest(FRAMEWORK_ROOT); + + const IGNORED_TOP = new Set(['.git', 'node_modules']); + // Framework-shipped files that are operator-owned by design: the identity + // seeds under defaults/, and the `.gitkeep` placeholder that lets the empty + // operator-owned memory/ directory exist in git. + function isOperatorShipped(rel: string): boolean { + if (rel === 'defaults/SOUL.md' || rel === 'defaults/USER.md') return true; + if (rel.startsWith('memory/')) return true; + return false; + } + + function walk(dir: string): string[] { + const out: string[] = []; + for (const entry of readdirSync(dir)) { + const abs = join(dir, entry); + const rel = relative(FRAMEWORK_ROOT, abs); + if (IGNORED_TOP.has(rel)) continue; + if (statSync(abs).isDirectory()) out.push(...walk(abs)); + else out.push(rel); + } + return out; + } + + it('every shipped framework file resolves to framework ownership', () => { + const shipped = walk(FRAMEWORK_ROOT); + const misclassified = shipped.filter( + (p) => !isOperatorShipped(p) && resolveOwnership(manifest, p) !== 'framework', + ); + expect(misclassified).toEqual([]); + }); + + it('the operator-owned surface from #791 resolves to operator', () => { + const operatorPaths = [ + 'agents/coder0.conf', + 'fleet/agents/coder0.env', + 'memory/note.md', + 'policy/custom.md', + 'SOUL.local.md', + 'USER.local.md', + 'STANDARDS.local.md', + 'tools/_lib/credentials.json', + 'fleet/roster.yaml', + 'fleet/roster.json', + 'fleet/run/coder0.hb', + 'fleet/backlog/data.db', + 'fleet/roles.local/custom.md', + ]; + for (const p of operatorPaths) { + expect(resolveOwnership(manifest, p), p).toBe('operator'); + } + }); +}); diff --git a/packages/mosaic/src/framework/manifest.ts b/packages/mosaic/src/framework/manifest.ts new file mode 100644 index 0000000..056a03b --- /dev/null +++ b/packages/mosaic/src/framework/manifest.ts @@ -0,0 +1,248 @@ +import { readFileSync } from 'node:fs'; + +/** + * Framework path-ownership manifest (#791). + * + * The updater must operate from an explicit framework-owned path manifest and + * NEVER write outside it. This module is the TypeScript reader for the shared + * SSOT manifest (`packages/mosaic/framework/framework-manifest.txt`) that the + * bash installer also consumes. Keeping both paths on one data file is what + * closes the two-copies-drift failure class (see #631 → #791). + * + * Everything here is pure (parse + resolve + plan) so the ownership guarantee + * is unit- and property-testable without touching the filesystem. + */ + +export type Ownership = 'framework' | 'operator'; + +/** + * Thrown when the manifest is missing, empty, or malformed. A distinct type lets + * callers (e.g. finalizeStage) tell a pre-sync validation abort — where NO files + * were touched — apart from a generic mid-sync filesystem failure, and message + * the user accurately (#791 blocker-C). + */ +export class ManifestError extends Error { + constructor(message: string) { + super(message); + this.name = 'ManifestError'; + } +} + +export interface FrameworkManifest { + /** Globs the updater MAY create/overwrite, and prune only when retired. */ + readonly framework: readonly string[]; + /** Globs the updater must NEVER write over or prune. Win over `framework`. */ + readonly operator: readonly string[]; +} + +type Section = 'framework' | 'operator' | null; + +/** + * Parse the line-oriented manifest text. `#` comments and blank lines are + * ignored; `[framework]` / `[operator]` headers switch the active section. + * Lines before any header are rejected — the format must be explicit. + */ +export function parseManifest(text: string): FrameworkManifest { + const framework: string[] = []; + const operator: string[] = []; + let section: Section = null; + + const lines = text.split(/\r?\n/); + for (let i = 0; i < lines.length; i++) { + const raw = lines[i] ?? ''; + const line = raw.trim(); + if (line === '' || line.startsWith('#')) continue; + + if (line === '[framework]') { + section = 'framework'; + continue; + } + if (line === '[operator]') { + section = 'operator'; + continue; + } + if (line.startsWith('[')) { + throw new ManifestError(`Unknown manifest section header on line ${i + 1}: ${line}`); + } + + if (section === null) { + throw new ManifestError( + `Manifest entry before any [section] header on line ${i + 1}: ${line}`, + ); + } + (section === 'framework' ? framework : operator).push(line); + } + + // Fail CLOSED on an empty or comment-only manifest. A manifest with zero + // framework-owned globs would make resolveOwnership() return `operator` for + // every path: an upgrade would prune nothing and refresh nothing — a silent + // no-op indistinguishable from success. Refuse loudly instead, mirroring the + // bash reader's `manifest_load` guard so both halves reject it identically (#791 B2). + if (framework.length === 0) { + throw new ManifestError( + 'Framework manifest defines no [framework] paths — refusing to proceed (empty or malformed manifest).', + ); + } + + // Fail CLOSED on framework entries that normalize to nothing usable. A manifest + // like `[framework]\n/` or `[framework]\n./` passes the length check above but + // every entry normalizes to an empty (or bare-dot) glob that matches no real + // path — so the compiled framework matcher is empty and every path resolves + // `operator`: the same silent no-op as an empty manifest. Require at least one + // entry with a real, non-dot character (the bash reader applies the identical + // `[^/.]` test, so both halves reject these inputs together — #791 blocker-B). + if (!framework.some(isUsableFrameworkGlob)) { + throw new ManifestError( + 'Framework manifest defines no usable [framework] paths (every entry is empty or a bare dot segment) — refusing to proceed (malformed manifest).', + ); + } + + return { framework, operator }; +} + +/** + * A framework glob is usable only if, once normalized, it still contains a + * character other than `/` or `.` — i.e. it names a real path segment or a + * wildcard. `''`, `/`, `./`, `.`, `..` are all unusable (they compile to a glob + * that matches nothing). Kept byte-compatible with the bash `[[ =~ [^/.] ]]` + * test so TS and bash accept/reject exactly the same manifests. + */ +function isUsableFrameworkGlob(glob: string): boolean { + return /[^/.]/.test(normalizeRel(glob)); +} + +/** Read and parse the manifest from a framework root directory. */ +export function loadManifest(frameworkRoot: string): FrameworkManifest { + const file = `${frameworkRoot}/framework-manifest.txt`; + let text: string; + try { + text = readFileSync(file, 'utf-8'); + } catch (err) { + // A missing/unreadable manifest must fail closed with a clear message, not a + // raw ENOENT that a caller might mistake for an empty result set (#791 B2/B3). + throw new ManifestError( + `Cannot read framework manifest at ${file}: ${(err as Error).message} — refusing to sync (fail-closed).`, + ); + } + return parseManifest(text); +} + +/** + * Match a mosaic-home-relative POSIX path against one glob. + * + * Supported: `**` (any depth, including zero segments) and `*` (any run of + * characters within a single segment, not crossing `/`). A glob with no + * wildcard matches either the exact path OR any path beneath it (so a bare + * directory entry like `memory` covers `memory/notes.md`). + */ +export function matchGlob(glob: string, relPath: string): boolean { + const path = normalizeRel(relPath); + const pattern = normalizeRel(glob); + if (pattern === '') return false; + + if (!pattern.includes('*')) { + // Exact file, or any descendant of a bare directory prefix. + return path === pattern || path.startsWith(`${pattern}/`); + } + + const re = new RegExp(`^${globToRegExpBody(pattern)}$`); + return re.test(path); +} + +/** True if the path matches any glob in the list. */ +export function matchesAny(globs: readonly string[], relPath: string): boolean { + return globs.some((g) => matchGlob(g, relPath)); +} + +/** + * Resolve ownership of a mosaic-home-relative path (deny-wins / fail-safe): + * operator globs win, then framework globs, else operator by default. + */ +export function resolveOwnership(manifest: FrameworkManifest, relPath: string): Ownership { + if (matchesAny(manifest.operator, relPath)) return 'operator'; + if (matchesAny(manifest.framework, relPath)) return 'framework'; + return 'operator'; +} + +/** + * The set of `[framework]` subtree roots that pruning is allowed to descend + * into (glob entries of the form `dir/**`). Single-file framework entries + * (e.g. `CONSTITUTION.md`) are reconcile-managed and never pruned. + */ +export function frameworkSubtreeRoots(manifest: FrameworkManifest): string[] { + const roots: string[] = []; + for (const g of manifest.framework) { + if (g.endsWith('/**')) roots.push(g.slice(0, -3)); + } + return roots; +} + +export interface PrunePlanInput { + readonly manifest: FrameworkManifest; + /** Mosaic-home-relative paths currently present in the target. */ + readonly targetPaths: readonly string[]; + /** Mosaic-home-relative paths the framework currently ships (source). */ + readonly sourcePaths: readonly string[]; +} + +/** + * Pure prune planner — the testable seam of the #791 fix. + * + * Returns the delete-set: target paths that are framework-owned, live inside a + * shipped framework subtree, and are absent from the current source (retired + * framework files). By construction the result never contains an operator-owned + * or unknown path: those either resolve to `operator` or fall outside every + * framework subtree root, so they are structurally unreachable by pruning. + */ +export function planPrune(input: PrunePlanInput): string[] { + const { manifest, targetPaths, sourcePaths } = input; + const source = new Set(sourcePaths.map(normalizeRel)); + const roots = frameworkSubtreeRoots(manifest); + + const deleteSet: string[] = []; + for (const raw of targetPaths) { + const path = normalizeRel(raw); + if (source.has(path)) continue; // still shipped — keep + if (resolveOwnership(manifest, path) !== 'framework') continue; // operator/unknown — never prune + if (!roots.some((root) => path === root || path.startsWith(`${root}/`))) continue; // outside shipped subtrees + deleteSet.push(path); + } + return deleteSet; +} + +function normalizeRel(p: string): string { + return p.replace(/\\/g, '/').replace(/^\.\//, '').replace(/^\/+/, '').replace(/\/+$/, ''); +} + +/** Translate a glob body (already normalized) into a RegExp source fragment. */ +function globToRegExpBody(pattern: string): string { + let out = ''; + for (let i = 0; i < pattern.length; i++) { + const c = pattern[i]; + if (c === undefined) continue; + if (c === '*') { + if (pattern[i + 1] === '*') { + // `**` — any depth. `a/**` must also match the bare root `a`, so when a + // literal `/` was just emitted, make it optional along with the rest. + i++; + let trailingSlash = false; + if (pattern[i + 1] === '/') { + i++; + trailingSlash = true; + } + if (out.endsWith('/')) { + out = `${out.slice(0, -1)}(?:/.*)?`; + } else if (trailingSlash) { + out += '(?:.*/)?'; + } else { + out += '.*'; + } + } else { + out += '[^/]*'; + } + } else { + out += c.replace(/[.+?^${}()|[\]\\]/g, '\\$&'); + } + } + return out; +} diff --git a/packages/mosaic/src/platform/file-ops.ts b/packages/mosaic/src/platform/file-ops.ts index cce7887..55a689e 100644 --- a/packages/mosaic/src/platform/file-ops.ts +++ b/packages/mosaic/src/platform/file-ops.ts @@ -62,16 +62,28 @@ function rotateBackups(filePath: string): void { /** * Sync a source directory to a target, with optional preserve paths. * Replaces the rsync/cp logic from install.sh. + * + * `isOperatorOwned` is the #791 ownership guard: when supplied, any source path + * it flags as operator-owned is never copied (the framework must never write an + * operator path). Callers derive it from the shared framework manifest so the TS + * and bash sync paths obey one source of truth. This copy is non-destructive — + * it never deletes a target file — so honoring the guard is sufficient to leave + * operator config untouched. */ export function syncDirectory( source: string, target: string, - options: { preserve?: string[]; excludeGit?: boolean } = {}, + options: { + preserve?: string[]; + excludeGit?: boolean; + isOperatorOwned?: (relPath: string) => boolean; + } = {}, ): void { // Guard: source and target are the same directory — nothing to sync if (resolve(source) === resolve(target)) return; const preserveSet = new Set(options.preserve ?? []); + const isOperatorOwned = options.isOperatorOwned ?? (() => false); // Collect files from source function copyRecursive(src: string, dest: string, relBase: string): void { @@ -86,7 +98,7 @@ export function syncDirectory( if (options.excludeGit && (dirName === '.git' || relPath.includes('/.git'))) return; // Skip preserved paths at top level - if (preserveSet.has(relPath) && existsSync(dest)) return; + if (relPath !== '' && preserveSet.has(relPath) && existsSync(dest)) return; mkdirSync(dest, { recursive: true }); for (const entry of readdirSync(src)) { @@ -101,6 +113,10 @@ export function syncDirectory( // Skip preserved files at top level if (preserveSet.has(relPath) && existsSync(dest)) return; + // #791: never write an operator-owned path (the framework owns only its + // own files; unknown paths resolve to operator and are skipped too). + if (isOperatorOwned(relPath)) return; + mkdirSync(dirname(dest), { recursive: true }); copyFileSync(src, dest); } diff --git a/packages/mosaic/src/runtime/update-checker.ts b/packages/mosaic/src/runtime/update-checker.ts index 492a0f2..6b1aee6 100644 --- a/packages/mosaic/src/runtime/update-checker.ts +++ b/packages/mosaic/src/runtime/update-checker.ts @@ -488,9 +488,13 @@ export function getInstallAllCommand(outdated: PackageUpdateResult[]): string { // `mosaic update` installs the new npm CLI but, on its own, leaves the framework // files in ~/.config/mosaic/ stale — so shipped launcher/runtime changes (e.g. // the agent-name export + native heartbeat) never ACTIVATE until a re-seed. -// These helpers run the package's own install.sh in sync-only mode (the P4 -// data-safe reconcile: framework-owned overwrite + backup-once; SOUL/USER/ -// *.local/credentials preserved) and, opt-in, relaunch durable agents. +// These helpers run the package's own install.sh in sync-only mode. The re-seed +// is manifest-driven (#791): keep mode writes ONLY framework-owned paths from the +// shared framework-manifest.txt and prunes only retired framework files inside +// shipped subtrees — every operator path (SOUL/USER/*.local/credentials, fleet +// roster + agents + backlog, and anything the manifest never anticipated) is +// left byte-identical. Contract files are still reconciled (overwrite + +// backup-once). Opt-in, this also relaunches durable agents. /** Resolve the framework/ directory bundled in the installed package. */ export function resolveBundledFrameworkRoot(): string { diff --git a/packages/mosaic/src/stages/finalize-sync-abort.spec.ts b/packages/mosaic/src/stages/finalize-sync-abort.spec.ts new file mode 100644 index 0000000..119ba94 --- /dev/null +++ b/packages/mosaic/src/stages/finalize-sync-abort.spec.ts @@ -0,0 +1,136 @@ +/** + * Tests for the framework-sync abort messaging (#791 B2 + blocker-C). + * + * finalizeStage runs `config.syncFramework()` first, inside a try/catch. If the + * sync throws, the wizard must: + * 1. NEVER fall through to "Installation complete" — the error is re-raised so + * the process exits non-zero (#791 B2). + * 2. Classify the failure so recovery advice is accurate (#791 blocker-C): + * - ManifestError → a PRE-sync validation abort; nothing was written, so + * the message states "no files were changed". + * - any other error → may surface mid-copy, so the message must NOT claim + * nothing changed; it warns the state "may be partially applied". + * + * We assert on the spinner's stop() message (the user-visible line) and that the + * original error is re-thrown unchanged in both cases. + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { mkdtempSync, rmSync } from 'node:fs'; +import { join } from 'node:path'; +import { tmpdir } from 'node:os'; +import type { WizardState } from '../types.js'; +import type { ConfigService } from '../config/config-service.js'; +import { ManifestError } from '../framework/manifest.js'; + +vi.mock('node:child_process', () => ({ + // eslint-disable-next-line @typescript-eslint/no-explicit-any + spawnSync: vi.fn().mockReturnValue({ status: 0, stdout: '', stderr: '' }), +})); + +vi.mock('../platform/detect.js', () => ({ + getShellProfilePath: () => null, +})); + +import { finalizeStage } from './finalize.js'; + +function makeState(mosaicHome: string): WizardState { + return { + mosaicHome, + sourceDir: mosaicHome, + mode: 'quick', + installAction: 'keep', + soul: { agentName: 'TestBot', communicationStyle: 'direct' }, + user: {}, + tools: {}, + runtimes: { detected: [], mcpConfigured: false }, + selectedSkills: [], + }; +} + +function buildPrompter() { + const stop = vi.fn(); + const update = vi.fn(); + const prompter = { + intro: vi.fn(), + outro: vi.fn(), + note: vi.fn(), + log: vi.fn(), + warn: vi.fn(), + text: vi.fn(), + confirm: vi.fn(), + select: vi.fn(), + multiselect: vi.fn(), + groupMultiselect: vi.fn(), + spinner: vi.fn().mockReturnValue({ update, stop }), + separator: vi.fn(), + }; + return { prompter, stop }; +} + +function makeConfigService(syncFramework: ConfigService['syncFramework']): ConfigService { + return { + readSoul: vi.fn().mockResolvedValue({}), + readUser: vi.fn().mockResolvedValue({}), + readTools: vi.fn().mockResolvedValue({}), + writeSoul: vi.fn().mockResolvedValue(undefined), + writeUser: vi.fn().mockResolvedValue(undefined), + writeTools: vi.fn().mockResolvedValue(undefined), + syncFramework, + get: vi.fn(), + set: vi.fn(), + getSection: vi.fn(), + } as unknown as ConfigService; +} + +describe('finalizeStage — framework sync abort (#791 B2 + blocker-C)', () => { + let tmp: string; + + beforeEach(() => { + tmp = mkdtempSync(join(tmpdir(), 'mosaic-sync-abort-')); + }); + + afterEach(() => { + rmSync(tmp, { recursive: true, force: true }); + vi.clearAllMocks(); + }); + + it('re-throws a ManifestError and reports that no files were changed', async () => { + const err = new ManifestError('Framework manifest defines no usable [framework] paths'); + const { prompter, stop } = buildPrompter(); + const config = makeConfigService(vi.fn().mockRejectedValue(err)); + + await expect(finalizeStage(prompter, makeState(tmp), config)).rejects.toBe(err); + + // The abort message must state nothing was written (pre-sync validation). + expect(stop).toHaveBeenCalledWith(expect.stringContaining('no files were changed')); + // It must NOT fall through to a success line. + expect(stop).not.toHaveBeenCalledWith(expect.stringContaining('Installation complete')); + }); + + it('re-throws a non-ManifestError and warns the state may be partially applied', async () => { + const err = new Error('cp: write error mid-sync (disk full)'); + const { prompter, stop } = buildPrompter(); + const config = makeConfigService(vi.fn().mockRejectedValue(err)); + + await expect(finalizeStage(prompter, makeState(tmp), config)).rejects.toBe(err); + + // A generic mid-sync failure must NOT claim nothing changed… + expect(stop).toHaveBeenCalledWith(expect.stringContaining('may be partially applied')); + expect(stop).not.toHaveBeenCalledWith(expect.stringContaining('no files were changed')); + expect(stop).not.toHaveBeenCalledWith(expect.stringContaining('Installation complete')); + }); + + it('does not proceed to config writes when the sync aborts', async () => { + const err = new ManifestError('malformed manifest'); + const { prompter } = buildPrompter(); + const config = makeConfigService(vi.fn().mockRejectedValue(err)); + + await expect(finalizeStage(prompter, makeState(tmp), config)).rejects.toBe(err); + + // writeSoul/writeUser/writeTools are only reached after a successful sync. + expect(config.writeSoul).not.toHaveBeenCalled(); + expect(config.writeUser).not.toHaveBeenCalled(); + expect(config.writeTools).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/mosaic/src/stages/finalize.ts b/packages/mosaic/src/stages/finalize.ts index 4835f6e..95dd503 100644 --- a/packages/mosaic/src/stages/finalize.ts +++ b/packages/mosaic/src/stages/finalize.ts @@ -6,6 +6,7 @@ import type { WizardPrompter } from '../prompter/interface.js'; import type { ConfigService } from '../config/config-service.js'; import type { WizardState } from '../types.js'; import { getShellProfilePath } from '../platform/detect.js'; +import { ManifestError } from '../framework/manifest.js'; function linkRuntimeAssets(mosaicHome: string, skipClaudeHooks: boolean): void { const script = join(mosaicHome, 'bin', 'mosaic-link-runtime-assets'); @@ -160,7 +161,26 @@ export async function finalizeStage( // 1. Sync framework files (before config writes so identity files aren't overwritten) spin.update('Syncing framework files...'); - await config.syncFramework(state.installAction); + try { + await config.syncFramework(state.installAction); + } catch (err) { + // Stop the spinner loudly and re-raise so the process exits non-zero — never + // fall through to "Installation complete" on an aborted sync (#791 B2). + // A ManifestError is a PRE-sync validation abort: the manifest is loaded and + // validated before any file is written, so nothing was touched. Any other + // error can surface AFTER files were partially copied, so we must NOT claim + // "no files were changed" for it — that would misdirect recovery (#791 blocker-C). + if (err instanceof ManifestError) { + spin.stop( + 'Framework sync aborted — the framework manifest is missing, empty, or malformed; no files were changed.', + ); + } else { + spin.stop( + 'Framework sync aborted — the update did not complete and may be partially applied; see the error below.', + ); + } + throw err; + } // 2. Write config files (after sync so they aren't overwritten by source templates) if (state.installAction !== 'keep') {