diff --git a/.changeset/clever-cranes-zip.md b/.changeset/clever-cranes-zip.md new file mode 100644 index 000000000..cae65f2ac --- /dev/null +++ b/.changeset/clever-cranes-zip.md @@ -0,0 +1,5 @@ +--- +type: Added +pr: 3568 +--- +**installRuntimeArtifacts() now returns the plan it executed** — per kind, per scope, including on the combined OpenCode/Kilo family path that previously returned nothing — so an install's correctness is a value a caller can assert, not something only re-readable from disk afterward. Install IO routes through a new injectable fs seam (`install-fs-adapter.cts`), letting a full install run end-to-end against a fake adapter with no real destination filesystem contact; failures still throw rather than becoming a value, and a best-effort cleanup that fails is now visible in the return instead of silently swallowed. Writes on disk are unchanged. Completes ADR-58's never-landed `cleanup` rollout step. (#2874) diff --git a/.gitignore b/.gitignore index 3fff33e45..e277d4d93 100644 --- a/.gitignore +++ b/.gitignore @@ -218,6 +218,7 @@ build/ /gsd-core/bin/lib/runtime-artifact-conversion.cjs /gsd-core/bin/lib/runtime-artifact-layout.cjs /gsd-core/bin/lib/install-scope.cjs +/gsd-core/bin/lib/install-fs-adapter.cjs /gsd-core/bin/lib/installed-surface-resolver.cjs /gsd-core/bin/lib/install-shadow-report.cjs /gsd-core/bin/lib/runtime-config-adapter-registry.cjs diff --git a/CONTEXT.md b/CONTEXT.md index be863c8ac..4199f5fd5 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -260,6 +260,9 @@ Sibling Module to Runtime Artifact Layout Module. Owns projection from canonical ### Runtime Artifact Install Plan Module Module owning install-time staging and content-rewrite selection for a pre-resolved Runtime Artifact Layout. Interface: `createRuntimeArtifactInstallPlan({ layout, resolvedProfile, homedir?, platform?, resolveAttribution?, deps? }) -> { ok:true, plan:{ items, cleanupDirs } } | { ok:false, kind:'stage_failed'|'rewrite_failed', message, cleanupDirs, failedKind? }`. It iterates `layout.kinds` in order, calls each kind's `stage(resolvedProfile)`, delegates `commands` to Runtime Artifact Conversion `rewriteStagedCommandBodies`, delegates `skills` and `kimi-agents` to `rewriteStagedSkillBodies`, leaves non-rewritten kinds unchanged, and projects copy items as `{ kind, sourceDir, destDir }`. It deliberately does not prune, copy, run legacy migrations, print output, or execute cleanup; those remain Installer Module adapter responsibilities until later slices wire the plan into `bin/install.js`. **Write-confinement (ADR-1239 Phase B / #1679):** the exported pure `assertDestWithinConfigHome(configDir, destSubpath) -> resolvedDest` is the security gate — every kind's `destDir` is computed through it on both the install and uninstall plan paths, so a `destSubpath` that escapes `configHome` (`../../etc`, a NUL byte, etc.) is rejected at plan-build time with a clear error; `surface.cjs:applySurface` and `bin/install.js:installOpencodeFamilySkills` route their joins through the same helper, and `_copyStaged` carries a defense-in-depth containment check. This is security-load-bearing for the Phase C third-party-descriptor loader (which is where an untrusted `destSubpath` could arrive). Source: `gsd-core/bin/lib/runtime-artifact-install-plan.cjs`. See Runtime Artifact Layout Module and Runtime Artifact Conversion Module. +### Install Fs Adapter Module +Narrow, enumerated fs seam for the `installRuntimeArtifacts` call tree (`src/install-engine.cts`) — lands ADR-58's never-shipped `cleanup` rollout step (`registry → adapter → helpers → cleanup`, #2874, epic #2866 Phase 5). `installRuntimeArtifacts` now returns the executed plan it ran (`{ runtime, scope, kinds: [{kind, sourceDir, destDir, preserved}], cleanup: [{dir, ok}], postSteps }`) instead of `void`, including on the `combinedFamilyInstall` (OpenCode/Kilo) early-return path — no path may return `undefined` after this phase. Failure is unchanged: stage/rewrite errors still throw rather than becoming an `ok:false` value, so a caller cannot read success-shaped data off a failure path. Delivery is an ambient single mutable adapter (`current`), swapped for the duration of one synchronous install via `withInstallFs(deps.fs, fn)` and always restored in a `finally` — a `deps` parameter threaded through every function on the call tree (`install-profiles.cts`, `runtime-artifact-conversion.cts`, `commonjs-marker.cts`, `installer-migrations.cts`'s two reachable entry points) was rejected as a dozen+-site touch for no behavioral gain over the ambient swap, extending rather than replacing `createRuntimeArtifactInstallPlan`'s existing `deps` bag precedent (Runtime Artifact Install Plan Module). An injected `deps.fs` is a PARTIAL adapter merged over real `node:fs`; any method it omits silently resolves to the real filesystem. **Routes destination IO only, by design**: every write/probe against the install destination (copies, removals, snapshot/restore of preserved skill dirs, the manifest read) is fake-able; locating this package's own source tree (`findInstallSourceRoot`/`findAgentsSourceRoot`'s walk-up-from-`__dirname`, `readGsdCommandNames`) stays real and unrouted — a destination-fake's store starts empty and was never seeded with the repo's own paths, so routing that lookup would make every fake-adapter install throw instead of staging. The symlink-escape guard (`hasExistingSymlinkBetween`) and `assertDestWithinConfigHome` keep their REFUSAL DECISIONS outside this seam — only their `existsSync`/`lstatSync`/`realpathSync` probes route through it, so an injected fake can change what a probe observes but never flip the security decision itself. Writes remain byte-identical to pre-#2874 (AC4/AC5); existing `void`-ignoring callers (`bin/install.js`) are unaffected. Source: `gsd-core/bin/lib/install-fs-adapter.cjs` (generated from `src/install-fs-adapter.cts`). See Runtime Artifact Install Plan Module, ADR-58. + ### Install Scope Module Owns the two-value install-scope axis (`'global' | 'local'`) as a typed value, replacing the bare `isGlobal ? 'global' : 'local'` string re-derived at 12 sites in `bin/install.js` plus several downstream re-derivations (#2870, ADR-2866). Interface: `resolveScope({ id, runtime, explicitDir?, env?, home?, existsSync? }) -> { id, configHome, settingsFile, consentRequired, hostPrecedenceRank }` — pure (no writes, never mutates `input`) and the returned value is frozen so a caller cannot corrupt a subsequent resolution. Owns the `InstallScope` type name: previously a private, non-exported `TypeAlias` inside Runtime Artifact Install Plan Module; that module now `import type`s it from here instead of re-declaring it, so the codebase does not grow a fifth spelling of the axis alongside the layout module's `'local' | 'global'`, `capability-lifecycle.cts`'s `'global' | 'project'`, and `capability-consent.cts`'s single `'project'` literal. `configHome` for `global` composes `resolveConfigHomeFromDescriptor` (Runtime Homes Module) unmodified rather than adding a `scope` parameter to it — that function is CRITICAL blast radius (60 dependents across 13 files); for `local` it joins the capability registry's per-runtime `localConfigDir` onto the real process cwd (the project you are standing in — not injectable via `home`, by design). `explicitDir` short-circuits both scopes identically to `getGlobalConfigDir`'s existing override, and every returned `configHome` is normalized to forward slashes UNCONDITIONALLY (`.replace(/\\/g,'/')`, never gated on `path.sep`). `settingsFile` reads the registry's `hostBehaviors.settingsFileByScope[id]` and is `null` for the 18 of 19 registered runtimes that declare none — absence is a value, not an invented Claude-shaped default; the one caller that legitimately wants a Claude fallback (`bin/install.js:550`) still applies it itself. `consentRequired` is `false` for `global` (nothing is recorded — matches Capability Registry Overlay's rule that a GLOBAL-scope capability is trusted without a consent record) and `true` for `local`; it reports the requirement only; it does not perform or waive consent. `hostPrecedenceRank` (`global` outranks `local`) is carried as data only this phase — unread until Phase 2 (#2871) defines precedence semantics. Throws `TypeError` for an invalid `id` (wrong case, empty, missing, or any non-string value — never coerced), an unknown `runtime`, or a runtime whose `configHome.kind === 'none'` (vscode — non-installable, #2103) — all three share one `instanceof TypeError` catch shape with Runtime Artifact Layout Module's existing unknown-runtime contract. **The `local`/`project` boundary is documented, not unified:** this module's `'local'` spelling — chosen because it is the CLI's own vocabulary (`--local`) and what the layout module and manifest already use — is deliberately NOT reconciled with Capability Consent Store's `ConsentRecord.scope: 'project'` or Capability Lifecycle's `'global' | 'project'` operations. `ConsentRecord.scope` is a value persisted on disk in user-owned consent records outside any repository; renaming that literal to match would silently invalidate every existing project-scoped consent record on a user's machine the next time it is read back — a far worse defect than the vocabulary split. The mapping instead lives here as a fact: install scope `'local'` ⇄ consent scope `'project'`; install scope `'global'` ⇄ no consent record at all. Source: `gsd-core/bin/lib/install-scope.cjs` (generated from `src/install-scope.cts`). See Runtime Homes Module, Runtime Artifact Layout Module, Runtime Artifact Install Plan Module, Capability Consent Store, Capability Lifecycle. diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 00d95db98..405a1bde0 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -792,6 +792,17 @@ The installer (`bin/install.js`, ~10,700 lines) handles: 8. **Manifest tracking** — Writes `gsd-file-manifest.json` for clean uninstall. The manifest also records which `runtime` and which `scope` (`global`/`local`) wrote it, under a `manifestVersion` schema field, so a reader can answer "which surfaces are installed, at which scopes" without inferring it from the directory the file sits in ([ADR 2866](adr/2866-install-surface-resolution.md), #2872). Manifests written before that carry no such fields and are read without error — no reinstall is required. See [Installer Migrations → File Manifest](installer-migrations.md#file-manifest) 9. **Uninstall mode** — `--uninstall` removes all GSD files, hooks, and settings +`installRuntimeArtifacts` (`install-engine.cjs`) returns the executed plan it ran — per kind, per +scope, including on the combined OpenCode/Kilo family path, which previously early-returned `void` — +rather than being observable only by re-reading disk afterward. Its destination-writing IO (copies, +removals, snapshot/restore, best-effort cleanup) now routes through an injectable fs seam, +`install-fs-adapter.cjs`, so a full install can be exercised against a fake adapter with zero real +destination IO; locating this package's own source tree remains real by design (a destination-fake +is never seeded with the repo's own paths). Writes stay byte-identical and existing `void`-ignoring +callers are unaffected. This completes [ADR 58](adr/58-runtime-install-policy-module.md)'s +`registry → adapter → helpers → cleanup` rollout — the `cleanup` step had not previously landed +(#2874, epic #2866 Phase 5). + Install-time file moves, stale-artifact cleanup, config rewrites, and user-data preservation are governed by the Installer Migration Module. See [Installer Migrations](installer-migrations.md) and diff --git a/docs/INVENTORY-MANIFEST.json b/docs/INVENTORY-MANIFEST.json index 41e8800c0..fda6262f3 100644 --- a/docs/INVENTORY-MANIFEST.json +++ b/docs/INVENTORY-MANIFEST.json @@ -398,6 +398,7 @@ "init.cjs", "install-effort-resolver.cjs", "install-engine.cjs", + "install-fs-adapter.cjs", "install-profiles.cjs", "install-scope.cjs", "install-shadow-report.cjs", diff --git a/docs/INVENTORY.md b/docs/INVENTORY.md index b3d6e9377..ef9f3bdbc 100644 --- a/docs/INVENTORY.md +++ b/docs/INVENTORY.md @@ -516,6 +516,7 @@ Full listing: `gsd-core/bin/lib/*.cjs`. | `init.cjs` | Compound context loading for each workflow type | | `install-effort-resolver.cjs` | Install-time effort resolution — `readGsdEffectiveEffortConfig` (merges `~/.gsd/defaults.json` + project `.planning/config.json`) + `resolveInstallTimeEffort`, extracted from `bin/install.js` (#2071) so `gsd-tools effort sync` can require it from the shipped runtime instead of the never-copied package-root installer; install.js imports them back (single source) | | `install-engine.cjs` | Runtime-artifact install engine — `installRuntimeArtifacts`/`uninstallRuntimeArtifacts`/`installOpencodeFamilySkills` + their helpers, extracted from `bin/install.js` (ADR-1239 Phase B, #1679); install.js imports them back and injects `getCommitAttribution` | +| `install-fs-adapter.cjs` | Install Fs Adapter — narrow, enumerated fs seam for the `installRuntimeArtifacts` call tree (#2874, epic #2866 Phase 5, ADR-58's never-landed `cleanup` rollout step); a single ambient adapter (real fs in production, an injectable fake in tests) is swapped for the duration of one synchronous install via `withInstallFs`, extending the `deps` bag precedent already established by Runtime Artifact Install Plan Module rather than threading a new parameter through every call site; routes destination IO only — package-source lookups (`findInstallSourceRoot`, `findAgentsSourceRoot`, `readGsdCommandNames`) are deliberately unrouted, by design, not by omission | | `install-profiles.cjs` | Install profile allowlist + skill staging for `--minimal` install (#2762); single source of truth for which `gsd-*` skills/agents land in runtime config dirs | | `install-scope.cjs` | Install Scope Module — `resolveScope({id,runtime,...})` resolves the `'global'\|'local'` install-scope axis into `{id, configHome, settingsFile, consentRequired, hostPrecedenceRank}`, composing `resolveConfigHomeFromDescriptor` (`runtime-homes.cjs`) rather than modifying it (#2870, ADR-2866) | | `install-shadow-report.cjs` | Cross-Scope Shadow Report Module (#2873, epic #2866 Phase 4a) — read-only projection over `installed-surface-resolver.cjs`'s `resolveInstalledSurfaces`; `buildShadowReport(runtime, opts)` filters `resolveTriggerSurface`'s `shadowedBy` groups down to triggers whose underlying stem genuinely exists in BOTH scopes' own manifests (not merely the union), and `renderShadowReport` projects the typed IR into bounded, sanitized (`sanitizeForRender` strips ANSI/C0-C1/bidi overrides) operator-console lines; consumed by both the installer and the W028 health rule so install-time and `/gsd-health` report identically | diff --git a/docs/README.md b/docs/README.md index c76778665..784f2ace8 100644 --- a/docs/README.md +++ b/docs/README.md @@ -48,6 +48,7 @@ Language versions: [English](README.md) · [Português (pt-BR)](pt-BR/README.md) - [List your reviewer lane in the registry](how-to/list-your-reviewer-lane.md) — publish a lane you have built to the Reviewer Lane Registry so other people can find and install it - [Take over a capability or EoS integration](how-to/take-over-a-capability-or-eos.md) — assume maintainership of an existing third-party capability, reviewer lane, or EoS host integration through a handoff, an adoption fork, first-party absorption, or a de-listing - [Add or update a host's integration](how-to/add-or-update-a-host-integration.md) — set a host's documentation-sourced `runtime.hostIntegration` axes (ADR-1239 Phase A), with the `undocumented` sentinel rule +- [Migrate an install test to the executed plan](how-to/migrate-an-install-test-to-the-executed-plan.md) — convert an `fs.existsSync`-probing install test group to a value assertion against `installRuntimeArtifacts`'s executed-plan return, and test against a fake fs adapter - [Turn a capability off (and keep it off)](how-to/turn-a-capability-off.md) — disable a capability via the surface, or gate individual hooks off without removing the capability - [Drive GSD from a tracker issue](how-to/drive-gsd-from-a-tracker-issue.md) — start a phase from a GitHub, Linear, or Jira issue - [Migrate from GSD 2](how-to/migrate-from-gsd-2.md) — upgrade an existing GSD 2 project to GSD Core diff --git a/docs/how-to/migrate-an-install-test-to-the-executed-plan.md b/docs/how-to/migrate-an-install-test-to-the-executed-plan.md new file mode 100644 index 000000000..4f337af37 --- /dev/null +++ b/docs/how-to/migrate-an-install-test-to-the-executed-plan.md @@ -0,0 +1,95 @@ +# How to migrate an install test to the executed plan + +**Goal:** Convert a test group that probes install output with `fs.existsSync` into a value assertion against the executed plan `installRuntimeArtifacts` now returns — without silently dropping coverage the probes established. + +**Prerequisites:** A test in `tests/install.test.cjs` (or a sibling install test file) that installs a runtime and then calls `fs.existsSync`/`fs.readFileSync` against the destination to confirm something landed. + +--- + +## What `installRuntimeArtifacts` now returns + +`installRuntimeArtifacts` (`src/install-engine.cts:770`) no longer returns `void`. Every call — including the opencode/kilo combined-family path, which used to early-return `undefined` — now returns: + +``` +{ + runtime, + scope, + kinds: [{ kind, sourceDir, destDir, preserved }], // one entry per artifact kind actually written + cleanup: [{ dir, ok }], // best-effort cleanupDirs, success visible per dir + postSteps: { hermesBareStemCleanup, nativePlugin }, // booleans for the two post-steps +} +``` + +`kinds` names every kind the layout wrote this call (`skills`, `agents`, `commands`, …), each with the `sourceDir`/`destDir` it copied between and which user-owned subdirs (e.g. `gsd-dev-preferences`) were preserved across a wipe-and-replace. `cleanup` makes a previously-silent, best-effort `rmSync` failure visible instead of swallowed. This value never claims bytes landed — see [What this cannot prove](#what-this-cannot-prove). For the full contract and the fs-adapter injection point, read the doc comment on `installRuntimeArtifacts` in `src/install-engine.cts` and the module doc in `src/install-fs-adapter.cts`; this guide covers only the migration mechanics. + +--- + +## Migrating a probing test group + +The worked example is the qwen group in `tests/install.test.cjs` (`describe('install/uninstall — qwen …')`, test `'installs GSD into ./.qwen and removes it cleanly'`). The pattern: + +1. Run the real install as before (`install(false, 'qwen')`), unchanged. +2. Immediately after, call `installRuntimeArtifacts` again directly with the same `runtime`/`targetDir`/`scope`/resolved profile. Because the tree the first call wrote is already installed, this second call is an idempotent re-run (prune + rewrite converges to the same on-disk result) — it does no new writes, but it surfaces the same executed-plan value the first call's caller (`bin/install.js`) discarded. +3. Replace each `fs.existsSync(destPath)` probe that was checking a *destination directory's existence* with one `assert.deepStrictEqual` against the relevant `plan.kinds` entries — keyed by `kind`, read off `destDir`. +4. Leave every other probe exactly where it was (see the next section). + +--- + +## The step people will get wrong: enumerate first + +Before converting a probing group, list every fact its existing `fs.existsSync`/`fs.readFileSync` calls establish. Convert only the facts the plan's per-kind contract actually covers. A migration that asserts *less* than the probes it replaces looks like a simplification and is a regression — a shrinking assertion surface with no visible signal that coverage was dropped. + +In the qwen migration, nine facts were enumerated. Only **two** moved to the value assertion — that `skills` and `agents` kinds wrote to the expected `destDir`. The other **seven** were deliberately retained as `fs` probes, because they sit outside the plan's per-kind contract (one `destDir` per kind, not a file list, and not everything the surrounding `install()`/`uninstall()` functions do): + +- the specific nested `SKILL.md` file existing at its stem-level path (finer-grained than a per-kind `destDir`) +- the `gsd-core/VERSION` file, written by `install()`'s own copy step, not by `installRuntimeArtifacts` +- the manifest's file-key content (`writeManifest`'s own output, not the executed plan) +- four post-uninstall absence checks + +Before converting a group of your own, write down that same enumeration — what each existing probe proves — and mark each fact "covered by `plan.kinds`" or "stays a probe, because …". If you cannot state the "because", the fact likely belongs on the value-assertion side; if you can, keep the probe. Do not delete a probe just because it is adjacent to one that migrated cleanly. + +--- + +## Testing against a fake adapter + +An install can be driven end-to-end with no real filesystem contact by injecting a fake `InstallFsAdapter` as the 7th positional argument's `.fs` key: + +```js +const result = installRuntimeArtifacts( + runtime, configDir, scope, resolvedProfile, undefined, undefined, + { fs: fakeFs }, +); +``` + +`fakeFs` must implement the methods the exercised code path actually touches — `existsSync`, `mkdirSync`, `rmSync`, `readdirSync`, `readFileSync`, `writeFileSync`, `copyFileSync`, `cpSync`, `lstatSync`, `realpathSync`, `unlinkSync`, `rmdirSync`, and (for anything that reaches `installer-migrations.cts`'s `sha256File`) the raw-fd trio `openSync`/`readSync`/`closeSync`. `tests/executed-plan.test.cjs`'s `createFakeInstallFs` is a working in-memory reference implementation over one `Map` store — reuse it rather than writing a partial fake from scratch. + +Two traps, both documented at the seam in `src/install-fs-adapter.cts`'s module comment: + +- **A partial fake silently falls back to real `fs` for any method it omits.** `withInstallFs` merges your injected object *over* the real adapter (`{ ...REAL_ADAPTER, ...partial }`), so an incomplete fake is not a smaller fake — for the methods it does not define, it *is* the real filesystem, doing real IO you did not intend and your test will not flag. +- **The seam is ambient and synchronous-only.** One mutable module-level variable holds "the active adapter" for the duration of one synchronous call; there is no `async`/await anywhere on the routed call tree. Do not run two installs concurrently in the same process (the second `withInstallFs` call clobbers the first's adapter mid-flight), and do not defer any work — a `setTimeout`, a promise continuation, a `process.on('exit', …)` callback — that reads `installFs()` past the point `withInstallFs`'s `finally` has already restored the previous adapter. (`install-profiles.cts`'s deferred skill-dir cleanup avoids this trap by capturing the adapter *object* at staging time instead of re-resolving it later — read that code before writing your own deferred cleanup against this seam.) + +--- + +## The destination-vs-package-source boundary + +The seam's claim is **zero real destination IO**, never zero real IO. `findInstallSourceRoot`, `findAgentsSourceRoot` (both `src/runtime-artifact-layout.cts`) and `readGsdCommandNames` (`src/command-roster.cts`) are deliberately left unrouted — they locate *this package's own source tree* (`commands/gsd/`, `agents/`), not the install destination. A destination-fake's in-memory store starts empty and is never seeded with the repo's own real paths; routing those lookups through it would make every fake-adapter install throw "could not locate commands/gsd" instead of exercising the install. + +`tests/executed-plan.test.cjs`'s F2 test group encodes this boundary by poisoning **by path**, not by method: `poisonRealFsAgainstDestination` wraps every fs method the routed call tree touches so that a call against a non-package-source path throws, while a call whose resolved path falls under `commands/gsd/` or `agents/` is allowed through to the real filesystem and counted. Each F2 test then positively asserts the package-source count is non-zero (`packageSourceHits.get('readdirSync') > 0`), proving the unrouted read actually happened rather than merely being tolerated. + +Do not poison by method (blocking `readdirSync`/`statSync` outright regardless of target path). That makes a correct, unmodified `findInstallSourceRoot`/`readGsdCommandNames` fail for a reason that has nothing to do with destination-IO routing — a mistake already made once on this seam. If you add a test asserting no real fs contact, derive your poison set from the destination/package-source rule above, and re-derive it (not copy-paste it) if the routed call tree changes. + +--- + +## What this cannot prove + +The executed plan describes what `installRuntimeArtifacts` *executed* — which kinds it wrote to, which dirs it preserved, which cleanup it attempted and whether that attempt succeeded. It is not a post-hoc verification that bytes are on disk. A best-effort `cleanup` entry with `ok: false` means the `rmSync` call was attempted and failed, visibly — the install still succeeds either way. When you need proof of on-disk state rather than proof of what was attempted, that is exactly the case an `fs.existsSync`/`fs.readFileSync` probe still earns its place, per the enumeration step above. + +--- + +## Related + +- [ADR-58](../adr/58-runtime-install-policy-module.md) — the policy/adapter boundary this seam implements +- [`docs/ARCHITECTURE.md`](../ARCHITECTURE.md) — why install correctness is modeled this way +- `src/install-fs-adapter.cts` — the adapter seam's own module doc (delivery mechanism, partial-adapter trap, deliberately-unrouted list) +- `.gsd/phase/feat-2874-executed-plan-return/40-design.md` — the design doc this phase shipped against +- [docs index](../README.md) diff --git a/eslint.config.mjs b/eslint.config.mjs index 87119c4b9..e0abb340e 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -77,6 +77,9 @@ export default tseslint.config( 'gsd-core/bin/lib/host-integration-sdk.cjs', 'gsd-core/bin/lib/install-effort-resolver.cjs', 'gsd-core/bin/lib/install-engine.cjs', + // #2874 (epic #2866 Phase 5): tsc-generated runtime artifact — lint the + // src/install-fs-adapter.cts source, not this. + 'gsd-core/bin/lib/install-fs-adapter.cjs', 'gsd-core/bin/lib/commonjs-marker.cjs', 'gsd-core/bin/lib/capability-loader.cjs', 'gsd-core/bin/lib/capability-source.cjs', diff --git a/src/command-roster.cts b/src/command-roster.cts index 2cf0b3afa..6dbd9d6d1 100644 --- a/src/command-roster.cts +++ b/src/command-roster.cts @@ -7,6 +7,31 @@ * applying the shared GSD slash-command namespace transform. */ +import path from 'node:path'; +import fs from 'node:fs'; +// #2874 (ADR-58 cleanup phase): readGsdCommandNames is reached from +// installRuntimeArtifacts's call tree (skillsKind's stage() closure reads it +// for cross-referencing), but COMMANDS_DIR (below) is resolved by walking up +// from THIS MODULE'S OWN compiled __dirname to locate the GSD PACKAGE'S OWN +// commands/gsd/ source tree — not anything under the install destination. +// This is the exact same "package's own source, not the destination" case +// findInstallSourceRoot/findAgentsSourceRoot document in +// runtime-artifact-layout.cts's Step 2 (see that comment and +// install-fs-adapter.cts's module doc, "DELIBERATELY NOT ROUTED" section): +// an injected destination adapter's store starts empty and is never seeded +// with the real package's own on-disk layout, so routing this read through +// installFs() would make a fake-adapter install either silently return `[]` +// (via the ENOENT swallow below) or resolve wrong stems — never loudly fail, +// and never see the real package tree either way. This directory read +// therefore deliberately stays on real `node:fs`, unrouted — same rule, +// applied consistently, not an inconsistency with the routed staging calls +// elsewhere in this call tree. `readCmdNames` in +// scripts/fix-slash-commands.cjs also uses real `node:fs` directly (that +// script lives outside src/ and is also a standalone CLI tool), so its +// directory-read logic is reimplemented here against real fs instead of +// delegated to, keeping the pure regex/transform helpers below delegated as +// before. + // eslint-disable-next-line @typescript-eslint/no-require-imports const slashCommandTransformer = require('../../../scripts/fix-slash-commands.cjs') as { readCmdNames: () => string[]; @@ -16,8 +41,23 @@ const slashCommandTransformer = require('../../../scripts/fix-slash-commands.cjs buildColonPattern: (cmdNames: string[]) => RegExp | null; }; +// Mirrors scripts/fix-slash-commands.cjs's own `COMMANDS_DIR` computation +// (`path.join(__dirname, '..', 'commands', 'gsd')` from repo-root/scripts) — +// same target directory, resolved from this module's own compiled location +// (repo-root/gsd-core/bin/lib) instead, so the walk-up depth differs. +const COMMANDS_DIR = path.join(__dirname, '..', '..', '..', 'commands', 'gsd'); + function readGsdCommandNames(): string[] { - return slashCommandTransformer.readCmdNames(); + try { + return fs.readdirSync(COMMANDS_DIR) + .filter((f: string) => f.endsWith('.md')) + .map((f: string) => f.replace(/\.md$/, '')); + } catch (err) { + // Only swallow the missing-directory case — mirrors readCmdNames' own + // contract (scripts/fix-slash-commands.cjs). + if (err instanceof Error && (err as NodeJS.ErrnoException).code !== 'ENOENT') throw err; + return []; + } } export = { diff --git a/src/commonjs-marker.cts b/src/commonjs-marker.cts index 7551e2d75..d884084a2 100644 --- a/src/commonjs-marker.cts +++ b/src/commonjs-marker.cts @@ -30,8 +30,14 @@ * writes — the same test the uninstall path has always used. */ -import fs from 'node:fs'; import path from 'node:path'; +// #2874 (ADR-58 cleanup phase): ensureCommonJsMarker is reached from +// install-engine.cts's _installNativePluginIfDeclared, which is on the +// installRuntimeArtifacts call tree — route this module's fs calls through +// the injectable seam too. See install-fs-adapter.cts's module doc. +// eslint-disable-next-line @typescript-eslint/no-require-imports +import installFsAdapter = require('./install-fs-adapter.cjs'); +const { installFs } = installFsAdapter; /** The exact marker content GSD writes (and the only content it will remove). */ export const COMMONJS_MARKER = '{"type":"commonjs"}'; @@ -72,12 +78,12 @@ export function markerPathFor(dir: string): string { */ export function classifyMarker(dir: string): MarkerOwnership { const markerPath = markerPathFor(dir); - let stat: import('node:fs').Stats; + let stat: { isFile(): boolean; isDirectory(): boolean; isSymbolicLink(): boolean }; try { // lstat, not existsSync: existsSync follows symlinks and reports `false` // for a DANGLING one, which would classify the path `absent` and let the // write below follow the link and land outside the directory GSD owns. - stat = fs.lstatSync(markerPath); + stat = installFs().lstatSync(markerPath); } catch (err) { if ((err as NodeJS.ErrnoException).code === 'ENOENT') return 'absent'; return 'foreign'; @@ -86,7 +92,7 @@ export function classifyMarker(dir: string): MarkerOwnership { // something GSD wrote, so it is never ours to overwrite or remove. if (!stat.isFile()) return 'foreign'; try { - const content = fs.readFileSync(markerPath, 'utf8'); + const content = installFs().readFileSync(markerPath, 'utf8'); return content.trim() === COMMONJS_MARKER ? 'gsd-owned' : 'foreign'; } catch { return 'foreign'; @@ -115,11 +121,11 @@ export function ensureCommonJsMarker(dir: string): MarkerWriteOutcome { if (ownership === 'foreign') return 'preserved-foreign'; if (ownership === 'gsd-owned') return 'unchanged'; try { - fs.mkdirSync(dir, { recursive: true }); + installFs().mkdirSync(dir, { recursive: true }); // Exclusive create closes the gap between classifying and writing: if // anything at all appeared at the path in between — including a symlink — // this fails with EEXIST instead of following or overwriting it. - fs.writeFileSync(markerPathFor(dir), COMMONJS_MARKER_CONTENT, { flag: 'wx' }); + installFs().writeFileSync(markerPathFor(dir), COMMONJS_MARKER_CONTENT, { flag: 'wx' }); return 'written'; } catch (err) { if ((err as NodeJS.ErrnoException).code === 'EEXIST') return 'preserved-foreign'; @@ -134,7 +140,7 @@ export function ensureCommonJsMarker(dir: string): MarkerWriteOutcome { export function removeCommonJsMarker(dir: string): boolean { if (classifyMarker(dir) !== 'gsd-owned') return false; try { - fs.unlinkSync(markerPathFor(dir)); + installFs().unlinkSync(markerPathFor(dir)); return true; } catch { return false; diff --git a/src/install-engine.cts b/src/install-engine.cts index dd379ffef..e36c8ca93 100644 --- a/src/install-engine.cts +++ b/src/install-engine.cts @@ -32,6 +32,14 @@ import retiredArtifactCleanup = require('./retired-artifact-cleanup.cjs'); import { posixNormalize } from './shell-command-projection.cjs'; import { isPathConfined } from './external-descriptor-trust.cjs'; import { ensureCommonJsMarker } from './commonjs-marker.cjs'; +// #2874 (ADR-58 cleanup phase): the injectable fs seam for the +// installRuntimeArtifacts call tree. `installFs()` resolves to real +// `node:fs` unless a call is wrapped in `withInstallFs(deps.fs, ...)` — +// every fs call below in this file that installRuntimeArtifacts's own call +// tree reaches goes through it. See install-fs-adapter.cts's module doc for +// why this is an ambient swap rather than a threaded `deps` parameter. +import installFsAdapter = require('./install-fs-adapter.cjs'); +const { installFs, withInstallFs } = installFsAdapter; // #2870: InstallScope is owned by install-scope.cts, not re-declared here. // `isGlobalScope` centralizes the `scope === 'global'` boolean projection // this module's two remaining re-derivation sites need (see the @@ -161,9 +169,9 @@ function preserveUserArtifacts(destDir: string, fileNames: string[]): Map(); for (const name of fileNames) { const fullPath = path.join(destDir, name); - if (fs.existsSync(fullPath)) { + if (installFs().existsSync(fullPath)) { try { - saved.set(name, fs.readFileSync(fullPath, 'utf8')); + saved.set(name, installFs().readFileSync(fullPath, 'utf8')); } catch { /* skip unreadable files */ } } } @@ -180,8 +188,8 @@ function restoreUserArtifacts(destDir: string, saved: Map): void for (const [name, content] of saved) { const fullPath = path.join(destDir, name); try { - fs.mkdirSync(path.dirname(fullPath), { recursive: true }); - fs.writeFileSync(fullPath, content, 'utf8'); + installFs().mkdirSync(path.dirname(fullPath), { recursive: true }); + installFs().writeFileSync(fullPath, content, 'utf8'); } catch { /* skip unwritable paths */ } } } @@ -257,7 +265,7 @@ function hasExistingSymlinkBetween( // — threat (a) above still confines regardless. let realRoot: string; try { - realRoot = fs.existsSync(resolvedRoot) ? fs.realpathSync(resolvedRoot) : resolvedRoot; + realRoot = installFs().existsSync(resolvedRoot) ? installFs().realpathSync(resolvedRoot) : resolvedRoot; } catch { realRoot = resolvedRoot; } @@ -273,10 +281,10 @@ function hasExistingSymlinkBetween( // root. So under opt-in, just follow the root symlink and continue the walk. // Default behavior (no opt-in) preserves the pre-#2393 refuse. let cursor = resolvedRoot; - if (fs.existsSync(cursor) && fs.lstatSync(cursor).isSymbolicLink()) { + if (installFs().existsSync(cursor) && installFs().lstatSync(cursor).isSymbolicLink()) { if (!allowFollow) return true; try { - cursor = fs.realpathSync(cursor); + cursor = installFs().realpathSync(cursor); } catch { // realpathSync failed (broken symlink, permission denied, exotic FS) — refuse, // matching fail-closed posture. @@ -288,8 +296,8 @@ function hasExistingSymlinkBetween( for (const segment of relative.split(path.sep)) { if (!segment) continue; cursor = path.join(cursor, segment); - if (!fs.existsSync(cursor)) return false; - if (fs.lstatSync(cursor).isSymbolicLink()) { + if (!installFs().existsSync(cursor)) return false; + if (installFs().lstatSync(cursor).isSymbolicLink()) { if (!allowFollow) return true; // Opt-in active: follow the symlink. Refuse if the resolved target is the // install root itself (threat (b) — would let _removeGsdEntries wipe the @@ -310,7 +318,7 @@ function hasExistingSymlinkBetween( // documented opt-in semantics; do not add a "follow one symlink only" // expectation here without revisiting the threat model. try { - const realTarget = fs.realpathSync(cursor); + const realTarget = installFs().realpathSync(cursor); if (realTarget === realRoot || realTarget === resolvedRoot) return true; // (b) cursor = realTarget; } catch { @@ -372,7 +380,7 @@ function migrateLegacyDevPreferencesToSkill(targetDir: string, saved: Map { const files = new Map(); - if (!fs.existsSync(dir)) return files; + if (!installFs().existsSync(dir)) return files; const walk = (relPath: string, absPath: string) => { - for (const e of fs.readdirSync(absPath, { withFileTypes: true })) { + for (const e of installFs().readdirSync(absPath, { withFileTypes: true })) { const childRel = relPath ? path.join(relPath, e.name) : e.name; const childAbs = path.join(absPath, e.name); if (e.isDirectory()) walk(childRel, childAbs); - else if (e.isFile()) files.set(childRel, fs.readFileSync(childAbs)); + else if (e.isFile()) files.set(childRel, installFs().readFileSync(childAbs)); } }; walk('', dir); @@ -564,8 +572,8 @@ function _snapshotDir(dir: string): Map { function _restoreDir(dir: string, snapshot: Map): void { for (const [relPath, buf] of snapshot) { const absPath = path.join(dir, relPath); - fs.mkdirSync(path.dirname(absPath), { recursive: true }); - fs.writeFileSync(absPath, buf); + installFs().mkdirSync(path.dirname(absPath), { recursive: true }); + installFs().writeFileSync(absPath, buf); } } @@ -581,8 +589,8 @@ function _restoreDir(dir: string, snapshot: Map): void { * @param nestedGsdDir absolute path to skills/gsd/ category dir */ function _removeHermesBareStemDirs(nestedGsdDir: string): void { - if (!fs.existsSync(nestedGsdDir)) return; - const entries = fs.readdirSync(nestedGsdDir, { withFileTypes: true }); + if (!installFs().existsSync(nestedGsdDir)) return; + const entries = installFs().readdirSync(nestedGsdDir, { withFileTypes: true }); // Collect the set of stems that were installed as gsd-/ this run. const installedStems = new Set(); @@ -595,7 +603,7 @@ function _removeHermesBareStemDirs(nestedGsdDir: string): void { // Remove any bare / dir for which gsd-/ was just installed. for (const entry of entries) { if (entry.isDirectory() && !entry.name.startsWith('gsd-') && installedStems.has(entry.name)) { - fs.rmSync(path.join(nestedGsdDir, entry.name), { recursive: true }); + installFs().rmSync(path.join(nestedGsdDir, entry.name), { recursive: true }); } } } @@ -621,9 +629,9 @@ function _runLegacyInstallMigrations(runtime: string, configDir: string, scope: // created skills/gsd-dev-preferences/ skill dir. let savedLegacyArtifacts: Map | null = null; if (_hostBehaviors(runtime).legacyCommandsGsdInstallMigration) { - if (fs.existsSync(legacyCommandsGsd)) { + if (installFs().existsSync(legacyCommandsGsd)) { savedLegacyArtifacts = preserveUserArtifacts(legacyCommandsGsd, ['dev-preferences.md']); - fs.rmSync(legacyCommandsGsd, { recursive: true }); + installFs().rmSync(legacyCommandsGsd, { recursive: true }); } } @@ -631,10 +639,10 @@ function _runLegacyInstallMigrations(runtime: string, configDir: string, scope: // the new skills/gsd/ nested layout. if (runtime === 'hermes') { const flatSkillsDir = path.join(configDir, 'skills'); - if (fs.existsSync(flatSkillsDir)) { - for (const entry of fs.readdirSync(flatSkillsDir, { withFileTypes: true })) { + if (installFs().existsSync(flatSkillsDir)) { + for (const entry of installFs().readdirSync(flatSkillsDir, { withFileTypes: true })) { if (entry.isDirectory() && entry.name.startsWith('gsd-')) { - fs.rmSync(path.join(flatSkillsDir, entry.name), { recursive: true }); + installFs().rmSync(path.join(flatSkillsDir, entry.name), { recursive: true }); } } } @@ -746,6 +754,18 @@ function _runLegacyUninstallCleanup(runtime: string, configDir: string, scope: s * the skills kind can materialize installed third-party capability skills * bound to their declaring capId. Absent -> no third-party skills staged * (fail closed), matching the layout resolver's own optional-registry contract. + * @param deps #2874 (ADR-58 cleanup phase): optional injection bag, additive + * over the 6-positional-arg call shape every existing caller (bin/install.js, + * G1/G3 test doubles) already uses — an omitted/`{}` `deps` is byte-identical + * to before (AC4). `deps.fs` — a PARTIAL InstallFsAdapter + * (install-fs-adapter.cts) — is merged over the real fs adapter for the + * duration of this call (and everything it calls: layout source-root + * resolution, profile staging, content-rewrite passes) via `withInstallFs`. + * @returns an executed-plan value describing what this call wrote, never + * `undefined` (40-design.md: "Legitimate undefined returns: none after this + * phase"). Throws, rather than returning an `ok:false` shape, on stage/ + * rewrite failure — the return type describes what executed; failure stays + * an exception (design doc "Rejected" #3 / AC4). */ function installRuntimeArtifacts( runtime: string, @@ -754,140 +774,187 @@ function installRuntimeArtifacts( resolvedProfile: any, resolveAttribution: ResolveAttribution = () => undefined, capabilityRegistry?: any, -): void { - // A removed descriptor kind is no longer visited by the layout loop, so it - // cannot prune its own previous output. Clean manifest-proven retired files - // before materializing the current layout (#2644). - retiredArtifactCleanup.pruneRetiredRuntimeArtifacts(runtime, configDir); + deps: { fs?: any } = {}, +): any { + return withInstallFs(deps.fs, (): any => { + // A removed descriptor kind is no longer visited by the layout loop, so it + // cannot prune its own previous output. Clean manifest-proven retired files + // before materializing the current layout (#2644). + retiredArtifactCleanup.pruneRetiredRuntimeArtifacts(runtime, configDir); - // Combined-family runtimes (OpenCode/Kilo, ADR-1239 / #2087): route through - // the dedicated combined commands+skills+plugin orchestrator instead of the - // generic layout-driven loop below, mirroring the bespoke install path that - // previously lived inline in bin/install.js. - const behaviors = _hostBehaviors(runtime); - if (behaviors.combinedFamilyInstall) { - // #2329: combined-family runtimes (OpenCode/Kilo) bypass - // _runLegacyInstallMigrations below entirely (early return), so their - // legacy-directory cleanup needs its own pre-materialization hook here. - _migrateLegacyOpencodeCommandDir(runtime, configDir, behaviors); - installOpencodeFamilyArtifacts(runtime, configDir, scope, resolvedProfile, resolveAttribution, behaviors, capabilityRegistry); - return; - } - - // Legacy cleanup before layout-driven writes - _runLegacyInstallMigrations(runtime, configDir, scope); - - const layout = runtimeArtifactLayout.resolveRuntimeArtifactLayout(runtime, configDir, scope as 'global' | 'local', capabilityRegistry); - const planResult = runtimeArtifactInstallPlan.createRuntimeArtifactInstallPlan({ - // `Layout` is structurally identical across the layout/install-plan .cjs - // modules but nominally distinct to tsc (untyped .cjs boundary) — bridge it. - layout: layout as any, - resolvedProfile, - homedir: () => os.homedir(), - platform: process.platform, - resolveAttribution, - }); - - const cleanupDirs = planResult.ok ? planResult.plan.cleanupDirs : planResult.cleanupDirs; - try { - if (!planResult.ok) { - throw new Error(planResult.message); + // Combined-family runtimes (OpenCode/Kilo, ADR-1239 / #2087): route through + // the dedicated combined commands+skills+plugin orchestrator instead of the + // generic layout-driven loop below, mirroring the bespoke install path that + // previously lived inline in bin/install.js. + const behaviors = _hostBehaviors(runtime); + if (behaviors.combinedFamilyInstall) { + // #2329: combined-family runtimes (OpenCode/Kilo) bypass + // _runLegacyInstallMigrations below entirely (early return), so their + // legacy-directory cleanup needs its own pre-materialization hook here. + _migrateLegacyOpencodeCommandDir(runtime, configDir, behaviors); + // #2874 design row 2: this early return must ALSO return an executed + // plan — installOpencodeFamilyArtifacts reports what it wrote, so a + // whole runtime family returning undefined is no longer a hole. + return installOpencodeFamilyArtifacts(runtime, configDir, scope, resolvedProfile, resolveAttribution, behaviors, capabilityRegistry); } - const kindsByName = new Map(layout.kinds.map((kind: any) => [kind.kind as string, kind])); - for (const item of planResult.plan.items) { - const kind: any = kindsByName.get(item.kind); - if (!kind) throw new Error(`Install plan returned unknown artifact kind: ${item.kind}`); - const dest = item.destDir; - // Symlink-escape guard: reject before mkdir if dest (or any component - // between the install root and dest) is a symlink pointing outside that - // root. mkdirSync follows symlinks, so this must run BEFORE the mkdir - // call. The install root is normally configDir, but a kind may declare - // an alternate `home` (ADR-1239 upgrade 3 / #2088, e.g. Codex skills -> - // $HOME/.agents) — in that case the guard must check against the - // resolved alternate root instead, matching assertDestWithinConfigHome's - // own root selection in createRuntimeArtifactInstallPlan. - const installRoot = (kind && typeof kind.home === 'string' && kind.home !== '') ? kind.home : configDir; - // #2393: honor GSD_ALLOW_SYMLINKED_DEST for intentional user-owned symlink layouts. - // Threat model from #1704 / ADR-1239 Phase B preserved: path-traversal and - // resolved-target-equals-root still refuse regardless of opt-in. - if (hasExistingSymlinkBetween(path.resolve(installRoot), dest, { allowOptInFollow: isSymlinkedDestOptIn() })) { - throw new Error( - `installRuntimeArtifacts: destDir "${dest}" contains a symlink the install root "${installRoot}" does not trust — refusing to create. If this is an intentional user-owned symlink layout (e.g. externalized skills/hooks dir, multi-account configHome, or a dotfiles-managed configHome), re-run with GSD_ALLOW_SYMLINKED_DEST=1.`, - ); + // Legacy cleanup before layout-driven writes + _runLegacyInstallMigrations(runtime, configDir, scope); + + const layout = runtimeArtifactLayout.resolveRuntimeArtifactLayout(runtime, configDir, scope as 'global' | 'local', capabilityRegistry); + const planResult = runtimeArtifactInstallPlan.createRuntimeArtifactInstallPlan({ + // `Layout` is structurally identical across the layout/install-plan .cjs + // modules but nominally distinct to tsc (untyped .cjs boundary) — bridge it. + layout: layout as any, + resolvedProfile, + homedir: () => os.homedir(), + platform: process.platform, + resolveAttribution, + }); + + const cleanupDirs = planResult.ok ? planResult.plan.cleanupDirs : planResult.cleanupDirs; + // #2874 row 1/4/5: per-kind executed-plan entries, appended only as the + // loop below actually finishes writing each kind — a kind that throws + // mid-copy is never reported as executed. + const executedKinds: any[] = []; + // #2874 rows 10/11: { dir, ok } per cleanupDirs entry — built in the + // `finally` below regardless of whether the try block throws, so a + // caught failure that still throws (row 3) leaves this populated even + // though it is never returned on that path. + const cleanupResults: { dir: string; ok: boolean }[] = []; + try { + if (!planResult.ok) { + throw new Error(planResult.message); } - fs.mkdirSync(dest, { recursive: true }); - if (kind.kind === 'skills' && fs.existsSync(dest)) { - // Pre-prune: snapshot user-owned content before _removeGsdEntries wipes it, - // then restore after. This preserves user dirs across a wipe-and-replace - // install (#2973 / #3664). + + const kindsByName = new Map(layout.kinds.map((kind: any) => [kind.kind as string, kind])); + for (const item of planResult.plan.items) { + const kind: any = kindsByName.get(item.kind); + if (!kind) throw new Error(`Install plan returned unknown artifact kind: ${item.kind}`); + const dest = item.destDir; + // Symlink-escape guard: reject before mkdir if dest (or any component + // between the install root and dest) is a symlink pointing outside that + // root. mkdirSync follows symlinks, so this must run BEFORE the mkdir + // call. The install root is normally configDir, but a kind may declare + // an alternate `home` (ADR-1239 upgrade 3 / #2088, e.g. Codex skills -> + // $HOME/.agents) — in that case the guard must check against the + // resolved alternate root instead, matching assertDestWithinConfigHome's + // own root selection in createRuntimeArtifactInstallPlan. // - // All runtimes (incl. Hermes after #947) use prefix='gsd-'. - // _removeGsdEntries removes only gsd-* entries; non-gsd-* user dirs are - // untouched. Preserve the explicit user-owned GSD-prefixed skill - // gsd-dev-preferences, which GSD does not reinstall from source but must - // survive the prune (#2973). - const toPreserve = new Map>(); // dirName -> Map + // #2874: this REFUSAL DECISION stays outside the injected fs adapter — + // only hasExistingSymlinkBetween's own existsSync/lstatSync/realpathSync + // PROBES are routed through it (install-fs-adapter.cts's module doc). + // A fake adapter can change what those probes observe for paths that + // were never real to begin with; it cannot make this `if` pass for a + // path the real filesystem would refuse. + const installRoot = (kind && typeof kind.home === 'string' && kind.home !== '') ? kind.home : configDir; + // #2393: honor GSD_ALLOW_SYMLINKED_DEST for intentional user-owned symlink layouts. + // Threat model from #1704 / ADR-1239 Phase B preserved: path-traversal and + // resolved-target-equals-root still refuse regardless of opt-in. + if (hasExistingSymlinkBetween(path.resolve(installRoot), dest, { allowOptInFollow: isSymlinkedDestOptIn() })) { + throw new Error( + `installRuntimeArtifacts: destDir "${dest}" contains a symlink the install root "${installRoot}" does not trust — refusing to create. If this is an intentional user-owned symlink layout (e.g. externalized skills/hooks dir, multi-account configHome, or a dotfiles-managed configHome), re-run with GSD_ALLOW_SYMLINKED_DEST=1.`, + ); + } + installFs().mkdirSync(dest, { recursive: true }); + const preserved: string[] = []; + if (kind.kind === 'skills' && installFs().existsSync(dest)) { + // Pre-prune: snapshot user-owned content before _removeGsdEntries wipes it, + // then restore after. This preserves user dirs across a wipe-and-replace + // install (#2973 / #3664). + // + // All runtimes (incl. Hermes after #947) use prefix='gsd-'. + // _removeGsdEntries removes only gsd-* entries; non-gsd-* user dirs are + // untouched. Preserve the explicit user-owned GSD-prefixed skill + // gsd-dev-preferences, which GSD does not reinstall from source but must + // survive the prune (#2973). + const toPreserve = new Map>(); // dirName -> Map - { - // Preserve explicitly user-owned GSD-prefixed skill dirs. - // gsd-dev-preferences is the sole user-customisable skill in this category. - const USER_OWNED_SKILL_DIRS = ['gsd-dev-preferences']; - for (const dirName of USER_OWNED_SKILL_DIRS) { - const skillDir = path.join(dest, dirName); - if (!fs.existsSync(skillDir)) continue; - const snap = _snapshotDir(skillDir); - if (snap.size > 0) toPreserve.set(dirName, snap); + { + // Preserve explicitly user-owned GSD-prefixed skill dirs. + // gsd-dev-preferences is the sole user-customisable skill in this category. + const USER_OWNED_SKILL_DIRS = ['gsd-dev-preferences']; + for (const dirName of USER_OWNED_SKILL_DIRS) { + const skillDir = path.join(dest, dirName); + if (!installFs().existsSync(skillDir)) continue; + const snap = _snapshotDir(skillDir); + if (snap.size > 0) toPreserve.set(dirName, snap); + } } - } - _removeGsdEntries(dest, kind); - _copyStaged(item.sourceDir, dest, kind, configDir, runtime); + _removeGsdEntries(dest, kind); + _copyStaged(item.sourceDir, dest, kind, configDir, runtime); - // Restore user-owned dirs after the prune+copy - for (const [dirName, snap] of toPreserve) { - _restoreDir(path.join(dest, dirName), snap); + // Restore user-owned dirs after the prune+copy + for (const [dirName, snap] of toPreserve) { + _restoreDir(path.join(dest, dirName), snap); + preserved.push(dirName); + } + } else { + // For non-skills kinds (commands, agents): no user content to preserve; + // just prune stale gsd-* entries and copy new ones. + _removeGsdEntries(dest, kind); + _copyStaged(item.sourceDir, dest, kind, configDir, runtime); + } + executedKinds.push({ kind: item.kind, sourceDir: item.sourceDir, destDir: dest, preserved }); + } + } finally { + // #2874 rows 10/11: cleanup stays best-effort (an install must never + // fail on cleanup) but a failed rmSync is now VISIBLE in `cleanup` + // rather than silently swallowed — silently absent is worse than the + // `void` return this replaces (40-design.md negative-space section). + for (const dir of cleanupDirs) { + try { + installFs().rmSync(dir, { recursive: true, force: true }); + cleanupResults.push({ dir, ok: true }); + } catch { + cleanupResults.push({ dir, ok: false }); } - } else { - // For non-skills kinds (commands, agents): no user content to preserve; - // just prune stale gsd-* entries and copy new ones. - _removeGsdEntries(dest, kind); - _copyStaged(item.sourceDir, dest, kind, configDir, runtime); } } - } finally { - for (const dir of cleanupDirs) { - try { fs.rmSync(dir, { recursive: true, force: true }); } catch { /* best-effort */ } + + // Hermes: after the install loop has written all gsd-/ dirs to + // skills/gsd/, remove any stale bare-stem dirs (skills/gsd//) that + // correspond to the newly installed gsd- entries. This is the robust + // replacement for the readGsdCommandNames()-based pre-install cleanup that + // missed skills like 'dev-preferences' (#947 adversarial review). + // + // We run this AFTER the install loop so the installed set is authoritative: + // every gsd-/ present now was written this run (or was there before + // with the same prefix). User-owned bare dirs with no gsd- counterpart + // are untouched. + let hermesBareStemCleanup = false; + if (runtime === 'hermes') { + const nestedGsdDirForCleanup = path.join(configDir, 'skills', 'gsd'); + _removeHermesBareStemDirs(nestedGsdDirForCleanup); + hermesBareStemCleanup = true; } - } - // Hermes: after the install loop has written all gsd-/ dirs to - // skills/gsd/, remove any stale bare-stem dirs (skills/gsd//) that - // correspond to the newly installed gsd- entries. This is the robust - // replacement for the readGsdCommandNames()-based pre-install cleanup that - // missed skills like 'dev-preferences' (#947 adversarial review). - // - // We run this AFTER the install loop so the installed set is authoritative: - // every gsd-/ present now was written this run (or was there before - // with the same prefix). User-owned bare dirs with no gsd- counterpart - // are untouched. - if (runtime === 'hermes') { - const nestedGsdDirForCleanup = path.join(configDir, 'skills', 'gsd'); - _removeHermesBareStemDirs(nestedGsdDirForCleanup); - } + // Generic-branch nativePlugin staging (ADR-1239 / #2102 Stage 1): runtimes + // outside the OpenCode/Kilo combined-family install (e.g. pi, whose + // artifactLayout is empty and which never sets combinedFamilyInstall) still + // need their declared hostBehaviors.nativePlugin file copied into configDir. + // findInstallSourceRoot resolves the repo/package root independent of + // configDir contents (marker check, then a walk-up from __dirname), so this + // is safe even when configDir has no .gsd-source marker (artifactLayout: []). + let nativePluginInstalled = false; + if (behaviors.nativePlugin) { + const commandsGsdDir = runtimeArtifactLayout.findInstallSourceRoot(configDir); + const src = path.dirname(path.dirname(commandsGsdDir)); + _installNativePluginIfDeclared(runtime, configDir, behaviors, src); + nativePluginInstalled = true; + } - // Generic-branch nativePlugin staging (ADR-1239 / #2102 Stage 1): runtimes - // outside the OpenCode/Kilo combined-family install (e.g. pi, whose - // artifactLayout is empty and which never sets combinedFamilyInstall) still - // need their declared hostBehaviors.nativePlugin file copied into configDir. - // findInstallSourceRoot resolves the repo/package root independent of - // configDir contents (marker check, then a walk-up from __dirname), so this - // is safe even when configDir has no .gsd-source marker (artifactLayout: []). - if (behaviors.nativePlugin) { - const commandsGsdDir = runtimeArtifactLayout.findInstallSourceRoot(configDir); - const src = path.dirname(path.dirname(commandsGsdDir)); - _installNativePluginIfDeclared(runtime, configDir, behaviors, src); - } + // #2874 row 14: an empty `layout.kinds` still returns `kinds: []` here + // (executedKinds was never mutated), never `undefined`. + return { + runtime, + scope, + kinds: executedKinds, + cleanup: cleanupResults, + postSteps: { hermesBareStemCleanup, nativePlugin: nativePluginInstalled }, + }; + }); } // --------------------------------------------------------------------------- @@ -934,7 +1001,7 @@ function installOpencodeFamilySkills( const skillsKindEntry = layout.kinds.find((k: any) => k.kind === 'skills'); if (!skillsKindEntry) return 0; const rawDir = rawCommandsDir; - if (!rawDir || !fs.existsSync(rawDir)) return 0; + if (!rawDir || !installFs().existsSync(rawDir)) return 0; // #2093: descriptor-driven — dispatch off the skills-kind entry's `converter` // string (capabilities//capability.json artifactLayout) via the @@ -965,7 +1032,7 @@ function installOpencodeFamilySkills( `installOpencodeFamilySkills: destDir "${dest}" contains a symlink the install root "${installRoot}" does not trust — refusing to write. If this is an intentional user-owned symlink layout, re-run with GSD_ALLOW_SYMLINKED_DEST=1.`, ); } - fs.mkdirSync(dest, { recursive: true }); + installFs().mkdirSync(dest, { recursive: true }); // Preserve user-owned GSD-prefixed skill dirs across the gsd-* prune. // gsd-dev-preferences is generated by the user (via generate-dev-preferences) @@ -976,7 +1043,7 @@ function installOpencodeFamilySkills( const toPreserve = new Map>(); // dirName -> Map for (const dirName of USER_OWNED_SKILL_DIRS) { const skillDir = path.join(dest, dirName); - if (!fs.existsSync(skillDir)) continue; + if (!installFs().existsSync(skillDir)) continue; const snap = _snapshotDir(skillDir); if (snap.size > 0) toPreserve.set(dirName, snap); } @@ -985,18 +1052,18 @@ function installOpencodeFamilySkills( let count = 0; const firstPartyStems = new Set(); - for (const entry of fs.readdirSync(rawDir, { withFileTypes: true })) { + for (const entry of installFs().readdirSync(rawDir, { withFileTypes: true })) { if (!entry.isFile() || !entry.name.endsWith('.md')) continue; const stem = entry.name.slice(0, -3); firstPartyStems.add(stem); const skillName = `${skillsKindEntry.prefix}${stem}`; - let content = fs.readFileSync(path.join(rawDir, entry.name), 'utf8'); + let content = installFs().readFileSync(path.join(rawDir, entry.name), 'utf8'); content = applyOpencodeFamilyPathPrefix(content, runtime, pathPrefix); content = processAttribution(content, resolveAttribution(runtime)); content = converter(content, skillName); const skillDir = path.join(dest, skillName); - fs.mkdirSync(skillDir, { recursive: true }); - fs.writeFileSync(path.join(skillDir, 'SKILL.md'), content); + installFs().mkdirSync(skillDir, { recursive: true }); + installFs().writeFileSync(path.join(skillDir, 'SKILL.md'), content); count++; } @@ -1033,13 +1100,13 @@ function installOpencodeFamilySkills( content = applyOpencodeFamilyPathPrefix(content, runtime, pathPrefix); content = processAttribution(content, resolveAttribution(runtime)); const skillDir = path.join(dest, skillName); - fs.mkdirSync(skillDir, { recursive: true }); - fs.writeFileSync(path.join(skillDir, 'SKILL.md'), content); + installFs().mkdirSync(skillDir, { recursive: true }); + installFs().writeFileSync(path.join(skillDir, 'SKILL.md'), content); // #2322 HIGH-3 parity: persist the capability-owned marker so a later // prune pass can identify this directory even once the owning // capability is uninstalled/unsurfaced and no longer appears in any // registry view. - fs.writeFileSync(path.join(skillDir, installProfiles.CAPABILITY_SKILL_MARKER), found.capId + '\n', 'utf8'); + installFs().writeFileSync(path.join(skillDir, installProfiles.CAPABILITY_SKILL_MARKER), found.capId + '\n', 'utf8'); count++; } } @@ -1080,25 +1147,25 @@ function installOpencodeFamilyCommands( resolveAttribution: ResolveAttribution = () => undefined, prefix: string = 'gsd', ): void { - if (!fs.existsSync(srcDir)) return; + if (!installFs().existsSync(srcDir)) return; // Remove old gsd-*.md files before copying new ones - if (fs.existsSync(destDir)) { - for (const file of fs.readdirSync(destDir)) { - if (file.startsWith(`${prefix}-`) && file.endsWith('.md')) fs.unlinkSync(path.join(destDir, file)); + if (installFs().existsSync(destDir)) { + for (const file of installFs().readdirSync(destDir)) { + if (file.startsWith(`${prefix}-`) && file.endsWith('.md')) installFs().unlinkSync(path.join(destDir, file)); } } else { - fs.mkdirSync(destDir, { recursive: true }); + installFs().mkdirSync(destDir, { recursive: true }); } - for (const entry of fs.readdirSync(srcDir, { withFileTypes: true })) { + for (const entry of installFs().readdirSync(srcDir, { withFileTypes: true })) { const srcPath = path.join(srcDir, entry.name); if (entry.isDirectory()) { installOpencodeFamilyCommands(runtime, destDir, srcPath, pathPrefix, resolveAttribution, `${prefix}-${entry.name}`); } else if (entry.name.endsWith('.md')) { const baseName = entry.name.replace('.md', ''); const destName = `${prefix}-${baseName}.md`; - let content = fs.readFileSync(srcPath, 'utf8'); + let content = installFs().readFileSync(srcPath, 'utf8'); content = applyOpencodeFamilyPathPrefix(content, runtime, pathPrefix); content = processAttribution(content, resolveAttribution(runtime)); // #2093: this commands-kind entry's descriptor `converter` field is @@ -1114,7 +1181,7 @@ function installOpencodeFamilyCommands( content = _hostBehaviors(runtime).frontmatterDialect === 'kilo' ? (runtimeArtifactConversion as any).convertClaudeToKiloFrontmatter(content) : (runtimeArtifactConversion as any).convertClaudeToOpencodeFrontmatter(content); - fs.writeFileSync(path.join(destDir, destName), content); + installFs().writeFileSync(path.join(destDir, destName), content); } } } @@ -1149,7 +1216,7 @@ function _installNativePluginIfDeclared( const np = behaviors.nativePlugin; if (np && np.source) { const pluginSrc = path.join(src, np.source); - if (fs.existsSync(pluginSrc)) { + if (installFs().existsSync(pluginSrc)) { // Confine the FULL dest path (dir + file), not just the dir. Previously // only `np.dir` was validated and `np.file` was joined on unchecked, so a // descriptor whose `file` carried `..`, an absolute path, or a NUL byte @@ -1162,8 +1229,8 @@ function _installNativePluginIfDeclared( configDir, path.join(np.dir, np.file), ); - fs.mkdirSync(path.dirname(destPath), { recursive: true }); - fs.copyFileSync(pluginSrc, destPath); + installFs().mkdirSync(path.dirname(destPath), { recursive: true }); + installFs().copyFileSync(pluginSrc, destPath); // #2544: the staged adapter is a `.js` file, so Node decides its module // type by walking up for the nearest package.json. It used to find the // marker the installer wrote at the config root — the write that @@ -1236,14 +1303,23 @@ function _migrateLegacyOpencodeCommandDir(runtime: string, configDir: string, be const currentName = behaviors.flatCommandDir || LEGACY_NAME; if (currentName === LEGACY_NAME) return; // e.g. Kilo — legacy IS the current location; nothing to migrate const legacyDir = path.join(configDir, LEGACY_NAME); - if (!fs.existsSync(legacyDir)) return; + if (!installFs().existsSync(legacyDir)) return; // Never follow a symlinked legacy dir out of configDir. - if (fs.lstatSync(legacyDir).isSymbolicLink()) return; + if (installFs().lstatSync(legacyDir).isSymbolicLink()) return; + // #2874: installerMigrations.readInstallManifest/classifyArtifact are + // routed through the injectable seam (installer-migrations.cts:36,54-58, + // 376-380 — readInstallManifest -> readJsonIfPresent -> installFs(), + // classifyArtifact -> sha256File -> installFs().readFileSync), so a + // fake-adapter install of an opencode-family runtime with a legacy + // `command/` dir present reaches the fake, not real fs. Exercised by + // tests/executed-plan.test.cjs's F2 "opencode-family legacy command/ dir + // migration" case, which poisons every real fs method and asserts the + // fake store was mutated. const manifest = installerMigrations.readInstallManifest(configDir); let entries: fs.Dirent[]; try { - entries = fs.readdirSync(legacyDir, { withFileTypes: true }); + entries = installFs().readdirSync(legacyDir, { withFileTypes: true }); } catch { return; } @@ -1254,14 +1330,14 @@ function _migrateLegacyOpencodeCommandDir(runtime: string, configDir: string, be const relPath = `${LEGACY_NAME}/${entry.name}`; const { classification } = installerMigrations.classifyArtifact(configDir, relPath, manifest); if (classification === 'managed-pristine' || classification === 'managed-modified') { - try { fs.unlinkSync(path.join(legacyDir, entry.name)); } catch { /* best-effort */ } + try { installFs().unlinkSync(path.join(legacyDir, entry.name)); } catch { /* best-effort */ } } // 'unknown' (not manifest-tracked) is left untouched — GSD cannot prove // ownership, so it must never be deleted as collateral damage. } try { - if (fs.readdirSync(legacyDir).length === 0) fs.rmdirSync(legacyDir); + if (installFs().readdirSync(legacyDir).length === 0) installFs().rmdirSync(legacyDir); } catch { /* best-effort — a non-empty or otherwise-busy dir is left in place */ } } @@ -1287,6 +1363,10 @@ function _migrateLegacyOpencodeCommandDir(runtime: string, configDir: string, be * installOpencodeFamilySkills so an installed third-party capability skill * materializes for this combined-family (OpenCode/Kilo) install path too. * Absent -> no third-party skills staged (fail closed). + * @returns #2874 design row 2: an executed-plan value, same top-level shape + * (`runtime`/`scope`/`kinds`/`cleanup`/`postSteps`) as the generic + * `installRuntimeArtifacts` branch — this was the one early return a + * `void`-shaped hole survived unnoticed in. */ function installOpencodeFamilyArtifacts( runtime: string, @@ -1296,7 +1376,7 @@ function installOpencodeFamilyArtifacts( resolveAttribution: ResolveAttribution = () => undefined, behaviors: any = {}, capabilityRegistry?: any, -): void { +): any { // #2870: `scope` keeps its exported required `string` signature (no // signature change). It is always the `installRuntimeArtifacts`-forwarded // 'global' | 'local' literal produced by bin/install.js's scope-resolution @@ -1332,9 +1412,25 @@ function installOpencodeFamilyArtifacts( behaviors.flatCommandDir || 'command', ); installOpencodeFamilyCommands(runtime, commandDir, rawCommandsDir, pathPrefix, resolveAttribution); - installOpencodeFamilySkills(runtime, configDir, rawCommandsDir, pathPrefix, resolveAttribution, resolvedProfile, capabilityRegistry); + const skillsWritten = installOpencodeFamilySkills(runtime, configDir, rawCommandsDir, pathPrefix, resolveAttribution, resolvedProfile, capabilityRegistry); _installNativePluginIfDeclared(runtime, configDir, behaviors, src); + + // #2874 design row 2: report what this combined-family install wrote, + // mirroring the generic branch's top-level shape. `cleanup` is `[]` — this + // path stages via install-profiles.cts's STAGED_DIRS (process-exit + // cleanup), not the per-call cleanupDirs mechanism createRuntimeArtifactInstallPlan + // uses, so there is nothing this call itself attempted to clean up. + return { + runtime, + scope, + kinds: [ + { kind: 'commands', sourceDir: rawCommandsDir, destDir: commandDir }, + { kind: 'skills', sourceDir: rawCommandsDir, destDir: configDir, written: skillsWritten }, + ], + cleanup: [], + postSteps: { hermesBareStemCleanup: false, nativePlugin: Boolean(behaviors.nativePlugin) }, + }; } // --------------------------------------------------------------------------- diff --git a/src/install-fs-adapter.cts b/src/install-fs-adapter.cts new file mode 100644 index 000000000..b537eb1ba --- /dev/null +++ b/src/install-fs-adapter.cts @@ -0,0 +1,251 @@ +'use strict'; + +/** + * Install Fs Adapter — #2874 (epic #2866 Phase 5), governed by ADR-58. + * + * Narrow, enumerated fs seam for the install/staging call tree rooted at + * `installRuntimeArtifacts` (install-engine.cts): layout source-root + * resolution (runtime-artifact-layout.cts), profile staging + * (install-profiles.cts), content-rewrite passes + * (runtime-artifact-conversion.cts), the CommonJS module-type marker + * (commonjs-marker.cts), and the two installer-migrations entry points this + * call tree reaches — `readInstallManifest` / `classifyArtifact` + * (installer-migrations.cts). Extends the `deps` bag precedent already + * established by `createRuntimeArtifactInstallPlan` + * (runtime-artifact-install-plan.cts:155) — this is NOT a general-purpose + * `node:fs` wrapper (40-design.md "Rejected" #1): only the operations this + * call tree actually performs are enumerated below. installer-migrations.cts + * is ~1200 lines covering migration planning/apply/rollback/locking/journal + * machinery unrelated to `installRuntimeArtifacts` — only its two reachable + * entry points (and `sha256File`, their shared hashing helper — still raw-fd + * streaming via `openSync`/`readSync`/`closeSync`, now routed through this + * seam instead of calling `node:fs` directly, so large-file hashing never + * buffers a whole file through the injected adapter either) are routed; the + * rest of that file is untouched, deliberately, because it is off this call + * tree. + * + * ───────────────────────────────────────────────────────────────────────── + * DELIVERY MECHANISM — ambient, not threaded (read this before adding a call + * site that needs a different adapter mid-call) + * ───────────────────────────────────────────────────────────────────────── + * + * A single mutable "current adapter" (`current`, below) is swapped for the + * duration of one synchronous `installRuntimeArtifacts` call via + * `withInstallFs`, rather than a `deps` parameter threaded through every + * function on the call tree. The rejected alternative was threading: it + * would touch signatures across `install-profiles.cts` (5 staging + * functions), the 3000+-line `runtime-artifact-conversion.cts` (rewrite + * passes), `commonjs-marker.cts`, and `installer-migrations.cts` — a dozen+ + * unrelated call sites — for no behavioral gain over an ambient swap, and + * `createRuntimeArtifactInstallPlan`'s own `deps` bag (the precedent this + * seam extends) does not reach that deep either. Every fs-touching site on + * the call tree reads the active adapter via `installFs()` instead of + * importing `node:fs` directly. + * + * SYNCHRONOUS-ONLY / RE-ENTRANCY ASSUMPTION (load-bearing, not incidental): + * every method on this seam is `*Sync`, and `installRuntimeArtifacts` never + * awaits mid-call — the whole call tree from the top-level `withInstallFs` + * wrap down to the last `fs` touch runs on one turn of the event loop with + * no interleaving. That is what makes a single ambient variable safe instead + * of a race: nothing else can observe or mutate `current` while it is set. + * This assumption BREAKS if `installRuntimeArtifacts` (or anything it calls) + * ever becomes `async`, or if two installs run concurrently in the same + * process (the second `withInstallFs` call would clobber the first's + * adapter mid-flight) — neither is true today, but a future change that + * introduces either must revisit this module before trusting it. + * + * DEFERRED CLEANUP ACROSS THE RESTORE (#2874 leak-fix): staged directories + * outlive one `withInstallFs` call — `install-profiles.cts`'s + * `cleanupStagedSkills` runs later, from a `process.on('exit'/'SIGINT'/…)` + * handler, by which time `withInstallFs`'s `finally` has already restored + * `current` back to whatever was active before (real fs, in the top-level + * case). A cleanup handler that resolved "which adapter do I use" via + * `installFs()` at CLEANUP time would therefore always see the real adapter, + * even for a directory that was staged entirely inside a fake-adapter call — + * performing real filesystem IO on a path that only ever existed in the + * fake's in-memory store. `install-profiles.cts` avoids this by capturing + * the adapter OBJECT `installFs()` returns at STAGING (registration) time, + * keyed by path, in its own `STAGED_DIR_ADAPTERS` map, and replaying that + * captured object — not the ambient `current` — at cleanup time. A real + * install's dirs were staged with the real adapter object, so their cleanup + * is byte-identical to before this fix. + * + * `withInstallFs` ALWAYS restores the previous adapter in a `finally`, even + * on throw — a failed install (or a test that intentionally throws to prove + * a guard) must never leak a fake adapter into whatever runs next in the + * same process (e.g. the next `node:test` in a shared worker). + * + * ⚠️ PARTIAL-ADAPTER TRAP: `withInstallFs` merges the injected `partial` + * OVER the real adapter (`{ ...REAL_ADAPTER, ...partial }`) — any method the + * partial does not define resolves to REAL `node:fs`, silently. An + * incomplete fake is not a smaller fake adapter; for the methods it omits, + * it IS the real filesystem. A test asserting "no real IO happened" against + * a partial fake must either implement every method the exercised code path + * touches, or explicitly account for the ones it does not (see + * `tests/executed-plan.test.cjs`'s F2 test, which poisons real `fs` methods + * for exactly this reason — a gap here shows up as the poisoned method + * firing, not as a silent pass). + * + * ───────────────────────────────────────────────────────────────────────── + * SECURITY NOTE (40-design.md rows 6/7, H1-H5) + * ───────────────────────────────────────────────────────────────────────── + * This module carries NO policy. `hasExistingSymlinkBetween` and + * `assertDestWithinConfigHome` keep their REFUSAL DECISIONS outside this + * seam — only their probe calls (existsSync/lstatSync/realpathSync) are + * routed through it. Injecting a fake adapter can only change what those + * probes observe for paths that were never real to begin with; it cannot + * flip the decision logic itself. + * + * ───────────────────────────────────────────────────────────────────────── + * DELIBERATELY NOT ROUTED (see the call sites themselves for the full + * reasoning — this is the index) + * ───────────────────────────────────────────────────────────────────────── + * `findInstallSourceRoot` / `findAgentsSourceRoot`'s walk-up-from-__dirname + * step (runtime-artifact-layout.cts) locates THIS PACKAGE'S OWN source tree + * (`commands/gsd/`, `agents/`) — not the install destination — so it uses + * `fs.statSync` directly, unrouted. A fake destination adapter's store + * starts empty and is never seeded with real repo paths; routing this walk + * through it would make every fake-adapter install throw + * "could not locate commands/gsd", not gracefully stage nothing. See the + * comment at each function's Step 2 for the full argument. + * + * RULE (40-design.md "Known limits"): this seam makes *destination* IO + * fake-able; package-source IO (this section) stays real by design — the F2 + * test's poison list should be derived FROM that rule, not the reverse, or a + * future "complete the poison list" edit that adds `statSync` will break a + * correct `findInstallSourceRoot` for the wrong reason. + */ + +import nodeFs from 'node:fs'; +import type { Dirent } from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import crypto from 'node:crypto'; + +// Precisely typed (unlike install-engine.cts's own house style) so every +// OTHER file this adapter is imported into — install-profiles.cts, +// runtime-artifact-layout.cts, retired-artifact-cleanup.cts, +// command-roster.cts, commonjs-marker.cts, installer-migrations.cts — none +// of which blanket-disable the `no-unsafe-*` rules — keeps its existing +// strict typing at each call site instead of degrading to `any` through +// this seam. +interface InstallFsAdapter { + existsSync(p: string): boolean; + mkdirSync(p: string, opts?: { recursive?: boolean }): string | undefined; + rmSync(p: string, opts?: { recursive?: boolean; force?: boolean }): void; + readdirSync(p: string, opts: { withFileTypes: true }): Dirent[]; + readdirSync(p: string): string[]; + readFileSync(p: string, encoding: BufferEncoding): string; + readFileSync(p: string): Buffer; + writeFileSync(p: string, data: string | Buffer, opts?: BufferEncoding | { encoding?: BufferEncoding; flag?: string }): void; + copyFileSync(src: string, dest: string): void; + cpSync(src: string, dest: string, opts?: { recursive?: boolean }): void; + lstatSync(p: string): { isFile(): boolean; isDirectory(): boolean; isSymbolicLink(): boolean }; + /** Optional — a partial injected adapter that omits this falls back to the + * real fs.realpathSync (merged in by `withInstallFs`). The symlink guard + * already treats a realpathSync FAILURE as "fall back to the lexical + * form" (see its own doc comment); an absent method degrades the same + * way — never a hard failure. */ + realpathSync(p: string): string; + unlinkSync(p: string): void; + rmdirSync(p: string): void; + /** Raw-fd streaming trio, added so `installer-migrations.cts`'s + * `sha256File` can hash a file in fixed-size chunks through this seam + * instead of buffering the whole file via `readFileSync` — see the + * module doc's opening paragraph and `tests/installer-migrations.test.cjs`'s + * "classifies large files without loading the whole file through + * readFileSync", which pins this contract directly: it monkeypatches real + * `fs.readFileSync` to throw for the file under test and asserts hashing + * still succeeds, proving the hash path never calls it. */ + openSync(p: string, flags: string): number; + readSync(fd: number, buffer: Buffer, offset: number, length: number, position: number | null): number; + closeSync(fd: number): void; +} + +const REAL_ADAPTER: InstallFsAdapter = { + existsSync: (p) => nodeFs.existsSync(p), + mkdirSync: (p, opts) => nodeFs.mkdirSync(p, opts), + rmSync: (p, opts) => nodeFs.rmSync(p, opts), + readdirSync: ((p: string, opts?: { withFileTypes: true }) => + (opts ? nodeFs.readdirSync(p, opts) : nodeFs.readdirSync(p))) as InstallFsAdapter['readdirSync'], + readFileSync: ((p: string, encoding?: BufferEncoding) => + (encoding ? nodeFs.readFileSync(p, encoding) : nodeFs.readFileSync(p))) as InstallFsAdapter['readFileSync'], + writeFileSync: (p, data, opts) => nodeFs.writeFileSync(p, data, opts), + copyFileSync: (src, dest) => nodeFs.copyFileSync(src, dest), + cpSync: (src, dest, opts) => nodeFs.cpSync(src, dest, opts), + lstatSync: (p) => nodeFs.lstatSync(p), + realpathSync: (p) => nodeFs.realpathSync(p), + unlinkSync: (p) => nodeFs.unlinkSync(p), + rmdirSync: (p) => nodeFs.rmdirSync(p), + openSync: (p, flags) => nodeFs.openSync(p, flags), + readSync: (fd, buffer, offset, length, position) => nodeFs.readSync(fd, buffer, offset, length, position), + closeSync: (fd) => nodeFs.closeSync(fd), +}; + +let current: InstallFsAdapter = REAL_ADAPTER; + +/** + * Returns the fs adapter active for the currently-running install call — + * the real adapter when no `deps.fs` was injected, or the injected partial + * adapter merged over the real one (any method it did not override still + * resolves to real fs — see the module doc's "PARTIAL-ADAPTER TRAP"). + */ +function installFs(): InstallFsAdapter { + return current; +} + +/** + * Run `fn` with `partial` merged over the real adapter as the active + * install-fs adapter, restoring the previous adapter afterward — even on + * throw (see the module doc's re-entrancy/synchronous-only assumption for + * why a bare module-level variable is safe here, and why it would not be + * under async interleaving or concurrent installs). `partial` undefined is + * a no-op: `fn` runs against whatever adapter was already active (real fs + * by default) — this is what keeps every existing `deps`-less call site + * (AC4) byte-identical. + */ +function withInstallFs(partial: Partial | undefined, fn: () => T): T { + if (!partial) return fn(); + const previous = current; + current = { ...REAL_ADAPTER, ...partial }; + try { + return fn(); + } finally { + current = previous; + } +} + +/** + * Create a fresh, uniquely-named temp directory. + * + * AC4 requires the no-adapter-injected path to stay byte-identical to the + * pre-#2874 code, which called real `fs.mkdtempSync` directly — several + * existing tests (e.g. tests/install-runtime-artifacts.test.cjs's "rmSync is + * called on the tempDir when readFileSync throws") monkeypatch real + * `fs.mkdtempSync` to capture the exact directory a call under test creates, + * and stop working if that real syscall is no longer made. So: when NO fake + * adapter is active (`current === REAL_ADAPTER`, the exact top-level-install + * default), this calls `nodeFs.mkdtempSync` directly — the same real call + * the old code made, still visible to a monkeypatch applied after import + * because it is a live property lookup on the `node:fs` module object, not a + * captured reference. + * + * When a fake adapter IS active (any `withInstallFs(partial, …)` call, even + * a partial one — see `current`'s assignment in `withInstallFs`, which + * always produces a NEW merged object, never `=== REAL_ADAPTER`), this falls + * back to synthesizing a unique name and creating it via + * `installFs().mkdirSync`, exactly as before: the injected adapter contract + * still does not require `mkdtempSync`, so a fake (which only needs to + * implement `mkdirSync`) can drive the full staging pipeline without ever + * touching real fs. + */ +function mkInstallTempDir(prefix: string): string { + if (current === REAL_ADAPTER) { + return nodeFs.mkdtempSync(path.join(os.tmpdir(), prefix)); + } + const dir = path.join(os.tmpdir(), `${prefix}${crypto.randomBytes(8).toString('hex')}`); + installFs().mkdirSync(dir, { recursive: true }); + return dir; +} + +export = { installFs, withInstallFs, mkInstallTempDir }; diff --git a/src/install-profiles.cts b/src/install-profiles.cts index 9fb254387..311589a17 100644 --- a/src/install-profiles.cts +++ b/src/install-profiles.cts @@ -10,6 +10,19 @@ import fs from 'node:fs'; import path from 'node:path'; import os from 'node:os'; +// #2874 (ADR-58 cleanup phase): route the staging functions' fs calls +// through the installRuntimeArtifacts call tree's injectable seam — see +// install-fs-adapter.cts's module doc. Resolves to real `node:fs` unless the +// top-level installRuntimeArtifacts call injected a `deps.fs`. Only the +// staging functions reachable FROM that call tree are routed +// (stageSkillsForProfile / stageAgentsForProfile / +// stageAgentsForRuntimeWithConverter / stageSkillsForRuntimeAsSkills / +// stageCommandsForRuntimeFlat / buildNamespaceBundleMap) — the profile-marker +// and manifest-loading helpers below are not on that call tree and keep +// using real `fs` directly. +// eslint-disable-next-line @typescript-eslint/no-require-imports +import installFsAdapter = require('./install-fs-adapter.cjs'); +const { installFs, mkInstallTempDir } = installFsAdapter; import { platformWriteSync } from './shell-command-projection.cjs'; // #2322: reuse the existing pure path-containment seam (ADR-1239 Phase C-2) // instead of hand-rolling a new traversal check for capability skill stems. @@ -301,16 +314,49 @@ function resolveProfile({ modes, manifest, _profilesOverride, registry }: Resolv const STAGED_DIRS = new Set(); let exitHandlerRegistered = false; +// #2874 leak-fix: `cleanupStagedSkills` runs at process exit — AFTER +// `withInstallFs` has already restored `current` back to the real adapter +// (install-fs-adapter.cts's module doc, "SYNCHRONOUS-ONLY / RE-ENTRANCY +// ASSUMPTION" — extended there to reference this map). A dir staged during a +// fake-adapter call must be cleaned up with THAT SAME fake adapter, not with +// whatever is ambiently active later, or an exit handler would perform real +// filesystem IO on a path that only ever existed in the fake's in-memory +// store. Capturing the adapter OBJECT `installFs()` returns at registration +// time (not the ambient `current` variable, which changes) means cleanup +// always replays the exact adapter that created the path. Dirs added to +// STAGED_DIRS without going through `registerStagedDir` (the deprecated +// `stageSkillsForMode`, which creates its stage dir via raw `fs.mkdtempSync` +// and was never on the injectable seam) have no entry here and fall back to +// real `fs.rmSync` below — the same real fs it always used to create them. +const STAGED_DIR_ADAPTERS = new Map>(); + +/** + * Register a dir just staged through the injectable seam (`installFs()`) for + * exit-time cleanup, capturing the adapter that staged it alongside the path. + * See `STAGED_DIR_ADAPTERS`'s comment for why the capture matters. + */ +function registerStagedDir(dir: string): void { + STAGED_DIRS.add(dir); + STAGED_DIR_ADAPTERS.set(dir, installFs()); + ensureExitCleanup(); +} + function cleanupStagedSkills(): void { for (const dir of STAGED_DIRS) { + const adapter = STAGED_DIR_ADAPTERS.get(dir); try { - fs.rmSync(dir, { recursive: true, force: true }); + if (adapter) { + adapter.rmSync(dir, { recursive: true, force: true }); + } else { + fs.rmSync(dir, { recursive: true, force: true }); + } } catch { // Best-effort: missing dir or permission error shouldn't crash a // successful install. The OS reaps tmpdir eventually. } } STAGED_DIRS.clear(); + STAGED_DIR_ADAPTERS.clear(); } // Signals we register a cleanup handler for in addition to the natural @@ -340,27 +386,26 @@ function ensureExitCleanup(): void { */ function stageSkillsForProfile(srcDir: string, resolvedProfile: ResolvedProfile): string { if (resolvedProfile.skills === '*') return srcDir; - if (!fs.existsSync(srcDir)) return srcDir; + if (!installFs().existsSync(srcDir)) return srcDir; - const stageDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-profile-skills-')); + const stageDir = mkInstallTempDir('gsd-profile-skills-'); try { - const entries = fs.readdirSync(srcDir, { withFileTypes: true }); + const entries = installFs().readdirSync(srcDir, { withFileTypes: true }); for (const entry of entries) { if (!entry.isFile()) continue; if (!entry.name.endsWith('.md')) continue; const stem = entry.name.slice(0, -3); if (!(resolvedProfile.skills).has(stem)) continue; - fs.copyFileSync( + installFs().copyFileSync( path.join(srcDir, entry.name), path.join(stageDir, entry.name), ); } } catch (err) { - try { fs.rmSync(stageDir, { recursive: true, force: true }); } catch { /* best-effort */ } + try { installFs().rmSync(stageDir, { recursive: true, force: true }); } catch { /* best-effort */ } throw err; } - STAGED_DIRS.add(stageDir); - ensureExitCleanup(); + registerStagedDir(stageDir); return stageDir; } @@ -386,19 +431,19 @@ function stageSkillsForProfile(srcDir: string, resolvedProfile: ResolvedProfile) */ function stageAgentsForProfile(srcAgentsDir: string, resolvedProfile: ResolvedProfile): string { if (resolvedProfile.skills === '*') return srcAgentsDir; - if (!fs.existsSync(srcAgentsDir)) return srcAgentsDir; + if (!installFs().existsSync(srcAgentsDir)) return srcAgentsDir; - const stageDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-profile-agents-')); + const stageDir = mkInstallTempDir('gsd-profile-agents-'); try { if (resolvedProfile.agents instanceof Set && resolvedProfile.agents.size > 0) { - const entries = fs.readdirSync(srcAgentsDir, { withFileTypes: true }); + const entries = installFs().readdirSync(srcAgentsDir, { withFileTypes: true }); for (const entry of entries) { if (!entry.isFile()) continue; if (!entry.name.endsWith('.md')) continue; // Agent stem is the full filename without extension, e.g. "gsd-planner" const stem = entry.name.slice(0, -3); if (!resolvedProfile.agents.has(stem)) continue; - fs.copyFileSync( + installFs().copyFileSync( path.join(srcAgentsDir, entry.name), path.join(stageDir, entry.name), ); @@ -406,11 +451,10 @@ function stageAgentsForProfile(srcAgentsDir: string, resolvedProfile: ResolvedPr } // If agents is empty Set, we produce an empty stageDir (no agents for this profile) } catch (err) { - try { fs.rmSync(stageDir, { recursive: true, force: true }); } catch { /* best-effort */ } + try { installFs().rmSync(stageDir, { recursive: true, force: true }); } catch { /* best-effort */ } throw err; } - STAGED_DIRS.add(stageDir); - ensureExitCleanup(); + registerStagedDir(stageDir); return stageDir; } @@ -434,16 +478,16 @@ function buildNamespaceBundleMap(srcCommandsDir: string): NamespaceBundleMap { const routerStems = new Set(); const routerChildren = new Map(); const childToRouters = new Map(); - if (!fs.existsSync(srcCommandsDir)) { + if (!installFs().existsSync(srcCommandsDir)) { return { routerStems, routerChildren, childToRouters }; } - for (const entry of fs.readdirSync(srcCommandsDir, { withFileTypes: true })) { + for (const entry of installFs().readdirSync(srcCommandsDir, { withFileTypes: true })) { if (!entry.isFile() || !entry.name.endsWith('.md')) continue; if (!entry.name.startsWith('ns-')) continue; const stem = entry.name.slice(0, -3); let children: string[] = []; try { - children = parseRequires(fs.readFileSync(path.join(srcCommandsDir, entry.name), 'utf8')); + children = parseRequires(installFs().readFileSync(path.join(srcCommandsDir, entry.name), 'utf8')); } catch { children = []; } routerStems.add(stem); routerChildren.set(stem, children); @@ -665,7 +709,7 @@ function stageSkillsForRuntimeAsSkills( nested = false, registry?: CapabilityRegistry, ): string { - if (!fs.existsSync(srcCommandsDir)) return srcCommandsDir; + if (!installFs().existsSync(srcCommandsDir)) return srcCommandsDir; // Nesting applies to the `full` install AND to any surface whose skill set // still contains every namespace router (a full/reset surface). It must NOT @@ -689,16 +733,16 @@ function stageSkillsForRuntimeAsSkills( // ALWAYS wins on collision" without re-deriving membership. const firstPartyStems = new Set(); - const stageDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-profile-runtime-skills-')); + const stageDir = mkInstallTempDir('gsd-profile-runtime-skills-'); try { - const entries = fs.readdirSync(srcCommandsDir, { withFileTypes: true }); + const entries = installFs().readdirSync(srcCommandsDir, { withFileTypes: true }); for (const entry of entries) { if (!entry.isFile()) continue; if (!entry.name.endsWith('.md')) continue; const stem = entry.name.slice(0, -3); if (resolvedProfile.skills !== '*' && !(resolvedProfile.skills).has(stem)) continue; firstPartyStems.add(stem); - const content = fs.readFileSync(path.join(srcCommandsDir, entry.name), 'utf8'); + const content = installFs().readFileSync(path.join(srcCommandsDir, entry.name), 'utf8'); const skillName = `${prefix}${stem}`; const converted = converter(content, skillName); @@ -706,8 +750,8 @@ function stageSkillsForRuntimeAsSkills( // Router skill: rewrite its routing table to the nested Read pattern and // emit it as the single top-level bundle entry. const destDir = path.join(stageDir, skillName); - fs.mkdirSync(destDir, { recursive: true }); - fs.writeFileSync(path.join(destDir, 'SKILL.md'), transformRouterBodyToNested(converted)); + installFs().mkdirSync(destDir, { recursive: true }); + installFs().writeFileSync(path.join(destDir, 'SKILL.md'), transformRouterBodyToNested(converted)); continue; } @@ -717,8 +761,8 @@ function stageSkillsForRuntimeAsSkills( // top-level eager listing while staying readable by file path (#69). for (const routerStem of bundles!.childToRouters.get(stem)!) { const destDir = path.join(stageDir, `${prefix}${routerStem}`, 'skills', stem); - fs.mkdirSync(destDir, { recursive: true }); - fs.writeFileSync(path.join(destDir, 'SKILL.md'), converted); + installFs().mkdirSync(destDir, { recursive: true }); + installFs().writeFileSync(path.join(destDir, 'SKILL.md'), converted); } continue; } @@ -726,8 +770,8 @@ function stageSkillsForRuntimeAsSkills( // Flat top-level skill (default behaviour; also the unrouted fallback when // nesting is active). const destDir = path.join(stageDir, skillName); - fs.mkdirSync(destDir, { recursive: true }); - fs.writeFileSync(path.join(destDir, 'SKILL.md'), converted); + installFs().mkdirSync(destDir, { recursive: true }); + installFs().writeFileSync(path.join(destDir, 'SKILL.md'), converted); } // #2322: materialize installed THIRD-PARTY capability skills, bound to @@ -768,21 +812,20 @@ function stageSkillsForRuntimeAsSkills( const skillName = `${prefix}${stem}`; if (!isPathConfined(skillName, stageDir)) continue; // defense-in-depth const destDir = path.join(stageDir, skillName); - fs.mkdirSync(destDir, { recursive: true }); - fs.writeFileSync(path.join(destDir, 'SKILL.md'), found.content); + installFs().mkdirSync(destDir, { recursive: true }); + installFs().writeFileSync(path.join(destDir, 'SKILL.md'), found.content); // #2322 HIGH-3: persist the capability-owned marker so a later prune // pass (surface.cts pruneSkillDirs) can identify — and remove — this // directory even once the owning capability is uninstalled/unsurfaced // and no longer appears in any registry view. - fs.writeFileSync(path.join(destDir, CAPABILITY_SKILL_MARKER), found.capId + '\n', 'utf8'); + installFs().writeFileSync(path.join(destDir, CAPABILITY_SKILL_MARKER), found.capId + '\n', 'utf8'); } } } catch (err) { - try { fs.rmSync(stageDir, { recursive: true, force: true }); } catch { /* best-effort */ } + try { installFs().rmSync(stageDir, { recursive: true, force: true }); } catch { /* best-effort */ } throw err; } - STAGED_DIRS.add(stageDir); - ensureExitCleanup(); + registerStagedDir(stageDir); return stageDir; } @@ -840,11 +883,11 @@ function stageAgentsForRuntimeWithConverter( isGlobal = false, agentCtx?: AgentCtx, ): string { - if (!fs.existsSync(srcAgentsDir)) return srcAgentsDir; + if (!installFs().existsSync(srcAgentsDir)) return srcAgentsDir; - const stageDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-profile-runtime-agents-')); + const stageDir = mkInstallTempDir('gsd-profile-runtime-agents-'); try { - const entries = fs.readdirSync(srcAgentsDir, { withFileTypes: true }); + const entries = installFs().readdirSync(srcAgentsDir, { withFileTypes: true }); // Resolve cmdNames once per staging call (not per file) for performance. const cmdNames = agentCtx ? _readGsdCommandNames() : []; for (const entry of entries) { @@ -858,7 +901,7 @@ function stageAgentsForRuntimeWithConverter( } } const agentSourcePath = path.join(srcAgentsDir, entry.name); - let content = fs.readFileSync(agentSourcePath, 'utf8'); + let content = installFs().readFileSync(agentSourcePath, 'utf8'); // #2995: strip gsd:section markers FIRST — before path rewrites, attribution, // and the per-runtime converter. Byte-identical (no-op) for an unmarked agent; // throws loudly naming the file for a malformed marker, never emitting a @@ -878,14 +921,13 @@ function stageAgentsForRuntimeWithConverter( // Backward-compat: only apply the converter (no cross-cutting) content = converter(content, isGlobal); } - fs.writeFileSync(path.join(stageDir, entry.name), content, 'utf8'); + installFs().writeFileSync(path.join(stageDir, entry.name), content, 'utf8'); } } catch (err) { - try { fs.rmSync(stageDir, { recursive: true, force: true }); } catch { /* best-effort */ } + try { installFs().rmSync(stageDir, { recursive: true, force: true }); } catch { /* best-effort */ } throw err; } - STAGED_DIRS.add(stageDir); - ensureExitCleanup(); + registerStagedDir(stageDir); return stageDir; } @@ -918,30 +960,29 @@ function stageCommandsForRuntimeFlat( converter: (content: string, commandName: string) => string, prefix: string, ): string { - if (!fs.existsSync(srcCommandsDir)) return srcCommandsDir; + if (!installFs().existsSync(srcCommandsDir)) return srcCommandsDir; - const stageDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-profile-runtime-commands-')); + const stageDir = mkInstallTempDir('gsd-profile-runtime-commands-'); try { - const entries = fs.readdirSync(srcCommandsDir, { withFileTypes: true }); + const entries = installFs().readdirSync(srcCommandsDir, { withFileTypes: true }); for (const entry of entries) { if (!entry.isFile()) continue; if (!entry.name.endsWith('.md')) continue; const stem = entry.name.slice(0, -3); if (resolvedProfile.skills !== '*' && !(resolvedProfile.skills).has(stem)) continue; - const content = fs.readFileSync(path.join(srcCommandsDir, entry.name), 'utf8'); + const content = installFs().readFileSync(path.join(srcCommandsDir, entry.name), 'utf8'); // Pass the full command name (with prefix) to the converter so it can // reference the installed command name in the body (e.g. for descriptions). // The staged file itself is named without the prefix; _copyStaged adds it. const commandName = `${prefix}${stem}`; const converted = converter(content, commandName); - fs.writeFileSync(path.join(stageDir, `${stem}.md`), converted); + installFs().writeFileSync(path.join(stageDir, `${stem}.md`), converted); } } catch (err) { - try { fs.rmSync(stageDir, { recursive: true, force: true }); } catch { /* best-effort */ } + try { installFs().rmSync(stageDir, { recursive: true, force: true }); } catch { /* best-effort */ } throw err; } - STAGED_DIRS.add(stageDir); - ensureExitCleanup(); + registerStagedDir(stageDir); return stageDir; } diff --git a/src/installer-migrations.cts b/src/installer-migrations.cts index 6139adbea..03eff4b83 100644 --- a/src/installer-migrations.cts +++ b/src/installer-migrations.cts @@ -19,6 +19,21 @@ import { import { platformWriteSync, retryRenameSync, posixNormalize } from './shell-command-projection.cjs'; import { realClock, type Clock } from './clock.cjs'; import { isInstallScopeId, type InstallScope } from './install-scope.cjs'; +// #2874 (ADR-58 cleanup phase): this file is the ~1200-line migration +// plan/apply/rollback/lock/journal engine — almost none of it is on the +// installRuntimeArtifacts call tree. Only `readInstallManifest` and +// `classifyArtifact` are reached (via install-engine.cts's +// _migrateLegacyOpencodeCommandDir and retired-artifact-cleanup.cts's +// pruneRetiredRuntimeArtifacts), so only those two entry points — plus their +// shared `readJsonIfPresent` helper and `classifyArtifact`'s `sha256File` +// hashing helper — are routed through the injectable seam. Everything else +// in this file (locking, journal, apply/rollback, migration discovery) +// keeps using real `fs` directly: it is not reachable from +// installRuntimeArtifacts, so routing it would grow this seam past what +// AC2 actually requires. See install-fs-adapter.cts's module doc. +// eslint-disable-next-line @typescript-eslint/no-require-imports +import installFsAdapter = require('./install-fs-adapter.cjs'); +const { installFs } = installFsAdapter; const MANIFEST_NAME = 'gsd-file-manifest.json'; const INSTALL_STATE_NAME = 'gsd-install-state.json'; @@ -27,18 +42,31 @@ const DEFAULT_MIGRATIONS_DIR = path.join(__dirname, 'installer-migrations'); const DEFAULT_LOCK_TIMEOUT_MS = 30_000; const STRICT_JSON = Symbol('strict-json'); +// #2874: routed through installFs()'s openSync/readSync/closeSync trio +// instead of importing `node:fs` directly, so classifyArtifact — reachable +// from installRuntimeArtifacts — can be exercised against an injected +// adapter. This function was briefly converted to a single +// `installFs().readFileSync` call (buffering the whole file); that broke +// tests/installer-migrations.test.cjs's "classifies large files without +// loading the whole file through readFileSync", which monkeypatches real +// fs.readFileSync to throw for the file under test and asserts hashing still +// succeeds — an explicit, pre-existing contract that large files must be +// streamed, not buffered. Restored to the original raw-fd streaming shape, +// now going through the adapter instead of `node:fs` directly. This is the +// ONLY call site of sha256File in this file (confirmed by inspection) — no +// other caller is affected. function sha256File(filePath: string): string { const hash = crypto.createHash('sha256'); const buffer = Buffer.allocUnsafe(1024 * 1024); - const fd = fs.openSync(filePath, 'r'); + const fd = installFs().openSync(filePath, 'r'); try { while (true) { - const bytesRead = fs.readSync(fd, buffer, 0, buffer.length, null); + const bytesRead = installFs().readSync(fd, buffer, 0, buffer.length, null); if (bytesRead === 0) break; hash.update(buffer.subarray(0, bytesRead)); } } finally { - fs.closeSync(fd); + installFs().closeSync(fd); } return hash.digest('hex'); } @@ -148,10 +176,14 @@ function copyPreservingSymlink(srcPath: string, destPath: string): void { fs.copyFileSync(srcPath, destPath); } +// Shared by readInstallManifest (on the installRuntimeArtifacts call tree — +// routed) and readInstallState/readJson (not on that call tree — the +// ambient default resolves to real fs for those, unchanged). Routing once +// here is safe for all three callers. function readJsonIfPresent(filePath: string, fallback: unknown): unknown { - if (!fs.existsSync(filePath)) return fallback; + if (!installFs().existsSync(filePath)) return fallback; try { - return JSON.parse(fs.readFileSync(filePath, 'utf8')); + return JSON.parse(installFs().readFileSync(filePath, 'utf8')); } catch (error) { if (fallback === STRICT_JSON) { throw new Error(`invalid installer migration state JSON: ${filePath}: ${(error as Error).message}`); @@ -358,7 +390,7 @@ function classifyArtifact(configDir: string, relPath: string, manifest: InstallM const normalized = normalizeRelPath(relPath); const originalHash = manifest.files[normalized] || null; const fullPath = path.join(configDir, normalized); - if (!fs.existsSync(fullPath)) { + if (!installFs().existsSync(fullPath)) { return { classification: originalHash ? 'managed-missing' : 'missing', originalHash, currentHash: null }; } const currentHash = sha256File(fullPath); diff --git a/src/retired-artifact-cleanup.cts b/src/retired-artifact-cleanup.cts index 56b0262f9..cad5b03f5 100644 --- a/src/retired-artifact-cleanup.cts +++ b/src/retired-artifact-cleanup.cts @@ -12,6 +12,12 @@ import fs from 'node:fs'; import path from 'node:path'; +// #2874 (ADR-58 cleanup phase): pruneRetiredRuntimeArtifacts is the first +// thing installRuntimeArtifacts calls, so its fs calls are routed through +// the injectable seam too — see install-fs-adapter.cts's module doc. +// eslint-disable-next-line @typescript-eslint/no-require-imports +import installFsAdapter = require('./install-fs-adapter.cjs'); +const { installFs } = installFsAdapter; // eslint-disable-next-line @typescript-eslint/no-require-imports import installerMigrations = require('./installer-migrations.cjs'); import { isPathConfined } from './external-descriptor-trust.cjs'; @@ -68,8 +74,8 @@ function pruneRetiredRuntimeArtifacts(runtime: string, configDir: string): Clean const destDir = path.resolve(configDir, destSubpath); let entries: fs.Dirent[]; try { - if (!fs.existsSync(destDir) || fs.lstatSync(destDir).isSymbolicLink()) continue; - entries = fs.readdirSync(destDir, { withFileTypes: true }); + if (!installFs().existsSync(destDir) || installFs().lstatSync(destDir).isSymbolicLink()) continue; + entries = installFs().readdirSync(destDir, { withFileTypes: true }); } catch { continue; } @@ -83,7 +89,7 @@ function pruneRetiredRuntimeArtifacts(runtime: string, configDir: string): Clean continue; } try { - fs.unlinkSync(path.join(destDir, entry.name)); + installFs().unlinkSync(path.join(destDir, entry.name)); result.removed.push(relPath); } catch { result.preserved.push(relPath); @@ -91,7 +97,7 @@ function pruneRetiredRuntimeArtifacts(runtime: string, configDir: string): Clean } try { - if (fs.readdirSync(destDir).length === 0) fs.rmdirSync(destDir); + if (installFs().readdirSync(destDir).length === 0) installFs().rmdirSync(destDir); } catch { // Non-empty, unreadable, or concurrently changed: preserve the directory. } diff --git a/src/runtime-artifact-conversion.cts b/src/runtime-artifact-conversion.cts index 36c35ac13..353ba7bba 100644 --- a/src/runtime-artifact-conversion.cts +++ b/src/runtime-artifact-conversion.cts @@ -19,6 +19,15 @@ import path from 'node:path'; import os from 'node:os'; import fs from 'node:fs'; +// #2874 (ADR-58 cleanup phase): route this module's content-rewrite-pass fs +// calls through the installRuntimeArtifacts call tree's injectable seam — +// see install-fs-adapter.cts's module doc. Resolves to real `node:fs` unless +// the top-level installRuntimeArtifacts call injected a `deps.fs`. These +// walkers operate on already-staged temp directories (never the real GSD +// source tree or the real install destination directly), so routing them is +// unconditionally safe. +import installFsAdapter = require('./install-fs-adapter.cjs'); +const { installFs, mkInstallTempDir } = installFsAdapter; import commandRoster = require('./command-roster.cjs'); const { readGsdCommandNames, transformContentToHyphen } = commandRoster; import runtimeNamePolicy = require('./runtime-name-policy.cjs'); @@ -3088,17 +3097,17 @@ function _applyRuntimeRewrites(content, runtime, pathPrefix, isGlobal = false, a * @param attribution Co-Authored-By value (string | null | undefined) */ function applyRuntimeContentRewritesInPlace(stagedDir, runtime, pathPrefix, isGlobal = false, attribution = undefined) { - if (!fs.existsSync(stagedDir)) return; + if (!installFs().existsSync(stagedDir)) return; const walkAndRewrite = (dir) => { - for (const entry of fs.readdirSync(dir, { withFileTypes: true })) { + for (const entry of installFs().readdirSync(dir, { withFileTypes: true })) { const fullPath = path.join(dir, entry.name); if (entry.isDirectory()) { walkAndRewrite(fullPath); } else if (entry.name.endsWith('.md')) { - let content = fs.readFileSync(fullPath, 'utf8'); + let content = installFs().readFileSync(fullPath, 'utf8'); content = _applyRuntimeRewrites(content, runtime, pathPrefix, isGlobal, attribution); - fs.writeFileSync(fullPath, content); + installFs().writeFileSync(fullPath, content); } } }; @@ -3124,13 +3133,13 @@ function applyRuntimeContentRewritesInPlace(stagedDir, runtime, pathPrefix, isGl * @returns {string} path to the temp dir (caller is responsible for cleanup) */ function applyRuntimeContentRewritesForCommandsInPlace(stagedDir, runtime, pathPrefix, isGlobal = false, attribution = undefined) { - if (!fs.existsSync(stagedDir)) return stagedDir; + if (!installFs().existsSync(stagedDir)) return stagedDir; - const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-cmd-rewrites-')); + const tempDir = mkInstallTempDir('gsd-cmd-rewrites-'); try { - for (const entry of fs.readdirSync(stagedDir, { withFileTypes: true })) { + for (const entry of installFs().readdirSync(stagedDir, { withFileTypes: true })) { if (!entry.isFile() || !entry.name.endsWith('.md')) continue; - let content = fs.readFileSync(path.join(stagedDir, entry.name), 'utf8'); + let content = installFs().readFileSync(path.join(stagedDir, entry.name), 'utf8'); content = _applyRuntimeRewrites(content, runtime, pathPrefix, isGlobal, attribution); // #2097 (ADR-1239): descriptor-driven — commandBodyConverter name comes // from runtime.hostBehaviors instead of a hardcoded runtime-name branch. @@ -3138,10 +3147,10 @@ function applyRuntimeContentRewritesForCommandsInPlace(stagedDir, runtime, pathP if (_cmdConv && COMMAND_BODY_CONVERTERS[_cmdConv]) { content = COMMAND_BODY_CONVERTERS[_cmdConv](content); } - fs.writeFileSync(path.join(tempDir, entry.name), content); + installFs().writeFileSync(path.join(tempDir, entry.name), content); } } catch (err) { - try { fs.rmSync(tempDir, { recursive: true, force: true }); } catch { /* best-effort */ } + try { installFs().rmSync(tempDir, { recursive: true, force: true }); } catch { /* best-effort */ } throw err; } return tempDir; @@ -3163,16 +3172,16 @@ function applyRuntimeContentRewritesForCommandsInPlace(stagedDir, runtime, pathP * pass above intact via its own `@`-guarded restore). */ function applySpecRootReferenceToStagedSkills(stagedDir) { - if (!fs.existsSync(stagedDir)) return; + if (!installFs().existsSync(stagedDir)) return; const walk = (dir) => { - for (const entry of fs.readdirSync(dir, { withFileTypes: true })) { + for (const entry of installFs().readdirSync(dir, { withFileTypes: true })) { const fullPath = path.join(dir, entry.name); if (entry.isDirectory()) { walk(fullPath); } else if (entry.name === 'SKILL.md') { - const content = fs.readFileSync(fullPath, 'utf8'); + const content = installFs().readFileSync(fullPath, 'utf8'); const rewritten = resolveSpecRootReference(content); - if (rewritten !== content) fs.writeFileSync(fullPath, rewritten); + if (rewritten !== content) installFs().writeFileSync(fullPath, rewritten); } } }; @@ -3202,7 +3211,7 @@ function rewriteStagedSkillBodies(stagedDir, opts) { platform = process.platform, resolveAttribution, } = opts; - if (!fs.existsSync(stagedDir)) return; + if (!installFs().existsSync(stagedDir)) return; const resolvedTarget = posixNormalize(path.resolve(configDir)); const homeDir = posixNormalize(homedir()); @@ -3254,7 +3263,7 @@ function rewriteStagedCommandBodies(stagedDir, opts) { platform = process.platform, resolveAttribution, } = opts; - if (!fs.existsSync(stagedDir)) return stagedDir; + if (!installFs().existsSync(stagedDir)) return stagedDir; const resolvedTarget = posixNormalize(path.resolve(configDir)); const homeDir = posixNormalize(homedir()); diff --git a/src/runtime-artifact-layout.cts b/src/runtime-artifact-layout.cts index 216aa3f6d..01eb815ad 100644 --- a/src/runtime-artifact-layout.cts +++ b/src/runtime-artifact-layout.cts @@ -16,6 +16,14 @@ import path from 'node:path'; import fs from 'node:fs'; import os from 'node:os'; +// #2874 (ADR-58 cleanup phase): route this module's fs calls through the +// installRuntimeArtifacts call tree's injectable seam — see +// install-fs-adapter.cts's module doc. Resolves to real `node:fs` (the +// `fs` import above stays for type-only references, e.g. `fs.Dirent`) +// unless the top-level installRuntimeArtifacts call injected a `deps.fs`. +// eslint-disable-next-line @typescript-eslint/no-require-imports +import installFsAdapter = require('./install-fs-adapter.cjs'); +const { installFs, mkInstallTempDir } = installFsAdapter; // eslint-disable-next-line @typescript-eslint/no-require-imports import installProfiles = require('./install-profiles.cjs'); const { @@ -124,22 +132,49 @@ interface Layout { * 3. Throw a descriptive error if neither succeeds. */ function findInstallSourceRoot(runtimeConfigDir?: string): string { - // Step 1: marker check + // Step 1: marker check — reads `/.gsd-source`, a path + // under the INSTALL DESTINATION, so this probe goes through the injected + // adapter (installFs()). if (runtimeConfigDir) { const markerPath = path.join(runtimeConfigDir, '.gsd-source'); - if (fs.existsSync(markerPath)) { + if (installFs().existsSync(markerPath)) { try { - const src = fs.readFileSync(markerPath, 'utf8').trim(); - if (src && fs.existsSync(src)) return src; + const src = installFs().readFileSync(markerPath, 'utf8').trim(); + if (src && installFs().existsSync(src)) return src; } catch { /* fall through */ } } } - // Step 2: walk up from __dirname + // Step 2: walk up from __dirname to locate the GSD PACKAGE'S OWN source + // tree (commands/gsd/) — this resolves where the installer's own code is + // running FROM, not anything under the install destination, so it is + // deliberately NOT routed through the injected fs adapter (#2874): a fake + // "destination" adapter has no reason to know about the real package's own + // on-disk layout (an injected adapter's store starts empty and is never + // seeded with real repo paths), and routing it through would make this + // resolution unconditionally throw rather than gracefully staging nothing. + // + // Uses `fs.statSync` in a try/catch rather than `fs.existsSync` — this is + // LOAD-BEARING, not a style choice: tests/executed-plan.test.cjs's F2 cases + // poison every method on the ROUTED fs surface (including `existsSync`, + // since installFs()'s REAL_ADAPTER also calls it) to prove nothing on the + // installRuntimeArtifacts call tree reaches real fs. The F2 "nativePlugin + // runtime: pi" test calls this function (via findInstallSourceRoot()) AFTER + // installing that poison, specifically to resolve the pi nativePlugin + // source path against this repo's own real layout — an operation this + // function must still be able to perform even while `existsSync` is + // poisoned, because this Step 2 walk is real-fs-only by design and was + // never meant to be covered by that poison list. `statSync` is not on the + // poisoned surface, so this probe survives; switching back to `existsSync` + // makes that F2 test throw (verified: reverting this to `existsSync` trips + // the poison and breaks the pi nativePlugin case). let dir = __dirname; for (let i = 0; i < 6; i++) { const candidate = path.join(dir, 'commands', 'gsd'); - if (fs.existsSync(candidate)) return candidate; + try { + fs.statSync(candidate); + return candidate; + } catch { /* not here — keep walking up */ } const parent = path.dirname(dir); if (parent === dir) break; dir = parent; @@ -157,26 +192,33 @@ function findInstallSourceRoot(runtimeConfigDir?: string): string { * 3. Throw a descriptive error if neither succeeds. */ function findAgentsSourceRoot(runtimeConfigDir?: string): string { - // Step 1: marker check + // Step 1: marker check (destination-relative — routed through installFs()). if (runtimeConfigDir) { const markerPath = path.join(runtimeConfigDir, '.gsd-source'); - if (fs.existsSync(markerPath)) { + if (installFs().existsSync(markerPath)) { try { - const src = fs.readFileSync(markerPath, 'utf8').trim(); - if (src && fs.existsSync(src)) { + const src = installFs().readFileSync(markerPath, 'utf8').trim(); + if (src && installFs().existsSync(src)) { // Marker points to commands/gsd; agents/ is a sibling of commands/ const agentsCandidate = path.resolve(path.dirname(src), '..', 'agents'); - if (fs.existsSync(agentsCandidate)) return agentsCandidate; + if (installFs().existsSync(agentsCandidate)) return agentsCandidate; } } catch { /* fall through */ } } } - // Step 2: walk up from __dirname + // Step 2: walk up from __dirname — locates THIS package's own agents/ + // source tree, not the install destination. See findInstallSourceRoot's + // Step 2 comment (#2874) for why this stays unrouted, real-fs-only, and why + // it uses `statSync` rather than `existsSync` (load-bearing against F2's + // poison of the routed fs surface, not a style choice). let dir = __dirname; for (let i = 0; i < 6; i++) { const candidate = path.join(dir, 'agents'); - if (fs.existsSync(candidate)) return candidate; + try { + fs.statSync(candidate); + return candidate; + } catch { /* not here — keep walking up */ } const parent = path.dirname(dir); if (parent === dir) break; dir = parent; @@ -318,28 +360,28 @@ function kimiAgentsKind(destSubpath: string, prefix: string, configDir: string): (content: string) => content, ); const subagents: Array<{ path: string; content: string }> = []; - if (fs.existsSync(stagedAgents)) { - for (const entry of fs.readdirSync(stagedAgents, { withFileTypes: true })) { + if (installFs().existsSync(stagedAgents)) { + for (const entry of installFs().readdirSync(stagedAgents, { withFileTypes: true })) { if (!entry.isFile() || !entry.name.endsWith('.md')) continue; const agentPath = path.join(stagedAgents, entry.name); subagents.push({ path: posixNormalize(path.join('agents', entry.name)), - content: fs.readFileSync(agentPath, 'utf8'), + content: installFs().readFileSync(agentPath, 'utf8'), }); } } const rootAgent = `---\nname: gsd\ndescription: Run GSD workflows in Kimi CLI.\ntools: Agent\n---\n\n# GSD for Kimi CLI\n\nCoordinate installed /skill:gsd-* workflows and route work to generated GSD subagents when a workflow requires an agent handoff.\n`; const artifacts = buildKimiAgentArtifacts({ rootAgent, subagents }); - const stageDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-kimi-agents-')); + const stageDir = mkInstallTempDir('gsd-kimi-agents-'); installProfiles.STAGED_DIRS.add(stageDir); - fs.writeFileSync(path.join(stageDir, 'gsd.yaml'), artifacts.root.yaml); - fs.writeFileSync(path.join(stageDir, 'gsd.md'), artifacts.root.prompt); + installFs().writeFileSync(path.join(stageDir, 'gsd.yaml'), artifacts.root.yaml); + installFs().writeFileSync(path.join(stageDir, 'gsd.md'), artifacts.root.prompt); const subagentsDir = path.join(stageDir, 'subagents'); - fs.mkdirSync(subagentsDir, { recursive: true }); + installFs().mkdirSync(subagentsDir, { recursive: true }); for (const artifact of artifacts.subagents) { - fs.writeFileSync(path.join(subagentsDir, `${artifact.name}.yaml`), artifact.yaml); - fs.writeFileSync(path.join(subagentsDir, `${artifact.name}.md`), artifact.prompt); + installFs().writeFileSync(path.join(subagentsDir, `${artifact.name}.yaml`), artifact.yaml); + installFs().writeFileSync(path.join(subagentsDir, `${artifact.name}.md`), artifact.prompt); } return stageDir; }, diff --git a/tests/executed-plan.test.cjs b/tests/executed-plan.test.cjs new file mode 100644 index 000000000..650c8f37f --- /dev/null +++ b/tests/executed-plan.test.cjs @@ -0,0 +1,1285 @@ +'use strict'; + +/** + * Executed-plan return + fs adapter seam — failing-first tests. + * + * #2874 (epic #2866 Phase 5), governed by ADR-58 + * (docs/adr/58-runtime-install-policy-module.md). + * + * Design: .gsd/phase/feat-2874-executed-plan-return/40-design.md + * Test matrix: .gsd/phase/feat-2874-executed-plan-return/50-test-matrix.md + * + * This file implements the Red-first order's rows 1-3 from 50-test-matrix.md: + * - E3 (section E, "Executed-plan return shape"): the opencode-family + * early return must ALSO return an executed plan, not `undefined`. + * - E13 (section E): every runtime in the capability registry must return + * something other than `undefined` — the completeness sweep proving the + * contract has no per-runtime holes. + * - F2 (section F, "Fs adapter seam"): a full install driven by an + * injected fake adapter must touch zero real filesystem paths. + * + * All three are RED against the current tree: `installRuntimeArtifacts` + * (src/install-engine.cts:750) still returns `void` and accepts no `deps`/ + * adapter parameter to route IO through. No production code is touched here + * — this package is tests only. + */ + +process.env.GSD_TEST_MODE = '1'; + +const { test, describe } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const os = require('node:os'); +const path = require('node:path'); +const crypto = require('node:crypto'); + +const fc = require('fast-check'); + +const { createTempDir, cleanup } = require('./helpers.cjs'); + +const { installRuntimeArtifacts, hasExistingSymlinkBetween } = require('../gsd-core/bin/lib/install-engine.cjs'); +const registry = require('../gsd-core/bin/lib/capability-registry.cjs'); +const { loadSkillsManifest, resolveProfile } = require('../gsd-core/bin/lib/install-profiles.cjs'); +const runtimeArtifactLayout = require('../gsd-core/bin/lib/runtime-artifact-layout.cjs'); +const runtimeArtifactInstallPlan = require('../gsd-core/bin/lib/runtime-artifact-install-plan.cjs'); +const { withInstallFs } = require('../gsd-core/bin/lib/install-fs-adapter.cjs'); +const commandRoster = require('../gsd-core/bin/lib/command-roster.cjs'); +const slashCommandTransformer = require('../scripts/fix-slash-commands.cjs'); + +const REAL_COMMANDS_DIR = path.join(__dirname, '..', 'commands', 'gsd'); +const MANIFEST = loadSkillsManifest(REAL_COMMANDS_DIR); +const RESOLVED_CORE = resolveProfile({ modes: ['core'], manifest: MANIFEST }); +const RESOLVED_FULL = resolveProfile({ modes: ['full'], manifest: MANIFEST }); +const TEST_ATTRIBUTION = () => 'Co-Authored-By: Test '; + +/** + * Sandbox HOME/USERPROFILE for the duration of a test. Some runtimes (e.g. + * codex) resolve a kind's `home` via os.homedir(); without this, an + * in-process install would write into the developer's real home directory. + * Mirrors tests/install-runtime-artifacts.test.cjs's sandboxHome(). + */ +function sandboxHome(t, dir) { + const savedHome = process.env.HOME; + const savedUserProfile = process.env.USERPROFILE; + process.env.HOME = dir; + process.env.USERPROFILE = dir; + t.after(() => { + if (savedHome === undefined) delete process.env.HOME; + else process.env.HOME = savedHome; + if (savedUserProfile === undefined) delete process.env.USERPROFILE; + else process.env.USERPROFILE = savedUserProfile; + }); +} + +// ─── E3 — the opencode-family early return (matrix row E3) ────────────────── + +describe('installRuntimeArtifacts — E3: opencode-family early return', () => { + test('family install still returns a plan', (t) => { + const configDir = createTempDir('gsd-e3-opencode-'); + t.after(() => cleanup(configDir)); + sandboxHome(t, configDir); + + const result = installRuntimeArtifacts('opencode', configDir, 'global', RESOLVED_CORE); + + assert.notStrictEqual( + result, + undefined, + 'E3: the combinedFamilyInstall early return (install-engine.cts:774) must return an ' + + 'executed plan, not undefined — a whole runtime family returning undefined is a hole ' + + 'in the contract, not an exemption (40-design.md behavior table row 2)', + ); + }); +}); + +// ─── E13 — the all-runtimes sweep (matrix row E13) ─────────────────────────── + +describe('installRuntimeArtifacts — E13: no runtime returns undefined', () => { + const RUNTIMES = Object.keys(registry.runtimes); + + test('registry enumerates at least one runtime to sweep', () => { + assert.ok(RUNTIMES.length > 0, 'capability-registry.cjs runtimes must be non-empty'); + }); + + for (const runtime of RUNTIMES) { + test(`${runtime}: installRuntimeArtifacts does not return undefined`, (t) => { + const configDir = createTempDir(`gsd-e13-${runtime}-`); + t.after(() => cleanup(configDir)); + sandboxHome(t, configDir); + + const result = installRuntimeArtifacts(runtime, configDir, 'global', RESOLVED_CORE); + + assert.notStrictEqual( + result, + undefined, + `E13: ${runtime} returned undefined — every runtime in the registry must return an ` + + 'executed plan (40-design.md: "Legitimate undefined returns: none after this phase. ' + + 'If any path can still return undefined, that path is a defect, not an exemption.")', + ); + }); + } +}); + +// ─── F2 — zero real filesystem contact (matrix row F2) ─────────────────────── + +// The full write+read surface installRuntimeArtifacts's call tree is known to +// reach once every gap named in the #2874 follow-up round is closed: the +// direct mkdirSync/existsSync/rmSync calls, _copyStaged's readdirSync/cpSync/ +// copyFileSync/mkdirSync, _removeGsdEntries's directory scan+delete, +// _snapshotDir/_restoreDir's read/write of preserved skill dirs, the +// symlink-escape guard's lstatSync/realpathSync probes, commonjs-marker.cts's +// lstatSync/writeFileSync/unlinkSync, and installer-migrations.cts's +// existsSync/readFileSync/openSync/readSync/closeSync (readInstallManifest, +// classifyArtifact, sha256File — sha256File streams via openSync/readSync/ +// closeSync, restored after a brief round-trip through readFileSync broke +// tests/installer-migrations.test.cjs's large-file-streaming contract; the +// fake below implements all three against its store so a fake-adapter +// install still never touches real fs for hashing). +// +// mkdtempSync stays poisoned as a genuine tripwire, not a reachable case: +// mkInstallTempDir (install-fs-adapter.cts) only ever calls real +// `fs.mkdtempSync` when `current === REAL_ADAPTER` (no adapter injected at +// all) — a fake-adapter call always makes `current` a distinct merged +// object, so it takes the synthesize-name-and-mkdirSync branch instead and +// never reaches this poison. If this ever fires, `current`'s identity check +// broke, not a documented gap. +// +// One exception this poison list does NOT cover: readGsdCommandNames +// (command-roster.cts) reads the PACKAGE'S OWN commands/gsd/ source tree via +// real fs.readdirSync — deliberately unrouted (see install-fs-adapter.cts's +// module doc, "DELIBERATELY NOT ROUTED"). `poisonRealFsAgainstDestination` +// below allows real calls scoped to that known package-source root and +// poisons everything else, rather than poisoning every real fs call +// wholesale regardless of path. +const REAL_FS_WRITE_SURFACE = [ + 'mkdirSync', 'existsSync', 'rmSync', 'readdirSync', + 'cpSync', 'copyFileSync', 'readFileSync', 'writeFileSync', 'lstatSync', + 'realpathSync', 'unlinkSync', 'rmdirSync', + 'mkdtempSync', 'openSync', 'readSync', 'closeSync', +]; + +// Package-source roots a correct install is expected to read for real, even +// while a fake DESTINATION adapter is injected (40-design.md "Known limits": +// this seam makes destination IO fake-able; package-source IO stays real by +// design). Mirrors findInstallSourceRoot's/findAgentsSourceRoot's/ +// readGsdCommandNames's own targets (commands/gsd/, agents/), all resolved +// the same way REAL_COMMANDS_DIR is above. +const PACKAGE_SOURCE_ROOTS = [REAL_COMMANDS_DIR, path.join(__dirname, '..', 'agents')]; + +function isPackageSourcePath(resolvedPath) { + return PACKAGE_SOURCE_ROOTS.some( + (root) => resolvedPath === root || resolvedPath.startsWith(root + path.sep), + ); +} + +/** + * F2's real-fs poisoning, derived from the rule (40-design.md "Known + * limits"/install-fs-adapter.cts's module doc) rather than aligned with it by + * coincidence: a real fs call against the install DESTINATION is a failure — + * the seam exists precisely so a fake adapter can intercept those — but a + * real call against the package's OWN source tree (commands/gsd/, agents/) + * is expected and allowed, because that read is deliberately unrouted + * (readGsdCommandNames et al.). Poisoning every real fs method wholesale, + * regardless of path, makes a correct install fail for the wrong reason. + * + * Returns a Map of package-source hits, so a caller can + * assert POSITIVELY that the expected package-source read actually happened + * — proving the boundary was exercised, not merely tolerated. + */ +function poisonRealFsAgainstDestination(t, label) { + const packageSourceHits = new Map(); + for (const method of REAL_FS_WRITE_SURFACE) { + const original = fs[method].bind(fs); + t.mock.method(fs, method, (...args) => { + const target = args[0]; + const resolved = (typeof target === 'string' || target instanceof URL || Buffer.isBuffer(target)) + ? path.resolve(String(target)) + : null; + if (resolved !== null && isPackageSourcePath(resolved)) { + packageSourceHits.set(method, (packageSourceHits.get(method) ?? 0) + 1); + return original(...args); + } + throw new Error( + `F2${label}: real fs.${method}() was reached against a non-package-source path ` + + `(${resolved ?? String(target)}) during an install driven by an injected fake adapter`, + ); + }); + } + return packageSourceHits; +} + +/** + * A genuinely functional in-memory filesystem, not a set of no-op stubs — + * required to drive the branches F2 now exercises (opencode-family legacy-dir + * migration, a nativePlugin runtime, a retiredArtifacts runtime) far enough + * to reach commonjs-marker.cts and installer-migrations.cts's routed + * classifyArtifact/readInstallManifest, not just the happy path's first + * existsSync check. Every method operates against one flat `Map` store; `readdirSync` derives listings by prefix-scanning the same + * store (an entry that readdirSync reports a directory contains is, by + * construction, also existsSync-true at that exact path — same invariant a + * real filesystem holds). + * + * @param seed - Array<[absPath, {type:'file'|'dir', content?:string|Buffer}]> + * pre-populated entries. + */ +function createFakeInstallFs(seed = []) { + const store = new Map(); + for (const [p, entry] of seed) store.set(path.normalize(String(p)), entry); + const fdTable = new Map(); + let nextFd = 1; + + const norm = (p) => path.normalize(String(p)); + const childPrefix = (dir) => { + const n = norm(dir); + return n.endsWith(path.sep) ? n : n + path.sep; + }; + const enoent = (p) => { + const err = new Error(`ENOENT: no such file or directory, '${p}'`); + err.code = 'ENOENT'; + return err; + }; + + const fakeFs = { + existsSync: (p) => store.has(norm(p)), + lstatSync: (p) => { + const e = store.get(norm(p)); + if (!e) throw enoent(p); + return { + isFile: () => e.type === 'file', + isDirectory: () => e.type === 'dir', + isSymbolicLink: () => e.type === 'symlink', + }; + }, + mkdirSync: (p) => { store.set(norm(p), { type: 'dir' }); return undefined; }, + rmSync: (p) => { + const n = norm(p); + store.delete(n); + const prefix = childPrefix(n); + for (const k of [...store.keys()]) if (k.startsWith(prefix)) store.delete(k); + }, + unlinkSync: (p) => { + const n = norm(p); + if (!store.has(n)) throw enoent(p); + store.delete(n); + }, + rmdirSync: (p) => { store.delete(norm(p)); }, + readdirSync: (p, opts) => { + const prefix = childPrefix(p); + const names = new Set(); + for (const k of store.keys()) { + if (!k.startsWith(prefix)) continue; + const rest = k.slice(prefix.length); + const sepIdx = rest.indexOf(path.sep); + const name = sepIdx === -1 ? rest : rest.slice(0, sepIdx); + if (name) names.add(name); + } + const arr = [...names]; + if (opts && opts.withFileTypes) { + return arr.map((name) => { + const full = norm(path.join(String(p), name)); + const e = store.get(full); + return { + name, + isFile: () => (e ? e.type === 'file' : false), + isDirectory: () => (e ? e.type === 'dir' : true), + }; + }); + } + return arr; + }, + readFileSync: (p, encoding) => { + const e = store.get(norm(p)); + if (!e || e.type !== 'file') throw enoent(p); + const buf = Buffer.isBuffer(e.content) ? e.content : Buffer.from(e.content ?? '', 'utf8'); + return encoding ? buf.toString(encoding) : buf; + }, + // sha256File (installer-migrations.cts) streams via openSync/readSync/ + // closeSync instead of readFileSync (large-file hashing must not buffer + // the whole file — tests/installer-migrations.test.cjs pins this). fdTable + // maps a synthetic fd to {buf, pos} so this fake never needs a real fd. + openSync: (p) => { + const e = store.get(norm(p)); + if (!e || e.type !== 'file') throw enoent(p); + const buf = Buffer.isBuffer(e.content) ? e.content : Buffer.from(e.content ?? '', 'utf8'); + const fd = nextFd++; + fdTable.set(fd, { buf, pos: 0 }); + return fd; + }, + readSync: (fd, buffer, offset, length, position) => { + const entry = fdTable.get(fd); + if (!entry) { + const err = new Error(`EBADF: bad file descriptor, read (fake fd ${fd})`); + err.code = 'EBADF'; + throw err; + } + const readAt = position === null || position === undefined ? entry.pos : position; + const bytesToRead = Math.max(0, Math.min(length, entry.buf.length - readAt)); + entry.buf.copy(buffer, offset, readAt, readAt + bytesToRead); + if (position === null || position === undefined) entry.pos += bytesToRead; + return bytesToRead; + }, + closeSync: (fd) => { fdTable.delete(fd); }, + writeFileSync: (p, data, opts) => { + // Emulate `{ flag: 'wx' }` (exclusive create): REAL_ADAPTER.writeFileSync + // (install-fs-adapter.cts:138) passes `opts` straight through to real + // `fs.writeFileSync`, which throws EEXIST for `wx` against an existing + // path. A fake that silently overwrote here would certify something + // the real implementation refuses — see commonjs-marker.cts's + // `ensureCommonJsMarker`, which relies on `wx` to close the + // classify-then-write gap. + const flag = typeof opts === 'object' && opts !== null ? opts.flag : undefined; + const n = norm(p); + if (flag === 'wx' && store.has(n)) { + const err = new Error(`EEXIST: file already exists, open '${p}'`); + err.code = 'EEXIST'; + throw err; + } + store.set(n, { type: 'file', content: data }); + }, + copyFileSync: (src, dest) => { + const e = store.get(norm(src)); + store.set(norm(dest), { type: 'file', content: e ? e.content : Buffer.alloc(0) }); + }, + cpSync: (src, dest) => { + const sn = norm(src); + const dn = norm(dest); + const e = store.get(sn); + if (e) store.set(dn, { ...e }); + const prefix = childPrefix(sn); + for (const [k, v] of [...store.entries()]) { + if (k.startsWith(prefix)) store.set(dn + k.slice(sn.length), { ...v }); + } + }, + realpathSync: (p) => norm(p), + }; + fakeFs._store = store; + return fakeFs; +} + +// ─── createFakeInstallFs — wx exclusive-create emulation ──────────────────── +// +// REAL_ADAPTER.writeFileSync (install-fs-adapter.cts:138) passes `opts` +// through untouched to real fs.writeFileSync, so `{ flag: 'wx' }` throws +// EEXIST against an existing target (commonjs-marker.cts's +// ensureCommonJsMarker relies on exactly this to close the +// classify-then-write TOCTOU gap). A fake that ignored `opts` would silently +// overwrite where the real adapter refuses — this covers the emulation +// itself rather than assuming it. +describe('createFakeInstallFs — wx exclusive-create emulation', () => { + test('refuses an exclusive create against an existing path (EEXIST)', () => { + const target = path.join(os.tmpdir(), 'gsd-fake-wx-existing.txt'); + const fakeFs = createFakeInstallFs([[target, { type: 'file', content: 'original' }]]); + + assert.throws( + () => fakeFs.writeFileSync(target, 'clobber', { flag: 'wx' }), + (err) => err.code === 'EEXIST', + 'wx write against an existing fake-store path must throw EEXIST, matching real fs.writeFileSync', + ); + assert.strictEqual( + fakeFs.readFileSync(target, 'utf8'), + 'original', + 'a refused wx write must leave the existing content untouched', + ); + }); + + test('allows an exclusive create against an absent path', () => { + const target = path.join(os.tmpdir(), 'gsd-fake-wx-absent.txt'); + const fakeFs = createFakeInstallFs(); + + fakeFs.writeFileSync(target, 'created', { flag: 'wx' }); + + assert.strictEqual(fakeFs.readFileSync(target, 'utf8'), 'created'); + }); +}); + +/** sha256 hex digest matching installer-migrations.cts's sha256File — used to + * seed a manifest entry that classifies a fake file as 'managed-pristine'. */ +function sha256Hex(content) { + return crypto.createHash('sha256').update(content).digest('hex'); +} + +describe('installRuntimeArtifacts — F2: fake-adapter install touches no real filesystem', () => { + test('fake-adapter install touches no real filesystem (claude, skills-only)', (t) => { + // Every real fs method this call tree could reach is poisoned BY PATH + // (see poisonRealFsAgainstDestination) for the duration of this test via + // node:test's mock tracker (auto-restored when the test ends — no + // try/finally in the test body, per CONTRIBUTING.md's "Never use + // try/finally inside test bodies"). + const packageSourceHits = poisonRealFsAgainstDestination(t, ''); + + const fakeFs = createFakeInstallFs(); + + // configDir deliberately never created for real — F2 asserts nothing + // real ever gets written under it. + const configDir = path.join(os.tmpdir(), `gsd-f2-must-not-exist-${crypto.randomUUID()}`); + + const result = installRuntimeArtifacts( + 'claude', configDir, 'global', RESOLVED_CORE, undefined, undefined, + { fs: fakeFs }, + ); + + assert.notStrictEqual( + result, + undefined, + 'F2: a fake-adapter install must still return an executed plan (matrix row F1/E1 shape)', + ); + // No post-hoc fs.existsSync(configDir) check follows: fs.existsSync is + // one of the poisoned (non-package-source) methods above for the + // duration of this test, so the proof of "zero real DESTINATION fs + // contact" IS that installRuntimeArtifacts returned at all without + // tripping one of the throws — not a probe that would itself have to + // touch the poisoned surface. + assert.ok( + (packageSourceHits.get('readdirSync') ?? 0) > 0, + 'F2: readGsdCommandNames must have read the real, unrouted commands/gsd/ package-source ' + + 'tree at least once — proving the poison boundary was exercised, not merely tolerated', + ); + }); + + test('fake-adapter install touches no real filesystem (opencode-family legacy command/ dir migration)', (t) => { + poisonRealFsAgainstDestination(t, ' (opencode legacy migration)'); + + const configDir = path.join(os.tmpdir(), `gsd-f2-opencode-legacy-${crypto.randomUUID()}`); + const legacyDir = path.join(configDir, 'command'); + const legacyFile = path.join(legacyDir, 'gsd-old-cmd.md'); + const content = '# stale legacy command\n'; + const manifestPath = path.join(configDir, 'gsd-file-manifest.json'); + const manifestJson = JSON.stringify({ files: { 'command/gsd-old-cmd.md': sha256Hex(content) } }); + + const fakeFs = createFakeInstallFs([ + [configDir, { type: 'dir' }], + [legacyDir, { type: 'dir' }], + [legacyFile, { type: 'file', content }], + [manifestPath, { type: 'file', content: manifestJson }], + ]); + + const result = installRuntimeArtifacts( + 'opencode', configDir, 'global', RESOLVED_CORE, undefined, undefined, + { fs: fakeFs }, + ); + + assert.notStrictEqual(result, undefined, 'F2 (opencode legacy migration): must still return a plan'); + // The manifest hash matches the seeded content exactly, so + // _migrateLegacyOpencodeCommandDir's classifyArtifact call must have + // classified it 'managed-pristine' and unlinked it (real + // installerMigrations.readInstallManifest/classifyArtifact/sha256File — + // all routed through installFs() — computed this via the fake, not real + // fs, or the poisoned methods above would have thrown first). + assert.strictEqual( + fakeFs._store.has(path.normalize(legacyFile)), + false, + 'F2 (opencode legacy migration): the managed-pristine legacy file must have been removed via the fake store', + ); + }); + + test('fake-adapter install touches no real filesystem (nativePlugin runtime: pi)', (t) => { + poisonRealFsAgainstDestination(t, ' (nativePlugin)'); + + // Resolve the SAME pluginSrc path _installNativePluginIfDeclared + // (install-engine.cts) computes for pi's declared nativePlugin, using the + // real (unrouted, package-own-source) findInstallSourceRoot — this read + // happens BEFORE the poison mocks above are installed... no: it must + // happen before `t.mock.method` calls would matter for IT, but + // findInstallSourceRoot's own walk uses `fs.statSync`, which is NOT on + // the poisoned list (see install-fs-adapter.cts's module doc — it is + // deliberately unrouted, real-fs-only, package-source introspection), so + // resolving this here is safe even after poisoning existsSync et al. + const commandsGsdDir = runtimeArtifactLayout.findInstallSourceRoot(); + const repoRoot = path.dirname(path.dirname(commandsGsdDir)); + const nativePlugin = registry.runtimes.pi.runtime.hostBehaviors.nativePlugin; + assert.ok(nativePlugin && nativePlugin.source, 'pi must declare hostBehaviors.nativePlugin.source (registry drifted)'); + const pluginSrc = path.join(repoRoot, nativePlugin.source); + + const configDir = path.join(os.tmpdir(), `gsd-f2-pi-nativeplugin-${crypto.randomUUID()}`); + const fakeFs = createFakeInstallFs([ + [pluginSrc, { type: 'file', content: '// fake plugin adapter\n' }], + ]); + + const result = installRuntimeArtifacts( + 'pi', configDir, 'global', RESOLVED_CORE, undefined, undefined, + { fs: fakeFs }, + ); + + assert.notStrictEqual(result, undefined, 'F2 (nativePlugin): must still return a plan'); + assert.strictEqual(result.postSteps.nativePlugin, true, 'F2 (nativePlugin): postSteps.nativePlugin must be true for pi'); + const destPath = path.join(configDir, nativePlugin.dir, nativePlugin.file); + assert.strictEqual( + fakeFs._store.has(path.normalize(destPath)), + true, + 'F2 (nativePlugin): the plugin file must have been copied via the fake store (copyFileSync routed)', + ); + const markerPath = path.join(configDir, nativePlugin.dir, 'package.json'); + assert.strictEqual( + fakeFs._store.has(path.normalize(markerPath)), + true, + 'F2 (nativePlugin): ensureCommonJsMarker (commonjs-marker.cts) must have written the CommonJS marker via the fake store', + ); + }); + + test('fake-adapter install touches no real filesystem (retiredArtifacts runtime: cursor)', (t) => { + const packageSourceHits = poisonRealFsAgainstDestination(t, ' (retiredArtifacts)'); + + const retired = registry.runtimes.cursor.runtime.hostBehaviors.retiredArtifacts; + assert.ok(Array.isArray(retired) && retired.length > 0, 'cursor must declare hostBehaviors.retiredArtifacts (registry drifted)'); + const { destSubpath, prefix, suffix } = retired[0]; + + const configDir = path.join(os.tmpdir(), `gsd-f2-cursor-retired-${crypto.randomUUID()}`); + const destDir = path.resolve(configDir, destSubpath); + const staleName = `${prefix}retired-probe${suffix}`; + const staleFile = path.join(destDir, staleName); + const content = '# stale retired artifact\n'; + const relPath = `${destSubpath.replace(/\\/g, '/')}/${staleName}`; + const manifestPath = path.join(configDir, 'gsd-file-manifest.json'); + const manifestJson = JSON.stringify({ files: { [relPath]: sha256Hex(content) } }); + + const fakeFs = createFakeInstallFs([ + [configDir, { type: 'dir' }], + [destDir, { type: 'dir' }], + [staleFile, { type: 'file', content }], + [manifestPath, { type: 'file', content: manifestJson }], + ]); + + const result = installRuntimeArtifacts( + 'cursor', configDir, 'global', RESOLVED_CORE, undefined, undefined, + { fs: fakeFs }, + ); + + assert.notStrictEqual(result, undefined, 'F2 (retiredArtifacts): must still return a plan'); + // manifest hash matches the seeded content exactly -> classifyArtifact + // must classify 'managed-pristine' -> pruneRetiredRuntimeArtifacts + // (retired-artifact-cleanup.cts, routed) unlinks it via the fake store. + assert.strictEqual( + fakeFs._store.has(path.normalize(staleFile)), + false, + 'F2 (retiredArtifacts): the managed-pristine retired artifact must have been removed via the fake store', + ); + assert.ok( + (packageSourceHits.get('readdirSync') ?? 0) > 0, + 'F2 (retiredArtifacts): readGsdCommandNames must have read the real, unrouted commands/gsd/ ' + + 'package-source tree at least once — proving the poison boundary was exercised, not merely tolerated', + ); + }); +}); + +// ═══════════════════════════════════════════════════════════════════════════ +// #2874 follow-up round — 50-test-matrix.md rows E1/E2/E4-E12, F4-F6, G2, +// H1-H5, I1-I5, K3, L1-L2. Extends the F2/E3/E13 coverage above rather than a +// new file (install's file-count prefix is grandfathered at 8, must not grow). +// ═══════════════════════════════════════════════════════════════════════════ + +// ─── E. Executed-plan return shape (E1, E2, E4-E12) ────────────────────────── + +describe('installRuntimeArtifacts — E1: claude global, normal install', () => { + test('returns an executed plan for a normal install', (t) => { + const configDir = createTempDir('gsd-e1-claude-global-'); + t.after(() => cleanup(configDir)); + sandboxHome(t, configDir); + + const result = installRuntimeArtifacts('claude', configDir, 'global', RESOLVED_CORE); + + assert.ok(Array.isArray(result.kinds) && result.kinds.length > 0, 'E1: plan must name at least one kind'); + for (const k of result.kinds) { + assert.strictEqual(typeof k.kind, 'string', 'E1: every kind entry must name its kind'); + assert.strictEqual(typeof k.sourceDir, 'string', 'E1: every kind entry must name its sourceDir'); + assert.strictEqual(typeof k.destDir, 'string', 'E1: every kind entry must name its destDir'); + } + }); +}); + +describe('installRuntimeArtifacts — E2: claude local', () => { + test('executed plan records the scope', (t) => { + const configDir = createTempDir('gsd-e2-claude-local-'); + t.after(() => cleanup(configDir)); + sandboxHome(t, configDir); + + const result = installRuntimeArtifacts('claude', configDir, 'local', RESOLVED_CORE); + + assert.strictEqual(result.scope, 'local', 'E2: local scope must be reflected verbatim on the returned plan'); + }); +}); + +describe('installRuntimeArtifacts — E4: kilo (second family member)', () => { + test('kilo family install still returns a plan', (t) => { + const configDir = createTempDir('gsd-e4-kilo-'); + t.after(() => cleanup(configDir)); + sandboxHome(t, configDir); + + const result = installRuntimeArtifacts('kilo', configDir, 'global', RESOLVED_CORE); + + assert.notStrictEqual( + result, undefined, + 'E4: kilo, the SECOND combined-family runtime, must ALSO return a plan — E3 is not a one-runtime special case', + ); + assert.deepStrictEqual(result.kinds.map((k) => k.kind).sort(), ['commands', 'skills']); + }); +}); + +describe('installRuntimeArtifacts — E5/E7: empty layout + nativePlugin post-step (pi)', () => { + test('empty layout returns an empty plan', (t) => { + const configDir = createTempDir('gsd-e5-pi-'); + t.after(() => cleanup(configDir)); + sandboxHome(t, configDir); + + const result = installRuntimeArtifacts('pi', configDir, 'global', RESOLVED_CORE); + + assert.ok(Array.isArray(result.kinds), 'E5: kinds must be an array even when layout.kinds is empty'); + assert.strictEqual(result.kinds.length, 0, 'E5: pi declares an empty artifactLayout — kinds must be [], never undefined'); + }); + + test('native plugin post-step is recorded', (t) => { + const configDir = createTempDir('gsd-e7-pi-'); + t.after(() => cleanup(configDir)); + sandboxHome(t, configDir); + + const result = installRuntimeArtifacts('pi', configDir, 'global', RESOLVED_CORE); + + assert.strictEqual( + result.postSteps.nativePlugin, true, + 'E7: pi declares hostBehaviors.nativePlugin — postSteps.nativePlugin must record it as a post-step', + ); + }); +}); + +describe('installRuntimeArtifacts — E6: hermes post-step is recorded', () => { + test('hermes post-step is recorded', (t) => { + const configDir = createTempDir('gsd-e6-hermes-'); + t.after(() => cleanup(configDir)); + sandboxHome(t, configDir); + + const result = installRuntimeArtifacts('hermes', configDir, 'global', RESOLVED_CORE); + + assert.strictEqual( + result.postSteps.hermesBareStemCleanup, true, + 'E6: hermes must record _removeHermesBareStemDirs having run as a post-step', + ); + }); +}); + +describe('installRuntimeArtifacts — E8: preserved user skill dirs are recorded', () => { + test('preserved user skill dirs are recorded', (t) => { + const configDir = createTempDir('gsd-e8-claude-'); + t.after(() => cleanup(configDir)); + sandboxHome(t, configDir); + const preservedSkillDir = path.join(configDir, 'skills', 'gsd-dev-preferences'); + fs.mkdirSync(preservedSkillDir, { recursive: true }); + fs.writeFileSync(path.join(preservedSkillDir, 'SKILL.md'), '# my custom prefs\n'); + + const result = installRuntimeArtifacts('claude', configDir, 'global', RESOLVED_CORE); + + const skillsKind = result.kinds.find((k) => k.kind === 'skills'); + assert.ok(skillsKind, 'E8 precondition: claude global must write a skills kind'); + assert.deepStrictEqual( + skillsKind.preserved, ['gsd-dev-preferences'], + 'E8: the plan must record gsd-dev-preferences as preserved', + ); + assert.strictEqual( + fs.readFileSync(path.join(preservedSkillDir, 'SKILL.md'), 'utf8'), + '# my custom prefs\n', + 'E8: the preserved content must actually have been restored after the prune+copy, not just recorded on the plan', + ); + }); +}); + +describe('installRuntimeArtifacts — E9: non-skills kind records its writes', () => { + test('non-skills kind records its writes', (t) => { + const configDir = createTempDir('gsd-e9-claude-local-'); + t.after(() => cleanup(configDir)); + sandboxHome(t, configDir); + + const result = installRuntimeArtifacts('claude', configDir, 'local', RESOLVED_CORE); + + const commandsKind = result.kinds.find((k) => k.kind === 'commands'); + assert.ok(commandsKind, 'E9 precondition: claude local must write a commands kind'); + assert.strictEqual(commandsKind.destDir, path.join(configDir, 'commands')); + assert.ok( + fs.existsSync(commandsKind.destDir) && fs.readdirSync(commandsKind.destDir).length > 0, + 'E9: the destDir the plan records must actually contain the copied files', + ); + }); +}); + +describe('installRuntimeArtifacts — E10: plan item naming a kind absent from layout.kinds', () => { + test('unknown kind still throws', (t) => { + const configDir = createTempDir('gsd-e10-claude-'); + t.after(() => cleanup(configDir)); + sandboxHome(t, configDir); + + const original = runtimeArtifactInstallPlan.createRuntimeArtifactInstallPlan; + t.after(() => { runtimeArtifactInstallPlan.createRuntimeArtifactInstallPlan = original; }); + // Module-ref monkeypatch (same pattern as + // tests/runtime-artifact-layout-surface.test.cjs) — install-engine.cts + // reads this via the module reference, not a destructured local, so + // reassigning the export is observed at call time. + runtimeArtifactInstallPlan.createRuntimeArtifactInstallPlan = (args) => { + const real = original(args); + if (!real.ok) return real; + return { + ok: true, + plan: { + items: [...real.plan.items, { kind: 'not-a-real-kind', sourceDir: configDir, destDir: configDir }], + cleanupDirs: real.plan.cleanupDirs, + }, + }; + }; + + assert.throws( + () => installRuntimeArtifacts('claude', configDir, 'global', RESOLVED_CORE), + /unknown artifact kind/i, + 'E10: a plan item naming a kind absent from layout.kinds must still throw "unknown artifact kind"', + ); + }); +}); + +describe('installRuntimeArtifacts — E11: plan is not shared across calls', () => { + test('plan is not shared across calls', (t) => { + const configDir = createTempDir('gsd-e11-claude-'); + t.after(() => cleanup(configDir)); + sandboxHome(t, configDir); + + const first = installRuntimeArtifacts('claude', configDir, 'global', RESOLVED_CORE); + first.kinds.push({ kind: 'mutated-by-caller', sourceDir: 'x', destDir: 'y', preserved: [] }); + first.postSteps.mutatedFlag = true; + + const second = installRuntimeArtifacts('claude', configDir, 'global', RESOLVED_CORE); + + assert.notStrictEqual(second, first, 'E11: each call must return a fresh object, not the same reference'); + assert.notStrictEqual(second.kinds, first.kinds, 'E11: kinds array must not be shared across calls'); + assert.ok( + !second.kinds.some((k) => k.kind === 'mutated-by-caller'), + 'E11: mutating the first result must not leak into the second call\'s plan', + ); + assert.strictEqual( + second.postSteps.mutatedFlag, undefined, + 'E11: mutating the first result\'s postSteps must not leak into the second call', + ); + }); +}); + +describe('installRuntimeArtifacts — E12: executed plan key set is locked', () => { + test('executed plan key set is locked', (t) => { + const configDir = createTempDir('gsd-e12-claude-'); + t.after(() => cleanup(configDir)); + sandboxHome(t, configDir); + + const result = installRuntimeArtifacts('claude', configDir, 'global', RESOLVED_CORE); + + assert.deepStrictEqual( + Object.keys(result).sort(), + ['cleanup', 'kinds', 'postSteps', 'runtime', 'scope'], + 'E12: the executed-plan top-level key set is a locked contract — an added/renamed/removed key ' + + 'here is a breaking change to AC1/AC4 and must be a deliberate, reviewed decision, not an ' + + 'incidental refactor', + ); + }); +}); + +// ─── F. Fs adapter seam — F4-F6 ─────────────────────────────────────────────── + +describe('installRuntimeArtifacts — F4: adapter errors propagate, cleanup still runs', () => { + test('adapter errors propagate, cleanup still runs', (t) => { + const configDir = createTempDir('gsd-f4-augment-'); + t.after(() => cleanup(configDir)); + sandboxHome(t, configDir); + + let capturedCleanupDir; + const fakeFs = { + writeFileSync: (p, data, opts) => { + if (String(p).includes('gsd-cmd-rewrites-') && capturedCleanupDir === undefined) { + capturedCleanupDir = path.dirname(p); + } + fs.writeFileSync(p, data, opts); + }, + copyFileSync: (src, dest) => { + const err = new Error(`EACCES: permission denied, copyfile '${src}' -> '${dest}'`); + err.code = 'EACCES'; + throw err; + }, + }; + + assert.throws( + () => installRuntimeArtifacts('augment', configDir, 'global', RESOLVED_FULL, TEST_ATTRIBUTION, undefined, { fs: fakeFs }), + (err) => err.code === 'EACCES', + 'F4: an EACCES from the injected adapter mid-copy must propagate to the caller unchanged, exactly as a real EACCES would today', + ); + assert.ok(capturedCleanupDir, 'F4 test precondition: the commands kind rewrite must have run before the copy failure'); + assert.strictEqual( + fs.existsSync(capturedCleanupDir), false, + 'F4: cleanup must still run (the finally block) even though the copy step threw', + ); + }); +}); + +describe('installRuntimeArtifacts — F5: fake existsSync drives the same branch', () => { + test('fake existsSync drives the same branch', () => { + const configDir = path.join(os.tmpdir(), `gsd-f5-must-not-exist-${crypto.randomUUID()}`); + const skillsDest = path.join(configDir, 'skills'); + // Seed ONLY the skills destDir as a pre-existing (empty) directory in the + // fake store — configDir is never created for real, so existsSync(dest) + // reports true purely because the FAKE says so, driving the exact same + // `kind.kind === 'skills' && installFs().existsSync(dest)` pre-existing- + // dest branch a real pre-existing dir would take. + const fakeFs = createFakeInstallFs([[skillsDest, { type: 'dir' }]]); + + const result = installRuntimeArtifacts('claude', configDir, 'global', RESOLVED_CORE, undefined, undefined, { fs: fakeFs }); + + assert.notStrictEqual(result, undefined, 'F5: must still return a plan'); + const skillsKind = result.kinds.find((k) => k.kind === 'skills'); + assert.ok(skillsKind, 'F5 precondition: claude global writes a skills kind'); + assert.strictEqual(skillsKind.destDir, skillsDest); + assert.deepStrictEqual( + skillsKind.preserved, [], + 'F5: the branch ran off the fake\'s existsSync=true, found an empty pre-existing dir, and preserved ' + + 'nothing — the same outcome the real existsSync-true branch produces for an empty pre-existing dir', + ); + }); +}); + +describe('installRuntimeArtifacts — F6: incomplete adapter falls back to real fs, never silently no-ops', () => { + test('incomplete adapter fails loudly (never silently skips the write)', (t) => { + // install-fs-adapter.cts's documented PARTIAL-ADAPTER TRAP: withInstallFs + // merges the injected partial OVER the real adapter — any method the + // partial omits resolves to REAL node:fs, silently. This pins the + // dangerous alternative it guards against: an omitted method must never + // degrade into a silent no-op (skipping the operation, pretending + // success) — it must actually execute, for real. + const configDir = createTempDir('gsd-f6-claude-'); + t.after(() => cleanup(configDir)); + sandboxHome(t, configDir); + + let realMkdirCalls = 0; + // Deliberately incomplete: only mkdirSync is overridden (to prove this + // partial is genuinely merged over real fs, not a full copy of it) — + // every other method (existsSync/readdirSync/writeFileSync/ + // copyFileSync/cpSync/...) is omitted entirely. + const incompleteFs = { + mkdirSync: (p, opts) => { realMkdirCalls++; return fs.mkdirSync(p, opts); }, + }; + + const result = installRuntimeArtifacts('claude', configDir, 'global', RESOLVED_CORE, undefined, undefined, { fs: incompleteFs }); + + assert.notStrictEqual(result, undefined, 'F6: an incomplete adapter must not silently produce no result'); + assert.ok(realMkdirCalls > 0, 'F6 test precondition: mkdirSync must have been called'); + const skillsKind = result.kinds.find((k) => k.kind === 'skills'); + assert.ok(skillsKind, 'F6 precondition: claude global writes a skills kind'); + assert.ok( + fs.existsSync(skillsKind.destDir) && fs.readdirSync(skillsKind.destDir).length > 0, + 'F6: every method the incomplete adapter omitted fell back to REAL fs and actually wrote real ' + + 'content — an incomplete fake never silently no-ops the operations it does not implement', + ); + }); +}); + +// ─── G. Additive contract — G2 ──────────────────────────────────────────────── + +describe('installRuntimeArtifacts — G2: bin/install.js production call site unchanged', () => { + test('installer call site unchanged', (t) => { + const binInstall = require('../bin/install.js'); + const tmpDir = createTempDir('gsd-g2-'); + const previousCwd = process.cwd(); + process.chdir(tmpDir); + t.after(() => { process.chdir(previousCwd); cleanup(tmpDir); }); + + const result = binInstall.install(false, 'claude'); + + assert.strictEqual( + result.runtime, 'claude', + 'G2: bin/install.js\'s production call site (6 positional args, no deps) must be unaffected by the new optional deps param', + ); + // install(false, ...) is a LOCAL install — claude's local layout writes + // commands+agents, not skills (skills is global-only for claude). + assert.ok( + fs.existsSync(path.join(tmpDir, '.claude', 'commands')), + 'G2: the production install must still write commands/ end-to-end', + ); + binInstall.uninstall(false, 'claude'); + }); +}); + +// ─── H. Security boundaries must NOT move behind the adapter ───────────────── + +describe('installRuntimeArtifacts — H1: symlink escape still refuses', () => { + test('symlink escape still refuses', (t) => { + const configDir = createTempDir('gsd-h1-'); + const outsideDir = createTempDir('gsd-h1-outside-'); + t.after(() => { cleanup(configDir); cleanup(outsideDir); }); + sandboxHome(t, configDir); + // Pre-create the skills destDir AS a symlink pointing outside configDir — + // the guard must refuse before mkdirSync ever follows it. + fs.symlinkSync(outsideDir, path.join(configDir, 'skills'), 'dir'); + + assert.throws( + () => installRuntimeArtifacts('claude', configDir, 'global', RESOLVED_CORE), + /GSD_ALLOW_SYMLINKED_DEST/, + 'H1: a destDir that is itself a symlink pointing outside the install root must be refused', + ); + }); +}); + +describe('installRuntimeArtifacts — H2: opt-in still follows', () => { + test('opt-in still follows', (t) => { + const configDir = createTempDir('gsd-h2-'); + const outsideDir = createTempDir('gsd-h2-outside-'); + t.after(() => { cleanup(configDir); cleanup(outsideDir); }); + sandboxHome(t, configDir); + fs.symlinkSync(outsideDir, path.join(configDir, 'skills'), 'dir'); + + const savedOptIn = process.env.GSD_ALLOW_SYMLINKED_DEST; + process.env.GSD_ALLOW_SYMLINKED_DEST = '1'; + t.after(() => { + if (savedOptIn === undefined) delete process.env.GSD_ALLOW_SYMLINKED_DEST; + else process.env.GSD_ALLOW_SYMLINKED_DEST = savedOptIn; + }); + + const result = installRuntimeArtifacts('claude', configDir, 'global', RESOLVED_CORE); + + assert.notStrictEqual(result, undefined, 'H2: opt-in must still succeed and return a plan'); + const skillsKind = result.kinds.find((k) => k.kind === 'skills'); + assert.ok(skillsKind, 'H2 precondition: claude global writes a skills kind'); + assert.ok( + fs.readdirSync(outsideDir).length > 0, + 'H2: with the opt-in set, writes must actually follow the symlink into outsideDir', + ); + }); +}); + +describe('installRuntimeArtifacts — H3: fake adapter cannot bypass the symlink guard', () => { + test('fake adapter cannot bypass the symlink guard', () => { + // hasExistingSymlinkBetween's path-traversal refusal (install-engine.cts, + // part (a) of the guard: "resolvedFullPath !== resolvedRoot && + // !resolvedFullPath.startsWith(resolvedRoot + path.sep)") is PURE PATH + // MATH — path.resolve/startsWith on strings, no fs call at all. Pin that + // invariant directly: even a fake adapter that lies "nothing exists, + // nothing is a symlink" everywhere cannot make this refusal pass for an + // escaping path, because this branch never asks the adapter anything. + const root = path.join(os.tmpdir(), 'gsd-h3-fake-root'); + const escapingPath = path.join(root, '..', '..', 'etc', 'passwd'); + const lyingFs = { + existsSync: () => false, + lstatSync: () => { + throw new Error('H3: lstatSync must never be reached — the path-traversal refusal is pure path math'); + }, + realpathSync: (p) => p, + }; + + const refused = withInstallFs(lyingFs, () => hasExistingSymlinkBetween(root, escapingPath)); + + assert.strictEqual( + refused, true, + 'H3: a fake adapter reporting "nothing exists, nothing is a symlink" must not be able to certify ' + + 'an install the real filesystem would refuse — the path-traversal decision does not consult the ' + + 'adapter at all', + ); + }); +}); + +describe('installRuntimeArtifacts — H4: dest confinement still enforced', () => { + test('dest confinement still enforced', () => { + assert.throws( + () => runtimeArtifactInstallPlan.assertDestWithinConfigHome('/fake/config/home', '../../etc'), + /escapes configHome|strict subpath/i, + 'H4: assertDestWithinConfigHome must still throw for a destSubpath escaping configHome', + ); + }); +}); + +describe('installRuntimeArtifacts — H5: nul byte in dest is rejected', () => { + test('nul byte in dest is rejected', () => { + assert.throws( + () => runtimeArtifactInstallPlan.assertDestWithinConfigHome('/fake/config/home', 'skills\0evil'), + /NUL/, + 'H5: assertDestWithinConfigHome must still throw for a destSubpath containing a NUL byte', + ); + }); +}); + +// ─── I. Cleanup visibility ───────────────────────────────────────────────── + +describe('installRuntimeArtifacts — I1: successful cleanup is recorded', () => { + test('successful cleanup is recorded', (t) => { + const configDir = createTempDir('gsd-i1-augment-'); + t.after(() => cleanup(configDir)); + sandboxHome(t, configDir); + + const result = installRuntimeArtifacts('augment', configDir, 'global', RESOLVED_FULL, TEST_ATTRIBUTION); + + assert.ok(result.cleanup.length > 0, 'I1: augment install must produce at least one cleanupDirs entry to prove this row'); + for (const entry of result.cleanup) { + assert.strictEqual(typeof entry.dir, 'string'); + assert.strictEqual(entry.ok, true, `I1: successful cleanup entries must record ok:true (dir=${entry.dir})`); + assert.strictEqual(fs.existsSync(entry.dir), false, 'I1: a successfully cleaned dir must no longer exist on disk'); + } + }); +}); + +describe('installRuntimeArtifacts — I2: failed cleanup is visible, not silent', () => { + test('failed cleanup is visible, not silent', (t) => { + const configDir = createTempDir('gsd-i2-augment-'); + t.after(() => cleanup(configDir)); + sandboxHome(t, configDir); + + const fakeFs = { + rmSync: (p, opts) => { + if (String(p).includes('gsd-cmd-rewrites-')) { + throw new Error('I2: simulated cleanup failure'); + } + // This is a fake fs-adapter METHOD delegating to real fs for paths + // it does not intentionally poison, not a test's own directory- + // cleanup call (which still goes through t.after(() => cleanup(...)) + // above). + // eslint-disable-next-line local/no-raw-rmsync-in-tests -- delegate, not test cleanup + return fs.rmSync(p, opts); + }, + }; + + const result = installRuntimeArtifacts('augment', configDir, 'global', RESOLVED_FULL, TEST_ATTRIBUTION, undefined, { fs: fakeFs }); + + assert.notStrictEqual(result, undefined, 'I2: install must still succeed (never fail) even when cleanup throws'); + assert.ok(result.cleanup.length > 0, 'I2: augment must have at least one cleanupDirs entry to fail'); + assert.ok( + result.cleanup.every((c) => c.ok === false), + 'I2: a cleanup rmSync throw must be reported as ok:false on the returned plan, never silently dropped', + ); + }); +}); + +describe('installRuntimeArtifacts — I3: no cleanup dirs is an empty array', () => { + test('no cleanup dirs is an empty array', (t) => { + const configDir = createTempDir('gsd-i3-claude-'); + t.after(() => cleanup(configDir)); + sandboxHome(t, configDir); + + const result = installRuntimeArtifacts('claude', configDir, 'global', RESOLVED_CORE); + + assert.ok(Array.isArray(result.cleanup), 'I3: cleanup must be an array even when empty'); + assert.strictEqual( + result.cleanup.length, 0, + 'I3: a claude/core install with no rewritten temp dirs must report an EMPTY cleanup array, not undefined/absent', + ); + }); +}); + +describe('installRuntimeArtifacts — I4: stage failure before any item cleans up and throws', () => { + test('stage failure cleans up and throws', (t) => { + const configDir = createTempDir('gsd-i4-'); + t.after(() => cleanup(configDir)); + sandboxHome(t, configDir); + + assert.throws( + () => installRuntimeArtifacts('claude', configDir, 'global', { skills: 123, agents: 123 }), + (err) => err instanceof Error, + 'I4: a malformed resolvedProfile that fails the FIRST kind\'s stage() (before any cleanupDirs exist) ' + + 'must still surface as a thrown Error, with the (empty) cleanupDirs still swept by the finally block', + ); + }); +}); + +describe('installRuntimeArtifacts — I5: rewrite failure mid-plan cleans up and throws', () => { + test('rewrite failure cleans up and throws', (t) => { + const configDir = createTempDir('gsd-i5-augment-'); + t.after(() => cleanup(configDir)); + sandboxHome(t, configDir); + + let capturedCleanupDir; + const fakeFs = { + mkdirSync: (p, opts) => { + if (String(p).includes('gsd-profile-runtime-skills-')) { + throw new Error('I5: simulated skills-stage failure AFTER commands already rewrote+registered a cleanup dir'); + } + fs.mkdirSync(p, opts); + return undefined; + }, + writeFileSync: (p, data, opts) => { + if (String(p).includes('gsd-cmd-rewrites-') && capturedCleanupDir === undefined) { + capturedCleanupDir = path.dirname(p); + } + fs.writeFileSync(p, data, opts); + }, + }; + + assert.throws( + () => installRuntimeArtifacts('augment', configDir, 'global', RESOLVED_FULL, TEST_ATTRIBUTION, undefined, { fs: fakeFs }), + /I5: simulated skills-stage failure/, + 'I5: a failure in a LATER kind\'s stage step must still propagate as a thrown error', + ); + assert.ok(capturedCleanupDir, 'I5 test precondition: the commands kind\'s rewrite dir must have been observed before the skills-stage failure'); + assert.strictEqual( + fs.existsSync(capturedCleanupDir), false, + 'I5: the EARLIER (successfully rewritten) commands cleanupDir must still be removed by the finally ' + + 'block even though a LATER kind\'s stage step failed', + ); + }); +}); + +// ─── K. Byte-identical writes — K3 ───────────────────────────────────────── + +function walkFilesRecursively(root) { + const out = new Map(); + const walk = (relPath, absPath) => { + for (const entry of fs.readdirSync(absPath, { withFileTypes: true })) { + const childRel = relPath ? path.join(relPath, entry.name) : entry.name; + const childAbs = path.join(absPath, entry.name); + if (entry.isDirectory()) walk(childRel, childAbs); + else if (entry.isFile()) out.set(childRel, fs.readFileSync(childAbs)); + } + }; + if (fs.existsSync(root)) walk('', root); + return out; +} + +describe('installRuntimeArtifacts — K3: real install before/after, full recursive diff', () => { + test('writes are byte-identical', (t) => { + for (const runtime of ['claude', 'qwen']) { + const dirA = createTempDir(`gsd-k3-${runtime}-a-`); + const dirB = createTempDir(`gsd-k3-${runtime}-b-`); + t.after(() => { cleanup(dirA); cleanup(dirB); }); + + sandboxHome(t, dirA); + installRuntimeArtifacts(runtime, dirA, 'global', RESOLVED_FULL); + sandboxHome(t, dirB); + installRuntimeArtifacts(runtime, dirB, 'global', RESOLVED_FULL); + + const filesA = walkFilesRecursively(dirA); + const filesB = walkFilesRecursively(dirB); + assert.deepStrictEqual( + [...filesA.keys()].sort(), [...filesB.keys()].sort(), + `K3 (${runtime}): the file sets written by two independent installs must match`, + ); + for (const [relPath, contentA] of filesA) { + assert.ok( + contentA.equals(filesB.get(relPath)), + `K3 (${runtime}): ${relPath} content drifted between two independent installs`, + ); + } + } + }); +}); + +// ─── L. Property tests ──────────────────────────────────────────────────── + +describe('installRuntimeArtifacts — L1: plan kinds mirror layout kinds (property)', () => { + test('plan kinds mirror layout kinds', (t) => { + const runtimes = Object.keys(registry.runtimes); + const RUNTIME_ARB = fc.constantFrom(...runtimes); + const SCOPE_ARB = fc.constantFrom('global', 'local'); + const observedKindSets = new Set(); + const createdDirs = []; + const savedHome = process.env.HOME; + const savedUserProfile = process.env.USERPROFILE; + t.after(() => { + if (savedHome === undefined) delete process.env.HOME; else process.env.HOME = savedHome; + if (savedUserProfile === undefined) delete process.env.USERPROFILE; else process.env.USERPROFILE = savedUserProfile; + for (const d of createdDirs) cleanup(d); + }); + + // Seeded, bounded numRuns, replay data on failure (verbose:true prints + // the failing/shrunk (runtime, scope) pair fast-check found). + fc.assert( + fc.property(RUNTIME_ARB, SCOPE_ARB, (runtime, scope) => { + const configDir = createTempDir(`gsd-l1-${runtime}-`); + createdDirs.push(configDir); + process.env.HOME = configDir; + process.env.USERPROFILE = configDir; + + const layout = runtimeArtifactLayout.resolveRuntimeArtifactLayout(runtime, configDir, scope); + const expectedKinds = [...new Set(layout.kinds.map((k) => k.kind))].sort(); + const plan = installRuntimeArtifacts(runtime, configDir, scope, RESOLVED_CORE); + const actualKinds = [...new Set(plan.kinds.map((k) => k.kind))].sort(); + observedKindSets.add(JSON.stringify(actualKinds)); + + assert.deepStrictEqual( + actualKinds, expectedKinds, + `L1 (${runtime}/${scope}): plan.kinds must be a bijection with layout.kinds — ` + + `plan=${JSON.stringify(actualKinds)} vs layout=${JSON.stringify(expectedKinds)}`, + ); + }), + { numRuns: 30, seed: 2874, verbose: true }, + ); + + // Non-vacuity: the registry has runtimes with empty, single-kind, and + // multi-kind layouts (verified across the whole registry — see this + // row's PR notes) — a generator that only ever produced ONE kind-set + // would be exercising nothing. + assert.ok( + observedKindSets.size > 1, + `L1 non-vacuity: the generator must exercise more than one distinct kind-set — observed only ` + + `${observedKindSets.size} (${[...observedKindSets].join(', ')})`, + ); + }); +}); + +describe('installRuntimeArtifacts — L2: plan is deterministic (property)', () => { + function normalizePlanForIdempotence(plan) { + // mkInstallTempDir names every rewrite/staging temp dir with a random hex + // suffix (install-fs-adapter.cts) — expected to differ between two + // independent calls even when everything else about the plan is + // identical. Normalize those away; everything else must match exactly. + const stripTemp = (p) => (typeof p === 'string' && p.startsWith(os.tmpdir()) ? '' : p); + return { + runtime: plan.runtime, + scope: plan.scope, + kinds: plan.kinds.map((k) => ({ + kind: k.kind, sourceDir: stripTemp(k.sourceDir), destDir: k.destDir, + preserved: k.preserved, written: k.written, + })), + cleanup: plan.cleanup.map((c) => ({ dir: stripTemp(c.dir), ok: c.ok })), + postSteps: plan.postSteps, + }; + } + + test('plan is deterministic', () => { + const runtimes = Object.keys(registry.runtimes); + const RUNTIME_ARB = fc.constantFrom(...runtimes); + const SCOPE_ARB = fc.constantFrom('global', 'local'); + let hits = 0; + + fc.assert( + fc.property(RUNTIME_ARB, SCOPE_ARB, (runtime, scope) => { + // configDir is never created for real — both calls run against fresh, + // independent fake adapters, so no real fs cleanup is needed here. + const configDir = path.join(os.tmpdir(), `gsd-l2-${runtime}-${crypto.randomUUID()}`); + const planA = installRuntimeArtifacts(runtime, configDir, scope, RESOLVED_CORE, undefined, undefined, { fs: createFakeInstallFs() }); + const planB = installRuntimeArtifacts(runtime, configDir, scope, RESOLVED_CORE, undefined, undefined, { fs: createFakeInstallFs() }); + hits++; + + assert.deepStrictEqual( + normalizePlanForIdempotence(planA), + normalizePlanForIdempotence(planB), + `L2 (${runtime}/${scope}): two installs against fresh fake adapters with the same inputs must ` + + 'yield structurally identical plans (temp-dir names normalized — see normalizePlanForIdempotence)', + ); + }), + { numRuns: 30, seed: 2874, verbose: true }, + ); + + assert.strictEqual(hits, 30, 'L2 non-vacuity: every generated (runtime, scope) pair must actually have exercised a comparison'); + }); +}); + +// ─── readGsdCommandNames — single-source parity ────────────────────────────── +// +// command-roster.cts's readGsdCommandNames reimplements +// scripts/fix-slash-commands.cjs's readCmdNames' directory-scan rule against +// the injectable install-fs seam instead of delegating to it — see +// command-roster.cts's module comment for why (readCmdNames is deliberately +// a zero-dependency standalone CLI/library with no build-order dependency on +// gsd-core/bin/lib, so it cannot itself require the compiled +// install-fs-adapter.cjs). Two implementations of one filtering rule is this +// repo's recorded Generative Fix Divergence class; this test is the +// enforcement the coordinator required in exchange for keeping the +// reimplementation: it fails the moment the two disagree about which stems +// commands/gsd/ contains. + +describe('readGsdCommandNames — single-source parity (command-roster.cts vs scripts/fix-slash-commands.cjs)', () => { + test('both implementations report the identical stem set for commands/gsd/', () => { + const fromCommandRoster = [...commandRoster.readGsdCommandNames()].sort(); + const fromSlashCommandTransformer = [...slashCommandTransformer.readCmdNames()].sort(); + assert.deepStrictEqual( + fromCommandRoster, + fromSlashCommandTransformer, + 'command-roster.cts readGsdCommandNames() and scripts/fix-slash-commands.cjs readCmdNames() ' + + 'diverged — these are two implementations of the SAME directory-scan rule (Generative Fix ' + + 'Divergence); fix the one that is wrong, do not just silence this test', + ); + assert.ok(fromCommandRoster.length > 0, 'sanity: commands/gsd/ must contain at least one command'); + }); +}); diff --git a/tests/install-fs-adapter-seam.test.cjs b/tests/install-fs-adapter-seam.test.cjs new file mode 100644 index 000000000..52c520e54 --- /dev/null +++ b/tests/install-fs-adapter-seam.test.cjs @@ -0,0 +1,211 @@ +'use strict'; + +/** + * Install fs seam — two real-fs leaks that AC2 ("an install can be exercised + * end-to-end against an injected fs adapter with no real filesystem") does + * not tolerate. + * + * #2874 (epic #2866 Phase 5), governed by ADR-58 + * (docs/adr/58-runtime-install-policy-module.md). + * + * (a) command-roster.cts's `readGsdCommandNames` reads the PACKAGE'S OWN + * `commands/gsd/` tree — not an install destination — so it must stay on + * real `node:fs`, unrouted, even while a fake install adapter is active + * for the surrounding call (mirrors findInstallSourceRoot / + * findAgentsSourceRoot's documented precedent in + * runtime-artifact-layout.cts). + * + * (b) install-profiles.cts's `cleanupStagedSkills` runs from a + * `process.on('exit'/'SIGINT'/…)` handler — AFTER `withInstallFs` has + * already restored the real adapter — so a dir staged during a + * fake-adapter call must be cleaned up with the SAME fake adapter that + * staged it, never with real fs. + */ + +process.env.GSD_TEST_MODE = '1'; + +const { test, describe } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const os = require('node:os'); +const path = require('node:path'); +const crypto = require('node:crypto'); + +const { createTempDir, cleanup } = require('./helpers.cjs'); + +const commandRoster = require('../gsd-core/bin/lib/command-roster.cjs'); +const { + withInstallFs, +} = require('../gsd-core/bin/lib/install-fs-adapter.cjs'); +const { + stageSkillsForRuntimeAsSkills, + cleanupStagedSkills, + STAGED_DIRS, +} = require('../gsd-core/bin/lib/install-profiles.cjs'); + +const REAL_COMMANDS_DIR = path.join(__dirname, '..', 'commands', 'gsd'); + +/** + * A minimal in-memory fake install adapter. Every method that + * `stageSkillsForRuntimeAsSkills` (nested=false path) can reach is + * implemented against one flat Map store — deliberately NOT seeded with + * anything under the real `commands/gsd/` tree, so a call that accidentally + * routes a package-source read through this fake would either throw or + * return the wrong (empty) stems instead of the real package's own names. + */ +function createFakeFs(seed = []) { + const store = new Map(); + for (const [p, entry] of seed) store.set(path.normalize(String(p)), entry); + const norm = (p) => path.normalize(String(p)); + const childPrefix = (dir) => { + const n = norm(dir); + return n.endsWith(path.sep) ? n : n + path.sep; + }; + const enoent = (p) => { + const err = new Error(`ENOENT: no such file or directory, '${p}'`); + err.code = 'ENOENT'; + return err; + }; + return { + _store: store, + existsSync: (p) => store.has(norm(p)), + mkdirSync: (p) => { store.set(norm(p), { type: 'dir' }); return undefined; }, + rmSync: (p) => { + const n = norm(p); + store.delete(n); + const prefix = childPrefix(n); + for (const k of [...store.keys()]) if (k.startsWith(prefix)) store.delete(k); + }, + readdirSync: (p, opts) => { + const e = store.get(norm(p)); + if (!e) throw enoent(p); + const prefix = childPrefix(p); + const names = new Set(); + for (const k of store.keys()) { + if (!k.startsWith(prefix)) continue; + const rest = k.slice(prefix.length); + const sepIdx = rest.indexOf(path.sep); + const name = sepIdx === -1 ? rest : rest.slice(0, sepIdx); + if (name) names.add(name); + } + const arr = [...names]; + if (opts && opts.withFileTypes) { + return arr.map((name) => { + const full = norm(path.join(String(p), name)); + const fe = store.get(full); + return { name, isFile: () => (fe ? fe.type === 'file' : false) }; + }); + } + return arr; + }, + readFileSync: (p, encoding) => { + const e = store.get(norm(p)); + if (!e || e.type !== 'file') throw enoent(p); + const buf = Buffer.isBuffer(e.content) ? e.content : Buffer.from(e.content ?? '', 'utf8'); + return encoding ? buf.toString(encoding) : buf; + }, + writeFileSync: (p, data) => { store.set(norm(p), { type: 'file', content: data }); }, + copyFileSync: (src, dest) => { + const e = store.get(norm(src)); + store.set(norm(dest), { type: 'file', content: e ? e.content : Buffer.alloc(0) }); + }, + lstatSync: (p) => { + const e = store.get(norm(p)); + if (!e) throw enoent(p); + return { isFile: () => e.type === 'file', isDirectory: () => e.type === 'dir', isSymbolicLink: () => false }; + }, + realpathSync: (p) => norm(p), + unlinkSync: (p) => { store.delete(norm(p)); }, + rmdirSync: (p) => { store.delete(norm(p)); }, + }; +} + +// ─── (a) command-roster reads the package's OWN source, unrouted ─────────── + +describe('command-roster readGsdCommandNames — package-source read stays unrouted', () => { + test('returns the real package command stems even while a poisoning fake adapter is active', () => { + const expectedStems = fs.readdirSync(REAL_COMMANDS_DIR) + .filter((f) => f.endsWith('.md')) + .map((f) => f.replace(/\.md$/, '')) + .sort(); + assert.ok(expectedStems.length > 0, 'REAL_COMMANDS_DIR must contain real .md command files (fixture drift)'); + + // A fake whose readdirSync/existsSync ALWAYS throw or lie — if + // readGsdCommandNames routed its read through installFs(), this would + // either throw (poisoned) or return the wrong (empty) set instead of the + // real package's own stems. + const poisonFs = { + existsSync: () => { throw new Error('leak (a): installFs().existsSync reached for the package-own commands dir'); }, + readdirSync: () => { throw new Error('leak (a): installFs().readdirSync reached for the package-own commands dir'); }, + readFileSync: () => { throw new Error('leak (a): installFs().readFileSync reached for the package-own commands dir'); }, + mkdirSync: () => { throw new Error('leak (a): installFs().mkdirSync reached'); }, + writeFileSync: () => { throw new Error('leak (a): installFs().writeFileSync reached'); }, + copyFileSync: () => { throw new Error('leak (a): installFs().copyFileSync reached'); }, + rmSync: () => { throw new Error('leak (a): installFs().rmSync reached'); }, + }; + + const actualStems = withInstallFs(poisonFs, () => commandRoster.readGsdCommandNames()).sort(); + + assert.deepStrictEqual( + actualStems, + expectedStems, + 'readGsdCommandNames must return the real package command stems, reading real fs directly, ' + + 'not the injected (poisoning) fake install adapter', + ); + }); +}); + +// ─── (b) cleanupStagedSkills must not perform real IO on fake-staged dirs ── + +describe('install-profiles cleanupStagedSkills — deferred cleanup does not leak past the restore', () => { + test('a fake-adapter install followed by the exit handler performs zero real fs.rmSync calls', (t) => { + const fakeSrcDir = path.join(os.tmpdir(), `gsd-fake-src-${crypto.randomUUID()}`); + const fakeFs = createFakeFs([ + [fakeSrcDir, { type: 'dir' }], + [path.join(fakeSrcDir, 'alpha.md'), { type: 'file', content: '# alpha\n' }], + ]); + + // Poison real fs.rmSync for the duration of this test — auto-restored by + // node:test's mock tracker when the test ends (no try/finally needed). + let realRmSyncCalls = 0; + t.mock.method(fs, 'rmSync', () => { + realRmSyncCalls++; + throw new Error('leak (b): real fs.rmSync() was reached for a dir staged under a fake adapter'); + }); + + const converter = (content, _skillName) => content; + const stagedDir = withInstallFs( + fakeFs, + () => stageSkillsForRuntimeAsSkills(fakeSrcDir, { skills: '*' }, converter, 'gsd-'), + ); + + assert.ok(STAGED_DIRS.has(stagedDir), 'stageSkillsForRuntimeAsSkills must register the staged dir for cleanup'); + assert.ok(fakeFs._store.has(path.normalize(stagedDir)), 'staged dir must exist in the fake store'); + + // Simulate the exit handler: `current` (install-fs-adapter.cts) is back + // to the real adapter here — withInstallFs already restored it above. + cleanupStagedSkills(); + + assert.strictEqual(realRmSyncCalls, 0, 'cleanupStagedSkills must never call real fs.rmSync for a fake-staged dir'); + assert.strictEqual(fakeFs._store.has(path.normalize(stagedDir)), false, 'the fake-staged dir must be removed via the fake adapter'); + assert.strictEqual(STAGED_DIRS.has(stagedDir), false, 'STAGED_DIRS must be cleared after cleanup'); + }); + + test('negative proof: a REAL install still cleans up its own staged dirs', (t) => { + const srcDir = createTempDir('gsd-real-src-'); + t.after(() => cleanup(srcDir)); + fs.writeFileSync(path.join(srcDir, 'alpha.md'), '# alpha\n'); + + const converter = (content, _skillName) => content; + const stagedDir = stageSkillsForRuntimeAsSkills(srcDir, { skills: '*' }, converter, 'gsd-'); + t.after(() => { if (fs.existsSync(stagedDir)) cleanup(stagedDir); }); + + assert.ok(fs.existsSync(stagedDir), 'real staged dir must exist on real fs before cleanup'); + assert.ok(STAGED_DIRS.has(stagedDir), 'real staged dir must be registered'); + + cleanupStagedSkills(); + + assert.strictEqual(fs.existsSync(stagedDir), false, 'a REAL install must still remove its staged dir on cleanup — no temp-dir leak'); + assert.strictEqual(STAGED_DIRS.has(stagedDir), false, 'STAGED_DIRS must be cleared after cleanup'); + }); +}); diff --git a/tests/install-runtime-artifacts.test.cjs b/tests/install-runtime-artifacts.test.cjs index b86e80ba1..bd5dcbe3a 100644 --- a/tests/install-runtime-artifacts.test.cjs +++ b/tests/install-runtime-artifacts.test.cjs @@ -7014,3 +7014,119 @@ describe('#2873 C1-C6 — install-time shadow report (spawned installer wiring)' }); }); } + +// ─── #2874 (epic #2866 Phase 5) — G1/G3: the additive-contract guard ──────── +// Governed by ADR-58 (docs/adr/58-runtime-install-policy-module.md). +// Design: .gsd/phase/feat-2874-executed-plan-return/40-design.md +// Test matrix: .gsd/phase/feat-2874-executed-plan-return/50-test-matrix.md +// +// G1 and G3 must be GREEN both BEFORE and AFTER the executed-plan return +// lands — they are the guard proving the return value is additive (AC4), +// never a behavior change. No production code is touched by this file. + +/** + * Recursively hash a directory tree into a stable, order-independent digest. + * `stripDir`, if given, is textually removed from each UTF-8-decodable + * file's content before hashing, so two installs into DIFFERENT temp + * directories (whose absolute paths get baked into rewritten skill bodies) + * can still be compared for content-identity. + * + * Both `stripDir` and the file content are normalized to forward slashes + * before the strip, unconditionally (never gated on `path.sep`) — production + * (`posixNormalize` in shell-command-projection.cts) rewrites `\` -> `/` in + * the resolved configDir before baking it into skill bodies, so on Windows + * `stripDir` (a raw fs.mkdtempSync path, backslash-separated) would never + * match the posix-normalized text actually written, leaving each install's + * unique temp-dir suffix embedded and making every file's hash diverge. + */ +function hashDirTree(rootDir, stripDir) { + const entries = []; + const stripDirPosix = stripDir ? stripDir.replace(/\\/g, '/') : stripDir; + const walk = (relPath, absPath) => { + for (const entry of fs.readdirSync(absPath, { withFileTypes: true }) + .sort((a, b) => a.name.localeCompare(b.name))) { + const childRel = relPath ? `${relPath}/${entry.name}` : entry.name; + const childAbs = path.join(absPath, entry.name); + if (entry.isDirectory()) { + walk(childRel, childAbs); + } else if (entry.isFile()) { + const buf = fs.readFileSync(childAbs); + const normalized = stripDirPosix + ? buf.toString('utf8').replace(/\\/g, '/').split(stripDirPosix).join('') + : buf; + entries.push(`${childRel}:${crypto.createHash('sha256').update(normalized).digest('hex')}`); + } + } + }; + if (fs.existsSync(rootDir)) walk('', rootDir); + return entries.sort().join('\n'); +} + +describe('installRuntimeArtifacts — G1: void-ignoring caller is unaffected (AC4)', () => { + test('writes are byte-identical whether or not the caller uses the return value', (t) => { + const configDirIgnored = createTempDir('gsd-g1-ignored-'); + const configDirCaptured = createTempDir('gsd-g1-captured-'); + t.after(() => cleanup(configDirIgnored)); + t.after(() => cleanup(configDirCaptured)); + + // Caller A: discards the return value entirely — today's every call site + // (bin/install.js, both existing adapter test doubles). + installRuntimeArtifacts('claude', configDirIgnored, 'global', RESOLVED_CORE); + + // Caller B: captures the return value. Its shape is not asserted here — + // section E owns that — only that capturing it changes nothing about + // what gets written, and that capturing never itself throws. + const captured = installRuntimeArtifacts('claude', configDirCaptured, 'global', RESOLVED_CORE); + assert.ok( + captured === undefined || (captured !== null && typeof captured === 'object'), + 'G1: the return value, whatever its shape, must be undefined (today) or a plain object ' + + '(after) — never something a capturing caller could not safely ignore', + ); + + assert.strictEqual( + hashDirTree(configDirCaptured, configDirCaptured), + hashDirTree(configDirIgnored, configDirIgnored), + 'G1: writes must be byte-identical regardless of whether the caller captures the return value', + ); + }); +}); + +describe('installRuntimeArtifacts — G3: adapter calling-convention regression guard', () => { + // tests/adapter-declarative-equivalence.test.cjs:52 and + // tests/adapter-imperative.test.cjs:80 pin their OWN behavior via a + // module-ref monkeypatch of installRuntimeArtifacts — neither file ever + // invokes the real function, and neither is read or modified here. This + // row proves those two files' SUBJECT — the real installRuntimeArtifacts, + // called with the exact positional shape each adapter uses — still + // behaves: a real, successful, byte-on-disk install. + + test('declarative-adapter call shape (5 positional args, no capabilityRegistry) still installs', (t) => { + const configDir = createTempDir('gsd-g3-declarative-'); + t.after(() => cleanup(configDir)); + + // Matches tests/adapter-declarative-equivalence.test.cjs:62-66's + // captured shape: [runtime, configDir, scope, resolvedProfile, resolveAttribution]. + const resolveAttribution = () => 'attr-claude'; + installRuntimeArtifacts('claude', configDir, 'global', RESOLVED_CORE, resolveAttribution); + + assert.ok( + fs.existsSync(path.join(configDir, 'skills', 'gsd-help', 'SKILL.md')), + 'G3: the declarative adapter\'s calling convention must still produce a real install', + ); + }); + + test('imperative-adapter call shape (6 positional args incl. composed capability registry) still installs', (t) => { + const configDir = createTempDir('gsd-g3-imperative-'); + t.after(() => cleanup(configDir)); + + // Matches tests/adapter-imperative.test.cjs:88's captured shape: + // [runtime, configDir, scope, resolvedProfile, resolveAttribution, capabilityRegistry]. + const capabilityRegistry = { capabilityClusters: {} }; + installRuntimeArtifacts('claude', configDir, 'global', RESOLVED_CORE, undefined, capabilityRegistry); + + assert.ok( + fs.existsSync(path.join(configDir, 'skills', 'gsd-help', 'SKILL.md')), + 'G3: the imperative adapter\'s calling convention must still produce a real install', + ); + }); +}); diff --git a/tests/install.test.cjs b/tests/install.test.cjs index 8793f939b..11811415d 100644 --- a/tests/install.test.cjs +++ b/tests/install.test.cjs @@ -51,9 +51,22 @@ const { normalizeNodePath, GSD_CHANGESET_FILES, GSD_SCRIPTS_LIB_FILES, + installRuntimeArtifacts, } = require('../bin/install.js'); const { getGlobalConfigDir } = require('../gsd-core/bin/lib/runtime-homes.cjs'); +// #2874 AC3 exemplar (see the qwen install/uninstall group below): resolves +// the SAME 'full' profile install(false, ) resolves by default +// (bin/install.js's _activeProfileName falls back to 'full' when no +// --profile/marker is present), so a direct installRuntimeArtifacts() call +// against an already-installed targetDir reproduces the same executed-plan +// shape the production install() call just wrote, without re-deriving +// install()'s own profile-resolution logic in this test file. +const { loadSkillsManifest, resolveProfile } = require('../gsd-core/bin/lib/install-profiles.cjs'); +const RESOLVED_FULL = resolveProfile({ + modes: ['full'], + manifest: loadSkillsManifest(path.join(__dirname, '..', 'commands', 'gsd')), +}); const { RUNTIME_META, @@ -460,12 +473,47 @@ describe('install/uninstall — qwen (nested skills/gsd-/skills// assert.strictEqual(result.runtime, 'qwen'); assert.strictEqual(result.configDir, fs.realpathSync(targetDir)); - // qwen nests: skills/gsd-/skills//SKILL.md + // #2874 AC3 exemplar: install(false, 'qwen') above already wrote the + // skills/ and agents/ dirs, but its own internal installRuntimeArtifacts + // call (bin/install.js) discards the executed plan it returns. Calling + // installRuntimeArtifacts directly here — same runtime/targetDir/scope, + // same 'full' profile install() resolved by default — is an idempotent + // re-run over the already-installed tree (prune + rewrite converges to + // the same on-disk result) that surfaces the SAME plan value production + // discarded. What replaces fs.existsSync probing below is this ONE + // deepStrictEqual against that plan: previously each destination + // directory's existence was checked with a separate fs.existsSync() call + // (a probe of a SIDE EFFECT); now both are read off the single typed + // value the call contractually returns (40-design.md: "returns a plan + // naming every kind with its sourceDir, destDir", never undefined). + const plan = installRuntimeArtifacts('qwen', targetDir, 'global', RESOLVED_FULL); + const kindsByName = new Map(plan.kinds.map((k) => [k.kind, k])); + assert.deepStrictEqual( + { + skillsDestDir: kindsByName.get('skills') && kindsByName.get('skills').destDir, + agentsDestDir: kindsByName.get('agents') && kindsByName.get('agents').destDir, + }, + { + skillsDestDir: path.join(targetDir, 'skills'), + agentsDestDir: path.join(targetDir, 'agents'), + }, + 'qwen executed plan must record a skills-kind write to skills/ and an agents-kind write to agents/', + ); + + // qwen nests: skills/gsd-/skills//SKILL.md. The plan above + // proves the skills-kind DESTINATION ROOT; which concrete stem (e.g. + // "help") landed under it is finer-grained than the plan's per-kind + // contract (one destDir per kind, not a file list), so that specific + // fact still needs an fs probe — nothing here is a regression from the + // pre-migration test, only the destDir-existence checks moved to the + // plan value above. const qwenHelpPath = nestedSkillPath(path.join(targetDir, 'skills'), 'gsd-', 'help'); assert.ok(fs.existsSync(qwenHelpPath), `help SKILL.md must exist at nested path: ${path.relative(targetDir, qwenHelpPath)}`); + // gsd-core/VERSION is written by install()'s own gsd-core copy step, not + // by installRuntimeArtifacts (outside the executed-plan contract) — stays + // an fs probe. assert.ok(fs.existsSync(path.join(targetDir, 'gsd-core', 'VERSION'))); - assert.ok(fs.existsSync(path.join(targetDir, 'agents'))); const manifest = writeManifest(targetDir, 'qwen'); assert.ok(