diff --git a/.changeset/quick-wasps-sing.md b/.changeset/quick-wasps-sing.md new file mode 100644 index 000000000..171eac9fa --- /dev/null +++ b/.changeset/quick-wasps-sing.md @@ -0,0 +1,5 @@ +--- +type: Changed +pr: 4249 +--- +**`gsd install` and `/gsd-update` now verify every GSD-managed runtime entrypoint before reporting success** — a hook script or its interpreter that is missing, unreadable, or not executable now fails the install with the offending paths named, instead of printing `Done!` over a configuration whose hooks can never fire. diff --git a/.github/workflows/install-smoke.yml b/.github/workflows/install-smoke.yml index 07e376ed9..677b9b8b5 100644 --- a/.github/workflows/install-smoke.yml +++ b/.github/workflows/install-smoke.yml @@ -144,6 +144,14 @@ jobs: if: steps.skip.outputs.skip != 'true' run: npm ci + # `npm pack` runs prepack/prepare (build:lib) but NOT prepublishOnly, so + # a packed tarball is missing hooks/dist — the very launch targets the + # published package ships and the lifecycle smoke is meant to exercise. + # Build them here so the smoked tarball is publish-shaped (#4154). + - name: Build hooks (prepublishOnly parity) + if: steps.skip.outputs.skip != 'true' + run: npm run build:hooks + - name: Pack root tarball if: steps.skip.outputs.skip != 'true' id: pack diff --git a/CONTEXT.md b/CONTEXT.md index 66eeebf05..9de41adf6 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -251,7 +251,7 @@ Module owning the `{"type":"commonjs"}` module-type marker GSD writes beside its Module owning validation for Installer Migration Module records and planned actions. It enforces migration metadata, explicit install scopes, ownership evidence for destructive/config actions, and runtime contract citations for runtime config rewrites before a migration can enter planning or apply. ### Installer Module -Primary installer for all runtimes. Single production file: `bin/install.js` (hand-authored JS — it is NOT generated from `src/*.cts`; ADR-1508 keeps it hand-authored deliberately, and no `npm run build` step emits it). Exports: `install(isGlobal, runtime[, configDir])` → typed result `{ runtime, configDir, settingsPath, settings, statuslineCommand, updateBannerCommand }`; `uninstall(isGlobal, runtime[, configDir])`; `installRuntimeArtifacts(runtime, configDir, scope, resolvedProfile)`; `uninstallRuntimeArtifacts(runtime, configDir, scope)`; `writeManifest(configDir, runtime)`. Runtime enum: `allRuntimes` (18 values: claude, antigravity, augment, cline, codebuddy, codex, copilot, cursor, hermes, kimi, kimi-code, kilo, opencode, pi, qwen, trae, windsurf, zcode). Directory helpers: `getDirName(runtime)` → local dir name; `getConfigDirFromHome(runtime, isGlobal)` → shell-quoted path fragment. Per-runtime global config-dir resolution is delegated to `gsd-core/bin/lib/runtime-homes.cjs:getGlobalConfigDir(runtime[, explicitDir])` — the canonical, env-var–aware projection (`explicitDir` override + opencode/kilo `*_CONFIG` file-path precedence); the legacy in-installer `getGlobalDir`/`getOpencodeGlobalDir`/`getKiloGlobalDir` were retired into it (#56). The same module exposes `detectAntigravityDirAmbiguity(opts)` — a side-effect-free probe reporting whether multiple `~/.gemini/antigravity{,-ide,-cli}` dirs coexist and which one GSD's `gsd-core/VERSION` marker (the `dot-home-nested` `probeExists`) resolves to, for installer / `/gsd-update` operator guidance when a pre-#217 install landed in the wrong sibling dir (#1441). Runtime-specific helpers: `resolveKiloConfigPath(configDir)`, `configureKiloPermissions(isGlobal[, explicitDir])`. Claude-specific permission helpers: `mergeClaudePermissions(settings)` — non-destructively appends GSD-owned allow entries (see `GSD_CLAUDE_ALLOW_PERMISSIONS`) to a Claude Code settings object and filters out the retired legacy forms (`GSD_CLAUDE_LEGACY_ALLOW_PERMISSIONS` #2278; `GSD_CLAUDE_LEGACY_DENY_PERMISSIONS` #4221 — the `Read(.env*)` deny rules are retired in favor of the managed `gsd-secret-read-guard.js` hook, and an emptied `deny` array is deleted); called from `finishInstall` for `runtime === 'claude'` only; uninstall removes exactly these entries (#768). Layout-driven artifact copy/removal delegates to `gsd-core/bin/lib/runtime-artifact-layout.cjs:resolveRuntimeArtifactLayout` (throws `TypeError` for unknown runtimes). Five runtimes with non-recursive skill loaders (cline, qwen, hermes, augment, trae) use a nested router layout: 6 `gsd-ns-*` router bundles emitted as top-level skills, with concrete skills nested at `/skills//SKILL.md` (hermes prefix='': `skills/gsd/ns-*/…`). claude (reverted from nested per #924 — the Skill tool errors on unrouted names) and antigravity (one-level scan, but concrete skills must be top-level discoverable) plus the remaining skills-runtimes (cursor, codex, copilot, windsurf, codebuddy, opencode, kilo) use the flat `skills/gsd-/` layout. See Skill Surface Budget Module and Runtime Artifact Layout Module. +Primary installer for all runtimes. Single production file: `bin/install.js` (hand-authored JS — it is NOT generated from `src/*.cts`; ADR-1508 keeps it hand-authored deliberately, and no `npm run build` step emits it). Exports: `install(isGlobal, runtime[, configDir])` → typed result `{ runtime, configDir, settingsPath, settings, statuslineCommand, updateBannerCommand, configuredEntrypoints }`; `uninstall(isGlobal, runtime[, configDir])`; `installRuntimeArtifacts(runtime, configDir, scope, resolvedProfile)`; `uninstallRuntimeArtifacts(runtime, configDir, scope)`; `writeManifest(configDir, runtime)`. Runtime enum: `allRuntimes` (18 values: claude, antigravity, augment, cline, codebuddy, codex, copilot, cursor, hermes, kimi, kimi-code, kilo, opencode, pi, qwen, trae, windsurf, zcode). Directory helpers: `getDirName(runtime)` → local dir name; `getConfigDirFromHome(runtime, isGlobal)` → shell-quoted path fragment. Per-runtime global config-dir resolution is delegated to `gsd-core/bin/lib/runtime-homes.cjs:getGlobalConfigDir(runtime[, explicitDir])` — the canonical, env-var–aware projection (`explicitDir` override + opencode/kilo `*_CONFIG` file-path precedence); the legacy in-installer `getGlobalDir`/`getOpencodeGlobalDir`/`getKiloGlobalDir` were retired into it (#56). The same module exposes `detectAntigravityDirAmbiguity(opts)` — a side-effect-free probe reporting whether multiple `~/.gemini/antigravity{,-ide,-cli}` dirs coexist and which one GSD's `gsd-core/VERSION` marker (the `dot-home-nested` `probeExists`) resolves to, for installer / `/gsd-update` operator guidance when a pre-#217 install landed in the wrong sibling dir (#1441). Runtime-specific helpers: `resolveKiloConfigPath(configDir)`, `configureKiloPermissions(isGlobal[, explicitDir])`. Claude-specific permission helpers: `mergeClaudePermissions(settings)` — non-destructively appends GSD-owned allow entries (see `GSD_CLAUDE_ALLOW_PERMISSIONS`) to a Claude Code settings object and filters out the retired legacy forms (`GSD_CLAUDE_LEGACY_ALLOW_PERMISSIONS` #2278; `GSD_CLAUDE_LEGACY_DENY_PERMISSIONS` #4221 — the `Read(.env*)` deny rules are retired in favor of the managed `gsd-secret-read-guard.js` hook, and an emptied `deny` array is deleted); called from `finishInstall` for `runtime === 'claude'` only; uninstall removes exactly these entries (#768). Layout-driven artifact copy/removal delegates to `gsd-core/bin/lib/runtime-artifact-layout.cjs:resolveRuntimeArtifactLayout` (throws `TypeError` for unknown runtimes). Five runtimes with non-recursive skill loaders (cline, qwen, hermes, augment, trae) use a nested router layout: 6 `gsd-ns-*` router bundles emitted as top-level skills, with concrete skills nested at `/skills//SKILL.md` (hermes prefix='': `skills/gsd/ns-*/…`). claude (reverted from nested per #924 — the Skill tool errors on unrouted names) and antigravity (one-level scan, but concrete skills must be top-level discoverable) plus the remaining skills-runtimes (cursor, codex, copilot, windsurf, codebuddy, opencode, kilo) use the flat `skills/gsd-/` layout. See Skill Surface Budget Module and Runtime Artifact Layout Module. `finishInstall` calls `assertConfiguredEntrypoints` (Runtime Hooks Surface Module, dedup'd by `(configPath, scriptPath)` first) before its own `writeSettings` — a failing settings-json-surface install/update never persists that write. `installAllRuntimes`'s `finalize()` runs the same check once over the aggregate set before its `printSummaries()` loop calls `finishInstall` per runtime, so the per-runtime slice is checked twice; the second pass is a dozen-odd `statSync`/`accessSync` calls and is left unconditional rather than gated on a caller-supplied bypass flag. Codex/Cursor/Windsurf/Kimi/Cline writers persist their own hooks.json/config.toml/`.clinerules/hooks/PreToolUse` directly inside `install()`, ahead of `installAllRuntimes`'s aggregate `assertConfiguredEntrypoints` call, so a validation failure there is reported (and blocks the completion message) before that file is reverted where a rollback exists. Codex's `install()` result exposes two rollbacks: `rollbackInstallerMigrations` — like every other runtime's — is the installer-migrations-only closure the name describes, and the Codex-only `rollbackPreInstallSnapshot` binds `restoreCodexSnapshot` (#3245's full pre-install snapshot restore — config.toml, hooks.json, skills/, agents/, VERSION). `installAllRuntimes`'s `rollbackFinalizedInstallerMigrations` picks `rollbackPreInstallSnapshot` only when the triggering error is tagged `configuredEntrypointValidation` (set by `assertConfiguredEntrypoints`); any other finalize-stage exception gets `rollbackInstallerMigrations`, so an unrelated sibling failure (e.g. EACCES on another runtime's permission write) cannot un-install a Codex install that already succeeded. The discriminator is the error's KIND, not which runtime owns the bad entrypoint: the aggregate gate is all-or-nothing, so ANY runtime's invalid entrypoint reverts the Codex snapshot — including when Codex's own entrypoints were fine and its "Done!" summary already printed (#4249). Cursor/Windsurf/Kimi/Cline have no equivalent snapshot, so their result binds only the narrow rollback and a validation failure leaves their just-written file unrevertable (#4249). `bannerOpts.configuredEntrypoints` is the only source `finishInstall` validates — a caller other than `installAllRuntimes` that omits it gets no entrypoint validation at all. ### I/O Module Module owning the tool's CLI I/O primitives: `output()` result emission (with large-payload temp-file spillover via `GSD_TEMP_DIR`/`ensureGsdTempDir`/`reapStaleTempFiles`), `error()` stderr emission with exit-code mapping, and the JSON-error-mode toggle (`setJsonErrorMode`/`getJsonErrorMode`, `ERROR_REASON`). **Degraded result vs fault (ADR-2980, #2980):** the two emitters are a deliberate two-channel failure contract, not a drift. A **fault** is `error(message, reason)` — stderr, exit **1**, structured `{ok:false,reason,message}` envelope under `--json-errors`. A **degraded result** is `output({ error: … })` — stdout, exit **0**, `--json-errors` does not apply — and means the command ran to completion and is reporting a condition (absent artifact, and in practice also missing-argument and unusable-input cases) through its result; a caller detects it by inspecting the payload, never by exit code. Ratified across **60 sites in 9 modules** (`state` 25, `verify` 8, `workstream` 7, `frontmatter` 6, `commands` 5, `template` 3, `gsd2-import` 2, `phase` 2, `roadmap` 2) because normalizing them to exit 1 is a Hyrum's Law break over a CRITICAL radius (`get_impact(cmdStateSnapshot)`; `output` has 170 direct callers). #2966/#2980 record "42 sites" — that counts only literals whose FIRST key is `error` (the `output\(\{\s*error:` regex); 18 more put another key first (`{found:false, error}`) and are identical in contract, so 60 is the population and 42 is a subset. New code prefers the fault path or a named-field result (`{updated:false, reason}`), not a 61st site. Known cost carried by the decision: the exit code does not distinguish absent from unusable, which is ADR-1411's "corrupt is not absent" open edge. Docs: `docs/json-errors.md` → "Degraded results vs faults". Extracted from the Core module per ADR-857 rollout phase 1 (#859) so feature modules (`graphify`, `intel`, `audit`, `profile-pipeline`) depend on a small I/O seam instead of the core god-module; the `core.cjs` re-export spine was retired in epic #1267, so callers import this leaf directly. Source of truth: `gsd-core/bin/lib/io.cjs` (generated from `src/io.cts`). @@ -461,7 +461,7 @@ A per-agent narrative entry written by `mempalace_diary_write`. GSD's `gsd-mempa The `mempalace.memory_mode` config key controlling how authoritative MemPalace is during recall/capture relative to GSD's native memory. Three wired values: `augment` (default — palace is an additive recall layer; native memory stays authoritative; lowest coupling), `kg_backend` (knowledge-graph queries resolve against MemPalace's temporal graph as the primary source, `.planning/graphs/` as fallback; non-KG drawer recall stays additive), `replace` (recall resolves through the palace as the source of truth, native artifacts as fallback). Every mode is `onError:skip` and default-resilient — an unreachable palace degrades to native memory and GSD keeps writing `.planning/graphs/`, so no mode loses memory. Read at hook-render time; switching is a config change, not a reinstall. Cross-mode migration of existing `.planning/graphs/` into the palace is a separate, not-yet-implemented concern (PRD/ADR §17 open question). See MemPalace Settings in `docs/CONFIGURATION.md`. ### Runtime Hooks Surface Module -Standalone hook-surface writer module extracted from `bin/install.js` as ADR-857 phase 5f-1 (behavior-preserving relocation, no logic change). Owns: Cline rules-body/agents-md/pre-tool-use hook generation (`buildClineRulesBody`, `buildClineAgentsMdBody`, `buildClinePreToolUseHook`, `mergeGsdAgentsMd`, `writeClineArtifacts`); Cursor `hooks.json` lifecycle (`buildCursorHookEntry`, `isManagedCursorHookEntry`, `reconcileCursorHooksJson`, `writeCursorHooksJson`, `removeCursorHooksJson`); Copilot session-hook config (`buildCopilotHookConfig`, `writeCopilotHookConfig`); Codex hook-block and event management (`buildCodexHookBlock`, `rewriteLegacyCodexHookBlock`, `reconcileCodexHooksJsonEvent`, `reconcileCodexHooksJsonSessionStart`, `ensureCodexHooksJsonSessionStart`, `removeCodexHooksJsonEvent`, `removeCodexHooksJsonSessionStart`, `buildCodexHookWindowsShimIR`, `cleanupOrphanedCodexContextMonitorScript`, `isGsdOwnedCodexContextMonitorScript`, `hooksJsonReferencesCodexContextMonitor` — #2586, the last three replacing the deleted `ensureCodexHooksJsonEvent`: GSD no longer adds Codex context-monitor hook-event registrations, only recognizes and removes stale ones left by a pre-#2586 install, then deletes the orphaned script/`.cmd` shim once unreferenced and content-verified as GSD-owned); Kimi native config.toml `[[hooks]]` lifecycle (`buildKimiHooksTomlBlock`, `stripKimiHooksTomlBlock`, `writeKimiHooksToml`, `removeKimiHooksToml` — #2095 EoS/kimi Upgrade 1, the first genuinely NEW hook surface added post-relocation rather than a behavior-preserving move: kimi's `[[hooks]]` array lives in its own native `config.toml`, resolved by `resolveKimiHooksTomlDir` in Runtime Homes Module to a directory deliberately separate from kimi's GSD configDir, wrapped in `# GSD Hooks BEGIN`/`END` marker comments for idempotent reinstall); and shared hook command helpers (`buildHookCommand`, `rewriteLegacyManagedNodeHookCommands`, `normalizeNodePath`, `resolveNodeRunner`, and — #3662 — `buildNodeRunnerChainToken`, the POSIX-sh runner token that resolves node at hook-fire time for non-portable managed JS hooks, plus the `NODE_RUNNER_RESOLVER_HOOK` basename of the staged `hooks/gsd-node-runner.sh` resolver that portable installs route through with the baked node path as its first argument). `bin/install.js` delegates to this module via thin wrappers and re-exports its functions unchanged so existing tests require no modification. Source: `src/runtime-hooks-surface.cts`. Built output: `gsd-core/bin/lib/runtime-hooks-surface.cjs`. +Standalone hook-surface writer module extracted from `bin/install.js` as ADR-857 phase 5f-1 (behavior-preserving relocation, no logic change). Owns: Cline rules-body/agents-md/pre-tool-use hook generation (`buildClineRulesBody`, `buildClineAgentsMdBody`, `buildClinePreToolUseHook`, `mergeGsdAgentsMd`, `writeClineArtifacts`); Cursor `hooks.json` lifecycle (`buildCursorHookEntry`, `isManagedCursorHookEntry`, `reconcileCursorHooksJson`, `writeCursorHooksJson`, `removeCursorHooksJson`); Copilot session-hook config (`buildCopilotHookConfig`, `writeCopilotHookConfig`); Codex hook-block and event management (`buildCodexHookBlock`, `rewriteLegacyCodexHookBlock`, `reconcileCodexHooksJsonEvent`, `reconcileCodexHooksJsonSessionStart`, `ensureCodexHooksJsonSessionStart`, `removeCodexHooksJsonEvent`, `removeCodexHooksJsonSessionStart`, `buildCodexHookWindowsShimIR`, `cleanupOrphanedCodexContextMonitorScript`, `isGsdOwnedCodexContextMonitorScript`, `hooksJsonReferencesCodexContextMonitor` — #2586, the last three replacing the deleted `ensureCodexHooksJsonEvent`: GSD no longer adds Codex context-monitor hook-event registrations, only recognizes and removes stale ones left by a pre-#2586 install, then deletes the orphaned script/`.cmd` shim once unreferenced and content-verified as GSD-owned); Kimi native config.toml `[[hooks]]` lifecycle (`buildKimiHooksTomlBlock`, `stripKimiHooksTomlBlock`, `writeKimiHooksToml`, `removeKimiHooksToml` — #2095 EoS/kimi Upgrade 1, the first genuinely NEW hook surface added post-relocation rather than a behavior-preserving move: kimi's `[[hooks]]` array lives in its own native `config.toml`, resolved by `resolveKimiHooksTomlDir` in Runtime Homes Module to a directory deliberately separate from kimi's GSD configDir, wrapped in `# GSD Hooks BEGIN`/`END` marker comments for idempotent reinstall); and shared hook command helpers (`buildHookCommand`, `rewriteLegacyManagedNodeHookCommands`, `normalizeNodePath`, `resolveNodeRunner`, and — #3662 — `buildNodeRunnerChainToken`, the POSIX-sh runner token that resolves node at hook-fire time for non-portable managed JS hooks, plus the `NODE_RUNNER_RESOLVER_HOOK` basename of the staged `hooks/gsd-node-runner.sh` resolver that portable installs route through with the baked node path as its first argument). `bin/install.js` delegates to this module via thin wrappers and re-exports its functions unchanged so existing tests require no modification. Also owns post-install entrypoint validation (#4154/#4249): `buildHookCommand` and every writer's `recordConfiguredHookCommand` record a `ConfiguredEntrypoint {runtime, configPath, scriptPath, interpreterCandidates?, selfExecutable?, platform, command?}` for each hook they compute — including an already-registered hook whose register-only-if-absent guard leaves its on-disk `command` stale, since `scriptPath`/`interpreterCandidates` are derived from `configDir`/`hookName`, not from that command string. `validateConfiguredEntrypoints(entries)` runs up to three checks per entry, none of which execute anything. First, always: `statSync`s `scriptPath` (must be a file) then `accessSync(R_OK)`s it — required even for a self-executable shebang script, since its kernel-invoked interpreter still opens and reads it, not just execs it. Second, only when `selfExecutable: true`: `accessSync(X_OK)`. Every producer that needs this sets the flag explicitly, rather than it being inferred from an absent `interpreterCandidates` — a Windows-Claude `.sh` hook direct-invoked via `shellHookOmitsBashRunner`, Codex's Windows `.cmd` shim (no shebang; relies on Windows' own extension-based dispatch), and Cline's hybrid `#!/usr/bin/env node` hook all set it; the check is skipped entirely when `entry.platform` is `win32` (POSIX mode bits don't mean executable there — mirroring `resolveExecutableBinary`'s own X_OK carve-out), so Cline is the only one of the three where this check runs. Third, only when `interpreterCandidates` is present: at least one must resolve via `resolveExecutableBinary(candidate, {requireExecutable:true})` (for a Node hook, the first candidate is `normalizeNodePath`'d — the SAME stable version-manager alias `buildNodeRunnerChainToken` bakes as its own first choice, not the raw, always-resolving `process.execPath`; a `.sh` hook's candidate is its resolved `bash` path instead; Cline's is `['node']`, since unlike every other GSD JS hook — which bakes an absolute install-time-resolved node path specifically to avoid this — its interpreter is looked up on PATH by `env` at hook-fire time). Failures carry `reason: 'missing'|'unreadable'|'wrong-file-type'|'unresolved-interpreter'|'not-executable'` (`unreadable` for an `EACCES` `statSync`, distinct from a genuinely absent path). See Installer Module for the caller-side gate, which deduplicates entries by `(configPath, scriptPath)` first (writers can push the same pair more than once, e.g. Kimi's context-monitor hook across several events). Source: `src/runtime-hooks-surface.cts`. Built output: `gsd-core/bin/lib/runtime-hooks-surface.cjs`. ### Runtime Config Adapter Registry Module owning the explicit per-runtime config-mutation dispatch table for the installer. `resolveRuntimeConfigIntent(runtime)` projects a typed config intent — `installSurface` (`settings-json` | `codex-toml` | `copilot-instructions` | `cline-rules` | `cursor-hooks-json` | `profile-marker-only`), `writesSharedSettings` (the `finishInstall` shared-settings write gate), and `finishPermissionWriter` (`opencode` | `kilo` | `antigravity` | none) — that `bin/install.js` dispatches on instead of inline `runtime === '...'` branching. Owns adapter selection only: it performs no filesystem IO and does not execute config mutations (the install/finishInstall handlers and the per-runtime writers do that). Unknown runtimes fail loudly with a `TypeError`, guarded by an `Object.hasOwn` own-property check so prototype-chain keys (`__proto__`, `constructor`) also throw. Also exports `resolveInstallPlan(runtime)` — the ADR-58 `InstallPlan` capstone — which collects the install-level descriptor axes (`installSurface`, `writesSharedSettings`, `finishPermissionWriter`, `hookEvents`, `extendedHookEvents`, `hooksSurface`, `sandboxTier`) into one typed `InstallPlan` value consumed by `install()` and `finishInstall()` in `bin/install.js`. `sandboxTier` (`none` | `codex-agent-sandbox`) gates per-agent `sandbox_mode` emission in the codex TOML path and fails loud on a missing/invalid value (#1151). The spatial axes (`configHome`, `artifactLayout`, `commandStyle`) remain behind their self-resolving adapter modules and are not part of the plan; they are the execution adapters. Realizes both the adapter-selection and plan-collection halves of the Runtime Install Policy Module boundary. Source: `gsd-core/bin/lib/runtime-config-adapter-registry.cjs`. See ADR-58, #60. diff --git a/bin/install.js b/bin/install.js index 8fa65f22c..5022c5c9d 100755 --- a/bin/install.js +++ b/bin/install.js @@ -10850,6 +10850,7 @@ function install(isGlobal, runtime = DEFAULT_RUNTIME, options = {}) { // Track installation failures const failures = []; + const configuredEntrypoints = []; let installerMigrationResult = null; const rollbackInstallerMigrations = () => { if (!installerMigrationResult || typeof installerMigrationResult.rollback !== 'function') return; @@ -10932,12 +10933,17 @@ function install(isGlobal, runtime = DEFAULT_RUNTIME, options = {}) { // destroyed by a wholesale delete whose snapshot lacked it. let codexManagedSnapshotCaptured = false; // null = the gate never ran; true/false = the gate ran and hooks/ (did|did - // not) exist as a directory pre-install. Three states are load-bearing: a + // not) exist as a directory pre-install. Two states are load-bearing: a // clean first install records false, so its rollback removes the staged - // hooks/ tree entirely; minimal mode records null, so rollback does nothing. + // hooks/ tree entirely; a non-Codex runtime records null, so rollback does + // nothing. let codexPreInstallHooksDirPreExisted = null; let codexPreInstallHooksCaptureIncomplete = false; - if (_hostBehaviors(runtime).tomlConfigInstall && !isMinimalMode(_effectiveInstallMode)) { + // #4249 CR: not gated on install mode. restoreCodexSnapshot is reachable for + // a core/--minimal install too (#2695), and its pass-2 sweeps remove every + // gsd-* skill dir / agent file the snapshot does not claim — so an empty + // minimal-mode snapshot deleted the whole surface with nothing to restore. + if (_hostBehaviors(runtime).tomlConfigInstall) { codexManagedSnapshotCaptured = true; const _preSkillsDir = _resolveSkillsRootDir(runtime, targetDir, _installScopeId); if (fs.existsSync(_preSkillsDir)) { @@ -12824,6 +12830,7 @@ function install(isGlobal, runtime = DEFAULT_RUNTIME, options = {}) { absoluteRunner: codexNodeRunner, platform: process.platform, }); + configuredEntrypoints.push(...(hookWrite.configuredEntrypoints || [])); if (hookWrite.wrote) { console.log(` ${green}✓${reset} Configured Codex hooks (SessionStart via hooks.json)`); } else { @@ -12903,7 +12910,21 @@ function install(isGlobal, runtime = DEFAULT_RUNTIME, options = {}) { } persistActiveProfileMarker(); - return { settingsPath: null, settings: null, statuslineCommand: null, updateBannerCommand: null, runtime, configDir: targetDir }; + // #4249: expose restoreCodexSnapshot (#3245) as a SECOND, separately named + // rollback rather than rebinding `rollbackInstallerMigrations` to it. A + // configured-entrypoint validation failure discovered later (outside this + // function, after Codex's own hooks.json/config.toml write already + // succeeded) previously had only the installer-migrations closure to call, + // leaving the just-written config.toml/hooks.json broken on disk despite + // Codex already owning a full pre-install snapshot/restore for exactly this. + // + // Every runtime's `rollbackInstallerMigrations` therefore still means what + // it says — the installer-migrations-only closure, which is what a + // finalize-stage failure that is NOT an entrypoint-validation failure gets + // (the Phase 4 contract). `rollbackPreInstallSnapshot` is Codex-only and is + // chosen only for entrypoint-validation failures. See the selection in + // installAllRuntimes' rollbackFinalizedInstallerMigrations. + return { settingsPath: null, settings: null, statuslineCommand: null, updateBannerCommand: null, runtime, configDir: targetDir, configuredEntrypoints, rollbackInstallerMigrations, rollbackPreInstallSnapshot: restoreCodexSnapshot }; } if (plan.installSurface === 'copilot-instructions') { @@ -12933,7 +12954,18 @@ function install(isGlobal, runtime = DEFAULT_RUNTIME, options = {}) { writeCopilotHookConfig(targetDir); console.log(` ${green}✓${reset} Configured Copilot lifecycle hook (sessionStart)`); persistActiveProfileMarker(); - return { settingsPath: null, settings: null, statuslineCommand: null, updateBannerCommand: null, runtime, configDir: targetDir }; + // #4249: `[]`, not omitted — every Copilot hook is an inline `printf` + // one-liner (GSD_COPILOT_*_HOOK_BASH/PWSH in src/runtime-hooks-surface.cts), + // so this runtime genuinely launches no GSD-managed script and has no + // interpreter to resolve. Stated explicitly like every other branch rather + // than leaning on installAllRuntimes' `|| []` defence. + // #4249 (antigravity review): `rollbackInstallerMigrations` was missing here + // — every other branch returns it. This PR's own aggregate entrypoint gate + // is what makes the gap reachable: an unrelated runtime's invalid entrypoint + // now triggers rollbackFinalizedInstallerMigrations for every result in the + // batch, and a Copilot result with no rollback function silently skips + // reverting Copilot's own installer migrations. + return { settingsPath: null, settings: null, statuslineCommand: null, updateBannerCommand: null, runtime, configDir: targetDir, configuredEntrypoints: [], rollbackInstallerMigrations }; } if (plan.installSurface === 'cursor-hooks-json') { @@ -12956,7 +12988,7 @@ function install(isGlobal, runtime = DEFAULT_RUNTIME, options = {}) { // The re-run is retained for parity with the settings.json install path. writeManifest(targetDir, runtime, { mode: _effectiveInstallMode, scope: _installScopeId }); persistActiveProfileMarker(); - return { settingsPath: null, settings: null, statuslineCommand: null, updateBannerCommand: null, runtime, configDir: targetDir }; + return { settingsPath: null, settings: null, statuslineCommand: null, updateBannerCommand: null, runtime, configDir: targetDir, configuredEntrypoints: cursorHookResult.configuredEntrypoints, rollbackInstallerMigrations }; } if (plan.installSurface === 'profile-marker-only') { @@ -13012,6 +13044,7 @@ function install(isGlobal, runtime = DEFAULT_RUNTIME, options = {}) { const kimiHookOpts = { portableHooks: hasPortableHooks, runtime }; const kimiHooksTomlPath = path.join(kimiHooksRoot, 'config.toml'); const kimiHooksResult = writeKimiHooksToml(kimiHooksTomlPath, kimiHooksRoot, { hookOpts: kimiHookOpts }); + configuredEntrypoints.push(...kimiHooksResult.configuredEntrypoints); if (kimiHooksResult.changed) { console.log(` ${green}✓${reset} Configured ${kimiHooksResult.entryCount} GSD hook(s) in ${kimiHooksTomlPath}`); } @@ -13068,6 +13101,7 @@ function install(isGlobal, runtime = DEFAULT_RUNTIME, options = {}) { const windsurfHookResult = writeWindsurfHooksJson(targetDir, src, { platform: process.platform, }); + configuredEntrypoints.push(...windsurfHookResult.configuredEntrypoints); if (windsurfHookResult.changed) { console.log(` ${green}✓${reset} Configured Windsurf lifecycle hooks (pre_write_code, pre_run_command)`); } else { @@ -13083,19 +13117,19 @@ function install(isGlobal, runtime = DEFAULT_RUNTIME, options = {}) { } persistActiveProfileMarker(); - return { settingsPath: null, settings: null, statuslineCommand: null, updateBannerCommand: null, runtime, configDir: targetDir }; + return { settingsPath: null, settings: null, statuslineCommand: null, updateBannerCommand: null, runtime, configDir: targetDir, configuredEntrypoints, rollbackInstallerMigrations }; } if (plan.installSurface === 'cline-rules') { // Cline uses the `.clinerules/` directory form (issue #787): GSD rules live // at .clinerules/gsd.md and a PreToolUse lifecycle hook at // .clinerules/hooks/PreToolUse. Global installs also get ~/.agents/AGENTS.md. - writeClineArtifacts(targetDir, isGlobal); + const clineArtifacts = writeClineArtifacts(targetDir, isGlobal); // Re-run the manifest pass: these artifacts are written *after* the earlier // writeManifest() call, so a second pass is needed to hash-track them. writeManifest(targetDir, runtime, { mode: _effectiveInstallMode, scope: _installScopeId }); persistActiveProfileMarker(); - return { settingsPath: null, settings: null, statuslineCommand: null, updateBannerCommand: null, runtime, configDir: targetDir }; + return { settingsPath: null, settings: null, statuslineCommand: null, updateBannerCommand: null, runtime, configDir: targetDir, configuredEntrypoints: clineArtifacts.configuredEntrypoints, rollbackInstallerMigrations }; } // Configure statusline and hooks in settings.json (or settings.local.json for local Claude installs). @@ -13220,8 +13254,11 @@ function install(isGlobal, runtime = DEFAULT_RUNTIME, options = {}) { persistActiveProfileMarker(); // Callers index this result by `runtime` (installAllRuntimes' statusline // lookup), so every early exit must return the full shape — a bare return - // crashes the install rather than skipping one file. - return { settingsPath: null, settings: null, statuslineCommand: null, updateBannerCommand: null, runtime, configDir: targetDir }; + // crashes the install rather than skipping one file. That includes + // configuredEntrypoints/rollbackInstallerMigrations: rollbackFinalizedInstallerMigrations + // reads result.rollbackInstallerMigrations unconditionally, and an omitted + // field there silently skips this runtime's rollback on a finalize-stage failure. + return { settingsPath: null, settings: null, statuslineCommand: null, updateBannerCommand: null, runtime, configDir: targetDir, configuredEntrypoints: [], rollbackInstallerMigrations }; } const settings = validateHookFields(cleanupOrphanedHooks(rawSettings)); // #3002 CR / #3662: rewrite legacy `node .../gsd-*.js` command strings (pre- @@ -13243,7 +13280,19 @@ function install(isGlobal, runtime = DEFAULT_RUNTIME, options = {}) { // runtime's hostBehaviors instead of a hardcoded `runtime === 'antigravity'` // check inside projectLocalHookPrefix. const localPrefix = projectLocalHookPrefix({ runtime, dirName, hookPathStyle: _hostBehaviors(runtime).hookPathStyle }); - const hookOpts = { portableHooks: hasPortableHooks, runtime }; + const settingsEntrypoints = []; + const hookOpts = { + portableHooks: hasPortableHooks, + runtime, + configPath: settingsPath, + // #4249: track unconditionally. Gating on `plan.hooksSurface === + // 'settings-json'` made tracking depend on an unasserted + // installSurface/hooksSurface coupling — a descriptor that broke it would + // silently drop this runtime out of validation. Everything recorded here + // lands in settings.json by construction, and the registered-command + // filter below already discards entries no hook actually references. + configuredEntrypoints: settingsEntrypoints, + }; // #2979: local-install hook commands also use a runner GUI/minimal-PATH // runtimes can resolve. Bare `node` fails when the host launches the // runtime with a stripped PATH (Finder/Antigravity/etc) — #3662 replaces @@ -13257,19 +13306,19 @@ function install(isGlobal, runtime = DEFAULT_RUNTIME, options = {}) { // `node` command that recreates the #2979 failure. const localCmd = (hookFile) => localNodeRunner === null ? null - : projectShellCommandText({ + : hooksSurface.recordConfiguredHookCommand(projectShellCommandText({ runnerToken: localNodeRunner, argTokens: [`${localPrefix}/hooks/${hookFile}`], runtime, platform: process.platform, - }); - const localShellCmd = (hookFile) => buildLocalShellHookCommand({ + }), targetDir, hookFile, hookOpts); + const localShellCmd = (hookFile) => hooksSurface.recordConfiguredHookCommand(buildLocalShellHookCommand({ localPrefix, hookFile, bashRunner: localBashRunner, runtime, platform: process.platform, - }); + }), targetDir, hookFile, hookOpts); const statuslineCommand = isGlobal ? buildHookCommand(targetDir, 'gsd-statusline.js', hookOpts) : localCmd('gsd-statusline.js'); @@ -13339,6 +13388,31 @@ function install(isGlobal, runtime = DEFAULT_RUNTIME, options = {}) { ? buildHookCommand(targetDir, 'gsd-update-banner.js', hookOpts) : localCmd('gsd-update-banner.js')); + const registeredHookCommands = Object.values(settings.hooks || {}) + .flatMap(groups => Array.isArray(groups) ? groups : []) + .flatMap(group => Array.isArray(group && group.hooks) ? group.hooks : []) + .map(hook => hook && hook.command) + .filter(command => typeof command === 'string'); + // #4249: match by the managed script's `/hooks/` path segment, not + // by exact command-string equality. The blocking-guard hooks above register + // only-if-absent, so a hook already present from a prior install keeps its + // OLD command untouched — but `track()` always records the FRESHLY computed + // command for it, which never equals what's actually persisted. Matching on + // the segment (present in the persisted command either way, since every + // entry.scriptPath is /hooks/ by construction) keeps an + // already-registered, still-active hook in the validated set instead of + // silently dropping it (#4154 Blocker) — anchored on `/hooks/` rather than a + // bare basename so an unrelated user command that merely mentions the same + // filename can't false-positive into GSD's validated set. + configuredEntrypoints.push( + ...settingsEntrypoints.filter(entry => { + const hooksSegment = '/hooks/' + path.basename(entry.scriptPath); + return registeredHookCommands.some(command => command.includes(hooksSegment)); + }), + ); + const statuslineEntrypoints = settingsEntrypoints.filter(entry => entry.command === statuslineCommand); + const updateBannerEntrypoints = settingsEntrypoints.filter(entry => entry.command === updateBannerCommand); + // #683: Set worktree.baseRef:"head" in settings.local.json for local Claude installs. // Both fresh and upgrade paths apply only when worktrees are enabled for the project. // Never applies to global installs, non-Claude runtimes, or when the user already @@ -13407,15 +13481,65 @@ function install(isGlobal, runtime = DEFAULT_RUNTIME, options = {}) { settings, statuslineCommand, updateBannerCommand, + statuslineEntrypoints, + updateBannerEntrypoints, runtime, configDir: targetDir, rollbackInstallerMigrations, + configuredEntrypoints, }; } -/** - * Apply statusline config, then print completion message - */ +// #4249 (review, Major): rollback consequence differs by runtime surface — +// see docs/how-to/update-gsd.md's rollback-matrix paragraph, which this +// mirrors. Codex reverts (pre-install snapshot restore); Cursor/Windsurf/ +// Kimi/Kimi Code/Cline already wrote their config file inside install(), +// ahead of this gate, with no revert path, so it is left on disk broken; +// every other (settings.json-based) runtime writes strictly after this gate, +// so a failure here means nothing new was persisted for it. +const ENTRYPOINT_LEFT_UNREVERTED_RUNTIMES = new Set(['cursor', 'windsurf', 'kimi', 'kimi-code', 'cline']); +function describeEntrypointConsequence(invalidRuntime) { + if (invalidRuntime === 'codex') return 'reverted: its pre-install snapshot was restored'; + if (ENTRYPOINT_LEFT_UNREVERTED_RUNTIMES.has(invalidRuntime)) return 'NOT reverted: its config file is already written and was left on disk — fix the reported path and rerun install'; + return 'not persisted: this runtime writes its config after this check'; +} + +function assertConfiguredEntrypoints(entries) { + // #4249: some writers push the same (configPath, scriptPath) pair more than + // once (e.g. Kimi's context-monitor hook registered under several events, + // or the portable resolver script shared by every portable JS hook) — keep + // one so a broken entry is reported once, not once per duplicate. + const seen = new Set(); + const deduped = (entries || []).filter((entry) => { + const key = JSON.stringify([entry.configPath, entry.scriptPath]); + if (seen.has(key)) return false; + seen.add(key); + return true; + }); + const validation = hooksSurface.validateConfiguredEntrypoints(deduped); + if (validation.ok) return; + + const error = new Error( + // #4249: lead each entry with its runtime, and name the actual consequence + // for that runtime (review, Major) — the aggregate gate is all-or-nothing + // across every runtime being installed, and a failure here can revert a + // runtime whose own entrypoints were fine (see + // rollbackFinalizedInstallerMigrations) while leaving another runtime's + // already-written config broken on disk with no revert at all, so an + // operator reading only this message must be able to tell WHOSE + // entrypoint broke and WHAT that means for their config, not just that + // something did. + `Configured entrypoint validation failed: ${validation.invalid.map(({ runtime: invalidRuntime, role, path: invalidPath, reason }) => `${invalidRuntime} ${role} ${invalidPath} (${reason}) [${describeEntrypointConsequence(invalidRuntime)}]`).join(', ')}`, + ); + error.configuredEntrypointValidation = validation; + throw error; +} + +// #4249: `bannerOpts.configuredEntrypoints` is the ONLY source assertConfiguredEntrypoints +// checks below — a caller that omits it (or calls finishInstall directly instead of +// through installAllRuntimes) gets zero entrypoint validation, silently. installAllRuntimes +// always passes the full set (per-runtime entries plus statusline/updateBanner); any other +// caller must do the same for this gate to mean anything. function finishInstall(settingsPath, settings, statuslineCommand, shouldInstallStatusline, runtime = DEFAULT_RUNTIME, isGlobal = true, configDir = null, bannerOpts = {}) { // #2093: isKilo dropped — the Kilo permissions-writer call below is gated // on plan.finishPermissionWriter === 'kilo' (descriptor-driven), not this flag. @@ -13429,6 +13553,19 @@ function finishInstall(settingsPath, settings, statuslineCommand, shouldInstallS const { isOpencode, isCodex, isCursor, isAugment, isQwen, isHermes, isCline } = runtimeFlags(runtime); const plan = resolveInstallPlan(runtime); + // #4249 Major: validate BEFORE this function's own settings.json write (and + // before writeNonClaudeDefaults) instead of after. Cursor/Windsurf/Kimi/Cline + // already persisted their config inside install() by this point, with no + // rollback path covering those writes; Codex also persists inside install() + // but its rollback binds to a full pre-install snapshot restore, so it IS + // covered (see docs/how-to/update-gsd.md). For the settings-json surface + // this ordering means a failing validation never reaches this function's + // own write at all. On the production path this is a redundant backstop — + // installAllRuntimes's own aggregate assertConfiguredEntrypoints call + // already validates the superset before finishInstall runs for any + // runtime — kept for a caller that invokes finishInstall directly. + assertConfiguredEntrypoints(bannerOpts.configuredEntrypoints); + if (shouldInstallStatusline && plan.writesSharedSettings && !_hostBehaviors(runtime).skipSettingsUi) { if (!isGlobal && !forceStatusline) { // Local installs skip statusLine by default: repo settings.json takes precedence over @@ -14326,10 +14463,32 @@ function installAllRuntimes(runtimes, isGlobal, isInteractive) { const rollbackFinalizedInstallerMigrations = (error) => { const rollbackFailures = []; + // #4249: this discriminates on the error's KIND, never on which runtime + // owns the failing entrypoint. `wide` is true for ANY entrypoint-validation + // failure from ANY runtime, by design: the aggregate gate exists so a + // multi-runtime install cannot report success while one of its entrypoints + // is broken, so an invalid Cline entrypoint reverts Codex's pre-install + // snapshot too — even though Codex itself was fine and its own "Done!" + // summary already printed. tests/configured-entrypoint-validation.test.cjs + // ('an aggregate entrypoint validation failure rolls the Codex install + // back') exercises exactly that, and it is the all-or-nothing behaviour + // docs/how-to/update-gsd.md documents. + // + // What this narrows is the OTHER axis: a finalize-stage exception that is + // not an entrypoint-validation failure at all — e.g. a sibling runtime's + // permission-config write dying with EACCES — gets only the + // installer-migrations-only rollback that Phase 4 specifies + // (docs/installer-migrations.md#phase-4-installupdate-integration). + // Un-installing (and, on update, downgrading) an already-"Done!" Codex over + // an unrelated error is not an outcome any doc promises, while the sibling + // surfaces that write config inside install() would keep theirs regardless. + const wide = !!(error && error.configuredEntrypointValidation); for (const result of [...results].reverse()) { - if (!result || typeof result.rollbackInstallerMigrations !== 'function') continue; + if (!result) continue; + const rollback = (wide && result.rollbackPreInstallSnapshot) || result.rollbackInstallerMigrations; + if (typeof rollback !== 'function') continue; try { - result.rollbackInstallerMigrations(); + rollback(); } catch (rollbackError) { rollbackFailures.push({ runtime: result.runtime, @@ -14357,6 +14516,19 @@ function installAllRuntimes(runtimes, isGlobal, isInteractive) { const finalize = (shouldInstallStatusline, shouldInstallBanner) => { try { + const selectedConfiguredEntrypoints = (result) => { + if (!result || result.skipped) return []; + const useStatusline = statuslineRuntimes.includes(result.runtime) + && shouldInstallStatusline + && (isGlobal || forceStatusline); + return [ + ...(result.configuredEntrypoints || []), + ...(useStatusline ? (result.statuslineEntrypoints || []) : []), + ...(shouldInstallBanner ? (result.updateBannerEntrypoints || []) : []), + ]; + }; + assertConfiguredEntrypoints(results.flatMap(selectedConfiguredEntrypoints)); + const printSummaries = () => { for (const result of results) { if (result && result.skipped) continue; @@ -14370,7 +14542,11 @@ function installAllRuntimes(runtimes, isGlobal, isInteractive) { result.runtime, isGlobal, result.configDir, - { shouldInstallBanner: !!shouldInstallBanner, bannerCommand: result.updateBannerCommand } + { + shouldInstallBanner: !!shouldInstallBanner, + bannerCommand: result.updateBannerCommand, + configuredEntrypoints: selectedConfiguredEntrypoints(result), + } ); } }; diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 955c24781..1cae2db30 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -476,7 +476,7 @@ Node.js CLI utility (`gsd-tools.cjs`) with domain modules split across `gsd-core | `graphify-command-router.cjs` | ADR-959 capability command router — first real capability command cutover (phase 4d-impl-2); extracted from the `case 'graphify':` arm in `gsd-tools.cjs`; dispatches build/query/status/diff subcommands; discovered via `commandFamilies` in the capability registry | | `audit-command-router.cjs` | ADR-959 capability command router (phase 4d-impl-3); extracted from the `case 'audit-uat':` and `case 'audit-open':` arms in `gsd-tools.cjs`; `routeAuditUat` → `uat.cjs:cmdAuditUat`, `routeAuditOpen` → `audit.cjs:{auditOpenArtifacts,formatAuditReport}`; discovered via `commandFamilies` in the capability registry | | `intel-command-router.cjs` | ADR-959 capability command router (phase 4d-impl-4, last first-party cutover); extracted from the `case 'intel':` arm in `gsd-tools.cjs`; `routeIntelCommand` → all 9 intel subcommands via lazy `require('./intel.cjs')`; preserves non-raw `timeAgo` transform on `status.files[*].updated_at`; discovered via `commandFamilies` in the capability registry | -| `runtime-hooks-surface.cjs` | Standalone hook-surface writer module (ADR-857 phase 5f-1); owns Cline rules/agents-md/pre-tool-use hook generation, Cursor `hooks.json` reconciliation, Copilot session-hook config, and Codex hook-block management; extracted verbatim from `bin/install.js` with no logic change. | +| `runtime-hooks-surface.cjs` | Hook-surface writer and configured-entrypoint validation module (ADR-857 phase 5f-1); owns Cline rules/agents-md/pre-tool-use hook generation, Cursor/Windsurf `hooks.json`, Kimi hook TOML, Copilot session-hook config, Codex hook-block management, and construction-time records for paths emitted into runtime configuration. Installer success requires those records to pass non-executing file-type and interpreter-resolution checks. | --- diff --git a/docs/how-to/update-gsd.md b/docs/how-to/update-gsd.md index 05fbaf4b1..3361fa635 100644 --- a/docs/how-to/update-gsd.md +++ b/docs/how-to/update-gsd.md @@ -26,7 +26,11 @@ GSD will: 8. Offer to restore the user-added files it backed up in step 5. 9. Report whether locally modified GSD files were backed up to `gsd-local-patches/`. -Restart your runtime after the update to pick up new commands and agents. +Before reporting completion, the installer checks each GSD-managed script and interpreter path written into runtime configuration, resolving the interpreter against the current install-time `PATH`. This only proves the path resolves now — a hook fired later under a different, more restricted `PATH` (e.g. a GUI launcher) can still fail even after this check passes. + +If a script is missing, unreadable, has the wrong file type, lacks a required execute permission, or its interpreter cannot be resolved, the update fails and reports every invalid path, each tagged with what happens to that runtime's config next. For Claude Code and other settings.json-based runtimes, this check runs before the update writes settings.json, so nothing new is persisted (an earlier settings.json/settings.local.json migration, if one applied, is the one exception and stays applied) — reported as "not persisted". For Codex, this reverts config.toml/hooks.json along with the rest of that runtime's pre-install snapshot (skills/, agents/, gsd-core/VERSION) — reported as "reverted". For Cursor, Windsurf, Kimi, and Cline, the runtime's config file is already written earlier in the update, ahead of this check, so a failure is reported but that file is left in place, broken — reported as "NOT reverted". Fix the reported path problem and rerun `/gsd-update` — do not restart into the incomplete update. + +Restart your runtime after a successful update to pick up new commands and agents. --- diff --git a/scripts/lib/macos-conformance-tier.generated.cjs b/scripts/lib/macos-conformance-tier.generated.cjs index da205a4ce..51e2ccc83 100644 --- a/scripts/lib/macos-conformance-tier.generated.cjs +++ b/scripts/lib/macos-conformance-tier.generated.cjs @@ -52,6 +52,7 @@ module.exports = { "tests/config-loader.test.cjs", "tests/config-schema.property.test.cjs", "tests/config.test.cjs", + "tests/configured-entrypoint-validation.test.cjs", "tests/contributor-standards.test.cjs", "tests/core-utils.test.cjs", "tests/cursor-subagent-isolation.test.cjs", diff --git a/scripts/lib/platform-conformance-tier.generated.cjs b/scripts/lib/platform-conformance-tier.generated.cjs index 02a7e850c..1e63fc957 100644 --- a/scripts/lib/platform-conformance-tier.generated.cjs +++ b/scripts/lib/platform-conformance-tier.generated.cjs @@ -62,6 +62,7 @@ module.exports = { "tests/config-loader.test.cjs", "tests/config-schema.property.test.cjs", "tests/config.test.cjs", + "tests/configured-entrypoint-validation.test.cjs", "tests/copilot-install.test.cjs", "tests/copilot-upgrades.test.cjs", "tests/core-utils.test.cjs", diff --git a/scripts/release-tarball-smoke.cjs b/scripts/release-tarball-smoke.cjs index 3dae43846..5b1236d81 100644 --- a/scripts/release-tarball-smoke.cjs +++ b/scripts/release-tarball-smoke.cjs @@ -37,6 +37,12 @@ * colon-namespace leaks (WORKFLOW_BODY_COLON_LEAK). * This check populates result.details with counters but does NOT return a * failure code by default; it is informational until enforcement is enabled. + * + * Configured-entrypoint checks (Cycle 4 — #4154): + * For each runtime in entrypointRuntimes, runs the tarball-installed + * installer into a throwaway HOME, then re-reads that runtime's own written + * config files and asserts every GSD-managed script path they name resolves + * to a file (ENTRYPOINT_UNRESOLVED). Requires fixtureDir. */ 'use strict'; @@ -47,6 +53,8 @@ const os = require('os'); const path = require('path'); const { PACKAGE_NAME } = require('../gsd-core/bin/lib/package-identity.cjs'); const { ExitError, runMain } = require('./lib/cli-exit.cjs'); +const shellCmdProjection = require('../gsd-core/bin/lib/shell-command-projection.cjs'); +const { escapeRegex } = require('../gsd-core/bin/lib/pattern.cjs'); // 120 s proved too tight for cold-cache `npm install -g` of a 1499-file tarball: // spawnSync fires SIGTERM at the deadline and returns { status: null, stdout: '', // stderr: '' } (Node docs: status is null when a subprocess is terminated by a @@ -86,6 +94,8 @@ const SMOKE = Object.freeze({ INIT_FAILED: 'init_failed', // Cycle 3 code WORKFLOW_BODY_COLON_LEAK: 'workflow_body_colon_leak', + // Cycle 4 code (#4154) + ENTRYPOINT_UNRESOLVED: 'entrypoint_unresolved', }); // --------------------------------------------------------------------------- @@ -274,6 +284,106 @@ function scanWorkflowColonLeak(filePath, cmdNames) { return null; } +// --------------------------------------------------------------------------- +// Cycle 4 helpers: configured-entrypoint resolution (#4154) +// --------------------------------------------------------------------------- + +/** + * Top-level config files this scan reads. These are the surfaces that carry + * every GSD-managed launch path for the runtimes Cycle 4 actually installs + * (`entrypointRuntimes`, default claude + codex): claude registers into + * settings.json, codex into hooks.json and config.toml. + * + * This is NOT an exhaustive map of where GSD writes launch paths across all + * runtimes, and the scan is top-level only by design. Two known surfaces sit + * outside it: Cline registers its hook at `.clinerules/hooks/PreToolUse` (a + * subdirectory, and not one of these names — see writeClineArtifacts in + * src/runtime-hooks-surface.cts), and Kimi's native `[[hooks]]` config.toml + * lives under `resolveKimiHooksTomlDir()` (`~/.kimi`), a directory separate + * from Kimi's own GSD configDir. Adding either runtime to entrypointRuntimes + * requires teaching scanConfiguredEntrypoints about its surface first, + * otherwise the scan reports zero entrypoints and silently proves nothing. + */ +const RUNTIME_CONFIG_FILES = Object.freeze(['settings.json', 'hooks.json', 'config.toml']); + +/** + * Extract every script path `text` names underneath `configDir`. + * + * #4249 (antigravity review): anchored on the literal, already-known + * `configDir` prefix instead of a generic "any absolute path" character + * class. The prior version excluded whitespace from the match to avoid + * swallowing a shell command's trailing args, which also truncated any + * legitimate path containing a space (e.g. `/Users/John Doe/.claude`) — + * `scanConfiguredEntrypoints` would then silently report zero checked + * paths. Anchoring on `configDir` removes the ambiguity outright: a match + * can only start where the known prefix literally occurs in the text, so + * an interpreter path concatenated ahead of it (`"/usr/bin/node + * /configDir/hooks/foo.js"`) is never swallowed either, and interior + * whitespace inside `configDir` or the script's own path segments is safe + * to allow. This still scans raw, unparsed config text on purpose (see + * scanConfiguredEntrypoints's doc comment) — it catches a writer that + * embeds a launch path without registering it, which a structured + * JSON.parse of the expected schema would miss entirely. + * + * Windows configs store paths with backslashes, which JSON/TOML doubles on + * write; collapsing `\\` to `\` first makes the raw text scan work on both + * platforms without parsing each config format separately (POSIX text has no + * backslashes, so the collapse is a no-op there). + * + * Every writer bakes `configDir` through the same posixNormalize seam + * (src/runtime-hooks-surface.cts) before writing it into config text, on + * every platform — so the anchor must match that projection, not the + * OS-native `configDir` string this function receives. + */ +function configuredEntrypointsIn(text, configDir) { + const normalizedPrefix = shellCmdProjection.posixNormalize(configDir).replace(/\/+$/, '') + '/'; + const scriptPathRe = new RegExp(`${escapeRegex(normalizedPrefix)}[^"']{0,400}?\\.(?:js|cjs|mjs|sh|cmd|ps1)`, 'g'); + const found = new Set(); + for (const match of text.replace(/\\\\/g, '\\').matchAll(scriptPathRe)) { + found.add(path.resolve(match[0])); + } + return [...found]; +} + +/** + * Throwaway HOME the Cycle 4 install for `runtime` runs against. Exported so a + * test can seed that runtime's config before the install rather than + * hard-coding the layout runSmoke picks. + */ +function entrypointFixtureHome(fixtureDir, runtime) { + return path.join(fixtureDir, `entrypoints-${runtime}`); +} + +/** + * Re-derive the configured entrypoints a completed install wrote into a + * runtime's own config files and report the ones that do not resolve to a + * file. Internal to Cycle 4; the smoke's verdict is the exported contract. + * + * This deliberately does NOT consult the installer's own entrypoint list (the + * assertConfiguredEntrypoints gate in bin/install.js). That gate can only + * validate paths a config writer remembered to register; reading the written + * config back is what catches a writer that emits a launch path without + * registering it, and a stale registration an install left behind. + * + * @param {string} configDir - Absolute path to the runtime config dir. + * @returns {{ checked: string[], unresolved: { configPath: string, scriptPath: string }[] }} + */ +function scanConfiguredEntrypoints(configDir) { + const checked = []; + const unresolved = []; + for (const name of RUNTIME_CONFIG_FILES) { + const configPath = path.join(configDir, name); + if (!fs.existsSync(configPath) || !fs.statSync(configPath).isFile()) continue; + const text = fs.readFileSync(configPath, 'utf-8'); + for (const scriptPath of configuredEntrypointsIn(text, configDir)) { + checked.push(scriptPath); + if (fs.existsSync(scriptPath) && fs.statSync(scriptPath).isFile()) continue; + unresolved.push({ configPath, scriptPath }); + } + } + return { checked, unresolved }; +} + // --------------------------------------------------------------------------- // Pure function: runSmoke // --------------------------------------------------------------------------- @@ -285,6 +395,8 @@ function scanWorkflowColonLeak(filePath, cmdNames) { * @param {string} opts.expectedVersion - semver string to assert (e.g. "1.50.0") * @param {string} [opts.fixtureDir] - Temp dir to run `init` into (must NOT be HOME) * @param {string[]} [opts.lifecycleCommands] - Commands to file-check (default: see below) + * @param {string[]} [opts.entrypointRuntimes] - Runtime profiles whose configured entrypoints are + * re-checked after a real install (default: see below). Requires fixtureDir; pass [] to skip. * @param {boolean} [opts.dryRun=false] - If true, skip actual npm install; validate input only * @param {object} [opts.npmEnv] - Optional env dict for the internal npm install * spawnSync call. Pass an isolated HOME env (e.g. from isolatedNpmEnv() in tests/helpers.cjs) @@ -298,6 +410,11 @@ function runSmoke({ expectedVersion, fixtureDir, lifecycleCommands = ['init', 'discuss-phase', 'plan-phase', 'execute-phase'], + // claude and codex cover the two top-level config surfaces this scan knows + // how to read (settings.json, and hooks.json + config.toml). Most other + // runtimes reuse one of those two shapes; Cline and Kimi do not (see + // RUNTIME_CONFIG_FILES), so they are out of scope here rather than covered. + entrypointRuntimes = ['claude', 'codex'], dryRun = false, npmEnv = undefined, }) { @@ -551,6 +668,75 @@ function runSmoke({ // NOTE: colonLeakCount is informational here. Once the backlog is fixed, // a future enforcement mode can fail on non-zero counts. + // ───────────────────────────────────────────────────────────────────────── + // Cycle 4: configured-entrypoint resolution (#4154) + // ───────────────────────────────────────────────────────────────────────── + + // The installer's own gate (assertConfiguredEntrypoints) runs in-process and + // only sees the paths a config writer registered with it. Installing the + // packed tarball for real and reading each runtime's written config back is + // what proves the launch paths a user's runtime will actually invoke exist + // in the shipped layout. + const entrypointProfiles = []; + if (fixtureDir && entrypointRuntimes.length > 0) { + for (const runtime of entrypointRuntimes) { + // Own HOME per runtime so a --global install cannot reach the real one. + const runtimeHome = entrypointFixtureHome(fixtureDir, runtime); + const configDir = path.join(runtimeHome, `.${runtime}`); + fs.mkdirSync(runtimeHome, { recursive: true }); + + const installResult = spawnSync( + process.execPath, + [path.join(pkg, 'bin', 'install.js'), `--${runtime}`, '--global', '--config-dir', configDir], + { + encoding: 'utf-8', + cwd: runtimeHome, + stdio: ['pipe', 'pipe', 'pipe'], + env: { + ...effectiveNpmEnv, + HOME: runtimeHome, + USERPROFILE: runtimeHome, + // Same reason as the init check above: install.js skips its main() + // block when GSD_TEST_MODE is set and would exit 0 writing nothing. + GSD_TEST_MODE: '', + NO_UPDATE_NOTIFIER: '1', + }, + timeout: CHILD_TIMEOUT_MS, + }, + ); + + if (installResult.status !== 0) { + // #4249 (antigravity review): this is the Cycle 4 per-runtime install, + // not Cycle 1's `gsd init` — SMOKE.INSTALL_FAILED is the code Cycle 2's + // identical spawnSync-failure check already uses for the same failure + // class; reusing SMOKE.INIT_FAILED here conflated the two lifecycle + // stages in the reported code. + return { + code: SMOKE.INSTALL_FAILED, + details: { + ...details, + runtime, + configDir, + stderr: installResult.stderr, + stdout: installResult.stdout, + }, + }; + } + + const scan = scanConfiguredEntrypoints(configDir); + if (scan.unresolved.length > 0) { + return { + code: SMOKE.ENTRYPOINT_UNRESOLVED, + details: { ...details, runtime, configDir, unresolved: scan.unresolved }, + }; + } + + entrypointProfiles.push({ runtime, configDir, entrypointsChecked: scan.checked.length }); + } + } + + details.entrypointProfiles = entrypointProfiles; + return { code: SMOKE.OK, details }; } @@ -635,7 +821,14 @@ function cleanup(...dirs) { // Exports // --------------------------------------------------------------------------- -module.exports = { SMOKE, runSmoke, binInvocation, CHILD_TIMEOUT_MS }; +module.exports = { + SMOKE, + runSmoke, + binInvocation, + entrypointFixtureHome, + CHILD_TIMEOUT_MS, + configuredEntrypointsIn, +}; if (require.main === module) { runMain(cliMain); diff --git a/src/runtime-hooks-surface.cts b/src/runtime-hooks-surface.cts index 4311ff9b6..fd1ecfe2a 100644 --- a/src/runtime-hooks-surface.cts +++ b/src/runtime-hooks-surface.cts @@ -56,6 +56,7 @@ const { shellHookOmitsBashRunner, escapeTomlDoubleQuotedString, escapePosixDoubleQuoted, + resolveExecutableBinary, } = shellCmdProjection as { isManagedHookBasename: (scriptPath: string, opts?: { surface?: string }) => boolean; isManagedHookCommand: (cmd: string | null | undefined, opts?: { surface?: string; includeLegacyAliases?: boolean; configDir?: string }) => boolean; @@ -66,6 +67,7 @@ const { shellHookOmitsBashRunner: (opts: { platform: string; runtime: string; isShellHook: boolean }) => boolean; escapeTomlDoubleQuotedString: (value: unknown) => string; escapePosixDoubleQuoted: (value: unknown) => string; + resolveExecutableBinary: (name: string, opts?: { platform?: string; requireExecutable?: boolean }) => string | null; }; // --------------------------------------------------------------------------- @@ -642,7 +644,7 @@ interface BashRunnerOpts { existsSync?: (p: string) => boolean; } -function resolveBashRunner(opts?: BashRunnerOpts): string | null { +function resolveBashExecutable(opts?: BashRunnerOpts): string | null { const platform = (opts && opts.platform) || process.platform; if (platform !== 'win32') return 'bash'; @@ -658,13 +660,19 @@ function resolveBashRunner(opts?: BashRunnerOpts): string | null { } for (const candidate of candidates) { - if (candidate && exists(candidate)) { - return JSON.stringify(shellCmdProjection.posixNormalize(candidate)); - } + if (candidate && exists(candidate)) return shellCmdProjection.posixNormalize(candidate); } return null; } +function resolveBashRunner(opts?: BashRunnerOpts): string | null { + const executable = resolveBashExecutable(opts); + if (executable === null) return null; + return ((opts && opts.platform) || process.platform) === 'win32' + ? JSON.stringify(executable) + : executable; +} + // --------------------------------------------------------------------------- // Shared: rewriteLegacyManagedNodeHookCommands // --------------------------------------------------------------------------- @@ -929,6 +937,7 @@ interface ReconcileResult { changed: boolean; wrote: boolean; path: string; + configuredEntrypoints?: ConfiguredEntrypoint[]; } /** @@ -1117,14 +1126,17 @@ interface ShimIR { render: { cmd: () => string }; } +function parseAbsoluteRunnerToken(absoluteRunnerToken: string): string { + try { + return JSON.parse(absoluteRunnerToken) as string; + } catch { + return absoluteRunnerToken; + } +} + function buildCodexHookWindowsShimIR(scriptAbsPath: string, absoluteRunnerToken: string | null): ShimIR | null { if (!absoluteRunnerToken) return null; - let interpreter: string; - try { - interpreter = JSON.parse(absoluteRunnerToken) as string; - } catch { - interpreter = absoluteRunnerToken; - } + const interpreter = parseAbsoluteRunnerToken(absoluteRunnerToken); const targetAbs = shellCmdProjection.posixNormalize(scriptAbsPath); const scriptQuoted = JSON.stringify(targetAbs); const cmdPath = scriptAbsPath.replace(/\.js$/, '.cmd'); @@ -1159,8 +1171,9 @@ function ensureCodexHooksJsonSessionStart(targetDir: string, opts: EnsureCodexSe const scriptPath = shellCmdProjection.posixNormalize(path.resolve(targetDir, 'hooks', 'gsd-check-update.js')); const cmdShimPath = scriptPath.replace(/\.js$/, '.cmd'); - + const configuredEntrypoints: ConfiguredEntrypoint[] = []; let managedCommand: string | undefined; + if (platform === 'win32') { const shimIR = buildCodexHookWindowsShimIR(scriptPath, absoluteRunner); if (!shimIR) return { changed: false, wrote: false, path: hooksJsonPath }; @@ -1176,6 +1189,10 @@ function ensureCodexHooksJsonSessionStart(targetDir: string, opts: EnsureCodexSe return { changed: false, wrote: false, path: hooksJsonPath }; } managedCommand = shimIR.hookCommand; + configuredEntrypoints.push( + { runtime: 'codex', configPath: hooksJsonPath, scriptPath: shimIR.cmdPath, platform, selfExecutable: true }, + { runtime: 'codex', configPath: hooksJsonPath, scriptPath, interpreterCandidates: [parseAbsoluteRunnerToken(absoluteRunner)], platform }, + ); } else { managedCommand = projectManagedHookCommand({ absoluteRunner, @@ -1183,15 +1200,23 @@ function ensureCodexHooksJsonSessionStart(targetDir: string, opts: EnsureCodexSe runtime: 'codex', platform, }) ?? undefined; + if (managedCommand) { + configuredEntrypoints.push({ + runtime: 'codex', + configPath: hooksJsonPath, + scriptPath, + interpreterCandidates: [parseAbsoluteRunnerToken(absoluteRunner)], + platform, + }); + } } if (!managedCommand) return { changed: false, wrote: false, path: hooksJsonPath }; - const commandWindows = platform === 'win32' ? JSON.stringify(shellCmdProjection.posixNormalize(cmdShimPath)) : undefined; - - return reconcileCodexHooksJsonSessionStart(targetDir, { managedCommand, commandWindows }); + const result = reconcileCodexHooksJsonSessionStart(targetDir, { managedCommand, commandWindows }); + return { ...result, configuredEntrypoints }; } // --------------------------------------------------------------------------- @@ -1397,7 +1422,77 @@ interface BuildHookCommandOpts { runtime?: string; hookShell?: string; env?: NodeJS.ProcessEnv; + execPath?: string; existsSync?: (p: string) => boolean; + configPath?: string; + configuredEntrypoints?: ConfiguredEntrypoint[]; +} + +function configuredEntrypointsForHook( + configDir: string, + hookName: string, + opts: BuildHookCommandOpts, +): ConfiguredEntrypoint[] { + const platform = opts.platform || process.platform; + const runtime = opts.runtime || 'generic'; + const configPath = opts.configPath || configDir; + const target: ConfiguredEntrypoint = { + runtime, + configPath, + scriptPath: path.join(configDir, 'hooks', hookName), + platform, + }; + const isShellHook = hookName.endsWith('.sh'); + + if (shellHookOmitsBashRunner({ platform, runtime, isShellHook })) return [{ ...target, selfExecutable: true }]; + + const bash = resolveBashExecutable(opts); + if (isShellHook) { + // An unresolved bash must still surface as an interpreterCandidates entry + // (the literal token, same as the portableHooks runner below) so + // validateConfiguredEntrypoints reports 'unresolved-interpreter' instead + // of silently skipping the check because the field is absent. + return [{ ...target, interpreterCandidates: [bash === null ? 'bash' : bash] }]; + } + + // #4249: check the SAME stable alias buildNodeRunnerChainToken bakes as its + // first candidate (normalizeNodePath rewrites a version-manager shim like + // fnm/nvm/mise/volta into its persistent path), not the raw, currently- + // running process.execPath — which always trivially resolves regardless of + // whether the alias actually baked into the persisted command still does. + const nodeCandidates = [ + normalizeNodePath(opts.execPath || process.execPath, opts), + 'node', + '/usr/local/bin/node', + '/usr/bin/node', + ].filter((candidate): candidate is string => Boolean(candidate)); + + if (!opts.portableHooks) { + return [{ ...target, interpreterCandidates: nodeCandidates }]; + } + + const runner: ConfiguredEntrypoint = { + runtime, + configPath, + scriptPath: path.join(configDir, 'hooks', NODE_RUNNER_RESOLVER_HOOK), + interpreterCandidates: bash === null ? ['bash'] : [bash], + platform, + }; + return [runner, { ...target, interpreterCandidates: nodeCandidates }]; +} + +function recordConfiguredHookCommand( + command: string | null, + configDir: string, + hookName: string, + opts: BuildHookCommandOpts, +): string | null { + if (command && opts.configuredEntrypoints) { + opts.configuredEntrypoints.push( + ...configuredEntrypointsForHook(configDir, hookName, opts).map(entry => ({ ...entry, command })), + ); + } + return command; } function buildHookCommand(configDir: string, hookName: string, opts?: BuildHookCommandOpts): string | null { @@ -1406,6 +1501,8 @@ function buildHookCommand(configDir: string, hookName: string, opts?: BuildHookC const runtime = opts.runtime || 'generic'; const hookShell = opts.hookShell; const isShellHook = hookName.endsWith('.sh'); + const track = (command: string | null): string | null => + recordConfiguredHookCommand(command, configDir, hookName, opts); if (shellHookOmitsBashRunner({ platform, runtime, isShellHook })) { if (opts.portableHooks) { @@ -1413,9 +1510,9 @@ function buildHookCommand(configDir: string, hookName: string, opts?: BuildHookC configDir, homeDir: os.homedir(), }); - return JSON.stringify(`${portableBaseDir}/hooks/${hookName}`); + return track(JSON.stringify(`${portableBaseDir}/hooks/${hookName}`)); } - return JSON.stringify(shellCmdProjection.posixNormalize(configDir) + '/hooks/' + hookName); + return track(JSON.stringify(shellCmdProjection.posixNormalize(configDir) + '/hooks/' + hookName)); } // .sh hooks keep the pre-#3662 shape everywhere: the bash runner resolves @@ -1423,30 +1520,42 @@ function buildHookCommand(configDir: string, hookName: string, opts?: BuildHookC // (the absolute Git-Bash discovery covers win32 — #580/#3393). if (isShellHook) { const runner = resolveBashRunner(opts); - if (runner === null) return null; + if (runner === null) { + // #4249 (antigravity review): this early return skips `track()` below, + // so an unresolved bash on win32 (no Git Bash found) previously left + // this hook silently unregistered with nothing for + // validateConfiguredEntrypoints to reject — configuredEntrypointsForHook's + // own 'unresolved bash must still surface' comment describes intent this + // return never reached. Push the entry directly (no `command`, since + // none was ever built) so the gate actually sees it. + if (opts.configuredEntrypoints) { + opts.configuredEntrypoints.push(...configuredEntrypointsForHook(configDir, hookName, opts)); + } + return null; + } if (opts.portableHooks) { const portableBaseDir = projectPortableHookBaseDir({ configDir, homeDir: os.homedir(), }); - return projectManagedHookCommand({ + return track(projectManagedHookCommand({ absoluteRunner: runner, scriptPath: `${portableBaseDir}/hooks/${hookName}`, runtime: opts.runtime || 'generic', platform, hookShell, - }); + })); } const hooksPath = shellCmdProjection.posixNormalize(configDir) + '/hooks/' + hookName; - return projectManagedHookCommand({ + return track(projectManagedHookCommand({ absoluteRunner: runner, scriptPath: hooksPath, runtime, platform, hookShell, - }); + })); } // JS hooks (#3662): the node runner is resolved at hook-fire time, never @@ -1470,7 +1579,7 @@ function buildHookCommand(configDir: string, hookName: string, opts?: BuildHookC // Absolute Git-Bash discovery on win32 when available (#580); `bash` on // PATH otherwise — the same assumption .sh hooks already make. const resolverRunner = resolveBashRunner(opts) || 'bash'; - return shellCmdProjection.projectShellCommandText({ + return track(shellCmdProjection.projectShellCommandText({ runnerToken: resolverRunner, argTokens: [ JSON.stringify(`${portableBaseDir}/hooks/${NODE_RUNNER_RESOLVER_HOOK}`), @@ -1480,19 +1589,19 @@ function buildHookCommand(configDir: string, hookName: string, opts?: BuildHookC runtime, platform, hookShell, - }); + })); } const chainRunner = buildNodeRunnerChainToken(opts); if (chainRunner === null) return null; const hooksPath = shellCmdProjection.posixNormalize(configDir) + '/hooks/' + hookName; - return shellCmdProjection.projectShellCommandText({ + return track(shellCmdProjection.projectShellCommandText({ runnerToken: chainRunner, argTokens: [JSON.stringify(hooksPath)], runtime, platform, hookShell, - }); + })); } // --------------------------------------------------------------------------- @@ -1600,8 +1709,9 @@ function mergeGsdAgentsMd(filePath: string, gsdContent: string): void { // writeClineArtifacts // --------------------------------------------------------------------------- -function writeClineArtifacts(targetDir: string, isGlobalInstall: boolean): string[] { +function writeClineArtifacts(targetDir: string, isGlobalInstall: boolean): { written: string[]; configuredEntrypoints: ConfiguredEntrypoint[] } { const written: string[] = []; + const configuredEntrypoints: ConfiguredEntrypoint[] = []; const clinerulesDir = path.join(targetDir, '.clinerules'); try { @@ -1626,6 +1736,20 @@ function writeClineArtifacts(targetDir: string, isGlobalInstall: boolean): strin try { fs.chmodSync(hookPath, 0o755); } catch { /* Windows: hooks unsupported anyway */ } written.push('.clinerules/hooks/PreToolUse'); console.log(` ${green}✓${reset} Wrote .clinerules/hooks/PreToolUse`); + // #4249 (CodeRabbit): Cline invokes this file directly via its own + // `#!/usr/bin/env node` shebang — a hybrid case. The script itself still + // needs the execute bit (selfExecutable), but unlike GSD's other JS hooks + // (which bake an absolute, install-time-resolved node path specifically to + // avoid this) its interpreter is looked up on PATH by `env` at hook-fire + // time, so `node` must also resolve or the hook can never run. + configuredEntrypoints.push({ + runtime: 'cline', + configPath: hookPath, + scriptPath: hookPath, + interpreterCandidates: ['node'], + selfExecutable: true, + platform: process.platform, + }); if (isGlobalInstall) { try { @@ -1637,7 +1761,7 @@ function writeClineArtifacts(targetDir: string, isGlobalInstall: boolean): strin } } - return written; + return { written, configuredEntrypoints }; } // --------------------------------------------------------------------------- @@ -1856,7 +1980,7 @@ function stageTransitiveHookLibs(opts: { return staged; } -function writeCursorHooksJson(targetDir: string, src: string, opts?: WriteCursorHooksJsonOpts): { hooksJsonPath: string; changed: boolean } { +function writeCursorHooksJson(targetDir: string, src: string, opts?: WriteCursorHooksJsonOpts): { hooksJsonPath: string; changed: boolean; configuredEntrypoints: ConfiguredEntrypoint[] } { opts = opts || {}; const hooksDir = path.join(targetDir, 'hooks'); fs.mkdirSync(hooksDir, { recursive: true }); @@ -1915,7 +2039,13 @@ function writeCursorHooksJson(targetDir: string, src: string, opts?: WriteCursor ensureCommonJsMarker(hooksDir); } - const hookOpts: BuildHookCommandOpts = { runtime: 'cursor', platform: opts.platform || process.platform }; + const configuredEntrypoints: ConfiguredEntrypoint[] = []; + const hookOpts: BuildHookCommandOpts = { + runtime: 'cursor', + platform: opts.platform || process.platform, + configPath: path.join(targetDir, 'hooks.json'), + configuredEntrypoints, + }; const commands: Record = {}; for (const ev of events) { const script = CURSOR_EVENT_SCRIPT_MAP[ev]; @@ -1929,7 +2059,7 @@ function writeCursorHooksJson(targetDir: string, src: string, opts?: WriteCursor const hooksJsonPath = path.join(targetDir, 'hooks.json'); const result = reconcileCursorHooksJson(hooksJsonPath, managedEntries); - return { hooksJsonPath, changed: result.changed }; + return { hooksJsonPath, changed: result.changed, configuredEntrypoints }; } function removeCursorHooksJson(targetDir: string): { changed: boolean } { @@ -2102,7 +2232,7 @@ interface WriteWindsurfHooksJsonOpts { * @param opts - `{ platform? }` * @returns `{ hooksJsonPath, changed }` */ -function writeWindsurfHooksJson(targetDir: string, src: string, opts?: WriteWindsurfHooksJsonOpts): { hooksJsonPath: string; changed: boolean } { +function writeWindsurfHooksJson(targetDir: string, src: string, opts?: WriteWindsurfHooksJsonOpts): { hooksJsonPath: string; changed: boolean; configuredEntrypoints: ConfiguredEntrypoint[] } { opts = opts || {}; const hooksDir = path.join(targetDir, 'hooks'); fs.mkdirSync(hooksDir, { recursive: true }); @@ -2154,7 +2284,13 @@ function writeWindsurfHooksJson(targetDir: string, src: string, opts?: WriteWind ensureCommonJsMarker(hooksDir); } - const hookOpts: BuildHookCommandOpts = { runtime: 'windsurf', platform: opts.platform || process.platform }; + const configuredEntrypoints: ConfiguredEntrypoint[] = []; + const hookOpts: BuildHookCommandOpts = { + runtime: 'windsurf', + platform: opts.platform || process.platform, + configPath: path.join(targetDir, 'hooks.json'), + configuredEntrypoints, + }; const commands: Record = {}; for (const ev of WINDSURF_HOOK_EVENTS) { const script = WINDSURF_EVENT_SCRIPT_MAP[ev]; @@ -2169,7 +2305,7 @@ function writeWindsurfHooksJson(targetDir: string, src: string, opts?: WriteWind const hooksJsonPath = path.join(targetDir, 'hooks.json'); const result = reconcileWindsurfHooksJson(hooksJsonPath, managedEntries); - return { hooksJsonPath, changed: result.changed }; + return { hooksJsonPath, changed: result.changed, configuredEntrypoints }; } /** @@ -3150,30 +3286,38 @@ function writeKimiHooksToml( configPath: string, targetDir: string, opts: { hookOpts: BuildHookCommandOpts }, -): { changed: boolean; path: string; entryCount: number } { +): { changed: boolean; path: string; entryCount: number; configuredEntrypoints: ConfiguredEntrypoint[] } { + const configuredEntrypoints: ConfiguredEntrypoint[] = []; + const trackedOpts = { + hookOpts: { + ...opts.hookOpts, + configPath, + configuredEntrypoints, + }, + }; const existing = fs.existsSync(configPath) ? fs.readFileSync(configPath, 'utf8') : ''; const stripped = stripKimiHooksTomlBlock(existing) ?? ''; - const block = buildKimiHooksTomlBlock(targetDir, opts); + const block = buildKimiHooksTomlBlock(targetDir, trackedOpts); const entryCount = block ? (block.match(/\[\[hooks\]\]/g) || []).length : 0; if (!block) { - if (stripped === existing) return { changed: false, path: configPath, entryCount: 0 }; + if (stripped === existing) return { changed: false, path: configPath, entryCount: 0, configuredEntrypoints }; if (stripped.trim() === '') { if (fs.existsSync(configPath)) fs.unlinkSync(configPath); } else { fs.mkdirSync(path.dirname(configPath), { recursive: true }); atomicWriteFileSync(configPath, stripped, 'utf8'); } - return { changed: true, path: configPath, entryCount: 0 }; + return { changed: true, path: configPath, entryCount: 0, configuredEntrypoints }; } const separator = stripped.trim() === '' ? '' : (stripped.endsWith('\n') ? '\n' : '\n\n'); const next = stripped.trim() === '' ? `${block}\n` : `${stripped}${separator}${block}\n`; - if (next === existing) return { changed: false, path: configPath, entryCount }; + if (next === existing) return { changed: false, path: configPath, entryCount, configuredEntrypoints }; fs.mkdirSync(path.dirname(configPath), { recursive: true }); atomicWriteFileSync(configPath, next, 'utf8'); - return { changed: true, path: configPath, entryCount }; + return { changed: true, path: configPath, entryCount, configuredEntrypoints }; } /** @@ -3220,6 +3364,101 @@ function referencesHook(h: Record, hookName: string): boolean { (typeof url === 'string' && url.includes(hookName)); } +type ConfiguredEntrypointFailureReason = 'missing' | 'unreadable' | 'wrong-file-type' | 'unresolved-interpreter' | 'not-executable'; + +interface ConfiguredEntrypoint { + runtime: string; + configPath: string; + scriptPath: string; + interpreterCandidates?: string[]; + // #4249 (CodeRabbit): true when the OS execs scriptPath directly (via a + // shebang, or Windows' own .cmd extension dispatch) — orthogonal to + // interpreterCandidates, which every producer that needs both sets + // alongside this rather than relying on their absence. Most self-executable + // entries have no candidates (a Windows-Claude .sh hook, Codex's .cmd shim); + // Cline's `#!/usr/bin/env node` is a hybrid needing both: the execute bit + // AND `node` resolving on PATH. + selfExecutable?: boolean; + platform?: string; + command?: string; +} + +interface ConfiguredEntrypointInvalid { + runtime: string; + configPath: string; + role: 'script' | 'interpreter'; + path: string; + reason: ConfiguredEntrypointFailureReason; +} + +type ConfiguredEntrypointValidationResult = + | { ok: true } + | { ok: false; invalid: ConfiguredEntrypointInvalid[] }; + +function validateConfiguredEntrypoints( + entries: ConfiguredEntrypoint[], + deps: { statSync?: typeof fs.statSync; accessSync?: typeof fs.accessSync; resolveExecutableBinary?: typeof resolveExecutableBinary } = {}, +): ConfiguredEntrypointValidationResult { + const statSync = deps.statSync ?? fs.statSync; + const accessSync = deps.accessSync ?? fs.accessSync; + const resolve = deps.resolveExecutableBinary ?? resolveExecutableBinary; + const invalid: ConfiguredEntrypointInvalid[] = []; + for (const entry of entries) { + let scriptOk = false; + try { + scriptOk = statSync(entry.scriptPath).isFile(); + if (!scriptOk) { + invalid.push({ runtime: entry.runtime, configPath: entry.configPath, role: 'script', path: entry.scriptPath, reason: 'wrong-file-type' }); + } else { + // #4249: statSync only needs search permission on the parent dirs, so + // it succeeds even for a chmod-000 file — the EACCES catch below + // never fires for that case. Read permission on the file itself must + // be checked explicitly: an interpreter opens the script directly, + // and even a self-executable shebang script is opened and read by + // its kernel-invoked interpreter, not just exec'd — X_OK alone does + // not prove it's readable. + try { + accessSync(entry.scriptPath, fs.constants.R_OK); + } catch { + scriptOk = false; + invalid.push({ runtime: entry.runtime, configPath: entry.configPath, role: 'script', path: entry.scriptPath, reason: 'unreadable' }); + } + } + } catch (statErr) { + // #4249 Nit: EACCES means a parent directory couldn't be searched — + // a real (if rare) permission problem, distinct from ENOENT's "missing". + // EPERM: Windows' equivalent permission-denied code for a directory a + // parent path couldn't be traversed into. + const code = (statErr as NodeJS.ErrnoException)?.code; + const reason = code === 'EACCES' || code === 'EPERM' ? 'unreadable' : 'missing'; + invalid.push({ runtime: entry.runtime, configPath: entry.configPath, role: 'script', path: entry.scriptPath, reason }); + } + // #4249: selfExecutable is the sole source of truth for whether the OS + // execs scriptPath directly via its own shebang (set explicitly by every + // producer that needs it — a Windows-Claude .sh hook, Codex's Windows + // .cmd shim, Cline's hybrid `env node` hook — rather than inferred from + // the absence of interpreterCandidates, which Cline's hybrid case also + // carries). Skip on win32 like resolveExecutableBinary's own X_OK + // carve-out does: POSIX mode bits don't mean executable on Windows, and a + // real accessSync(X_OK) there would fail a .cmd shim under a test that + // simulates win32 on a POSIX runner (Node's own no-op only protects an + // actual Windows machine). Cline is the only producer where this runs. + if (scriptOk && entry.selfExecutable && (entry.platform ?? process.platform) !== 'win32') { + try { + accessSync(entry.scriptPath, fs.constants.X_OK); + } catch { + invalid.push({ runtime: entry.runtime, configPath: entry.configPath, role: 'script', path: entry.scriptPath, reason: 'not-executable' }); + } + } + if (entry.interpreterCandidates && !entry.interpreterCandidates.some(candidate => + resolve(candidate, { platform: entry.platform, requireExecutable: true }) !== null, + )) { + invalid.push({ runtime: entry.runtime, configPath: entry.configPath, role: 'interpreter', path: entry.interpreterCandidates.join(' | '), reason: 'unresolved-interpreter' }); + } + } + return invalid.length === 0 ? { ok: true } : { ok: false, invalid }; +} + // --------------------------------------------------------------------------- // Exports // --------------------------------------------------------------------------- @@ -3295,7 +3534,9 @@ export = { // Shared stageTransitiveHookLibs, buildHookCommand, + recordConfiguredHookCommand, applySettingsJsonHooks, + validateConfiguredEntrypoints, referencesHook, rewriteLegacyManagedNodeHookCommands, reconcileManagedShellHookCommands, diff --git a/src/shell-command-projection.cts b/src/shell-command-projection.cts index fa1c53dcf..ccf833b01 100644 --- a/src/shell-command-projection.cts +++ b/src/shell-command-projection.cts @@ -249,6 +249,7 @@ const MANAGED_HOOK_COMMAND_BASENAMES_BY_SURFACE: Record> = { 'gsd-session-state.sh', 'gsd-validate-commit.sh', 'gsd-phase-boundary.sh', + 'gsd-graphify-update.sh', // #3662: same three guards as MANAGED_HOOK_BASENAMES_BY_SURFACE above — // their absence here meant isManagedHookCommand never recognized them, so // the settings.json→settings.local.json migration filter (and uninstall diff --git a/tests/codex-config-hooks.test.cjs b/tests/codex-config-hooks.test.cjs index 9fe3bdf26..ee337abc4 100644 --- a/tests/codex-config-hooks.test.cjs +++ b/tests/codex-config-hooks.test.cjs @@ -2361,6 +2361,95 @@ describe('#3245 — idempotent rollback reverts skills/, agents/, and VERSION', }); } +// ──────────────────────────────────────────────────────────────────────── +// #4249 — install() exposes the full snapshot restore, not just migrations rollback +// ──────────────────────────────────────────────────────────────────────── +{ + const { test, describe, beforeEach, afterEach } = require('node:test'); + const os = require('os'); + const { cleanup } = require('./helpers.cjs'); + const previousGsdTestMode = process.env.GSD_TEST_MODE; + process.env.GSD_TEST_MODE = '1'; + const { install } = require('../bin/install.js'); + if (previousGsdTestMode === undefined) { + delete process.env.GSD_TEST_MODE; + } else { + process.env.GSD_TEST_MODE = previousGsdTestMode; + } + + // concurrency: false — drives the real install pipeline like the block above. + describe('#4249 — install() exposes the full snapshot restore, not just migrations rollback', { concurrency: false }, () => { + let tmpDir; + let codexHome; + + beforeEach(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-4249-codex-rollback-')); + codexHome = path.join(tmpDir, 'codex-home'); + fs.mkdirSync(codexHome, { recursive: true }); + }); + + afterEach(() => cleanup(tmpDir)); + + test('result.rollbackPreInstallSnapshot() reverts skills/, agents/, and VERSION', () => { + // Skills resolve $HOME-relative independent of CODEX_HOME (#2088), so the + // rollback closure must run before this sandboxing is torn down — inline + // runCodexInstall's env dance instead of using the auto-restoring helper, + // matching how installAllRuntimes' real aggregate gate calls it: in the + // same process env install() itself ran in, never after it's restored. + const previousHome = process.env.HOME; + const previousUserProfile = process.env.USERPROFILE; + const previousCodexHome = process.env.CODEX_HOME; + const previousCwd = process.cwd(); + process.env.HOME = codexHome; + process.env.USERPROFILE = codexHome; + process.env.CODEX_HOME = codexHome; + process.chdir(path.join(__dirname, '..')); + try { + const result = install(true, 'codex'); + + // A configured-entrypoint validation failure discovered outside install() + // (installAllRuntimes' aggregate assertConfiguredEntrypoints, run after + // this function already returned) reaches for this field. Before #4249 + // the only rollback Codex exposed was rollbackInstallerMigrations, which + // reverts installer-migration state only and leaves the skills/agents/ + // VERSION this successful install just wrote untouched. + result.rollbackPreInstallSnapshot(); + + const skillsDir = codexSkillsRoot(codexHome); + const gsdSkills = fs.existsSync(skillsDir) + ? fs.readdirSync(skillsDir, { withFileTypes: true }).filter(e => e.isDirectory() && e.name.startsWith('gsd-')) + : []; + assert.strictEqual(gsdSkills.length, 0, 'rollback must remove all gsd-* skill directories: ' + gsdSkills.map(e => e.name).join(', ')); + + const versionPath = path.join(codexHome, 'gsd-core', 'VERSION'); + assert.strictEqual(fs.existsSync(versionPath), false, 'rollback must remove gsd-core/VERSION'); + + // #4249 (agy adversarial review): the whole point of this describe block + // is that rollback covers the full pre-install snapshot, not just + // installer migrations — config.toml/hooks.json must revert too. Both + // were absent before this fresh install, so rollback must remove them. + assert.strictEqual( + fs.existsSync(path.join(codexHome, 'config.toml')), + false, + 'rollback must remove config.toml (absent before this fresh install)' + ); + assert.strictEqual( + fs.existsSync(path.join(codexHome, 'hooks.json')), + false, + 'rollback must remove hooks.json (absent before this fresh install)' + ); + } finally { + process.chdir(previousCwd); + if (previousHome === undefined) delete process.env.HOME; + else process.env.HOME = previousHome; + if (previousUserProfile === undefined) delete process.env.USERPROFILE; + else process.env.USERPROFILE = previousUserProfile; + if (previousCodexHome === undefined) delete process.env.CODEX_HOME; + else process.env.CODEX_HOME = previousCodexHome; + } + }); + }); +} // ──────────────────────────────────────────────────────────────────────── // Folded from tests/bug-3285-codex-hooks-state-allowed.test.cjs — consolidation epic #1969 (B1 #1970) diff --git a/tests/configured-entrypoint-validation.test.cjs b/tests/configured-entrypoint-validation.test.cjs new file mode 100644 index 000000000..dd10950ad --- /dev/null +++ b/tests/configured-entrypoint-validation.test.cjs @@ -0,0 +1,558 @@ +'use strict'; + +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const os = require('node:os'); +const path = require('node:path'); +const test = require('node:test'); + +const helpers = require('./helpers.cjs'); + +const hooksSurface = require('../gsd-core/bin/lib/runtime-hooks-surface.cjs'); +const { install, installAllRuntimes, finishInstall } = require('../bin/install.js'); + +/** + * Run `fn` with HOME/USERPROFILE pointed at a fresh temp dir and every + * config-location env var scrubbed, so a real install() never touches the + * developer's own config. Restores all of it, and cleans the dir, afterwards. + */ +function withSandboxedHome(t, prefix, fn) { + const root = fs.mkdtempSync(path.join(os.tmpdir(), prefix)); + t.after(() => helpers.cleanup(root)); + const savedHome = process.env.HOME; + const savedUserProfile = process.env.USERPROFILE; + process.env.HOME = root; + process.env.USERPROFILE = root; + const restoreConfigLocationEnv = helpers.scrubConfigLocationEnv(); + try { + return fn(root); + } finally { + if (savedHome === undefined) delete process.env.HOME; + else process.env.HOME = savedHome; + if (savedUserProfile === undefined) delete process.env.USERPROFILE; + else process.env.USERPROFILE = savedUserProfile; + restoreConfigLocationEnv(); + } +} + +test('configured entrypoint validation exposes an aggregate typed boundary', () => { + assert.equal( + typeof hooksSurface.validateConfiguredEntrypoints, + 'function', + 'the Runtime Hooks Surface must export configured-entrypoint validation', + ); +}); + +test('finishInstall rejects an invalid configured entrypoint before Done output', (t) => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'configured-entrypoint-finish-')); + t.after(() => helpers.cleanup(root)); + const logs = []; + const originalLog = console.log; + console.log = (...args) => logs.push(args.join(' ')); + // #2665/#4249: finishInstall asserts configured entrypoints before any of its + // own writes now, but still sandbox HOME (+ USERPROFILE for os.homedir() on + // Windows) and config-location env defensively, so a future reordering that + // reintroduces a pre-assertion write can never redirect it to a live config dir. + const savedHome = process.env.HOME; + const savedUserProfile = process.env.USERPROFILE; + process.env.HOME = root; + process.env.USERPROFILE = root; + const restoreConfigLocationEnv = helpers.scrubConfigLocationEnv(); + try { + assert.throws(() => finishInstall(null, null, null, false, 'cline', false, root, { + configuredEntrypoints: [{ runtime: 'cline', configPath: path.join(root, 'config'), scriptPath: path.join(root, 'missing.js') }], + }), /Configured entrypoint validation failed/); + // #4249 review, Major: the message must name the actual consequence for + // the failing runtime, not just that something failed — Cline has no + // revert path, so its entry must be flagged "NOT reverted". + assert.throws(() => finishInstall(null, null, null, false, 'cline', false, root, { + configuredEntrypoints: [{ runtime: 'cline', configPath: path.join(root, 'config'), scriptPath: path.join(root, 'missing.js') }], + }), /NOT reverted/); + } finally { + console.log = originalLog; + restoreConfigLocationEnv(); + if (savedHome === undefined) delete process.env.HOME; + else process.env.HOME = savedHome; + if (savedUserProfile === undefined) delete process.env.USERPROFILE; + else process.env.USERPROFILE = savedUserProfile; + } + assert.equal(logs.some(line => line.includes('Done!')), false); +}); + +test('a hook already registered under a stale command is still tracked for validation on re-install (#4154 Blocker)', (t) => { + withSandboxedHome(t, 'configured-entrypoint-stale-', () => { + const first = install(true, 'claude'); + assert.ok(first.settingsPath, 'a fresh global install must produce a settings path'); + finishInstall(first.settingsPath, first.settings, first.statuslineCommand, false, 'claude', true, first.configDir, { + configuredEntrypoints: first.configuredEntrypoints, + }); + + // Simulate an entry registered by an older installer under a DIFFERENT + // node install (e.g. an nvm switch, #4087/#4098/#4137): same real + // scriptPath under /hooks/ (that never changes across + // installer versions) and still shaped as the modern runtime-resolving + // chain (rewriteLegacyManagedNodeHookCommands deliberately never touches + // an already-current-format entry — #3662), but baked with a node path + // this install would never produce. `hasGsdUpdateHook` still finds it and + // applySettingsJsonHooks takes its register-only-if-absent branch on the + // next install (never rewriting it). + const onDisk = JSON.parse(fs.readFileSync(first.settingsPath, 'utf8')); + const staleEntry = (onDisk.hooks.SessionStart || []).find(entry => + entry.hooks && entry.hooks.some(h => h.command && h.command.includes('gsd-check-update.js')) + ); + assert.ok(staleEntry, 'a fresh install must register the check-update hook'); + const staleCommand = hooksSurface.buildHookCommand(first.configDir, 'gsd-check-update.js', { + execPath: '/old/nvm/pinned/node', + platform: process.platform, + runtime: 'claude', + }); + for (const h of staleEntry.hooks) { + if (h.command && h.command.includes('gsd-check-update.js')) { + h.command = staleCommand; + } + } + fs.writeFileSync(first.settingsPath, JSON.stringify(onDisk, null, 2)); + + const second = install(true, 'claude'); + const trackedNames = (second.configuredEntrypoints || []).map(entry => path.basename(entry.scriptPath)); + assert.ok( + trackedNames.includes('gsd-check-update.js'), + `a hook already registered under a stale command must still be tracked for validation, got: ${trackedNames.join(', ')}`, + ); + }); +}); + +test('configured entrypoint validation aggregates file and interpreter failures without execution', (t) => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'configured-entrypoint-')); + t.after(() => helpers.cleanup(root)); + const directory = path.join(root, 'directory'); + fs.mkdirSync(directory); + + const unreadablePath = path.join(root, 'unreadable.js'); + const notExecutablePath = path.join(root, 'not-executable.js'); + + const result = hooksSurface.validateConfiguredEntrypoints([ + { runtime: 'claude', configPath: path.join(root, 'settings.json'), scriptPath: path.join(root, 'missing.js') }, + { runtime: 'claude', configPath: path.join(root, 'settings.json'), scriptPath: directory }, + { runtime: 'claude', configPath: path.join(root, 'settings.json'), scriptPath: __filename, interpreterCandidates: ['missing-node'] }, + { runtime: 'claude', configPath: path.join(root, 'settings.json'), scriptPath: unreadablePath }, + // #4249: selfExecutable means this entry is invoked directly via its own + // shebang (e.g. a Windows-Claude .sh hook) — must itself be +x. + // platform pinned to non-win32: the X_OK check itself is a POSIX-only + // concept (skipped entirely on win32, matching production) — this case + // must exercise it deterministically regardless of which OS runs the test. + { runtime: 'claude', configPath: path.join(root, 'settings.json'), scriptPath: notExecutablePath, selfExecutable: true, platform: 'linux' }, + ], { + resolveExecutableBinary: () => null, + statSync: (p) => { + if (p === unreadablePath) { + const err = new Error('EACCES: permission denied'); + err.code = 'EACCES'; + throw err; + } + return fs.statSync(p === notExecutablePath ? __filename : p); + }, + accessSync: (p, mode) => { + if (p === notExecutablePath && mode === fs.constants.X_OK) { + const err = new Error('EACCES: permission denied'); + err.code = 'EACCES'; + throw err; + } + // notExecutablePath is never written to disk (its statSync mock above + // redirects to a real file instead) — its R_OK call must redirect too, + // or this falls through to a real accessSync on a nonexistent path. + return fs.accessSync(p === notExecutablePath ? __filename : p, mode); + }, + }); + + assert.equal(result.ok, false); + assert.deepEqual(result.invalid.map(({ role, reason }) => [role, reason]), [ + ['script', 'missing'], + ['script', 'wrong-file-type'], + ['interpreter', 'unresolved-interpreter'], + ['script', 'unreadable'], + ['script', 'not-executable'], + ]); +}); + +test('an interpreter-invoked script that exists but has no read permission is reported unreadable, not ok (#4249 agy review)', (t) => { + // statSync only needs search (+x) permission on the parent directories, so + // it succeeds on a chmod-000 file even though `node