From 909a3b180b4b968c787f37f1d0b2234061f0134c Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Tue, 21 Jul 2026 08:26:47 -0400 Subject: [PATCH] fix(#2470): install pi's extension as gsd.js so pi actually discovers it (#2478) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * test(#2470): failing-first — pi extension must satisfy pi's auto-discovery filter pi auto-discovers extensions/ entries through isExtensionFile(), which accepts only .ts and .js. GSD installs its extension as gsd.cjs, so pi silently skips it: no /gsd command, no error, no log line. Encodes pi's discovery PREDICATE rather than a literal filename, so the contract keeps holding across future renames, and adds the migration-006 test matrix for retiring the stale gsd.cjs left in pre-fix installs. Red until the fix lands. Co-Authored-By: Claude Opus 4.8 * fix(#2470): install pi's extension as gsd.js so pi actually discovers it pi auto-discovers extensions/ entries via isExtensionFile(), which accepts only .ts and .js and skips everything else silently. capabilities/pi declared the dest as gsd.cjs, so the extension installed correctly and was then ignored forever: no /gsd command, no error, no log line. Install it as gsd.js. The in-repo source stays pi/gsd.cjs — tests require() it directly and .cjs is unambiguous CommonJS; only the installed name has to satisfy pi, and pi loads accepted files through jiti, which handles CJS and ESM alike. (The reporter's premise that ~/.pi/agent/package.json declares "type":"commonjs" does not hold — pi never writes that file.) Renaming an installed artifact requires a migration record, so add 006 to retire the stale gsd.cjs from pre-fix installs; without it the old path drops out of the manifest and uninstall can never remove it. The migration plans nothing for an unmanifested gsd.cjs: emitting remove-managed there would have the executor downgrade it to preserve-user and mark it blocked, failing the install for anyone who hand-placed their own file. Also pins body-parser >=2.3.0 (GHSA-v422-hmwv-36x6). The advisory reaches the production tree transitively via the Claude Agent SDK and fails the npm-integrity gate, blocking any PR; pinned via the existing overrides idiom. Co-Authored-By: Claude Opus 4.8 * fix(#2470): address orthogonal review findings + register migration checksum Code review: - pi/gsd.cjs's install docstring still told readers to copy the file to extensions/gsd.cjs — the exact silently-broken state this PR fixes. Anyone following it recreated the bug. - Two stale extensions/gsd.cjs comments in install-minimal-hooks.test.cjs. Security review: - _installNativePluginIfDeclared confined nativePlugin.dir but joined nativePlugin.file onto the validated directory unchecked, so a descriptor whose file carried .., an absolute path, or a NUL byte would have written outside configHome. Not reachable in a shipped build (descriptors are first-party and compiled into the capability registry), but file is exactly the field this PR changes. Confine the full dest path instead; for a well-formed descriptor this resolves identically to the previous mkdir(dir) + join(dir, file). Covered by four new write-confinement tests. Also register migration 006 in the #670 EXPECTED_CHECKSUMS baseline — shipped migration bodies are locked to a committed checksum and a new migration fails CI until it is listed. Co-Authored-By: Claude Opus 4.8 * fix(#2470): never dereference a symlinked managed path when snapshotting fs.copyFileSync follows symlinks, so a managed path replaced by a link had the REFERENT's bytes copied into the migration journal's rollback and backup trees — a gsd.cjs symlinked at a private key would land that key's contents under gsd-migration-journal/. Deletion was already safe (fs.rmSync unlinks the link, never the target); the copy was not. Nothing GSD installs is ever a symlink, so the faithful snapshot of a symlinked managed path is the link itself. copyPreservingSymlink recreates it, which keeps rollback fidelity (restore re-creates the same link) while never reading the referent. Scoped the pre-delete to the symlink branch only, so the regular-file path keeps copyFileSync's overwrite-in-place and a mid-restore failure cannot destroy the destination. The restore-side existence check moves to lstat, since existsSync follows a link whose target is gone and would silently skip the restore. This lives in the engine all six migrations share, so 000-005 are hardened too. Also regenerates the pi golden-parity hash: correcting pi/gsd.cjs's own install docstring changes the extension's content, which the golden suite caught. Co-Authored-By: Claude Opus 4.8 * fix(#2470): symlink-preserve the in-apply failure-recovery restore too The previous commit routed three copy sites through copyPreservingSymlink but missed a fourth: the catch block inside applyInstallerMigrationPlan, which replays rollback snapshots taken earlier in the SAME apply attempt. Those snapshots are symlinks precisely because of that commit, so the raw copyFileSync there dereferenced them and wrote the referent's bytes to the LIVE install path — worse than the journal-tree leak it was meant to fix, since it is user-visible and at a predictable location. Verified by experiment rather than assertion: with the pre-fix line restored, the managed path comes back as a REGULAR FILE containing the referent's bytes; with the fix it comes back as a symlink and the bytes appear nowhere. The accompanying test injects the failure by letting the delete succeed and then throwing once, modelling a later step failing after the delete. That ordering is load-bearing — an earlier draft injected before the delete, which leaves the live path in place, so the pre-fix copyFileSync hit a same-file collision and threw instead of leaking. That draft passed against the bug it was written to catch; this one fails against it. Adds the missing rollback() coverage as well: a restored symlinked managed path must come back as a link pointing at its original target. Co-Authored-By: Claude Opus 4.8 * test(#2470): read the backup location from the journal, not the plan The new backup-content assertion read backupRelPath off result.plan.actions, where it is always null: the planner reserves the field and apply chooses the concrete location, recording it in the journal. The assertion therefore failed on "backup path must be recorded for the user" rather than on anything about the behavior it was written to check. Read it from the journal, which is the authoritative record. Verified by executing all four new test bodies in-process against the built engine — the backup file exists and holds the locally patched content. Co-Authored-By: Claude Opus 4.8 * chore(#2470): backfill changeset pr number to 2478 * chore(#2470): backfill changeset pr number to 2478 --------- Co-authored-by: Claude Opus 4.8 --- .changeset/happy-bears-gather.md | 5 + .changeset/noble-newts-roam.md | 5 + .gitignore | 1 + bin/install.js | 3 +- capabilities/pi/capability.json | 4 +- docs/how-to/install-on-your-runtime.md | 4 +- docs/installer-migrations.md | 2 + .../host-integration-capability-matrix.md | 6 +- gsd-core/bin/lib/capability-registry.cjs | 8 +- package.json | 3 +- pi/gsd.cjs | 10 +- src/install-engine.cts | 17 +- src/installer-migrations.cts | 50 +++- .../006-pi-extension-cjs-to-js.cts | 129 +++++++++ tests/fixtures/golden-install-parity/pi.json | 2 +- tests/fixtures/install-tree/pi.json | 2 +- tests/install-minimal-hooks.test.cjs | 23 +- tests/install-write-confinement.test.cjs | 101 ++++++++ ...ler-migration-install.integration.test.cjs | 14 +- ...taller-migration-pi-extension-ext.test.cjs | 231 +++++++++++++++++ tests/installer-migrations.test.cjs | 245 ++++++++++++++++++ tests/pi-upgrades.test.cjs | 50 ++++ 22 files changed, 885 insertions(+), 30 deletions(-) create mode 100644 .changeset/happy-bears-gather.md create mode 100644 .changeset/noble-newts-roam.md create mode 100644 src/installer-migrations/006-pi-extension-cjs-to-js.cts create mode 100644 tests/installer-migration-pi-extension-ext.test.cjs diff --git a/.changeset/happy-bears-gather.md b/.changeset/happy-bears-gather.md new file mode 100644 index 000000000..dd4170e4c --- /dev/null +++ b/.changeset/happy-bears-gather.md @@ -0,0 +1,5 @@ +--- +type: Security +pr: 2478 +--- +**Patched a transitive denial-of-service advisory in the production dependency tree** — `body-parser` reached GSD via the Claude Agent SDK's MCP dependency and, on versions through 2.2.2, silently stopped enforcing request size limits when given an invalid limit value (GHSA-v422-hmwv-36x6). Pinned to >=2.3.0. (#2470) diff --git a/.changeset/noble-newts-roam.md b/.changeset/noble-newts-roam.md new file mode 100644 index 000000000..8362aeccc --- /dev/null +++ b/.changeset/noble-newts-roam.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 2478 +--- +**`/gsd` now registers on pi** — installing GSD for pi wrote its extension as `gsd.cjs`, a suffix pi's extension auto-discovery skips silently, so `/gsd` never appeared and nothing reported an error. The extension now installs as `gsd.js`, and upgrading removes the stale `gsd.cjs`. (#2470) diff --git a/.gitignore b/.gitignore index 98e345a05..53c018a18 100644 --- a/.gitignore +++ b/.gitignore @@ -152,6 +152,7 @@ build/ /gsd-core/bin/lib/installer-migrations/003-rename-get-shit-done-to-gsd-core.cjs /gsd-core/bin/lib/installer-migrations/004-prune-stale-pristine-snapshots.cjs /gsd-core/bin/lib/installer-migrations/005-opencode-baseline-commands-dir.cjs +/gsd-core/bin/lib/installer-migrations/006-pi-extension-cjs-to-js.cjs /gsd-core/bin/lib/observability/logger.cjs /gsd-core/bin/lib/active-workstream-store.cjs /gsd-core/bin/lib/adr-parser.cjs diff --git a/bin/install.js b/bin/install.js index d219cefce..bb05b8a2d 100755 --- a/bin/install.js +++ b/bin/install.js @@ -10524,7 +10524,8 @@ function install(isGlobal, runtime = DEFAULT_RUNTIME, options = {}) { } else if (_hostBehaviors(runtime).pluginOnlyInstall) { // pi (ADR-1239 / #2102 Stage 1): plugin-only install — pi's /gsd command is // registered programmatically by the native extension (pi/gsd.cjs → - // extensions/gsd.cjs, staged separately below) and dispatches in-process + // extensions/gsd.js, staged separately below; the dest suffix must be + // .ts/.js or pi's auto-discovery skips it silently — #2470) and dispatches in-process // through the embedded gsd-core command-routing hub. pi has no host-read // markdown surface (unlike Claude/OpenCode/etc., which scan commands/ or // command/ directories), so writing flat gsd-.md files here would be diff --git a/capabilities/pi/capability.json b/capabilities/pi/capability.json index a48edcfd8..045198a5e 100644 --- a/capabilities/pi/capability.json +++ b/capabilities/pi/capability.json @@ -3,7 +3,7 @@ "role": "runtime", "version": "1.7.0", "title": "pi", - "description": "pi (pi.dev) — bun-runtime programmatic-CLI; TS ExtensionAPI (registerCommand/registerTool/registerProvider/pi.on); single native-extension file at ~/.pi/agent/extensions/gsd.cjs; no shared-settings hook surface; tier-2 support.", + "description": "pi (pi.dev) — bun-runtime programmatic-CLI; TS ExtensionAPI (registerCommand/registerTool/registerProvider/pi.on); single native-extension file at ~/.pi/agent/extensions/gsd.js (.js, not .cjs — pi's extension auto-discovery accepts only .ts/.js, #2470); no shared-settings hook surface; tier-2 support.", "tier": "core", "requires": [], "engines": { @@ -51,7 +51,7 @@ "hostBehaviors": { "nativePlugin": { "dir": "extensions", - "file": "gsd.cjs", + "file": "gsd.js", "source": "pi/gsd.cjs" }, "pluginOnlyInstall": true diff --git a/docs/how-to/install-on-your-runtime.md b/docs/how-to/install-on-your-runtime.md index 4b77719ed..830e37cf7 100644 --- a/docs/how-to/install-on-your-runtime.md +++ b/docs/how-to/install-on-your-runtime.md @@ -469,7 +469,9 @@ npx @opengsd/gsd-core@latest --pi --global [pi](https://pi.dev) is a bun-runtime programmatic CLI whose extensions implement pi's own `ExtensionAPI` (`registerCommand`/`registerTool`/`registerProvider`/`pi.on`) rather than a settings-file or slash-markdown surface. GSD ships a single native-extension file: -- **Extension** → `~/.pi/agent/extensions/gsd.cjs` (global) or `.pi/extensions/gsd.cjs` (local) +- **Extension** → `~/.pi/agent/extensions/gsd.js` (global) or `.pi/extensions/gsd.js` (local) + +The `.js` suffix is load-bearing: pi auto-discovers extensions by scanning that directory and keeping only names ending in `.ts` or `.js`, and it skips anything else **silently** — no error, no log line. GSD shipped the file as `gsd.cjs` through 1.7.0, which pi therefore never loaded, so `/gsd` never appeared ([#2470](https://github.com/open-gsd/gsd-core/issues/2470)). Upgrading removes the stale `gsd.cjs`; if you had added a manual `extensions` entry in `~/.pi/agent/settings.json` as a workaround, you can drop it. The extension registers a `/gsd` command and a `gsd_invoke` tool that dispatch GSD commands via a bounded subprocess call to `gsd-core/bin/gsd-tools.cjs` (no fully-populated in-process command-routing hub exists — see the matrix's Stage 2 note). This is a **plugin-only install**: pi has no shared-settings hook surface (`hooksSurface: none`) and, unlike Claude/OpenCode/Kilo, no host-read markdown surface at all — pi's `/gsd` command is registered programmatically by the extension, not discovered from files, so GSD installs the extension plus its universal `gsd-core/` engine payload and the shared `hooks/`/`hooks/lib/` bundle (spawned by the extension itself, not by any config-file hook bus), and does **not** write any `commands/`, `agents/`, or `skills/` directory for pi. The extension bridges GSD's `session_start`/`before_agent_start`/`session_before_compact`/`tool_call` lifecycle events to those staged `hooks/` scripts as bounded, fail-open subprocesses, and steers pi's active model (`modelMode: active`) to a tier-resolved bare anthropic id via `pi.on('before_provider_request', ...)`. See the [`## pi`](host-integration-capability-matrix.md#pi) section of the host-integration capability matrix for the negotiated axes and citations. diff --git a/docs/installer-migrations.md b/docs/installer-migrations.md index 6fdae1b39..d15cb82e8 100644 --- a/docs/installer-migrations.md +++ b/docs/installer-migrations.md @@ -378,6 +378,7 @@ for the new shape before changing migration behavior. | Qwen Code | Claude-compatible skills in `skills/gsd-*/SKILL.md`; agents in `agents/`; optional common hook/settings integration through GSD | Global `QWEN_CONFIG_DIR` or `~/.qwen`; local `./.qwen` | GSD owns generated skills/agents/hooks and GSD settings entries only | [Qwen commands and skills](https://qwenlm.github.io/qwen-code-docs/en/users/features/commands/); docs last updated 2026-05-06 | | Hermes Agent | Category skills under `skills/gsd/` with `DESCRIPTION.md` plus nested `gsd-*/SKILL.md`; agents in `agents/`; optional common hook/settings integration through GSD | Global `HERMES_HOME` or `~/.hermes`; local `./.hermes` | GSD owns generated `skills/gsd/` category content, generated agents, and GSD settings entries only | [Hermes configuration](https://hermes-agent.nousresearch.com/docs/user-guide/configuration), [Hermes skills](https://hermes-agent.nousresearch.com/docs/zh-Hans/user-guide/features/skills), [working with skills](https://hermes-agent.nousresearch.com/docs/guides/work-with-skills); docs checked 2026-05-11 | | CodeBuddy | Skills in `skills/gsd-*/SKILL.md`; agents in `agents/`; optional common hook/settings integration through GSD | Global `CODEBUDDY_CONFIG_DIR` or `~/.codebuddy`; local `./.codebuddy` | GSD owns generated skills/agents/hooks and GSD settings entries only | [CodeBuddy CLI skills](https://www.codebuddy.ai/docs/cli/skills), [CodeBuddy IDE skills](https://www.codebuddy.ai/docs/ide/Features/Skills); docs checked 2026-05-11 | +| pi | A single native extension at `extensions/gsd.js` (registers `/gsd` + the `gsd_invoke` tool programmatically); the shared `hooks/` + `hooks/lib/` bundle the extension spawns as bounded subprocesses; the `gsd-core/` payload. No commands/agents/skills surface (`pluginOnlyInstall`) | Global `~/.pi/agent`; local `./.pi` | GSD owns only the generated extension file, the installed `hooks/`/`hooks/lib/` bundle, and the `gsd-core/` payload. GSD writes **no** pi config: `configFormat: "none"`, `hooksSurface: "none"`, `writesSharedSettings: false` — `~/.pi/agent/settings.json` is entirely user-owned and must never be rewritten, including its `extensions` array. Other users' extensions in `extensions/` are unknown files and are preserved | [pi extension loader](https://github.com/earendil-works/pi/blob/main/packages/coding-agent/src/core/extensions/loader.ts): `discoverExtensionsInDir()` scans `/extensions/` and keeps only names passing `isExtensionFile()` (`.ts`/`.js`); accepted files load through `jiti`, which handles CommonJS and ESM alike, so the suffix — not the module format — is what gates discovery. Explicit paths in `settings.json` bypass the filter. Source read 2026-07-20 against `@earendil-works/pi-coding-agent` 0.80.10 (#2470) | | Cline | Rule-based integration via `.clinerules` for current installer output | Global `CLINE_CONFIG_DIR` or `~/.cline`; local project root `.clinerules` | GSD owns the generated `.clinerules` file only when it created or manifest-tracked it; no hooks/statusline ownership | [Cline rules](https://docs.cline.bot/customization/cline-rules); docs prefer `.clinerules/` directory and still detect legacy rule files, checked 2026-05-11 | ### Registry Authoring Rules @@ -502,6 +503,7 @@ Each row corresponds to one migration record in `src/installer-migrations/`. | `2026-06-02-rename-get-shit-done-to-gsd-core` | `003-rename-get-shit-done-to-gsd-core.cts` | 1.2.0 | global, local | Yes | Removes managed files from the stale `get-shit-done/` runtime directory after the rename to `gsd-core/` (#604). User-added files are preserved; emptied directories may remain (framework limitation). | | `2026-06-09-prune-stale-pristine-get-shit-done` | `004-prune-stale-pristine-snapshots.cts` | 1.4.3 | global, local | Yes | Removes stale `gsd-pristine/get-shit-done/` snapshot files left behind by migration 003, which caused false `verify-reapply-patches` failures (#934). | `2026-07-17-opencode-baseline-commands-dir` | `005-opencode-baseline-commands-dir.cts` | 1.7.0 | global, local | No | Baselines pre-existing files under OpenCode's `commands/` (plural) directory during the first-time scan. #2329 moved OpenCode command materialization to `commands/`, but 000's `RUNTIME_SURFACES.opencode` is a shipped, immutable body that still only names the legacy `command/` alias, so this fix-forward migration widens the scanned surface. OpenCode only; Kilo is unaffected. | +| `2026-07-20-pi-extension-cjs-to-js` | `006-pi-extension-cjs-to-js.cts` | 1.7.1 | global, local | Yes | Removes the stale `extensions/gsd.cjs` left by pre-#2470 pi installs. pi's extension auto-discovery (`isExtensionFile()`) accepts only `.ts`/`.js`, so the `.cjs` file was never loaded and `/gsd` never registered; #2470 renamed the installed artifact to `extensions/gsd.js`, orphaning the old path. Locally modified copies are backed up rather than deleted; an unmanifested `gsd.cjs` is preserved as a user file. pi only. | ## Prior Art diff --git a/docs/reference/host-integration-capability-matrix.md b/docs/reference/host-integration-capability-matrix.md index 819e1fead..b5f8d0b3e 100644 --- a/docs/reference/host-integration-capability-matrix.md +++ b/docs/reference/host-integration-capability-matrix.md @@ -657,7 +657,7 @@ EoS migration status (#2101, ADR-1239): ZCode's install is fully dogfooded throu ## pi -> pi (pi.dev) is a bun-runtime Programmatic-CLI: it exposes an in-process TypeScript `ExtensionAPI` (`registerCommand`/`registerTool`/`registerProvider`/`pi.on`) rather than a settings-file or slash-markdown surface. GSD ships a single native-extension file (`pi/gsd.cjs`) installed to `~/.pi/agent/extensions/gsd.cjs` (global) or `.pi/extensions/gsd.cjs` (local) — the programmatic-CLI peer of the OpenCode/Kilo native-plugin binding. **Sourcing note:** the citations below are the pi.dev documentation pages named in ADR-1239 Stage 1 (#2102) as the source for each axis; this environment did not have live doc-fetch access at authoring time, so the Evidence column below is a paraphrase of pi's documented extension model rather than a verbatim excerpt — a maintainer with Context7/web access should verify the exact wording before treating this section as fully cited (flagged in the #2102 PR). +> pi (pi.dev) is a bun-runtime Programmatic-CLI: it exposes an in-process TypeScript `ExtensionAPI` (`registerCommand`/`registerTool`/`registerProvider`/`pi.on`) rather than a settings-file or slash-markdown surface. GSD ships a single native-extension file (`pi/gsd.cjs`) installed to `~/.pi/agent/extensions/gsd.js` (global) or `.pi/extensions/gsd.js` (local) — the programmatic-CLI peer of the OpenCode/Kilo native-plugin binding. **Sourcing note:** the citations below are the pi.dev documentation pages named in ADR-1239 Stage 1 (#2102) as the source for each axis; this environment did not have live doc-fetch access at authoring time, so the Evidence column below is a paraphrase of pi's documented extension model rather than a verbatim excerpt — a maintainer with Context7/web access should verify the exact wording before treating this section as fully cited (flagged in the #2102 PR). **Partially discharged (#2470, 2026-07-20):** pi's extension-loader contract specifically has now been read at source — `packages/coding-agent/src/core/extensions/loader.ts` in `earendil-works/pi` — confirming `discoverExtensionsInDir()` keeps only names passing `isExtensionFile()` (`.ts`/`.js`, everything else skipped silently), that accepted files load via `jiti` (CommonJS and ESM alike), and that explicit `settings.json` paths bypass the filter. The remaining axes below are still paraphrase. | Axis | Value | Source | Evidence | |---|---|---|---| @@ -685,11 +685,11 @@ Documentation gaps: - dispatch.maxDepth / dispatch.background / dispatch.backgroundDispatch — recorded as `0`/`false`/`false` (not `undocumented`) because the absence of any dispatch primitive is itself the documented ceiling, matching `shouldFlattenDispatch`'s fail-closed default. - This section's Evidence-column wording was authored without live Context7/web-fetch access (see the sourcing note above the table) — verify against the cited pi.dev pages before relying on it for a future capability upgrade. -EoS migration status (#2102 Stage 1, ADR-1239): pi lands as a NET-NEW installable runtime — pure additive descriptor + installer wiring, no prior `runtime === 'pi'` branches existed to fold. `artifactLayout` is declared empty (`global: []`, `local: []`) — pi has no skills/commands/agents layout, and installs as **PLUGIN-ONLY**: `hostBehaviors.pluginOnlyInstall: true` explicitly skips `bin/install.js`'s generic flat-commands-and-agents fallback (the legacy path Claude Code's LOCAL layout also uses), which would otherwise write inert `commands/gsd-.md` + `agents/gsd-.md` reference files no part of pi ever reads. pi's `/gsd` command and `gsd_invoke` tool are registered **programmatically** by the native extension (`pi/gsd.cjs` → `extensions/gsd.cjs`, mirroring OpenCode/Kilo's `nativePlugin` shape) — pi has no host-read markdown surface at all (unlike Claude/OpenCode/Kilo, which scan a `commands/`/`command/` directory), so a declarative artifact surface would be dead weight, not merely unused. `dispatch.subagentToolkit: "undocumented"` and `dispatch.backgroundDispatch: false` are both required by the capability validator's dispatch schema and reflect that pi has no documented named-dispatch primitive at all. (Stage 1 originally also set `hostBehaviors.skipSharedHooksInstall:true`, reasoning the staged `hooks/*.js` bundle would be dead weight for pi the way it genuinely is for Kilo/ZCode — **corrected in Stage 2 below**: pi's native extension DOES spawn them, so they are live, not dead, and the flag was removed.) +EoS migration status (#2102 Stage 1, ADR-1239): pi lands as a NET-NEW installable runtime — pure additive descriptor + installer wiring, no prior `runtime === 'pi'` branches existed to fold. `artifactLayout` is declared empty (`global: []`, `local: []`) — pi has no skills/commands/agents layout, and installs as **PLUGIN-ONLY**: `hostBehaviors.pluginOnlyInstall: true` explicitly skips `bin/install.js`'s generic flat-commands-and-agents fallback (the legacy path Claude Code's LOCAL layout also uses), which would otherwise write inert `commands/gsd-.md` + `agents/gsd-.md` reference files no part of pi ever reads. pi's `/gsd` command and `gsd_invoke` tool are registered **programmatically** by the native extension (`pi/gsd.cjs` → `extensions/gsd.js`, mirroring OpenCode/Kilo's `nativePlugin` shape) — pi has no host-read markdown surface at all (unlike Claude/OpenCode/Kilo, which scan a `commands/`/`command/` directory), so a declarative artifact surface would be dead weight, not merely unused. `dispatch.subagentToolkit: "undocumented"` and `dispatch.backgroundDispatch: false` are both required by the capability validator's dispatch schema and reflect that pi has no documented named-dispatch primitive at all. (Stage 1 originally also set `hostBehaviors.skipSharedHooksInstall:true`, reasoning the staged `hooks/*.js` bundle would be dead weight for pi the way it genuinely is for Kilo/ZCode — **corrected in Stage 2 below**: pi's native extension DOES spawn them, so they are live, not dead, and the flag was removed.) EoS migration status (#2102 Stage 2, ADR-1239): Stage 1's "in-process `gsd-core` command-routing hub" framing was aspirational and is corrected here — no fully-populated hub factory exists anywhere in gsd-core (every `createHub()` caller in the tree builds a single-family hub for its own narrow purpose), so `/gsd` and `gsd_invoke` instead dispatch via **SUBPROCESS REUSE**: `dispatchGsdCommand` (`src/shell-command-projection.cts`) spawns `gsd-core/bin/gsd-tools.cjs [subcommand] ... --cwd --raw --json-errors` bounded and non-throwing, mirroring the precedent already established for the OpenCode/Kilo hook bridge (`.opencode/plugins/gsd-core.js`'s "Architecture: SUBPROCESS REUSE" header). The companion MCP server's `gsd_invoke_command` tool dispatches through the SAME shared helper (it had the identical `createHub()`-with-no-args bug). `/gsd`'s command handler is `handler(args, ctx)` (pi's real ExtensionAPI shape — a raw args string, not `execute(ctx)`); `gsd_invoke`'s tool handler is the real 5-arg `execute(toolCallId, params, signal, onUpdate, ctx)`. The event surface (`EXTENSION_EVENT_SURFACES.pi`, `src/host-integration.cts`) now declares the full ~30-event pi ExtensionAPI vocabulary (was a placeholder `['tool_call']`), and `pi/gsd.cjs` binds `session_start` (→ `gsd-ensure-canonical-path.js`), `before_agent_start` (→ `gsd-workflow-guard.js`, a forward-compatible no-op today since that hook's triggers are tool-scoped), `session_before_compact` (→ `gsd-context-monitor.js`), and `tool_call`, each as a bounded fail-open `spawnSync` subprocess (mirroring `.opencode/plugins/gsd-core.js`'s `runHook`). `modelMode: active` is realized via `pi.on('before_provider_request', ...)`, which resolves a tier through the model-catalog's now-populated `runtimeTierDefaults.pi` entries (bare anthropic ids — `claude-opus-4-8`/`claude-sonnet-5`/`claude-haiku-4-5`, matching the `claude` runtime's own ids since pi talks the anthropic API) and returns a modified payload, or `undefined` (fail-open, pi's model left untouched) when resolution comes back null — **not** `registerProvider`, which would register a new model provider rather than steering pi's existing built-in anthropic models. -**Adversarial-review correction (#2102 Stage 2, post-review):** the event bridges above and the `/gsd` tokenizer's `hooks/lib/git-cmd.js` require were DEAD in a real install — Stage 1's `hostBehaviors.skipSharedHooksInstall:true` meant pi shipped NO `hooks/` directory at all, so `runHook('gsd-ensure-canonical-path.js', ...)` etc. always hit the "hook file absent → silent no-op" branch, and the tokenizer always fell back to plain whitespace-splitting. The tests masked this because they run against the dev tree, where `hooks/` genuinely exists. **Fix:** `capabilities/pi/capability.json` no longer sets `skipSharedHooksInstall` — pi is architecturally identical to OpenCode here (`hooksSurface: "none"` + a native extension that spawns the staged hooks), not to Kilo/ZCode (`hooksSurface: "none"` with NO plugin surface, where the same hooks genuinely are dead weight). pi now installs `hooks/` + `hooks/lib/` (27 entries: the same `INSTALLED_HOOK_FILES` set OpenCode gets) alongside `extensions/gsd.cjs`, verified end-to-end via a real `node bin/install.js --pi --global`/`--local` — `resolveEngineRoot`'s walk-up from the installed extension's own directory finds `ENGINE_ROOT/hooks/{gsd-ensure-canonical-path.js,gsd-workflow-guard.js,gsd-context-monitor.js,lib/git-cmd.js}`, and each bridge/`runHook` call exits 0 against the real installed files. `hooksSurface: "none"` + `configFormat: "none"` + `writesSharedSettings: false` are unaffected — no settings/hooks.json/config.toml is written for pi; the extension spawns hooks by absolute path, not via a config-file hook bus. `tests/fixtures/golden-install-parity/pi.json` grew from 292 → 320 entries (the 28 new `hooks/`/`hooks/lib/` files); `commands/`, `agents/`, `skills/` remain absent (`pluginOnlyInstall` is untouched — it only gates the declarative-markdown surfaces, not hooks). `tests/install-minimal-hooks.test.cjs`'s #1821 suite moved pi from the Kilo/ZCode (no-hooks) group into the OpenCode (ships-hooks) group accordingly. +**Adversarial-review correction (#2102 Stage 2, post-review):** the event bridges above and the `/gsd` tokenizer's `hooks/lib/git-cmd.js` require were DEAD in a real install — Stage 1's `hostBehaviors.skipSharedHooksInstall:true` meant pi shipped NO `hooks/` directory at all, so `runHook('gsd-ensure-canonical-path.js', ...)` etc. always hit the "hook file absent → silent no-op" branch, and the tokenizer always fell back to plain whitespace-splitting. The tests masked this because they run against the dev tree, where `hooks/` genuinely exists. **Fix:** `capabilities/pi/capability.json` no longer sets `skipSharedHooksInstall` — pi is architecturally identical to OpenCode here (`hooksSurface: "none"` + a native extension that spawns the staged hooks), not to Kilo/ZCode (`hooksSurface: "none"` with NO plugin surface, where the same hooks genuinely are dead weight). pi now installs `hooks/` + `hooks/lib/` (27 entries: the same `INSTALLED_HOOK_FILES` set OpenCode gets) alongside `extensions/gsd.js`, verified end-to-end via a real `node bin/install.js --pi --global`/`--local` — `resolveEngineRoot`'s walk-up from the installed extension's own directory finds `ENGINE_ROOT/hooks/{gsd-ensure-canonical-path.js,gsd-workflow-guard.js,gsd-context-monitor.js,lib/git-cmd.js}`, and each bridge/`runHook` call exits 0 against the real installed files. `hooksSurface: "none"` + `configFormat: "none"` + `writesSharedSettings: false` are unaffected — no settings/hooks.json/config.toml is written for pi; the extension spawns hooks by absolute path, not via a config-file hook bus. `tests/fixtures/golden-install-parity/pi.json` grew from 292 → 320 entries (the 28 new `hooks/`/`hooks/lib/` files); `commands/`, `agents/`, `skills/` remain absent (`pluginOnlyInstall` is untouched — it only gates the declarative-markdown surfaces, not hooks). `tests/install-minimal-hooks.test.cjs`'s #1821 suite moved pi from the Kilo/ZCode (no-hooks) group into the OpenCode (ships-hooks) group accordingly. ## vscode diff --git a/gsd-core/bin/lib/capability-registry.cjs b/gsd-core/bin/lib/capability-registry.cjs index d24f37c7d..1a40c9116 100644 --- a/gsd-core/bin/lib/capability-registry.cjs +++ b/gsd-core/bin/lib/capability-registry.cjs @@ -2130,7 +2130,7 @@ const capabilities = { "role": "runtime", "version": "1.7.0", "title": "pi", - "description": "pi (pi.dev) — bun-runtime programmatic-CLI; TS ExtensionAPI (registerCommand/registerTool/registerProvider/pi.on); single native-extension file at ~/.pi/agent/extensions/gsd.cjs; no shared-settings hook surface; tier-2 support.", + "description": "pi (pi.dev) — bun-runtime programmatic-CLI; TS ExtensionAPI (registerCommand/registerTool/registerProvider/pi.on); single native-extension file at ~/.pi/agent/extensions/gsd.js (.js, not .cjs — pi's extension auto-discovery accepts only .ts/.js, #2470); no shared-settings hook surface; tier-2 support.", "tier": "core", "requires": [], "engines": { @@ -2178,7 +2178,7 @@ const capabilities = { "hostBehaviors": { "nativePlugin": { "dir": "extensions", - "file": "gsd.cjs", + "file": "gsd.js", "source": "pi/gsd.cjs" }, "pluginOnlyInstall": true @@ -5159,7 +5159,7 @@ const runtimes = { "role": "runtime", "version": "1.7.0", "title": "pi", - "description": "pi (pi.dev) — bun-runtime programmatic-CLI; TS ExtensionAPI (registerCommand/registerTool/registerProvider/pi.on); single native-extension file at ~/.pi/agent/extensions/gsd.cjs; no shared-settings hook surface; tier-2 support.", + "description": "pi (pi.dev) — bun-runtime programmatic-CLI; TS ExtensionAPI (registerCommand/registerTool/registerProvider/pi.on); single native-extension file at ~/.pi/agent/extensions/gsd.js (.js, not .cjs — pi's extension auto-discovery accepts only .ts/.js, #2470); no shared-settings hook surface; tier-2 support.", "tier": "core", "requires": [], "engines": { @@ -5207,7 +5207,7 @@ const runtimes = { "hostBehaviors": { "nativePlugin": { "dir": "extensions", - "file": "gsd.cjs", + "file": "gsd.js", "source": "pi/gsd.cjs" }, "pluginOnlyInstall": true diff --git a/package.json b/package.json index 4aa9f1425..b50241829 100644 --- a/package.json +++ b/package.json @@ -70,7 +70,8 @@ "typescript-eslint": "^8.60.0" }, "overrides": { - "qs": ">=6.15.2" + "qs": ">=6.15.2", + "body-parser": ">=2.3.0" }, "optionalDependencies": { "fallow": "^2.70.0" diff --git a/pi/gsd.cjs b/pi/gsd.cjs index 9953d35fe..b1ea2b6c1 100644 --- a/pi/gsd.cjs +++ b/pi/gsd.cjs @@ -9,8 +9,14 @@ * This extension binds GSD's command surface to pi via the imperative adapter * path — the programmatic-CLI peer of the OpenCode worked binding. * - * Installation: copy this file to ~/.pi/agent/extensions/gsd.cjs (pi loads - * extensions via jiti from that dir). The engine is resolved from the installed + * Installation: copy this file to ~/.pi/agent/extensions/gsd.js — note the + * `.js` DEST suffix, not `.cjs`. pi auto-discovers extensions/ entries through + * `isExtensionFile()`, which accepts only `.ts`/`.js` and skips anything else + * SILENTLY (no error, no log line), so a `.cjs` dest installs fine and is then + * never loaded (#2470). This source file keeps its `.cjs` suffix on purpose — + * tests `require()` it directly and `.cjs` is unambiguous CommonJS — and pi + * loads the copied file via jiti, which handles CommonJS and ESM alike, so the + * suffix gates discovery, not parsing. The engine is resolved from the installed * GSD tree (walk-up like the OpenCode plugin). pi's shared hooks/ bundle * (hooks/*.js + hooks/lib/git-cmd.js) is installed alongside the extension — * capabilities/pi/capability.json does NOT set diff --git a/src/install-engine.cts b/src/install-engine.cts index ee9387325..cf1ceab01 100644 --- a/src/install-engine.cts +++ b/src/install-engine.cts @@ -1106,9 +1106,20 @@ function _installNativePluginIfDeclared( if (np && np.source) { const pluginSrc = path.join(src, np.source); if (fs.existsSync(pluginSrc)) { - const destDir = runtimeArtifactInstallPlan.assertDestWithinConfigHome(configDir, np.dir); - fs.mkdirSync(destDir, { recursive: true }); - fs.copyFileSync(pluginSrc, path.join(destDir, np.file)); + // 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 + // would have written outside configHome. Not reachable today — descriptors + // are first-party and compiled into the capability registry at build time — + // but `np.file` is exactly the field #2470 changes, and the guard costs + // nothing. For a well-formed descriptor this resolves identically to the + // previous mkdir(dir) + join(dir, file). + const destPath = runtimeArtifactInstallPlan.assertDestWithinConfigHome( + configDir, + path.join(np.dir, np.file), + ); + fs.mkdirSync(path.dirname(destPath), { recursive: true }); + fs.copyFileSync(pluginSrc, destPath); } } } diff --git a/src/installer-migrations.cts b/src/installer-migrations.cts index 2232401f2..fd4e7e2f7 100644 --- a/src/installer-migrations.cts +++ b/src/installer-migrations.cts @@ -46,6 +46,38 @@ function sha256Text(value: string): string { return crypto.createHash('sha256').update(value).digest('hex'); } +/** + * Copy a managed path for the rollback snapshot or the user-facing backup, + * WITHOUT dereferencing a symlink. + * + * `fs.copyFileSync` follows symlinks, so a managed path that has been replaced + * by a link (tampering, or an unexpected user layout) would have had the + * LINK TARGET's bytes copied into `gsd-migration-journal/…-backups/` — e.g. a + * `gsd.cjs` symlinked at `~/.ssh/id_rsa` would land that key's contents in the + * backup tree. Nothing GSD installs is ever a symlink, so the faithful snapshot + * of a symlinked managed path is the link itself: recreating it preserves + * rollback fidelity (restore re-creates the same link) while never reading the + * referent. Deletion was already safe — `fs.rmSync` unlinks the link, never the + * target. + * + * Windows note: `fs.symlinkSync` can throw EPERM for unprivileged users. That + * surfaces as an apply failure and triggers the normal rollback path, which is + * the correct outcome — refusing to proceed beats silently copying referent + * bytes. + */ +function copyPreservingSymlink(srcPath: string, destPath: string): void { + if (fs.lstatSync(srcPath).isSymbolicLink()) { + // symlinkSync fails with EEXIST on an occupied path, so clear it first. + // Scoped to this branch on purpose: the regular-file path below keeps + // copyFileSync's overwrite-in-place, so a mid-restore failure cannot leave + // the destination destroyed. + fs.rmSync(destPath, { force: true }); + fs.symlinkSync(fs.readlinkSync(srcPath), destPath); + return; + } + fs.copyFileSync(srcPath, destPath); +} + function readJsonIfPresent(filePath: string, fallback: unknown): unknown { if (!fs.existsSync(filePath)) return fallback; try { @@ -627,9 +659,12 @@ function rollbackAppliedMigrationResult({ configDir, journal, journalPath, rollb const rollbackPath = path.join(configDir, action.rollbackRelPath as string); const dest = path.join(configDir, action.relPath as string); try { - if (fs.existsSync(rollbackPath)) { + // lstat-based existence check: a snapshot of a symlinked managed path is + // itself a link, and existsSync() follows it — a link whose target is + // gone would read as "missing" and silently skip the restore. + if (fs.lstatSync(rollbackPath, { throwIfNoEntry: false })) { fs.mkdirSync(path.dirname(dest), { recursive: true }); - fs.copyFileSync(rollbackPath, dest); + copyPreservingSymlink(rollbackPath, dest); } } catch (error) { failures.push({ relPath: action.relPath as string, error: (error as Error).message }); @@ -743,7 +778,7 @@ function applyInstallerMigrationPlan({ const rollbackPath = path.join(rollbackRoot, normalized); fs.mkdirSync(path.dirname(rollbackPath), { recursive: true }); - fs.copyFileSync(fullPath, rollbackPath); + copyPreservingSymlink(fullPath, rollbackPath); rollback.push({ relPath: normalized, rollbackPath }); if (action.type === 'rewrite-json') { @@ -765,7 +800,7 @@ function applyInstallerMigrationPlan({ const backupRelPath = action.backupRelPath || path.posix.join(backupRootRelPath, normalized); const backupPath = path.join(configDir, backupRelPath); fs.mkdirSync(path.dirname(backupPath), { recursive: true }); - fs.copyFileSync(fullPath, backupPath); + copyPreservingSymlink(fullPath, backupPath); journal.actions.push(journalAction(action, 'removed', { backupRelPath, rollbackRelPath: path.posix.join(rollbackRootRelPath, normalized), @@ -817,7 +852,12 @@ function applyInstallerMigrationPlan({ const dest = path.join(configDir, entry.relPath); try { fs.mkdirSync(path.dirname(dest), { recursive: true }); - fs.copyFileSync(entry.rollbackPath, dest); + // Symlink-preserving, same as the forward path: `entry.rollbackPath` is + // itself a link whenever the managed path was one, so a raw copy here + // would dereference it and write the referent's bytes back to the LIVE + // install path — a worse leak than the journal-tree one, since it is + // user-visible and at a predictable location. + copyPreservingSymlink(entry.rollbackPath, dest); } catch (rollbackError) { rollbackFailures.push({ relPath: entry.relPath, diff --git a/src/installer-migrations/006-pi-extension-cjs-to-js.cts b/src/installer-migrations/006-pi-extension-cjs-to-js.cts new file mode 100644 index 000000000..ad890757b --- /dev/null +++ b/src/installer-migrations/006-pi-extension-cjs-to-js.cts @@ -0,0 +1,129 @@ +/** + * Installer migration: retire pi's stale `extensions/gsd.cjs` after #2470 + * renamed the installed native extension to `extensions/gsd.js`. + * + * What old artifact is being retired? + * `extensions/gsd.cjs` — the pre-#2470 dest filename for pi's native + * extension. pi auto-discovers extensions by scanning `/extensions/` + * and keeping only names accepted by its own predicate + * (`isExtensionFile()` in @earendil-works/pi-coding-agent: + * `name.endsWith(".ts") || name.endsWith(".js")`). A `.cjs` file is skipped + * SILENTLY — no `/gsd` command, no error, no log line. The file is therefore + * permanently inert, not merely redundant. + * + * How do we prove it is GSD-owned? + * The installer records the native plugin in the install manifest as + * `/` (bin/install.js, the + * `_hostBehaviors(runtime).nativePlugin` manifest block), so a pre-#2470 pi + * install carries `extensions/gsd.cjs` as a manifest-managed entry. Only a + * manifest-managed classification produces an action here; an unmanifested + * `gsd.cjs` is treated as a user's own file and preserved. + * + * What happens if the user modified it? + * `backup-and-remove` instead of `remove-managed`, so a patched extension is + * recoverable from the backup rather than silently destroyed. + * + * What happens if it is missing? + * No actions — fresh (post-#2470) installs and already-migrated installs both + * plan empty, so the migration is idempotent. + * + * What runtime and scope does it affect? + * pi only, global and local. No other runtime ever installed this path: + * OpenCode and Kilo — the only other runtimes declaring + * `hostBehaviors.nativePlugin` — both ship `plugins/gsd-core.js`. + * + * Is the action safe in non-interactive install? + * Yes. Both emitted action types are non-interactive and journaled; neither + * requires a user choice, and unknown files never produce an action. + * + * Why not `move-managed`? The installer materializes the new `extensions/gsd.js` + * from the package payload in the same run, so moving the stale file onto that + * path would just be overwritten. Retiring the old path is the accurate + * description of the change. + * + * See docs/installer-migrations.md#shipped-migrations and the pi row of + * docs/installer-migrations.md#runtime-configuration-contract-registry. + */ + +type ArtifactClassification = string; + +interface ClassifiedArtifact { + classification: ArtifactClassification; + [key: string]: unknown; +} + +type ActionType = 'remove-managed' | 'backup-and-remove'; + +interface MigrationAction { + type: ActionType; + relPath: string; + reason: string; + ownershipEvidence: string; +} + +interface MigrationPlanContext { + classifyArtifact(relPath: string): ClassifiedArtifact; +} + +interface InstallerMigration { + id: string; + title: string; + description: string; + introducedIn: string; + runtimes: string[]; + scopes: string[]; + destructive: boolean; + plan: (ctx: MigrationPlanContext) => MigrationAction[]; +} + +/** Pre-#2470 dest filename for pi's native extension. */ +const STALE_PI_EXTENSION = 'extensions/gsd.cjs'; + +const OWNERSHIP_EVIDENCE = + 'pre-#2470 pi installs record the native extension at extensions/gsd.cjs in ' + + 'gsd-file-manifest.json (installer nativePlugin manifest entry)'; + +const REASON = + 'pi cannot auto-discover a .cjs extension (isExtensionFile accepts only .ts/.js), ' + + 'so this file is inert; superseded by extensions/gsd.js (#2470)'; + +const migration: InstallerMigration = { + id: '2026-07-20-pi-extension-cjs-to-js', + title: 'Retire pi\'s undiscoverable extensions/gsd.cjs', + description: + 'Remove the stale extensions/gsd.cjs left by pre-#2470 pi installs, superseded by ' + + 'extensions/gsd.js — the suffix pi\'s extension auto-discovery actually accepts.', + introducedIn: '1.7.1', + runtimes: ['pi'], + scopes: ['global', 'local'], + destructive: true, + plan: (ctx: MigrationPlanContext): MigrationAction[] => { + const artifact = ctx.classifyArtifact(STALE_PI_EXTENSION); + if (artifact.classification === 'managed-pristine') { + return [ + { + type: 'remove-managed', + relPath: STALE_PI_EXTENSION, + reason: REASON, + ownershipEvidence: OWNERSHIP_EVIDENCE, + }, + ]; + } + if (artifact.classification === 'managed-modified') { + return [ + { + type: 'backup-and-remove', + relPath: STALE_PI_EXTENSION, + reason: REASON, + ownershipEvidence: OWNERSHIP_EVIDENCE, + }, + ]; + } + // 'unknown' (never GSD-managed), 'missing', and 'managed-missing' all plan + // nothing: unknown files are preserved by policy, and an absent file needs + // no retirement. + return []; + }, +}; + +export = migration; diff --git a/tests/fixtures/golden-install-parity/pi.json b/tests/fixtures/golden-install-parity/pi.json index 80143d591..d6dcae10d 100644 --- a/tests/fixtures/golden-install-parity/pi.json +++ b/tests/fixtures/golden-install-parity/pi.json @@ -1,7 +1,7 @@ { ".gsd-profile": "0e716a5fef4e6dc1", ".gsd/defaults.json": "615261cfd3ae1c96", - "extensions/gsd.cjs": "619cec0af9cfdadf", + "extensions/gsd.js": "976c6546391caf8f", "gsd-core/.gsd-runtime": "94e95f0bb38f8e0f", "gsd-core/VERSION": "ef0deccd81a6723c", "gsd-core/bin/check-latest-version.cjs": "e4a224058c8f4d74", diff --git a/tests/fixtures/install-tree/pi.json b/tests/fixtures/install-tree/pi.json index 05793d57c..9c216cd10 100644 --- a/tests/fixtures/install-tree/pi.json +++ b/tests/fixtures/install-tree/pi.json @@ -1,7 +1,7 @@ [ ".gsd-profile", ".gsd/defaults.json", - "extensions/gsd.cjs", + "extensions/gsd.js", "gsd-core/.gsd-runtime", "gsd-core/VERSION", "gsd-core/bin/check-latest-version.cjs", diff --git a/tests/install-minimal-hooks.test.cjs b/tests/install-minimal-hooks.test.cjs index cccc896e0..7fae2a031 100644 --- a/tests/install-minimal-hooks.test.cjs +++ b/tests/install-minimal-hooks.test.cjs @@ -678,7 +678,7 @@ describe('#1755: .sh hooks are copied and executable after install', () => { // hooksSurface:'none', but each has a native plugin adapter that spawns the // staged hooks/*.js scripts as subprocesses (OpenCode's #1914 // plugins/gsd-core.js via OpenCode's event bus; pi's #2102 Stage 2 pi/gsd.cjs -// → extensions/gsd.cjs via pi.on(...) bridges; Kilo's plugins/gsd-core.js, +// → extensions/gsd.js via pi.on(...) bridges; Kilo's plugins/gsd-core.js, // byte-identical to OpenCode's) — so for all three, the hooks are LIVE and // must keep being copied. ZCode has no plugin surface at all, so its staged // hooks are genuinely dead: that is the case #1821's fix removes. (#1821 @@ -766,7 +766,7 @@ describe('#1821/#2305: ZCode receives no dead hook files; Kilo/OpenCode/Claude k }); // pi ALSO declares hooksSurface:'none', but — like OpenCode — it is NOT a - // dead-weight case: pi's native extension (pi/gsd.cjs → extensions/gsd.cjs) + // dead-weight case: pi's native extension (pi/gsd.cjs → extensions/gsd.js) // spawns the staged hooks/*.js scripts as bounded subprocesses (session_start // → gsd-ensure-canonical-path.js, before_agent_start → gsd-workflow-guard.js, // session_before_compact → gsd-context-monitor.js — #2102 Stage 2), and its @@ -775,8 +775,20 @@ describe('#1821/#2305: ZCode receives no dead hook files; Kilo/OpenCode/Claude k // pi (unlike Kilo/ZCode/Cursor/Cline/Trae/Copilot/Windsurf/Kimi) — pi is in // the OpenCode group, not the Kilo/ZCode group. test('pi --global install still copies hooks (spawned by the native extension) + hooks/lib/git-cmd.js + the extension itself', () => { + // #2470: derive the extension filename from pi's own descriptor rather than + // hardcoding it, and assert it satisfies pi's isExtensionFile() discovery + // filter (.ts/.js only) — a dest pi cannot discover installs "successfully" + // while /gsd never registers, which is exactly how this shipped broken. + const piNativePlugin = JSON.parse( + fs.readFileSync(path.join(__dirname, '..', 'capabilities', 'pi', 'capability.json'), 'utf8'), + ).runtime.hostBehaviors.nativePlugin; + assert.ok( + piNativePlugin.file.endsWith('.ts') || piNativePlugin.file.endsWith('.js'), + `pi's installed extension "${piNativePlugin.file}" must end in .ts or .js — pi silently ` + + 'skips any other suffix during extensions/ auto-discovery (#2470)', + ); const { hookFiles, hooksLibExists, gitCmdExists, pluginExists } = installAndCollect('pi', { - pluginRelPath: path.join('extensions', 'gsd.cjs'), + pluginRelPath: path.join(piNativePlugin.dir, piNativePlugin.file), }); const basenames = hookFiles.map((f) => path.basename(f)); for (const expected of ['gsd-ensure-canonical-path.js', 'gsd-workflow-guard.js', 'gsd-context-monitor.js']) { @@ -787,7 +799,10 @@ describe('#1821/#2305: ZCode receives no dead hook files; Kilo/OpenCode/Claude k } assert.ok(hooksLibExists, 'pi install must create hooks/lib/'); assert.ok(gitCmdExists, 'pi install must copy hooks/lib/git-cmd.js (the /gsd command tokenizer)'); - assert.ok(pluginExists, 'pi install must install extensions/gsd.cjs (the native-extension hook bridge)'); + assert.ok( + pluginExists, + `pi install must install ${piNativePlugin.dir}/${piNativePlugin.file} (the native-extension hook bridge)`, + ); }); // Positive control: guards against over-exclusion breaking runtimes that diff --git a/tests/install-write-confinement.test.cjs b/tests/install-write-confinement.test.cjs index ac77e3b63..5a8345032 100644 --- a/tests/install-write-confinement.test.cjs +++ b/tests/install-write-confinement.test.cjs @@ -2964,3 +2964,104 @@ describe('#2393: GSD_ALLOW_SYMLINKED_DEST opt-in for intentional symlinked-dest } }); }); + +// --------------------------------------------------------------------------- +// _installNativePluginIfDeclared write-confinement (#2470) +// --------------------------------------------------------------------------- +// +// The native-plugin copy previously confined only `nativePlugin.dir`, then +// joined `nativePlugin.file` onto the validated directory unchecked. #2470 +// makes `file` a field we actively change (pi: gsd.cjs -> gsd.js), so the full +// dest path is now confined. Descriptors are first-party and compiled into the +// capability registry at build time, so this was never reachable in a shipped +// build — these tests keep it that way. + +describe('_installNativePluginIfDeclared write-confinement', () => { + const engine = require('../gsd-core/bin/lib/install-engine.cjs'); + + /** Build a src tree containing the declared plugin source. */ + function stageSource() { + const src = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-np-src-')); + const full = path.join(src, 'pi', 'gsd.cjs'); + fs.mkdirSync(path.dirname(full), { recursive: true }); + fs.writeFileSync(full, "'use strict';\n// plugin\n", 'utf8'); + return src; + } + + const behaviorsWith = (file) => ({ + nativePlugin: { dir: 'extensions', file, source: 'pi/gsd.cjs' }, + }); + + test('happy path: declared dir/file lands inside configDir', () => { + const configDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-np-cfg-')); + const src = stageSource(); + try { + engine._installNativePluginIfDeclared('pi', configDir, behaviorsWith('gsd.js'), src); + assert.ok( + fs.existsSync(path.join(configDir, 'extensions', 'gsd.js')), + 'plugin should be copied to /extensions/gsd.js', + ); + } finally { + cleanup(configDir); + cleanup(src); + } + }); + + const ESCAPE_CASES = [ + ['traversal', '../../evil.js'], + ['deep traversal', '../../../../../../tmp/evil.js'], + ['NUL byte', 'gsd\u0000.js'], + ]; + + for (const [label, file] of ESCAPE_CASES) { + test(`escape rejected: nativePlugin.file with ${label} → throws`, () => { + const configDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-np-esc-')); + const src = stageSource(); + try { + assert.throws( + () => engine._installNativePluginIfDeclared('pi', configDir, behaviorsWith(file), src), + /escap|strict subpath|configHome|NUL byte/i, + `nativePlugin.file=${JSON.stringify(file)} must be rejected, not joined onto the validated dir`, + ); + } finally { + cleanup(configDir); + cleanup(src); + } + }); + } + + test('nothing is written outside configDir when file tries to escape', () => { + const configDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-np-out-')); + const outside = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-np-outside-')); + const src = stageSource(); + try { + const escapeFile = path.join('..', '..', path.basename(outside), 'pwned.js'); + assert.throws( + () => engine._installNativePluginIfDeclared('pi', configDir, behaviorsWith(escapeFile), src), + /escap|strict subpath|configHome/i, + ); + assert.ok( + !fs.existsSync(path.join(outside, 'pwned.js')), + 'nothing may be written outside configDir', + ); + } finally { + cleanup(configDir); + cleanup(outside); + cleanup(src); + } + }); + + test('missing source is a silent no-op (unchanged behavior)', () => { + const configDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-np-nosrc-')); + const src = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-np-emptysrc-')); + try { + assert.doesNotThrow(() => + engine._installNativePluginIfDeclared('pi', configDir, behaviorsWith('gsd.js'), src), + ); + assert.ok(!fs.existsSync(path.join(configDir, 'extensions', 'gsd.js'))); + } finally { + cleanup(configDir); + cleanup(src); + } + }); +}); diff --git a/tests/installer-migration-install.integration.test.cjs b/tests/installer-migration-install.integration.test.cjs index 57667c291..ddb5e3992 100644 --- a/tests/installer-migration-install.integration.test.cjs +++ b/tests/installer-migration-install.integration.test.cjs @@ -292,9 +292,19 @@ function assertFreshInstallContract(runtime, targetDir) { // adversarial-review fix — hooksSurface:'none' no longer implies // skipSharedHooksInstall for pi, mirroring OpenCode), so hooks/ + the // git-cmd.js tokenizer helper ARE part of the artifact surface now. + // #2470: the dest filename comes from pi's descriptor, and must satisfy + // pi's isExtensionFile() auto-discovery filter (.ts/.js only) — otherwise + // the file installs but pi never loads it and /gsd never registers. + const piNativePlugin = JSON.parse( + fs.readFileSync(path.join(__dirname, '..', 'capabilities', 'pi', 'capability.json'), 'utf8') + ).runtime.hostBehaviors.nativePlugin; assert.ok( - fs.existsSync(path.join(targetDir, 'extensions', 'gsd.cjs')), - `${runtime} should install the native extension file at extensions/gsd.cjs` + piNativePlugin.file.endsWith('.ts') || piNativePlugin.file.endsWith('.js'), + `${runtime}'s extension "${piNativePlugin.file}" must end in .ts or .js for pi to discover it (#2470)` + ); + assert.ok( + fs.existsSync(path.join(targetDir, piNativePlugin.dir, piNativePlugin.file)), + `${runtime} should install the native extension file at ${piNativePlugin.dir}/${piNativePlugin.file}` ); assert.ok( fs.existsSync(path.join(targetDir, 'hooks', 'gsd-ensure-canonical-path.js')), diff --git a/tests/installer-migration-pi-extension-ext.test.cjs b/tests/installer-migration-pi-extension-ext.test.cjs new file mode 100644 index 000000000..26b3c4214 --- /dev/null +++ b/tests/installer-migration-pi-extension-ext.test.cjs @@ -0,0 +1,231 @@ +'use strict'; + +/** + * TDD tests for installer migration 006: + * 2026-07-20-pi-extension-cjs-to-js (#2470) + * + * #2470 renamed pi's installed native extension from `extensions/gsd.cjs` to + * `extensions/gsd.js`, because pi's own auto-discovery filter + * (`isExtensionFile()` in @earendil-works/pi-coding-agent) accepts only `.ts` + * and `.js` and silently skips everything else. Installs made before that fix + * still carry the stale, permanently-inert `extensions/gsd.cjs`; the installer + * writes the new `.js` alongside it and would otherwise orphan the old file + * forever (it drops out of the manifest, so uninstall never removes it). + * + * This migration retires the stale copy. Coverage follows the matrix in + * docs/installer-migrations.md#authoring-workflow: + * 1. metadata / authoring-guard conformance + * 2. stale file absent -> empty plan (idempotent, fresh installs) + * 3. stale file managed-pristine -> remove-managed + * 4. stale file managed-modified -> backup-and-remove (never silent delete) + * 5. stale file unknown -> NO action (never remove unowned files) + * 6. the replacement gsd.js and neighbouring user files are never touched + * 7. runtime scoping: pi only + */ + +const { describe, test } = 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 migration = require('../gsd-core/bin/lib/installer-migrations/006-pi-extension-cjs-to-js.cjs'); + +const { + classifyArtifact: realClassifyArtifact, + readInstallManifest, +} = require('../gsd-core/bin/lib/installer-migrations.cjs'); + +const STALE_REL = 'extensions/gsd.cjs'; +const CURRENT_REL = 'extensions/gsd.js'; + +function createTempDir() { + return fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-migration-006-test-')); +} + +function cleanup(dir) { + // eslint-disable-next-line local/no-raw-rmsync-in-tests -- local cleanup in migration test; no helpers import available + fs.rmSync(dir, { recursive: true, force: true }); +} + +function writeFile(root, relPath, content) { + const fullPath = path.join(root, relPath); + fs.mkdirSync(path.dirname(fullPath), { recursive: true }); + fs.writeFileSync(fullPath, content, 'utf8'); +} + +function writeManifest(root, files) { + fs.writeFileSync( + path.join(root, 'gsd-file-manifest.json'), + JSON.stringify( + { + version: '1.7.0', + timestamp: '2026-07-20T00:00:00.000Z', + mode: 'full', + files, + }, + null, + 2, + ), + 'utf8', + ); +} + +function makePlanCtx(configDir) { + const manifest = readInstallManifest(configDir); + return { + configDir, + classifyArtifact: (relPath) => realClassifyArtifact(configDir, relPath, manifest), + }; +} + +/** sha256 hash in the manifest's own format, so a file reads as pristine. */ +function hashOf(root, relPath) { + const crypto = require('node:crypto'); + return crypto + .createHash('sha256') + .update(fs.readFileSync(path.join(root, relPath))) + .digest('hex'); +} + +// --------------------------------------------------------------------------- +// 1. Metadata +// --------------------------------------------------------------------------- + +describe('migration 006 metadata', () => { + test('exports a single migration object with the required authoring fields', () => { + assert.equal(typeof migration, 'object'); + assert.equal(typeof migration.id, 'string'); + assert.ok(migration.id.length > 0, 'id must be non-empty'); + assert.equal(typeof migration.title, 'string'); + assert.equal(typeof migration.description, 'string'); + assert.equal(typeof migration.introducedIn, 'string'); + assert.ok(Array.isArray(migration.scopes), 'scopes must be an array'); + assert.ok(migration.scopes.includes('global'), 'scopes must include global'); + assert.ok(migration.scopes.includes('local'), 'scopes must include local'); + assert.strictEqual(migration.destructive, true); + assert.equal(typeof migration.plan, 'function'); + }); + + test('is scoped to pi only (no other runtime installs this artifact)', () => { + assert.ok(Array.isArray(migration.runtimes), 'runtimes must be an explicit array'); + assert.deepEqual(migration.runtimes, ['pi']); + }); + + test('id carries the expected date prefix and names the retired artifact', () => { + assert.ok( + migration.id.startsWith('2026-07-20-'), + `id should start with the date prefix, got: ${migration.id}`, + ); + assert.match(migration.id, /pi-extension/); + }); +}); + +// --------------------------------------------------------------------------- +// 2-5. plan() behaviour by classification +// --------------------------------------------------------------------------- + +describe('migration 006 plan()', () => { + test('emits no actions when the stale extension is absent (fresh install, idempotent)', (t) => { + const dir = createTempDir(); + t.after(() => cleanup(dir)); + + writeFile(dir, CURRENT_REL, '// current extension\n'); + writeManifest(dir, { [CURRENT_REL]: hashOf(dir, CURRENT_REL) }); + + const actions = migration.plan(makePlanCtx(dir)); + assert.deepEqual(actions, [], 'no stale file -> no actions'); + }); + + test('emits remove-managed for a pristine manifest-managed stale extension', (t) => { + const dir = createTempDir(); + t.after(() => cleanup(dir)); + + writeFile(dir, STALE_REL, '// stale pre-#2470 extension\n'); + writeManifest(dir, { [STALE_REL]: hashOf(dir, STALE_REL) }); + + const actions = migration.plan(makePlanCtx(dir)); + assert.equal(actions.length, 1, `expected exactly one action, got ${JSON.stringify(actions)}`); + assert.equal(actions[0].type, 'remove-managed'); + assert.equal(actions[0].relPath, STALE_REL); + assert.ok( + typeof actions[0].ownershipEvidence === 'string' && actions[0].ownershipEvidence.length > 0, + 'destructive actions require ownershipEvidence (authoring guard)', + ); + assert.ok( + typeof actions[0].reason === 'string' && actions[0].reason.length > 0, + 'action must carry a human-readable reason for dry-run output', + ); + }); + + test('emits backup-and-remove when the user locally modified the stale extension', (t) => { + const dir = createTempDir(); + t.after(() => cleanup(dir)); + + writeFile(dir, STALE_REL, '// stale pre-#2470 extension\n'); + // Manifest records a DIFFERENT hash -> managed-modified. + writeManifest(dir, { [STALE_REL]: 'a'.repeat(64) }); + + const actions = migration.plan(makePlanCtx(dir)); + assert.equal(actions.length, 1); + assert.equal( + actions[0].type, + 'backup-and-remove', + 'a locally patched managed file must be backed up, never silently deleted', + ); + assert.equal(actions[0].relPath, STALE_REL); + assert.ok(actions[0].ownershipEvidence); + }); + + test('emits NO action for an unknown (non-manifest) gsd.cjs — never remove unowned files', (t) => { + const dir = createTempDir(); + t.after(() => cleanup(dir)); + + // File present but absent from the manifest -> classification 'unknown'. + writeFile(dir, STALE_REL, '// hand-placed by the user\n'); + writeManifest(dir, {}); + + const actions = migration.plan(makePlanCtx(dir)); + assert.deepEqual( + actions, + [], + 'unknown files are preserved (docs/installer-migrations.md#ownership)', + ); + }); + + test('never targets the replacement extension or neighbouring user files', (t) => { + const dir = createTempDir(); + t.after(() => cleanup(dir)); + + writeFile(dir, STALE_REL, '// stale\n'); + writeFile(dir, CURRENT_REL, '// current\n'); + writeFile(dir, 'extensions/my-own-extension.js', '// user-authored\n'); + writeManifest(dir, { + [STALE_REL]: hashOf(dir, STALE_REL), + [CURRENT_REL]: hashOf(dir, CURRENT_REL), + }); + + const actions = migration.plan(makePlanCtx(dir)); + const targeted = actions.map((a) => a.relPath); + assert.deepEqual(targeted, [STALE_REL]); + assert.ok(!targeted.includes(CURRENT_REL), 'must not remove the replacement extension'); + assert.ok( + !targeted.includes('extensions/my-own-extension.js'), + "must not touch a user's own extension sitting in the same directory", + ); + }); + + test('plan() does not mutate disk (planning is pure)', (t) => { + const dir = createTempDir(); + t.after(() => cleanup(dir)); + + writeFile(dir, STALE_REL, '// stale\n'); + writeManifest(dir, { [STALE_REL]: hashOf(dir, STALE_REL) }); + + migration.plan(makePlanCtx(dir)); + assert.ok( + fs.existsSync(path.join(dir, STALE_REL)), + 'plan() must not remove anything — the executor owns mutation', + ); + }); +}); diff --git a/tests/installer-migrations.test.cjs b/tests/installer-migrations.test.cjs index d1b30279f..337ec7a61 100644 --- a/tests/installer-migrations.test.cjs +++ b/tests/installer-migrations.test.cjs @@ -1665,6 +1665,14 @@ test('shipped installer-migration checksums are locked to a committed baseline ( // fix-forward migration widens the scanned surface without touching 000. '2026-07-17-opencode-baseline-commands-dir': 'sha256:0f6080b5f9b75fb5adbe9664a71152e23a5336813453b0a77e4df6fd483ad38e', + // Migration 006 (NEW, added here per this test's own sanctioned "adding a new + // migration" case — not a shipped-body edit): retire pi's stale + // extensions/gsd.cjs. #2470 renamed the installed extension to + // extensions/gsd.js because pi's isExtensionFile() auto-discovery accepts + // only .ts/.js and silently skips everything else; without this migration the + // old path drops out of the manifest and uninstall can never remove it. + '2026-07-20-pi-extension-cjs-to-js': + 'sha256:185fa926ae24d83cbdd95c31a9ad2cc8d123e176ad543669b3b0ed75e6ca6f4a', }; const { DEFAULT_MIGRATIONS_DIR, migrationChecksum: computeChecksum } = require('../gsd-core/bin/lib/installer-migrations.cjs'); @@ -2565,3 +2573,240 @@ describe('migration.plan()', () => { }); }); } + +// --------------------------------------------------------------------------- +// Symlinked managed path: backup must never dereference (#2470 security review) +// --------------------------------------------------------------------------- +// +// `fs.copyFileSync` follows symlinks. Before this hardening, a managed path +// replaced by a link would have had the LINK TARGET's bytes copied into the +// journal's backup tree — e.g. a `gsd.cjs` symlinked at a private key would +// land that key's contents under gsd-migration-journal/. Nothing GSD installs +// is ever a symlink, so the faithful snapshot is the link itself. + +{ + const { describe, test } = require('node:test'); + describe('symlinked managed path is snapshotted as a link, never dereferenced', () => { + const piExtensionMigration = require('../gsd-core/bin/lib/installer-migrations/006-pi-extension-cjs-to-js.cjs'); + const SECRET = 'TOP-SECRET-PRIVATE-KEY-MATERIAL\n'; + + test('backup-and-remove on a symlinked managed file copies the link, not the referent', (t) => { + const configDir = createTempInstall(); + const secretDir = createTempInstall(); + try { + const secretPath = path.join(secretDir, 'id_rsa'); + fs.writeFileSync(secretPath, SECRET, 'utf8'); + + const linkPath = path.join(configDir, 'extensions', 'gsd.cjs'); + fs.mkdirSync(path.dirname(linkPath), { recursive: true }); + try { + fs.symlinkSync(secretPath, linkPath); + } catch { + t.skip('symlink creation unsupported on this platform/privilege'); + return; + } + + // Manifest records the path as managed with a hash that cannot match the + // referent -> classification 'managed-modified' -> backup-and-remove. + writeManifest(configDir, { 'extensions/gsd.cjs': sha256('the original extension\n') }); + + const result = runInstallerMigrations({ + configDir, + runtime: 'pi', + scope: 'global', + migrations: [piExtensionMigration], + now: () => '2026-07-20T00:00:00.000Z', + }); + + const backupAction = result.plan.actions.find((a) => a.type === 'backup-and-remove'); + assert.ok(backupAction, 'expected a backup-and-remove action for the modified managed file'); + + // The referent is untouched and still holds its content. + assert.ok(fs.existsSync(secretPath), 'symlink target must survive'); + assert.equal(fs.readFileSync(secretPath, 'utf8'), SECRET, 'symlink target content must be unchanged'); + + // The link itself is gone from the install tree. + assert.equal( + fs.lstatSync(linkPath, { throwIfNoEntry: false }), + undefined, + 'the symlink at the managed path must be removed', + ); + + // Nothing anywhere under configDir may contain the referent's bytes. + const leaked = []; + const walk = (dir) => { + for (const entry of fs.readdirSync(dir, { withFileTypes: true })) { + const full = path.join(dir, entry.name); + if (entry.isSymbolicLink()) continue; // a link is fine; its content is not copied + if (entry.isDirectory()) { walk(full); continue; } + let body; + try { body = fs.readFileSync(full, 'utf8'); } catch { continue; } + if (body.includes('TOP-SECRET')) leaked.push(path.relative(configDir, full)); + } + }; + walk(configDir); + assert.deepEqual(leaked, [], `symlink referent content leaked into: ${leaked.join(', ')}`); + } finally { + cleanup(configDir); + cleanup(secretDir); + } + }); + + test('a regular managed file is still backed up by content (no behavior change)', (t) => { + const configDir = createTempInstall(); + try { + writeFile(configDir, 'extensions/gsd.cjs', 'locally patched extension\n'); + writeManifest(configDir, { 'extensions/gsd.cjs': sha256('the original extension\n') }); + + const result = runInstallerMigrations({ + configDir, + runtime: 'pi', + scope: 'global', + migrations: [piExtensionMigration], + now: () => '2026-07-20T00:00:00.000Z', + }); + + const backupAction = result.plan.actions.find((a) => a.type === 'backup-and-remove'); + assert.ok(backupAction, 'expected backup-and-remove for the locally patched file'); + + // The PLAN carries backupRelPath: null — the concrete backup location is + // chosen during apply and recorded in the journal, so read it from there. + const journal = JSON.parse(fs.readFileSync(path.join(configDir, result.journalRelPath), 'utf8')); + const journalled = journal.actions.find((a) => a.backupRelPath); + assert.ok(journalled, 'apply must record the backup path in the journal for the user'); + const backupPath = path.join(configDir, journalled.backupRelPath); + assert.equal( + fs.readFileSync(backupPath, 'utf8'), + 'locally patched extension\n', + 'a real file must still be backed up by content so the user can recover it', + ); + assert.ok(!fs.existsSync(path.join(configDir, 'extensions', 'gsd.cjs'))); + } finally { + cleanup(configDir); + } + }); + + // In-flight failure recovery: when a later step of the SAME apply() attempt + // throws, the catch block replays the rollback snapshots it already took. + // Those snapshots are themselves symlinks, so a raw copy there dereferences + // and writes the referent's bytes back to the LIVE install path — worse than + // the journal-tree leak, because it is user-visible at a predictable path. + // + // The failure is injected by monkeypatching fs.rmSync (restored in finally) + // rather than by chmod/permission tricks: deterministic, root- and + // OS-independent. The delete of the managed path is allowed to SUCCEED and + // then throws once, modelling a later step failing after the delete. That + // ordering is load-bearing: if the live path still existed, the pre-fix + // copyFileSync would hit a same-file collision and throw instead of leaking, + // and this test would pass against the very bug it exists to catch. + test('apply failure after a symlinked snapshot does not leak the referent into the live tree', (t) => { + const configDir = createTempInstall(); + const secretDir = createTempInstall(); + const realRmSync = fs.rmSync; + try { + const secretPath = path.join(secretDir, 'id_rsa'); + fs.writeFileSync(secretPath, SECRET, 'utf8'); + + const linkPath = path.join(configDir, 'extensions', 'gsd.cjs'); + fs.mkdirSync(path.dirname(linkPath), { recursive: true }); + try { + fs.symlinkSync(secretPath, linkPath); + } catch { + t.skip('symlink creation unsupported on this platform/privilege'); + return; + } + writeManifest(configDir, { 'extensions/gsd.cjs': sha256('the original extension\n') }); + + let fired = false; + fs.rmSync = function patched(target, options) { + const result = realRmSync.call(fs, target, options); + if (!fired && path.resolve(String(target)) === path.resolve(linkPath)) { + fired = true; + throw new Error('injected post-delete failure'); + } + return result; + }; + + assert.throws(() => runInstallerMigrations({ + configDir, + runtime: 'pi', + scope: 'global', + migrations: [piExtensionMigration], + now: () => '2026-07-20T00:00:00.000Z', + }), /injected post-delete failure/, 'the injected failure must propagate, not be swallowed'); + + fs.rmSync = realRmSync; + + // The referent is untouched... + assert.ok(fs.existsSync(secretPath)); + assert.equal(fs.readFileSync(secretPath, 'utf8'), SECRET); + + // ...the managed path is restored as a LINK, not a dereferenced copy... + const restored = fs.lstatSync(linkPath, { throwIfNoEntry: false }); + assert.ok(restored, 'failure recovery must restore the managed path'); + assert.ok( + restored.isSymbolicLink(), + 'restored path must be a symlink — a regular file here means the referent was dereferenced into the live tree', + ); + + // ...and its bytes appear nowhere under the install tree. + const leaked = []; + const walk = (dir) => { + for (const entry of fs.readdirSync(dir, { withFileTypes: true })) { + const full = path.join(dir, entry.name); + if (entry.isSymbolicLink()) continue; + if (entry.isDirectory()) { walk(full); continue; } + let body; + try { body = fs.readFileSync(full, 'utf8'); } catch { continue; } + if (body.includes('TOP-SECRET')) leaked.push(path.relative(configDir, full)); + } + }; + walk(configDir); + assert.deepEqual(leaked, [], `referent content leaked into: ${leaked.join(', ')}`); + } finally { + fs.rmSync = realRmSync; + cleanup(configDir); + cleanup(secretDir); + } + }); + + test('rollback() restores a symlinked managed path as a link, not a dereferenced copy', (t) => { + const configDir = createTempInstall(); + const secretDir = createTempInstall(); + try { + const targetPath = path.join(secretDir, 'id_rsa'); + fs.writeFileSync(targetPath, SECRET, 'utf8'); + + const linkPath = path.join(configDir, 'extensions', 'gsd.cjs'); + fs.mkdirSync(path.dirname(linkPath), { recursive: true }); + try { + fs.symlinkSync(targetPath, linkPath); + } catch { + t.skip('symlink creation unsupported on this platform/privilege'); + return; + } + writeManifest(configDir, { 'extensions/gsd.cjs': sha256('the original extension\n') }); + + const result = runInstallerMigrations({ + configDir, + runtime: 'pi', + scope: 'global', + migrations: [piExtensionMigration], + now: () => '2026-07-20T00:00:00.000Z', + }); + assert.equal(fs.lstatSync(linkPath, { throwIfNoEntry: false }), undefined, 'link removed by apply'); + + result.rollback(); + + const restored = fs.lstatSync(linkPath, { throwIfNoEntry: false }); + assert.ok(restored, 'rollback must restore the managed path'); + assert.ok(restored.isSymbolicLink(), 'restored path must be a symlink, not a dereferenced copy'); + assert.equal(fs.readlinkSync(linkPath), targetPath, 'restored link must point at the original target'); + assert.equal(fs.readFileSync(targetPath, 'utf8'), SECRET, 'target content must be untouched throughout'); + } finally { + cleanup(configDir); + cleanup(secretDir); + } + }); +}); +} diff --git a/tests/pi-upgrades.test.cjs b/tests/pi-upgrades.test.cjs index 8d1cbac94..1b705c8b8 100644 --- a/tests/pi-upgrades.test.cjs +++ b/tests/pi-upgrades.test.cjs @@ -170,3 +170,53 @@ test('pi axes negotiate modelMode:"active" (the active-model steering axis)', () const result = negotiateHostCapabilities(PI_AXES); assert.equal(result.effective.modelMode, 'active'); }); + +// -- native-extension auto-discovery contract (#2470) ------------------------- +// +// pi auto-discovers extensions by scanning /extensions/ and keeping +// only names its `isExtensionFile()` predicate accepts: +// +// function isExtensionFile(name) { +// return name.endsWith(".ts") || name.endsWith(".js"); +// } +// +// (@earendil-works/pi-coding-agent, packages/coding-agent/src/core/extensions/ +// loader.ts — verified upstream 2026-07-20.) A dest filename outside that set +// is skipped SILENTLY: no /gsd command, no error, no log line. pi loads the +// accepted file through jiti, which handles CommonJS and ESM alike, so the +// extension's module format is irrelevant to discovery — only the suffix is. +// +// These assertions deliberately encode pi's PREDICATE rather than the literal +// filename, so they keep protecting the contract if the extension is ever +// renamed again, and they state the reason the rename mattered. + +/** pi's upstream discovery predicate, mirrored verbatim. */ +function piIsExtensionFile(name) { + return name.endsWith('.ts') || name.endsWith('.js'); +} + +test('pi capability declares a native-extension dest filename pi will auto-discover (#2470)', () => { + const np = PI_CAP.runtime.hostBehaviors.nativePlugin; + assert.ok(np && np.file, 'pi must declare hostBehaviors.nativePlugin.file'); + assert.ok( + piIsExtensionFile(np.file), + `pi's installed extension "${np.file}" must end in .ts or .js — pi's isExtensionFile() ` + + 'auto-discovery filter silently skips every other suffix, so /gsd never registers (#2470)', + ); +}); + +test('pi native-extension source stays CommonJS-explicit while the dest satisfies pi (#2470)', () => { + const np = PI_CAP.runtime.hostBehaviors.nativePlugin; + // The in-repo source keeps its .cjs suffix on purpose: tests require() it + // directly and .cjs is unambiguous CommonJS regardless of any future + // package.json "type" flip. Only the INSTALLED name must satisfy pi, and + // jiti parses the copied file by content, not by suffix. + assert.ok( + np.source.endsWith('.cjs'), + `pi's in-repo extension source should stay .cjs (explicit CommonJS), got "${np.source}"`, + ); + assert.ok( + fs.existsSync(path.join(__dirname, '..', np.source)), + `pi's declared nativePlugin.source "${np.source}" must exist in the repo`, + ); +});