diff --git a/.changeset/2088-eos-codex-declarative-adapter.md b/.changeset/2088-eos-codex-declarative-adapter.md new file mode 100644 index 000000000..e929c3ddc --- /dev/null +++ b/.changeset/2088-eos-codex-declarative-adapter.md @@ -0,0 +1,5 @@ +--- +type: Changed +pr: 2110 +--- +**Codex is now driven through the public Host-Integration Interface, with three capability upgrades (ADR-1239 / EoS).** Codex previously installed via hardcoded `runtime === 'codex'`/`isCodex` projection in `bin/install.js`; its `config.toml` / agent-`.toml` / `hooks.json` install now runs through the declarative embedding adapter and descriptor-driven `runtime.hostBehaviors`, with **zero** positive `isCodex` gates and **zero** `runtime === 'codex'` branches remaining (source-guarded). Install/uninstall output stays byte-parity-gated (`tests/fixtures/golden-install-parity/codex.json`). Three Context7-verified upgrades land, each with a test driving the user-reachable surface: (1) **skill root** — GSD skills now install to Codex's canonical `$HOME/.agents/skills` (via a skills-kind `home` override) instead of the deprecated `$CODEX_HOME/skills` fallback, and pre-move installs are migrated (stale `~/.codex/skills/gsd-*` cleaned on both install and uninstall, user-owned content preserved); (2) **hook events** — GSD registers the six documented Codex lifecycle events it previously skipped (`PreToolUse`, `PermissionRequest`, `PreCompact`, `PostCompact`, `SubagentStop`, `UserPromptSubmit`, in addition to the existing `SessionStart`/`SubagentStart`/`Stop`/`PostToolUse`) in `hooks.json`, so `gsd-context-monitor` fires at the same points as in Claude Code, and the descriptor `extendedHookEvents` is reconciled from `[]` to the schema-valid wired subset; (3) **dispatch tuning** — `[agents] max_depth = 1` is written explicitly into the managed `config.toml` block to pin the negotiated `dispatch.maxDepth: 1` axis (`degradationFor` flattens GSD-hosted waves to single-level), and `validateCodexConfigSchema` now permits a known-scalar-only `[agents]` AgentsToml table (coexisting with the flattened `[agents.gsd-*]` role sub-tables) while still rejecting the `[[agents]]` and unknown-key break-forms from #2760. (#2088) diff --git a/.changeset/2089-eos-cursor-imperative-adapter.md b/.changeset/2089-eos-cursor-imperative-adapter.md new file mode 100644 index 000000000..870d57748 --- /dev/null +++ b/.changeset/2089-eos-cursor-imperative-adapter.md @@ -0,0 +1,5 @@ +--- +type: Changed +pr: 2120 +--- +**Cursor is now driven through the public Host-Integration Interface, with two capability upgrades (ADR-1239 / EoS).** Cursor previously installed via hardcoded `runtime === 'cursor'`/`isCursor` branches in `bin/install.js`; its install/uninstall now runs through the imperative adapter, and every hardcoded cursor branch is folded into descriptor-driven `runtime.hostBehaviors` (reapplyCommand, frontmatterDialect, hooksJsonSurface, skipSharedHooksInstall, reportCommandsDir, managedHookEvents). Install/uninstall output is **byte-identical** (golden parity asserted for all 16 runtimes). Two Context7-verified upgrades land: (1) **expanded hook-bus coverage** — GSD registers all 6 managed lifecycle events in Cursor's `hooks.json` (`preToolUse`, `stop`, `subagentStart`, `subagentStop` in addition to the original `sessionStart`/`postToolUse`), driven by a new descriptor-driven adapter module (`src/host-integration-adapters/imperative-hook-bus.cts`) that reads `hostBehaviors.managedHookEvents` instead of a hardcoded event pair; cite https://cursor.com/docs/hooks. (2) **named/background nested subagent dispatch** — Cursor's `dispatch.background`/`backgroundDispatch`/`nested` are all `true` with `maxDepth: 2`, so `shouldFlattenDispatch(cursor)` returns `false` and GSD's wave-based execution drives Cursor's native background + depth-2 nested subagent invocation instead of flattening to inline sequential calls; cite https://cursor.com/docs/subagents + https://cursor.com/docs/sdk/typescript. (#2089) diff --git a/.gitignore b/.gitignore index c70665f48..37d552553 100644 --- a/.gitignore +++ b/.gitignore @@ -69,6 +69,7 @@ build/ /tsconfig.build.tsbuildinfo /gsd-core/bin/lib/host-integration.cjs /gsd-core/bin/lib/host-integration-sdk.cjs +/gsd-core/bin/lib/host-integration-adapters/imperative-hook-bus.cjs /gsd-core/bin/lib/handshake-serialized.cjs /gsd-core/bin/lib/install-effort-resolver.cjs /gsd-core/bin/lib/install-engine.cjs diff --git a/bin/install.js b/bin/install.js index b8e9fa279..16cc410c6 100755 --- a/bin/install.js +++ b/bin/install.js @@ -102,6 +102,45 @@ const reset = '\x1b[0m'; // Codex config.toml constants const GSD_CODEX_MARKER = '# GSD Agent Configuration \u2014 managed by gsd-core installer'; const GSD_CODEX_HOOKS_OWNERSHIP_PREFIX = '# GSD codex_hooks ownership: '; +// Known scalar fields of Codex's `AgentsToml` struct (codex-rs/config/src/ +// config_toml.rs \u2014 `[agents]` table). Codex marks the struct +// `#[schemars(deny_unknown_fields)]`, so a bare `[agents]` table is valid ONLY +// when every direct key is one of these (named agent roles live in the flattened +// `[agents.]` sub-tables, a separate `AgentRoleToml`). GSD writes only +// `max_depth` (ADR-1239 upgrade 2 / #2088); the full set is enumerated so the +// schema check accepts a user's other legitimate AgentsToml scalars too. +const CODEX_AGENTS_TOML_SCALAR_KEYS = new Set([ + 'max_threads', + 'max_depth', + 'job_max_runtime_seconds', + 'interrupt_message', +]); +// GSD's managed dispatch-depth value. Codex's implicit default is also 1 (root +// sessions start at depth 0); writing it EXPLICITLY pins the negotiated +// `dispatch.maxDepth: 1` axis instead of relying on codex-cli's implicit default +// (ADR-1239 upgrade 2 / #2088). Per the negotiated capability, GSD-hosted Codex +// dispatch is single-level (maxDepth === 1 \u2192 `degradationFor` flattens waves). +const GSD_CODEX_AGENTS_MAX_DEPTH = 1; +// Codex hooks.json lifecycle events GSD registers beyond SessionStart (which has +// its own dedicated path). This is Codex's OWN hook-event vocabulary (per +// developers.openai.com/codex/config-reference), distinct from the cross-runtime +// settings.json `extendedHookEvents` descriptor field (a claude/gemini-family +// allowlist consumed only by hooksSurface==='settings-json' runtimes — Codex is +// codex-hooks-json). All route through gsd-context-monitor.js. #772 wired the +// first three; #2088 adds the remaining six documented events so GSD's monitor +// fires at the same lifecycle points as in Claude Code. Install and uninstall +// share this list so the registered set and the removed set never diverge. +const CODEX_EXTENDED_HOOK_EVENTS = [ + 'SubagentStart', + 'Stop', + 'PostToolUse', + 'PreToolUse', + 'PermissionRequest', + 'PreCompact', + 'PostCompact', + 'SubagentStop', + 'UserPromptSubmit', +]; // Codex's hook-enabling feature flag (issue #3566). Codex itself marks // `codex_hooks` as a `legacy_key` in codex-rs/features/src/legacy.rs; the // canonical current key under [features] is `hooks`. The installer always @@ -216,12 +255,30 @@ const GSD_COPILOT_SESSION_HOOK_PWSH = // Cursor reads hook configs from /.cursor/hooks.json (local) or // ~/.cursor/hooks.json (global) with the shape { version: 1, hooks: { : [...] } }. // Events use camelCase: sessionStart, postToolUse, preToolUse, etc. -// A `command` hook entry runs an external script. GSD registers two managed hooks: -// sessionStart → gsd-cursor-session-start.js (context injection) -// postToolUse → gsd-cursor-post-tool.js (STATE.md update monitor) +// A `command` hook entry runs an external script. GSD registers six managed hooks +// (AC4a upgrade, #2089 — ADR-1239): +// sessionStart → gsd-cursor-session-start.js (context injection) +// postToolUse → gsd-cursor-post-tool.js (STATE.md update monitor) +// preToolUse → gsd-cursor-pre-tool.js (write-path guard) +// stop → gsd-cursor-stop.js (verify-work reminder) +// subagentStart → gsd-cursor-subagent-start.js (subagent context injection) +// subagentStop → gsd-cursor-subagent-stop.js (subagent completion reminder) // Cursor docs: https://cursor.com/docs/hooks const GSD_CURSOR_SESSION_HOOK_SCRIPT = 'gsd-cursor-session-start.js'; const GSD_CURSOR_POST_TOOL_HOOK_SCRIPT = 'gsd-cursor-post-tool.js'; +const GSD_CURSOR_PRE_TOOL_HOOK_SCRIPT = 'gsd-cursor-pre-tool.js'; +const GSD_CURSOR_STOP_HOOK_SCRIPT = 'gsd-cursor-stop.js'; +const GSD_CURSOR_SUBAGENT_START_HOOK_SCRIPT = 'gsd-cursor-subagent-start.js'; +const GSD_CURSOR_SUBAGENT_STOP_HOOK_SCRIPT = 'gsd-cursor-subagent-stop.js'; +// All GSD-managed Cursor hook scripts (used by uninstall cleanup). +const GSD_CURSOR_HOOK_SCRIPTS = [ + GSD_CURSOR_SESSION_HOOK_SCRIPT, + GSD_CURSOR_POST_TOOL_HOOK_SCRIPT, + GSD_CURSOR_PRE_TOOL_HOOK_SCRIPT, + GSD_CURSOR_STOP_HOOK_SCRIPT, + GSD_CURSOR_SUBAGENT_START_HOOK_SCRIPT, + GSD_CURSOR_SUBAGENT_STOP_HOOK_SCRIPT, +]; // Marker comment embedded in managed hook entries so GSD can find+remove them. const GSD_CURSOR_HOOK_MARKER = 'gsd-managed'; @@ -357,6 +414,22 @@ function _hostBehaviors(runtime) { return _resolveHostBehaviors(runtime, _capabilityRegistry); } +/** + * Resolve the ACTUAL on-disk skills-install directory for a runtime, honoring a + * skills-kind `home` override (ADR-1239 upgrade 3 / #2088: e.g. Codex skills -> + * $HOME/.agents/skills instead of the runtime's configDir). Descriptor-driven + * (no runtime === '' check) so the snapshot/rollback machinery and post-install + * verification look where the skills actually landed. Falls back to /skills. + */ +function _resolveSkillsRootDir(runtime, targetDir, scope) { + try { + const layout = resolveRuntimeArtifactLayout(runtime, targetDir, scope); + const skillsKind = layout.kinds.find((k) => k.kind === 'skills'); + if (skillsKind) return path.join(skillsKind.home || targetDir, skillsKind.destSubpath); + } catch (_e) { /* fall through to the configDir default */ } + return path.join(targetDir, 'skills'); +} + /** * Construct the imperative Host-Integration adapter (ADR-1239 / #2086), FAIL-OPEN. * `createImperativeAdapter` composes the capability registry via @@ -3147,6 +3220,77 @@ function cleanupWindsurfLegacyDevinSkills(workspaceDir) { return removed; } +/** + * Migrate a skills kind that moved to an alternate `home` (ADR-1239 split-home): + * remove now-stale `*` skill dirs left at the OLD configDir-rooted + * location by installs from before the move. Without this, upgrading (e.g. Codex + * relocating skills to ~/.agents/skills) orphans the pre-move dirs at + * ~/.codex/skills. Only managed `*` dirs are touched; user-owned content + * (non-prefixed dirs, gsd-dev-preferences, symlinks) is preserved. Fail-open. + * @param {string} oldSkillsDir absolute path to the pre-move skills location + * @param {string} prefix managed skill-dir prefix (e.g. 'gsd-') + * @returns {number} count of stale dirs removed + */ +function cleanupMovedSkillsOldLocation(oldSkillsDir, prefix) { + if (!fs.existsSync(oldSkillsDir)) return 0; + + // Mirror the user-owned list from cleanupCodexSkillMetadataSidecars (#2973). + const _userOwnedSkillDirs = new Set(['gsd-dev-preferences']); + let removed = 0; + + for (const entry of fs.readdirSync(oldSkillsDir, { withFileTypes: true })) { + if (!entry.isDirectory() || !entry.name.startsWith(prefix)) continue; + if (_userOwnedSkillDirs.has(entry.name)) continue; + + const dirToRemove = path.join(oldSkillsDir, entry.name); + try { + // Symlink guard (mirrors cleanupWindsurfLegacyDevinSkills): never delete + // through a symlinked gsd-* dir — it could escape the tree. + const stat = fs.lstatSync(dirToRemove); + if (stat.isSymbolicLink()) continue; + + fs.rmSync(dirToRemove, { recursive: true, force: true }); + removed++; + } catch (_err) { + // Fail open — a single bad dir must not block install/uninstall. + } + } + + // Prune the old skills dir if now empty — leaves the configHome clean. + // Never remove a non-empty container (user may keep other content there). + try { + if (fs.existsSync(oldSkillsDir) && fs.readdirSync(oldSkillsDir).length === 0) { + fs.rmdirSync(oldSkillsDir); + } + } catch (_err) { + // best-effort container cleanup + } + + return removed; +} + +/** + * When a runtime's skills kind declares an alternate `home` (split-home move), + * return the now-stale configDir-rooted skills location that installs before the + * move used; null when no move is in effect (no home override, or home resolves + * to the same path). Descriptor-driven — no per-runtime hardcoding. + * @returns {string|null} + */ +function _resolveMovedSkillsOldDir(runtime, targetDir, scope) { + try { + const layout = resolveRuntimeArtifactLayout(runtime, targetDir, scope); + const skillsKind = layout.kinds.find((k) => k.kind === 'skills'); + if (skillsKind && skillsKind.home) { + const oldDir = path.join(targetDir, skillsKind.destSubpath); + const newDir = path.join(skillsKind.home, skillsKind.destSubpath); + if (path.resolve(oldDir) !== path.resolve(newDir)) return oldDir; + } + } catch (_e) { + // No migration when the layout can't resolve — never block on this. + } + return null; +} + /** * Generate the GSD config block for Codex config.toml. * @param {Array<{name: string, description: string}>} agents @@ -3162,6 +3306,17 @@ function generateCodexConfigBlock(agents, targetDir) { '', ]; + // ADR-1239 upgrade 2 / #2088 — explicit dispatch tuning. Pin `max_depth` on the + // `[agents]` (AgentsToml) table rather than relying on codex-cli's implicit + // default, realizing the negotiated `dispatch.maxDepth: 1` axis. This bare + // `[agents]` scalar table coexists with the flattened `[agents.]` role + // sub-tables below (validated by validateCodexConfigSchema, which permits a + // known-scalar-only `[agents]`). Emitted before the role tables so the parent + // table is opened first. + lines.push('[agents]'); + lines.push(`max_depth = ${GSD_CODEX_AGENTS_MAX_DEPTH}`); + lines.push(''); + for (const { name, description } of agents) { // #2727 — Codex 0.124.0 requires [agents.] struct format, not [[agents]] sequence. // [[agents]] (introduced in #2645) is rejected by codex-cli 0.124.0 with @@ -3175,6 +3330,52 @@ function generateCodexConfigBlock(agents, targetDir) { return lines.join('\n'); } +/** + * Extract a user's pre-existing AgentsToml scalar assignments from a bare + * `[agents]` table — every known scalar EXCEPT `max_depth` (which GSD manages + * and always re-emits as 1). Returned as raw `key = value` line strings so + * mergeCodexConfig can PRESERVE them in the managed block instead of silently + * dropping the user's tuning when the bare `[agents]` table is purged (#2088 + * review finding: the loosened validator declares such a table legitimate, so + * install must not destroy it). Only the first bare `[agents]` section is read; + * `[agents.]` role tables are ignored. Fail-open → []. + * @returns {string[]} + */ +function extractCodexUserAgentsScalars(content) { + const preserved = []; + let section; + try { + section = getTomlTableSections(content).find((s) => !s.array && s.path === 'agents'); + } catch (_e) { + return preserved; + } + if (!section) return preserved; + const body = content.slice(section.headerEnd, section.end); + for (const record of getTomlLineRecords(body)) { + if (record.startsInMultilineString || record.tableHeader) continue; + const trimmed = record.text.trim(); + if (!trimmed || trimmed.startsWith('#')) continue; + if (!record.keySegments || record.keySegments.length !== 1) continue; + const key = record.keySegments[0]; + if (key === 'max_depth') continue; // GSD-managed — GSD's value wins. + if (!CODEX_AGENTS_TOML_SCALAR_KEYS.has(key)) continue; + preserved.push(trimmed); + } + return preserved; +} + +/** + * Splice preserved user AgentsToml scalar lines into the managed GSD config + * block, immediately after the `[agents]` header and before GSD's `max_depth` + * line. Operates on the pre-EOL-normalization block (LF joins), matching only + * the bare `[agents]` header (never `[agents.]`). Returns the block + * unchanged when there is nothing to preserve or the anchor is absent. + */ +function spliceCodexAgentsScalars(block, scalarLines) { + if (!scalarLines || scalarLines.length === 0) return block; + return block.replace(/(\n\[agents\]\n)(max_depth = )/, `$1${scalarLines.join('\n')}\n$2`); +} + /** * Strip any managed GSD agent sections from a TOML string. * @@ -3197,6 +3398,16 @@ function stripCodexGsdAgentSections(content) { return true; } + // GSD's managed `[agents]` scalar block (ADR-1239 upgrade 2 / #2088 — the + // `max_depth` dispatch-tuning table). Install purges any pre-existing bare + // `[agents]` and writes its own, so a known-scalar-only bare `[agents]` is + // GSD-owned; strip it on uninstall. (The marker path already removes it via + // the marker-to-EOF cut; this covers the no-marker fallback.) + if (!section.array && section.path === 'agents') { + const body = content.slice(section.headerEnd, section.end); + return codexBareAgentsHasOnlyKnownScalars(body); + } + // Legacy `[[agents]]` array-of-tables (#2645) — only strip blocks whose // `name = "gsd-..."`, preserving user-authored [[agents]] entries. if (section.array && section.path === 'agents') { @@ -3224,7 +3435,11 @@ function stripGsdFromCodexConfig(content) { const codexHooksOwnership = getManagedCodexHooksOwnership(content); if (markerIndex !== -1) { - // Has GSD marker — remove everything from marker to EOF + // Has GSD marker — remove everything from marker to EOF. First recover the + // user's own AgentsToml scalars (max_threads etc.) that install folded into + // the managed [agents] block (#2088), so a full install→uninstall cycle + // round-trips the user's tuning. GSD-managed max_depth is dropped. + const preservedScalars = extractCodexUserAgentsScalars(content.slice(markerIndex)); let before = content.substring(0, markerIndex); before = stripCodexHooksFeatureAssignments(before, codexHooksOwnership); // Also strip GSD-injected feature keys above the marker (Case 3 inject) @@ -3233,6 +3448,9 @@ function stripGsdFromCodexConfig(content) { before = before.replace(/^\[features\]\s*\n(?=\[|$)/m, ''); before = before.replace(/^\[agents\]\s*\n(?=\[|$)/m, ''); before = before.replace(/^(?:\r?\n)+/, '').trimEnd(); + if (preservedScalars.length > 0) { + before = (before ? before + eol + eol : '') + '[agents]' + eol + preservedScalars.join(eol); + } if (!before) return null; return before + eol; } @@ -3243,7 +3461,11 @@ function stripGsdFromCodexConfig(content) { cleaned = cleaned.replace(/^multi_agent\s*=\s*true\s*(?:\r?\n)?/m, ''); cleaned = cleaned.replace(/^default_mode_request_user_input\s*=\s*true\s*(?:\r?\n)?/m, ''); - // Remove [agents.gsd-*] sections (from header to next section or EOF) + // #2088: recover the user's own AgentsToml scalars before the [agents] table is + // stripped, so they survive uninstall even in the no-marker fallback path. + const preservedScalars = extractCodexUserAgentsScalars(cleaned); + + // Remove [agents.gsd-*] sections + the managed known-scalar [agents] table. cleaned = stripCodexGsdAgentSections(cleaned); // Remove [features] section if now empty (only header, no keys before next section) @@ -3254,6 +3476,10 @@ function stripGsdFromCodexConfig(content) { cleaned = cleaned.replace(/^(?:\r?\n)+/, '').trimEnd(); + if (preservedScalars.length > 0) { + cleaned = (cleaned ? cleaned + eol + eol : '') + '[agents]' + eol + preservedScalars.join(eol); + } + if (!cleaned) return null; return cleaned + eol; } @@ -4725,6 +4951,33 @@ function parseTomlToObject(content) { * - `hooks.` MUST be an array of tables when present (Codex ≥0.124 * rejects bare `[hooks.]` single-bracket maps). */ +/** + * True when a bare `[agents]` table body contains ONLY known AgentsToml scalar + * keys (CODEX_AGENTS_TOML_SCALAR_KEYS) — i.e. it is a valid AgentsToml struct + * that Codex's `deny_unknown_fields` will accept, not the break-causing form + * (#2760) that carries an unknown key. Comments and blank lines are ignored; an + * empty body is trivially valid. Mirrors isLegacyGsdAgentsSection's line scan. + */ +function codexBareAgentsHasOnlyKnownScalars(body) { + const lineRecords = getTomlLineRecords(body); + for (const record of lineRecords) { + // Conservative reject of anything not positively a single known-scalar + // assignment. A multiline-string value cannot be a valid AgentsToml scalar + // (max_threads/max_depth/job_max_runtime_seconds are integers, + // interrupt_message is a bool — none are strings), so codex would reject it + // too; rejecting here is correct, not a false negative. + if (record.startsInMultilineString) return false; + if (record.tableHeader) return false; + const trimmed = record.text.trim(); + if (!trimmed || trimmed.startsWith('#')) continue; + if (!record.keySegments || record.keySegments.length !== 1 || + !CODEX_AGENTS_TOML_SCALAR_KEYS.has(record.keySegments[0])) { + return false; + } + } + return true; +} + function validateCodexConfigSchema(content) { let parsed; try { @@ -4753,10 +5006,21 @@ function validateCodexConfigSchema(content) { } if (!section.array && section.path === 'agents') { - return { - ok: false, - reason: 'bare [agents] table is invalid in current Codex schema (expected [agents.] struct form)', - }; + // #2760 rejected ALL bare `[agents]` tables because a bare table holding a + // non-AgentsToml key (`default = "x"`, a role name, etc.) triggers Codex's + // "invalid type: ..., expected struct AgentsToml" and breaks every CLI + // invocation. But a bare `[agents]` whose keys are all valid AgentsToml + // scalars (max_depth/max_threads/...) IS a valid struct — that is exactly + // GSD's managed `max_depth` dispatch-tuning block (ADR-1239 upgrade 2 / + // #2088), and a user's own scalar tuning. Permit known-scalar-only; still + // reject any bare `[agents]` carrying an unknown key. + const body = content.slice(section.headerEnd, section.end); + if (!codexBareAgentsHasOnlyKnownScalars(body)) { + return { + ok: false, + reason: 'bare [agents] table with a non-AgentsToml key is invalid in current Codex schema (expected [agents.] struct form, or only AgentsToml scalars like max_depth/max_threads)', + }; + } } // hooks.state.* is Codex's persistent hook-trust namespace (added in @@ -5032,7 +5296,13 @@ function mergeCodexConfig(configPath, gsdBlock) { const existing = fs.readFileSync(configPath, 'utf8'); const eol = detectLineEnding(existing); - const normalizedGsdBlock = gsdBlock.replace(/\r?\n/g, eol); + // #2088 review: the bare `[agents]` table is purged below (Case 2/3 via + // stripLeakedGsdCodexSections) to keep a single managed `[agents]`. Preserve + // the user's own AgentsToml scalar tuning (max_threads, job_max_runtime_seconds, + // interrupt_message — everything except GSD-managed max_depth) by re-emitting + // it inside the managed block, so install never silently drops it. + const mergedGsdBlock = spliceCodexAgentsScalars(gsdBlock, extractCodexUserAgentsScalars(existing)); + const normalizedGsdBlock = mergedGsdBlock.replace(/\r?\n/g, eol); const markerIndex = existing.indexOf(GSD_CODEX_MARKER); // Case 2: Has GSD marker — truncate and re-append @@ -6565,8 +6835,24 @@ function uninstall(isGlobal, runtime = DEFAULT_RUNTIME) { } removedCount++; + // ADR-1239 split-home migration: the adapter/plan uninstall targets the new + // `home` location (e.g. Codex → ~/.agents/skills). A user who installed + // BEFORE the move and never reinstalled still has managed gsd-* skill dirs at + // the old configDir-rooted location (~/.codex/skills) — remove those too so + // uninstall leaves nothing behind. User-owned content is preserved. + { + const _movedOldSkillsDir = _resolveMovedSkillsOldDir(runtime, targetDir, scope); + if (_movedOldSkillsDir) { + const migrated = cleanupMovedSkillsOldLocation(_movedOldSkillsDir, 'gsd-'); + if (migrated > 0) { + removedCount++; + console.log(` ${green}✓${reset} Removed ${migrated} legacy skill dir(s) from ${_movedOldSkillsDir}`); + } + } + } + // 1a. Non-layout Codex side-effects: agent .toml files, config.toml sections, hooks.json - if (isCodex) { + if (_hostBehaviors(runtime).tomlConfigInstall) { const codexAgentsDir = path.join(targetDir, 'agents'); if (fs.existsSync(codexAgentsDir)) { const tomlFiles = fs.readdirSync(codexAgentsDir); @@ -6605,8 +6891,10 @@ function uninstall(isGlobal, runtime = DEFAULT_RUNTIME) { console.log(` ${green}✓${reset} Removed managed Codex SessionStart hook from hooks.json`); } - // #772: remove new Codex hook event registrations added by this enhancement. - for (const eventName of ['SubagentStart', 'Stop', 'PostToolUse']) { + // #772/#2088: remove every managed Codex extended hook-event registration. + // Shares CODEX_EXTENDED_HOOK_EVENTS with the install loop — removal set == + // registration set, so no managed event is ever orphaned. + for (const eventName of CODEX_EXTENDED_HOOK_EVENTS) { const eventCleanup = removeCodexHooksJsonEvent(targetDir, eventName); if (eventCleanup.changed) { removedCount++; @@ -6699,17 +6987,20 @@ function uninstall(isGlobal, runtime = DEFAULT_RUNTIME) { } } - // 1b-cursor. Non-layout Cursor side-effects (issue #777): remove GSD-managed - // hook entries from hooks.json and clean up the managed hook scripts. - if (isCursor) { + // 1b-cursor. Descriptor-driven hook-bus cleanup (ADR-1239 / #2089): remove + // GSD-managed hook entries from hooks.json and clean up the managed hook + // scripts. Gated by the hostBehaviors.hooksJsonSurface descriptor axis, not a + // hardcoded `isCursor` branch. + if (_hostBehaviors(runtime).hooksJsonSurface) { const hooksJsonCleanup = removeCursorHooksJson(targetDir); if (hooksJsonCleanup.changed) { removedCount++; console.log(` ${green}✓${reset} Removed GSD-managed Cursor hooks from hooks.json`); } - // Remove the managed hook scripts (session-start + post-tool). + // Remove all GSD-managed hook scripts (sessionStart, postToolUse, preToolUse, + // stop, subagentStart, subagentStop — AC4a, #2089). const hooksDir = path.join(targetDir, 'hooks'); - for (const script of [GSD_CURSOR_SESSION_HOOK_SCRIPT, GSD_CURSOR_POST_TOOL_HOOK_SCRIPT]) { + for (const script of GSD_CURSOR_HOOK_SCRIPTS) { const p = path.join(hooksDir, script); try { if (fs.existsSync(p)) { @@ -7514,11 +7805,16 @@ function writeManifest(configDir, runtime = DEFAULT_RUNTIME, options = {}) { // Claude local uses flatCommandsDir instead for manifest recording. const flatCommandsDir = path.join(configDir, 'commands'); const opencodeCommandDir = path.join(configDir, _hostBehaviors(runtime).flatCommandDir || 'command'); - // Hermes nests GSD skills under skills/gsd/ as a single category (#2841). + // Hermes nests GSD skills under skills/gsd/ as a single category (#2841) — + // already encoded in its layout descriptor's destSubpath ('skills/gsd'). // All other runtimes that use the Codex-style skills layout use a flat skills/ root. - const codexSkillsDir = isHermes - ? path.join(configDir, 'skills', 'gsd') - : path.join(configDir, 'skills'); + // ADR-1239 upgrade 3 (#2088): honor a skills-kind `home` override (e.g. Codex + // skills -> $HOME/.agents/skills instead of configDir/skills) via the same + // descriptor-driven helper used by the snapshot/rollback/verification paths, + // so the manifest records what's actually on disk. _resolveSkillsRootDir already + // resolves destSubpath (which includes hermes's 'skills/gsd' nesting) — do not + // re-append 'gsd' or the hermes dir gets double-nested to skills/gsd/gsd. + const codexSkillsDir = _resolveSkillsRootDir(runtime, configDir, options.scope === 'local' ? 'local' : 'global'); const codexSkillsManifestPrefix = isHermes ? 'skills/gsd/' : 'skills/'; const agentsDir = path.join(configDir, 'agents'); const manifest = { @@ -7970,13 +8266,9 @@ function reportLocalPatches(configDir, runtime = DEFAULT_RUNTIME) { if (meta.files && meta.files.length > 0) { const reapplyCommand = _hostBehaviors(runtime).reapplyCommand ? _hostBehaviors(runtime).reapplyCommand - : runtime === 'codex' - ? '$gsd-update --reapply' - : runtime === 'cursor' - ? 'gsd-update --reapply (mention the skill name)' - : runtime === 'kimi' - ? '/skill:gsd-update --reapply' - : '/gsd-update --reapply'; + : runtime === 'kimi' + ? '/skill:gsd-update --reapply' + : '/gsd-update --reapply'; console.log(''); console.log(' ' + yellow + 'Local patches detected' + reset + ' (from v' + meta.from_version + '):'); for (const f of meta.files) { @@ -8209,8 +8501,8 @@ function install(isGlobal, runtime = DEFAULT_RUNTIME, options = {}) { // Map — content snapshot of each pre-existing gsd-* agent file. const codexPreInstallAgentContents = new Map(); let codexPreInstallVersionBytes = null; - if (isCodex && !isMinimalMode(_effectiveInstallMode)) { - const _preSkillsDir = path.join(targetDir, 'skills'); + if (_hostBehaviors(runtime).tomlConfigInstall && !isMinimalMode(_effectiveInstallMode)) { + const _preSkillsDir = _resolveSkillsRootDir(runtime, targetDir, isGlobal ? 'global' : 'local'); if (fs.existsSync(_preSkillsDir)) { for (const entry of fs.readdirSync(_preSkillsDir, { withFileTypes: true })) { if (entry.isDirectory() && entry.name.startsWith('gsd-')) { @@ -8263,10 +8555,10 @@ function install(isGlobal, runtime = DEFAULT_RUNTIME, options = {}) { // atomic-write temp files. It is safe to call before any writes have happened. // The full restoreCodexSnapshot() (defined inside the config block) additionally // handles config.toml, which is not yet touched at this point in the pipeline. - const _codexPreConfigRollback = !isCodex || isMinimalMode(_effectiveInstallMode) ? null : () => { + const _codexPreConfigRollback = !_hostBehaviors(runtime).tomlConfigInstall || isMinimalMode(_effectiveInstallMode) ? null : () => { rollbackInstallerMigrations(); // skills/gsd-* — pass 1: restore snapshot entries (may be absent if deleted mid-install). - const _earlySkillsDir = path.join(targetDir, 'skills'); + const _earlySkillsDir = _resolveSkillsRootDir(runtime, targetDir, isGlobal ? 'global' : 'local'); for (const skillName of codexPreInstallSkillNames) { const skillDirPath = path.join(_earlySkillsDir, skillName); const fileMap = codexPreInstallSkillContents.get(skillName); @@ -8459,6 +8751,13 @@ function install(isGlobal, runtime = DEFAULT_RUNTIME, options = {}) { if (_isSkillsRuntime) { // Layout-driven install for skills-based runtimes (full and minimal modes) const scope = isGlobal ? 'global' : 'local'; + // ADR-1239 upgrade 3 / #2088: a kind may declare an alternate install `home` + // (e.g. Codex skills -> $HOME/.agents/skills) instead of the runtime's normal + // configDir. Resolve the ACTUAL on-disk skills root here, descriptor-driven + // (no isCodex check), so downstream sidecar-cleanup and post-install + // verification look in the right place regardless of which runtime declares + // an alternate home for its skills kind. + const _skillsRootDir = _resolveSkillsRootDir(runtime, targetDir, scope); // ADR-1239 / #2086: drive install through the public Host-Integration Interface // (imperative adapter). The adapter delegates to the SAME installRuntimeArtifacts // engine call -> byte-identical output (gated by golden-install-parity). Fail-open @@ -8481,8 +8780,23 @@ function install(isGlobal, runtime = DEFAULT_RUNTIME, options = {}) { // index BOTH SKILL.md and the sidecar, causing each GSD skill to appear twice // in autocomplete. Cleaning them up fixes the duplication; SKILL.md alone is // sufficient for Codex discovery. User-owned dirs are never touched. - if (isCodex) { - cleanupCodexSkillMetadataSidecars(path.join(targetDir, 'skills')); + if (_hostBehaviors(runtime).cleanupSkillSidecars) { + cleanupCodexSkillMetadataSidecars(_skillsRootDir); + } + + // ADR-1239 split-home migration: when a runtime's skills kind moved to an + // alternate `home` (e.g. Codex → ~/.agents/skills), pre-move installs left + // managed gsd-* skill dirs at the old configDir-rooted location + // (~/.codex/skills). Reinstalling here writes the new location but would + // otherwise orphan the old one — clean up the stale gsd-* dirs. + { + const _movedOldSkillsDir = _resolveMovedSkillsOldDir(runtime, targetDir, scope); + if (_movedOldSkillsDir) { + const migrated = cleanupMovedSkillsOldLocation(_movedOldSkillsDir, 'gsd-'); + if (migrated > 0) { + console.log(` ${green}✓${reset} Migrated ${migrated} skill dir(s) off the legacy ${_movedOldSkillsDir} location`); + } + } } // #1629 Finding B: Windsurf local only — remove legacy .devin/skills/gsd-* @@ -8554,7 +8868,7 @@ function install(isGlobal, runtime = DEFAULT_RUNTIME, options = {}) { } } } else { - const skillsDir = path.join(targetDir, 'skills'); + const skillsDir = _skillsRootDir; if (fs.existsSync(skillsDir)) { const count = fs.readdirSync(skillsDir, { withFileTypes: true }) .filter(e => e.isDirectory() && e.name.startsWith('gsd-')).length; @@ -8582,8 +8896,9 @@ function install(isGlobal, runtime = DEFAULT_RUNTIME, options = {}) { } } - // Cursor only: also report the commands/ output (#785 — Cursor 1.6 slash commands) - if (isCursor) { + // Descriptor-driven commands/ output report (#785 — Cursor 1.6 slash commands). + // Gated by hostBehaviors.reportCommandsDir, not a hardcoded `isCursor` branch (#2089). + if (_hostBehaviors(runtime).reportCommandsDir) { const commandsDir = path.join(targetDir, 'commands'); if (fs.existsSync(commandsDir)) { const cmdCount = fs.readdirSync(commandsDir) @@ -8809,7 +9124,7 @@ function install(isGlobal, runtime = DEFAULT_RUNTIME, options = {}) { for (const file of fs.readdirSync(agentsDest)) { if ( file.startsWith('gsd-') && - (file.endsWith('.md') || (isCodex && file.endsWith('.toml'))) + (file.endsWith('.md') || (_hostBehaviors(runtime).agentTomlFiles && file.endsWith('.toml'))) ) { fs.unlinkSync(path.join(agentsDest, file)); } @@ -8827,7 +9142,7 @@ function install(isGlobal, runtime = DEFAULT_RUNTIME, options = {}) { // Without stripping them here, a full → minimal reinstall would leave the // runtime advertising the old full agent surface even though the agent // files are gone. Reuse the same helper that powers `--uninstall`. - if (isCodex) { + if (_hostBehaviors(runtime).tomlConfigInstall) { const codexConfigPath = path.join(targetDir, 'config.toml'); if (fs.existsSync(codexConfigPath)) { const existing = fs.readFileSync(codexConfigPath, 'utf8'); @@ -8883,14 +9198,12 @@ function install(isGlobal, runtime = DEFAULT_RUNTIME, options = {}) { content = convertClaudeToOpencodeFrontmatter(content, { isAgent: true, modelOverride: _ocModelOverride }); } else if (_hostBehaviors(runtime).frontmatterDialect === 'kilo') { content = convertClaudeToKiloFrontmatter(content, { isAgent: true }); - } else if (isCodex) { + } else if (_hostBehaviors(runtime).frontmatterDialect === 'codex') { content = convertClaudeAgentToCodexAgent(content); } else if (isCopilot) { content = convertClaudeAgentToCopilotAgent(content, isGlobal); } else if (isAntigravity) { content = convertClaudeAgentToAntigravityAgent(content, isGlobal); - } else if (isCursor) { - content = convertClaudeAgentToCursorAgent(content); } else if (isWindsurf) { content = convertClaudeAgentToWindsurfAgent(content); } else if (isAugment) { @@ -8971,7 +9284,9 @@ function install(isGlobal, runtime = DEFAULT_RUNTIME, options = {}) { // its native plugin adapter (#1914, installed above under plugins/gsd-core.js) // spawns the staged hooks/*.js scripts via OpenCode's event bus and needs both // them and the CommonJS package.json marker written below. - if (!isCodex && !isCopilot && !isCursor && !isWindsurf && !isTrae && !isCline && !isKimi && !isKilo && !isZcode) { + // #2089: Cursor's exclusion is now descriptor-driven via + // hostBehaviors.skipSharedHooksInstall (was hardcoded !isCursor). + if (!isCodex && !isCopilot && _hostBehaviors(runtime).skipSharedHooksInstall !== true && !isWindsurf && !isTrae && !isCline && !isKimi && !isKilo && !isZcode) { // Write package.json to force CommonJS mode for GSD scripts // Prevents "require is not defined" errors when project has "type": "module" // Node.js walks up looking for package.json - this stops inheritance from project @@ -9062,7 +9377,8 @@ function install(isGlobal, runtime = DEFAULT_RUNTIME, options = {}) { // Gate hooks/lib/ install on the same runtimes that receive hooks (see line ~8702). // Codex/Copilot/Cursor/Windsurf/Trae/Cline do not use the shared hooks/lib/ helpers - // (Cursor uses standalone .js hook scripts registered via hooks.json; Codex uses + // (Cursor uses standalone .js hook scripts registered via hooks.json — gated + // descriptor-driven via hostBehaviors.skipSharedHooksInstall, #2089; Codex uses // hooks.json directly; the others skip hooks entirely); Kilo and ZCode also skip // hooks entirely (hooksSurface:'none' with no plugin surface — #1821). OpenCode // is NOT excluded: its #1914 plugin adapter spawns the staged hooks and requires @@ -9070,7 +9386,7 @@ function install(isGlobal, runtime = DEFAULT_RUNTIME, options = {}) { // helpers — otherwise the Codex comment downstream ("we deliberately do *not* // copy hooks/lib/ for Codex") is contradicted in practice. const hooksLibSrc = path.join(src, 'hooks', 'lib'); - if (!isCodex && !isCopilot && !isCursor && !isWindsurf && !isTrae && !isCline && !isKimi && !isKilo && !isZcode && fs.existsSync(hooksLibSrc)) { + if (!isCodex && !isCopilot && _hostBehaviors(runtime).skipSharedHooksInstall !== true && !isWindsurf && !isTrae && !isCline && !isKimi && !isKilo && !isZcode && fs.existsSync(hooksLibSrc)) { const hooksLibDest = path.join(targetDir, 'hooks', 'lib'); fs.mkdirSync(hooksLibDest, { recursive: true }); copyLibDir(hooksLibSrc, hooksLibDest, GSD_HOOK_LIB_FILES); @@ -9197,7 +9513,7 @@ function install(isGlobal, runtime = DEFAULT_RUNTIME, options = {}) { } // Write file manifest for future modification detection - writeManifest(targetDir, runtime, { mode: _effectiveInstallMode }); + writeManifest(targetDir, runtime, { mode: _effectiveInstallMode, scope: isGlobal ? 'global' : 'local' }); console.log(` ${green}✓${reset} Wrote file manifest (${MANIFEST_NAME})`); // Report any backed-up local patches @@ -9340,7 +9656,7 @@ function install(isGlobal, runtime = DEFAULT_RUNTIME, options = {}) { // (copyCommandsAsCodexSkills removes pre-existing gsd-* dirs before re-writing) // are restored even when they are absent from disk at rollback time (#3245 CR). // • Dirs that did not pre-exist: remove entirely. - const _rollbackSkillsDir = path.join(targetDir, 'skills'); + const _rollbackSkillsDir = _resolveSkillsRootDir(runtime, targetDir, isGlobal ? 'global' : 'local'); // Pass 1 — restore snapshot entries (may be absent from disk if deleted mid-install). for (const skillName of codexPreInstallSkillNames) { const skillDirPath = path.join(_rollbackSkillsDir, skillName); @@ -9451,7 +9767,7 @@ function install(isGlobal, runtime = DEFAULT_RUNTIME, options = {}) { // Re-write the manifest now that .toml agent files exist on disk. // The initial writeManifest call (before Codex config generation) could // not include agents/gsd-*.toml because those files did not yet exist. - writeManifest(targetDir, runtime, { mode: _effectiveInstallMode }); + writeManifest(targetDir, runtime, { mode: _effectiveInstallMode, scope: isGlobal ? 'global' : 'local' }); } else { console.log(` ${dim}↳${reset} Skipping Codex agent config generation (minimal install)`); } @@ -9591,24 +9907,23 @@ function install(isGlobal, runtime = DEFAULT_RUNTIME, options = {}) { } } - // ── Codex extended hook events (#772) ──────────────────────────────── - // Codex CLI stabilised a full hook-event set in rust-v0.137.0. Register - // three new high-value lifecycle events — all routed through - // gsd-context-monitor.js so context-headroom warnings surface at: - // SubagentStart — subagent session open (environment / agent-name aware) - // Stop — model stop / session final-response moment - // PostToolUse — after each tool invocation (mirrors Claude baseline) - // - // Note: UserPromptSubmit is NOT wired — gsd-prompt-guard exits unless - // tool_name is Write|Edit (PreToolUse payload shape), so it would be a - // silent no-op for the UserPromptSubmit payload. Registration deferred - // to a follow-on issue. + // ── Codex extended hook events (#772, #2088) ───────────────────────── + // Codex CLI stabilised a full hook-event set in rust-v0.137.0. GSD + // registers CODEX_EXTENDED_HOOK_EVENTS (#2088 adds the 6 documented + // events beyond the original #772 three) — all routed through + // gsd-context-monitor.js so context-headroom warnings surface at each + // lifecycle point: SubagentStart/SubagentStop (subagent open/close), + // Stop (final-response), PreToolUse/PostToolUse (tool boundaries), + // PermissionRequest (approval prompts), Pre/PostCompact (context + // compaction), and UserPromptSubmit (per-turn context injection). The + // context-monitor script decides per-payload what to do; unregistered + // events simply never fire. // // Guard: only register when the context-monitor file exists and the node // runner is available — same guards as the SessionStart path above. const contextMonitorFile = path.join(targetDir, 'hooks', 'gsd-context-monitor.js'); if (codexNodeRunner && fs.existsSync(contextMonitorFile)) { - for (const codexEvent of ['SubagentStart', 'Stop', 'PostToolUse']) { + for (const codexEvent of CODEX_EXTENDED_HOOK_EVENTS) { const eventWrite = ensureCodexHooksJsonEvent(targetDir, codexEvent, { absoluteRunner: codexNodeRunner, platform: process.platform, @@ -9620,7 +9935,7 @@ function install(isGlobal, runtime = DEFAULT_RUNTIME, options = {}) { } } } else if (!codexNodeRunner) { - console.warn(` ${yellow}⚠${reset} Skipped Codex SubagentStart/Stop/PostToolUse hook registration — Node runner unavailable.`); + console.warn(` ${yellow}⚠${reset} Skipped Codex extended hook-event registration — Node runner unavailable.`); } // ── end Codex extended hook events ──────────────────────────────────── } @@ -9685,16 +10000,20 @@ function install(isGlobal, runtime = DEFAULT_RUNTIME, options = {}) { } if (plan.installSurface === 'cursor-hooks-json') { - // #777: Cursor v2.4+ supports hooks.json. Register sessionStart + postToolUse. - // Hook scripts are copied to /hooks/ and referenced by hooks.json. - const cursorHookResult = writeCursorHooksJson(targetDir, src, {}); + // ADR-1239 / #2089: Cursor hooks.json driven by the descriptor-managed hook-bus + // adapter. Registers all 6 managed events (sessionStart, postToolUse, preToolUse, + // stop, subagentStart, subagentStop) via runtime-hooks-surface.cts, which reads + // the event list from the descriptor-driven adapter module. + const cursorHookResult = writeCursorHooksJson(targetDir, src, { + managedHookEvents: _hostBehaviors(runtime).managedHookEvents, + }); if (cursorHookResult.changed) { - console.log(` ${green}✓${reset} Configured Cursor lifecycle hooks (sessionStart, postToolUse)`); + console.log(` ${green}✓${reset} Configured Cursor lifecycle hooks (sessionStart, postToolUse, preToolUse, stop, subagentStart, subagentStop)`); } else { console.log(` ${green}✓${reset} Cursor lifecycle hooks already up to date`); } // Re-run the manifest pass so the hook scripts + hooks.json are hash-tracked. - writeManifest(targetDir, runtime, { mode: _effectiveInstallMode }); + writeManifest(targetDir, runtime, { mode: _effectiveInstallMode, scope: isGlobal ? 'global' : 'local' }); persistActiveProfileMarker(); return { settingsPath: null, settings: null, statuslineCommand: null, updateBannerCommand: null, runtime, configDir: targetDir }; } @@ -9712,7 +10031,7 @@ function install(isGlobal, runtime = DEFAULT_RUNTIME, options = {}) { 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 }); + writeManifest(targetDir, runtime, { mode: _effectiveInstallMode, scope: isGlobal ? 'global' : 'local' }); persistActiveProfileMarker(); return { settingsPath: null, settings: null, statuslineCommand: null, updateBannerCommand: null, runtime, configDir: targetDir }; } @@ -11009,6 +11328,13 @@ module.exports = { generateCodexAgentToml, cleanupCodexSkillMetadataSidecars, cleanupWindsurfLegacyDevinSkills, + cleanupMovedSkillsOldLocation, + _resolveMovedSkillsOldDir, + _resolveSkillsRootDir, + codexBareAgentsHasOnlyKnownScalars, + extractCodexUserAgentsScalars, + spliceCodexAgentsScalars, + CODEX_EXTENDED_HOOK_EVENTS, generateCodexConfigBlock, stripGsdFromCodexConfig, migrateCodexHooksMapFormat, @@ -11095,6 +11421,11 @@ module.exports = { mergeGsdAgentsMd, GSD_CURSOR_SESSION_HOOK_SCRIPT, GSD_CURSOR_POST_TOOL_HOOK_SCRIPT, + GSD_CURSOR_PRE_TOOL_HOOK_SCRIPT, + GSD_CURSOR_STOP_HOOK_SCRIPT, + GSD_CURSOR_SUBAGENT_START_HOOK_SCRIPT, + GSD_CURSOR_SUBAGENT_STOP_HOOK_SCRIPT, + GSD_CURSOR_HOOK_SCRIPTS, GSD_CURSOR_HOOK_MARKER, buildCursorHookEntry, isManagedCursorHookEntry, diff --git a/capabilities/codex/capability.json b/capabilities/codex/capability.json index db6e68df3..c067d9564 100644 --- a/capabilities/codex/capability.json +++ b/capabilities/codex/capability.json @@ -27,7 +27,8 @@ "prefix": "gsd-", "nesting": "flat", "recursive": false, - "converter": "convertClaudeCommandToCodexSkill" + "converter": "convertClaudeCommandToCodexSkill", + "home": ".agents" } ], "local": [ @@ -37,7 +38,8 @@ "prefix": "gsd-", "nesting": "flat", "recursive": false, - "converter": "convertClaudeCommandToCodexSkill" + "converter": "convertClaudeCommandToCodexSkill", + "home": ".agents" } ] }, @@ -49,7 +51,11 @@ "installSurface": "codex-toml", "writesSharedSettings": false, "permissionWriter": null, - "extendedHookEvents": [], + "extendedHookEvents": [ + "SubagentStop", + "Stop", + "PreCompact" + ], "hostIntegration": { "embeddingMode": "declarative", "commandSurface": "slash-file", @@ -66,6 +72,13 @@ "stateIO": "filesystem", "transport": "mcp", "runtime": "node" + }, + "hostBehaviors": { + "reapplyCommand": "$gsd-update --reapply", + "tomlConfigInstall": true, + "cleanupSkillSidecars": true, + "agentTomlFiles": true, + "frontmatterDialect": "codex" } } } diff --git a/capabilities/cursor/capability.json b/capabilities/cursor/capability.json index b4877e40f..1d58b7100 100644 --- a/capabilities/cursor/capability.json +++ b/capabilities/cursor/capability.json @@ -98,6 +98,23 @@ "stateIO": "filesystem", "transport": "mcp", "runtime": "node" + }, + "hostBehaviors": { + "reapplyCommand": "gsd-update --reapply (mention the skill name)", + "frontmatterDialect": "cursor", + "hooksJsonSurface": true, + "skipSharedHooksInstall": true, + "reportCommandsDir": true, + "skipUpdateBannerCommand": true, + "skipSettingsUi": true, + "managedHookEvents": [ + "sessionStart", + "postToolUse", + "preToolUse", + "stop", + "subagentStart", + "subagentStop" + ] } } } diff --git a/docs/INVENTORY-MANIFEST.json b/docs/INVENTORY-MANIFEST.json index 4628c7b77..046d19ce2 100644 --- a/docs/INVENTORY-MANIFEST.json +++ b/docs/INVENTORY-MANIFEST.json @@ -452,7 +452,11 @@ "gsd-config-reload.js", "gsd-context-monitor.js", "gsd-cursor-post-tool.js", + "gsd-cursor-pre-tool.js", "gsd-cursor-session-start.js", + "gsd-cursor-stop.js", + "gsd-cursor-subagent-start.js", + "gsd-cursor-subagent-stop.js", "gsd-ensure-canonical-path.js", "gsd-graphify-update.sh", "gsd-phase-boundary.sh", diff --git a/docs/INVENTORY.md b/docs/INVENTORY.md index af1075362..63f4d67a9 100644 --- a/docs/INVENTORY.md +++ b/docs/INVENTORY.md @@ -548,6 +548,10 @@ Full listing: `hooks/`. | `gsd-update-banner.js` | `SessionStart` | Opt-in banner surfacing update availability when GSD statusline isn't used (PR #2795) | | `gsd-cursor-session-start.js` | Cursor `sessionStart` | Cursor-native context injection at session start (issue #777) | | `gsd-cursor-post-tool.js` | Cursor `postToolUse` | Cursor-native STATE.md update monitor after tool calls (issue #777) | +| `gsd-cursor-pre-tool.js` | Cursor `preToolUse` | Cursor-native write-path guard for `.planning/` (ADR-1239 / #2089) | +| `gsd-cursor-stop.js` | Cursor `stop` | Cursor-native verify-work reminder on agent stop (ADR-1239 / #2089) | +| `gsd-cursor-subagent-start.js` | Cursor `subagentStart` | Cursor-native subagent context injection (ADR-1239 / #2089) | +| `gsd-cursor-subagent-stop.js` | Cursor `subagentStop` | Cursor-native subagent completion reminder (ADR-1239 / #2089) | | `gsd-prompt-guard.js` | `PreToolUse` | Scans `.planning/` writes for prompt-injection patterns (advisory) | | `gsd-workflow-guard.js` | `PreToolUse` | Detects file edits outside GSD workflow context (advisory, opt-in) | | `gsd-read-guard.js` | `PreToolUse` | Advisory guard preventing Edit/Write on unread files | diff --git a/docs/adr/2121-phase-identifier-parsing-consolidation.md b/docs/adr/2121-phase-identifier-parsing-consolidation.md new file mode 100644 index 000000000..6dd67b024 --- /dev/null +++ b/docs/adr/2121-phase-identifier-parsing-consolidation.md @@ -0,0 +1,168 @@ +# ADR-2121: Phase-Identifier Parsing Consolidation + +- **Status:** Accepted (Phase 0 — ADR only; locks the contract Phases 1–4 execute against. No production code lands in this PR.) +- **Date:** 2026-07-09 +- **Issue:** [#2121](https://github.com/open-gsd/gsd-core/issues/2121) — epic (tech-debt / root-cause consolidation, `type: chore` + `approved-enhancement`) +- **Supersedes:** nothing +- **Relationship to prior work:** completes [#1455](https://github.com/open-gsd/gsd-core/issues/1455) (which introduced the prefix-tolerant lookup source but only in `roadmap-parser.cts`); it is the parser-layer analog of the `package-identity.cjs` single-source seam (ADR-referenced by `scripts/lint-package-identity-drift.cjs`). + +## Context + +Phase-identifier parsing — turning a phase reference (`3`, `03`, `12A`, `2.7`, `2-01`, `CK-01`, `AB-29`, `Milestone v0.5 complete`) into a normalized identity, a ROADMAP heading match, or a resolved phase — is **implemented independently in at least six modules**. `src/phase-id.cts` exists and is *meant* to be the canonical normalizer, but the surrounding modules each roll their own regex instead of delegating. A fix or invariant lands on one surface and its siblings silently diverge, so the same defect keeps re-surfacing under new issue numbers. + +This is exactly the class `CLAUDE.md` warns about under **Generative Fix Divergence**: + +> When sharing constants/arrays/parsers between parallel surfaces, add a parity assertion test that fails if they diverge. + +The guard rule exists in the standards, but it is not applied to phase-ID parsing. Three confirmed bugs are the direct consequence: + +| Symptom issue | Site | Divergent behavior | Blocked? | +|---|---|---|---| +| [#2111](https://github.com/open-gsd/gsd-core/issues/2111) | `state.cts:parseProsePhaseField` (`state.cts:1118-1131`) | `/\b(\d+[A-Z]?(?:\.\d+)*)\b/i` mines the *first* numeral in a prose `Phase:` line; `Milestone v0.5 complete` → `5`, `v1.0` → `0` (a reserved sentinel). `milestone complete v0.5` writes `current_phase: 5` instead of the real last phase. | No | +| [#2114](https://github.com/open-gsd/gsd-core/issues/2114) | `roadmap.cts:cmdRoadmapGetPhase` (`roadmap.cts:238-303`) + `getRoadmapPhaseWithFallback` (`roadmap.cts:209-234`) | Both hand-roll a **2-source** lookup (exact → numeric). `getRoadmapPhaseInternal` (`roadmap-parser.cts:262-287`) loops a **3-source** pass (exact → numeric → prefix-tolerant) via `roadmapPhaseLookupSources`. `roadmap get-phase 29` returns empty for `### Phase AB-29:` while `init.phase-op 29` resolves it. | No | +| [#2104](https://github.com/open-gsd/gsd-core/issues/2104) | `phase-id.cts:normalizePhaseName` / `stripProjectCodePrefix` (`phase-id.cts:44-77`) | `PROJECT_CODE_PREFIX_STRIP_RE_I = /^[A-Z][A-Z0-9_]*-(?=\d)/i` strips *any* prefix-shaped token with no check against the configured `project_code`; `MEM-01` collapses to bare `01` even when the project code is `LKML`. The #2056 guard was added to `cmdInitPlanPhase` only; the three sibling init commands still collapse foreign prefixes. | **Yes — sequenced after PR #2105 (#2056)** | + +### Why #1455 did not close the loop + +`git show 2dedbdd11` (fix(#1455)) touched `phase-id.cts`, `phase.cts`, `roadmap-parser.cts`, `roadmap-upgrade.cts`, `validate.cts` — **not `roadmap.cts`**. It added `OPTIONAL_PROJECT_CODE_PREFIX_SOURCE` (`phase-id.cts:24`) and the third lookup source inside `roadmapPhaseLookupSources` (`roadmap-parser.cts:245-260`), but `roadmap.cts` never imported the constant. The mechanical root of #2114 is an import-list asymmetry: `roadmap.cts:16-17` destructures seven names from `phase-id.cjs` but omits `OPTIONAL_PROJECT_CODE_PREFIX_SOURCE`, so it *structurally cannot* build the prefix-tolerant source today. A point fix on `roadmap.cts` would leave the divergence itself — one seam missing, N call sites free to re-diverge — fully intact. + +### The full divergent surface (broader than the three modules the issue names) + +Memtrace blast-radius analysis (`get_impact normalizePhaseName` → **risk CRITICAL, 84 affected symbols, 20 direct callers across 19 files**) and a symbol sweep surfaced the complete surface. This matters because it bounds both the back-compat risk and the guard's scope: + +- **`phase-id.cts` (the partial canonical seam, 272 lines, pure — "no Node built-ins").** Already owns: `escapeRegex` (`:15`), `OPTIONAL_PROJECT_CODE_PREFIX_SOURCE` (`:24`), `OPTIONAL_PHASE_TAG_SOURCE` (`:42`), `stripProjectCodePrefix` (`:44`), `normalizePhaseName` (`:54`), `getMilestoneFromPhaseId` (`:79`), `getPhaseDirFromPhaseId` (`:88`), `phaseMarkdownRegexSource` (`:107`), `phaseMarkdownRegexSourceExact` (`:138`), `comparePhaseNum` (`:144`), `extractPhaseToken` (`:197`), `phaseTokenMatches` (`:247`). It does **not** parse-from-prose, and its regex-source builders are consumed by callers, not applied here. +- **`state.cts` — five independently-maintained phase-token regex shapes:** `parseProsePhaseField` (`:1120`, the `\b…\b` miner), its unanchored twins `resolvePhaseIdForCompletePhase` (`:2722`) and `cmdStateCompletePhase` (`:2750`), the `Phase`-anchored `extractRetiredPhaseNumbers` (`:1319`), the strip/pad idiom in `cmdStateValidate` (`:2291`,`:2294`), and the digits-only dir shape in `phaseInventoryProvider` (`:2642`). Only `phaseKeyFromToken`/`phaseKeyFromDir` (`:1283-1288`) delegate to `phase-id.cjs`. +- **`roadmap.cts` — four regex-construction sites** (`:410`, `:528`, `:787`, plus the two named CLI functions). The three standalone sites already use the canonical `phaseMarkdownRegexSource` builder; only the two named functions diverge (no prefix-tolerant source). +- **`roadmap-parser.cts`** — `roadmapPhaseLookupSources` (`:245`, the canonical 3-source ordering), `getRoadmapPhaseInternal` (`:262`, the one impure resolver), `findRoadmapPhaseInContent` (`:214`). +- **`init.cts`** — on `next` today, init resolves a phase query through the config-blind `stripProjectCodePrefix` / `normalizePhaseName` path (`init.cts:76`, `:1184`), so its three sibling commands (`cmdInitExecutePhase`, `cmdInitVerifyWork`, `cmdInitPhaseOp`) collapse foreign-prefixed IDs (the #2104 symptom). The config-aware guard family (`parsePhasePrefix` / `isForeignPrefixedPhaseQuery` / `roadmapPhaseMatchesExactPrefix`, with `/^([A-Z][A-Z0-9_]*)-(?=\d)/i`) is being introduced by #2056 on the **unmerged PR #2105** (`fix/2056-plan-phase-foreign-prefix`) and is **not yet on `next`** — it is **Cluster 1 / #2104 domain**, and its line numbers are omitted here deliberately because they will drift when #2105 lands. +- **`validate.cts`** — `buildNotStartedPhaseVariants` with its own `Phase\s+([\w][\w.-]*)` regex; `phaseVariants`. +- **Two distinct ROADMAP content matchers:** `searchPhaseInContent` (`roadmap.cts:126`, uses `OPTIONAL_PHASE_TAG_SOURCE` + a checklist fallback + `tokenizeHeadings`) vs `findRoadmapPhaseInContent` (`roadmap-parser.cts:214`). +- **`escapeRegex` is duplicated** in `phase-id.cts:15` and `state-document.cts:11`. + +Per `CONTRIBUTING.md`: + +> **One issue = one ADR-or-PRD = one PR.** Do not batch multiple decisions into one file or one PR. + +This ADR is that one file. It decides and **locks** the target seam; it ships no production code. Phases 1–4 execute against it as separate PRs. + +## Decision + +Make `src/phase-id.cts` the **single canonical owner** of every phase-identifier operation, migrate the divergent consumers to delegate to it, and add a machine-enforced anti-divergence guard so no future module can re-implement phase-ID parsing without failing CI. Seven decisions, locked below. + +### 1. `phase-id.cts` is the sole owner of phase-identifier parsing + +"Phase-identifier parsing" is defined as the closed set of operations: **normalize**, **compare**, **project-code prefix policy**, **parse-from-prose**, **parse-from-heading (regex-source construction + lookup-source ordering)**, **parse-from-dir-name (token extraction + match predicate)**, and **parse-from-CLI-query**. Every function in that set lives in `phase-id.cts` (or, for the one operation that must touch the filesystem, in the single resolver named in Decision 5). Consumers `require`/`import` the canonical functions; **no consumer defines a phase-identifier regex locally.** + +*Rejected:* (B) a new `phase-resolver.cts` module — rejected because `phase-id.cts` already holds twelve of these functions and 20 direct callers; a new module would create a *second* seam and worsen the divergence it aims to fix. (C) leave parsing distributed but add a lint that all sites match a golden regex — rejected because it enforces textual sameness, not single-ownership, and cannot cover the semantic divergences (`\b…\b` vs unanchored vs `Phase`-anchored are all "valid" regex). + +### 2. Extend, never mutate — the backward-compatibility guarantee (Hyrum's Law) + +`normalizePhaseName` has a **CRITICAL** blast radius (84 affected symbols, 20 direct callers, 19 files). Its observable behavior — zero-padding, unconditional prefix stripping, letter-case preservation (`#1962`), milestone-form decomposition — is depended upon everywhere. **Locked:** Phases 1–4 may only *add* exported functions to `phase-id.cts`; they may **not** change the observable behavior of any of the twelve existing exports. Any behavior change to an existing export (including "fixing" the config-blind strip in place) is out of scope for this epic and requires its own ADR. The #2104 fix is delivered as a **new, config-aware** function (Decision 4), leaving the existing config-blind path untouched for its 20 callers. + +### 3. The locked canonical surface (the exports Phase 1 adds) + +Phase 1 adds exactly these pure functions to `phase-id.cts`. Signatures and contracts are **locked**; Phases 2–4 consume them verbatim. + +**`parsePhaseFromProse(value: string | null): { phase: string | null; name: string | null }`** +The anchored replacement for `state.cts:parseProsePhaseField`. It extracts a phase identifier **only** from a genuine phase reference — the literal token `Phase ` (optionally `Phase : `, `Phase — `, or `Phase of `). Invariants this seam pins: +- A milestone-completion string carries **no** phase: `parsePhaseFromProse('Milestone v0.5 complete')` → `{ phase: null, name: null }` (fixes #2111). Likewise `v1.0`, `v2.10`, and any `Milestone v…` form. +- A real reference parses: `'Phase 3A — Delta (executing)'` → `{ phase: '3A', name: 'Delta' }`. +- It never mines a stray numeral from surrounding prose; absence of a `Phase` anchor yields `{ phase: null }`, not a guessed number. + +**`stripConfiguredProjectCodePrefix(value: unknown, projectCode: string | null | undefined): string`** +The config-aware prefix stripper. Strips a leading `-` **only** when `` case-insensitively equals `projectCode`; a foreign prefix (`MEM-` when the code is `LKML`) or an absent/empty `projectCode` leaves the value **verbatim**. This is the canonical home for the #2104 fix. It *complements* — does not replace — the existing config-blind `stripProjectCodePrefix` (Decision 2). + +**`isForeignPrefixedPhaseQuery(phase: unknown, projectCode: unknown): boolean`** +The canonical predicate that #2056's guard family — arriving on the **unmerged PR #2105**, not yet on `next` — will delegate to once it lands: `true` when `phase` carries a prefix that is not the configured `projectCode`. Locking it here means #2105's `cmdInitPlanPhase` guard and the three #2104 sibling commands share **one** foreign-prefix rule instead of the divergent copies they would otherwise seed. + +**`roadmapPhaseLookupSources(phaseNum: unknown): string[]`** *(moved from `roadmap-parser.cts:245-260`)* +The canonical heading lookup-source builder becomes an owned export of `phase-id.cts` (it is already pure — it only composes regex-source strings). All three roadmap call sites consume it, so the ordering (Decision 5) has exactly one definition. + +**Parse-from-CLI-query — no new function (locked).** A CLI-supplied phase argument (`gsd-tools … `) is resolved by *composing existing locked primitives*, not a new parser: `extractPhaseToken` / `normalizePhaseName` (token + normalize, Decision 2) → `isForeignPrefixedPhaseQuery` / `stripConfiguredProjectCodePrefix` (config-aware prefix policy, Decision 4) → `phaseTokenMatches` for dir-name resolution or `roadmapPhaseLookupSources` → `getRoadmapPhaseInternal` for heading resolution. This is deliberately *not* a distinct `parseCliQuery` function: callers already know they hold a CLI arg, and a discriminated god-parser would re-widen the accept surface (Postel's Law). The lock is that CLI-query resolution routes through these primitives only — no consumer re-derives a phase from a CLI arg with its own regex. + +*Rejected:* (B) fixing `parseProsePhaseField` in place with a tighter regex but leaving it in `state.cts` — rejected because the fix would not be reusable by the other prose sites and would re-seed the divergence. (C) a single mega-parser `parsePhaseId(input, kind)` with a `kind` discriminator — rejected (Postel's Law / interface clarity): callers already know whether they hold prose, a heading, a dir name, or a CLI arg; a discriminated god-function hides that and widens the accept surface. + +### 4. Project-code prefix policy — config-aware stripping is the resolution path + +**Locked policy:** a project-code prefix is a *display* prefix. For **identity/normalization** where config is unavailable, the config-blind `stripProjectCodePrefix` remains (back-compat). For **resolution of a caller-supplied query** (init commands, roadmap lookup) the config-aware `stripConfiguredProjectCodePrefix` / `isForeignPrefixedPhaseQuery` are the path: a query whose prefix is *not* this project's code must not collapse to a bare number and match a foreign phase. This tightens an over-liberal accept surface (Postel's Law) without touching the 20 callers of the blind stripper. + +### 5. Lookup-source ordering — the locked invariant + +The canonical resolution tries sources in this exact, de-duplicated order (as `roadmapPhaseLookupSources` implements today at `roadmap-parser.cts:251-259`): + +1. **Exact** — `phaseMarkdownRegexSourceExact(phaseNum)` — non-null only when the query itself carries a prefix; matches `### Phase PROJ-42:` verbatim. +2. **Numeric / padding-tolerant** — `phaseMarkdownRegexSource(phaseNum)` — the canonical bare heading (`### Phase 42:`), padding-tolerant (`0*N`). +3. **Prefix-tolerant** — `` `${OPTIONAL_PROJECT_CODE_PREFIX_SOURCE}${numericSource}` `` — the drifted-only fallback (`### Phase MANIFOLD-117:` for a bare `117` query), de-duplicated via `[...new Set(sources)]`. + +**Order matters and is locked:** bare-numeric is tried *before* prefix-tolerant so a canonical heading wins over a drifted one when both exist. The single impure ROADMAP resolver is **`getRoadmapPhaseInternal` (`roadmap-parser.cts:262`)** — it reads `ROADMAP.md` and loops these sources. `roadmap.cts`'s CLI siblings (`cmdRoadmapGetPhase`, `getRoadmapPhaseWithFallback`) **delegate to it** rather than re-scanning content, collapsing the `searchPhaseInContent` vs `findRoadmapPhaseInContent` duplication onto one resolution path — this is precisely the delegation #2114 requests. + +### 6. Migration order & backward-compatibility guarantees + +Each phase is its own small PR, opened under a fresh `chore(#2121): … — Phase N` sub-issue, and lands **in order** — Phase N+1 does not begin until Phase N merges. No phase changes any observable CLI output **except** the corrected resolution for the cited symptom cases. + +| Phase | Scope | Drives green | Sub-issue | +|---|---|---|---| +| **0** | This ADR — lock the contract. No production code. | — | Closes #2121 | +| **1** | Add the Decision-3 functions to `phase-id.cts`; move `roadmapPhaseLookupSources` in. Exhaustive unit tests + boundary cases (`v0.5`, `v1.0`, `MEM-01`, `AB-29`, bare `29`, zero-padded `029`) + ≥1 `fast-check` property test for the parse↔normalize contract. **No consumer changes.** | — | new | +| **2** | Migrate `state.cts` prose/number sites (`parseProsePhaseField` → `parsePhaseFromProse`; the unanchored twins `resolvePhaseIdForCompletePhase`, `cmdStateCompletePhase`; align the dir/pad shapes on `extractPhaseToken`/`normalizePhaseName`). Regression-first: assert `current_phase` survives a `milestone complete v0.5` close unchanged. | **#2111** | new | +| **3** | Delegate `roadmap.cts:cmdRoadmapGetPhase` + `getRoadmapPhaseWithFallback` to `getRoadmapPhaseInternal` / `roadmapPhaseLookupSources`. Regression-first: `roadmap get-phase ` resolves `### Phase AB-N:`, and both CLI siblings route through the same lookup sources as the internal resolver. | **#2114** | new | +| **4** | Add the Decision-7 anti-divergence guard; inventory-sweep the remaining sites. | closes the recurrence loop | new | + +**#2104 disposition (locked):** Phase 1 builds the config-aware prefix API (Decision 4) so #2104's fix has a canonical home, but **#2104's own migration** (applying the guard to `cmdInitExecutePhase` / `cmdInitVerifyWork` / `cmdInitPhaseOp`) is **blocked on PR #2105 (#2056)** — the guard helpers it must reuse do not exist on `next` yet — and stays tracked on #2104, **outside this epic's critical path**. Phases 1–4 must not block on #2104, and #2104's init sites are allowlisted by the Phase-4 guard (Decision 7) until #2105 lands. + +### 7. The anti-divergence contract (the parity guard) + +**Locked mechanism**, modeled on the repo's two proven single-source patterns — `tests/capability-precedence-parity.test.cjs` (identity guard) and `scripts/lint-package-identity-drift.cjs` + `tests/issue-498-identity-drift-lint.test.cjs` (drift scanner): + +1. **Identity guard test** — for every consumer that re-exports a canonical phase-ID function, assert reference identity: `assert.strictEqual(consumer.fn, phaseId.fn)`. A pasted re-implementation is a different function object and fails instantly (the mechanism `capability-precedence-parity.test.cjs:44-51` uses). +2. **Drift scanner** — `scripts/lint-phase-id-drift.cjs` exports a **pure** `findPhaseIdRegexDrift(text, opts)` that flags phase-ID-shaped regex literals (`\d+[A-Z]?(?:\.\d+)*`, `[A-Z][A-Z0-9_]*-`, and `Phase\s+…:` heading builders) defined in any `src/*.cts` other than `phase-id.cts`. It is wired to `npm run check:phase-id-drift` and asserted zero via a `scanRepo(ROOT)` integration test. A narrow allowlist keyed by an explicit `// phase-id-owner: ` comment covers sanctioned exceptions (e.g. Cluster-1 / #2104 init sites, `// phase-id-owner: cluster-1-#2104`) until they migrate. + +**Locked constraints on the guard's own implementation** (so it does not become new tech debt): +- It must be **behavioral**, not a `readFileSync(path).includes(...)` inside a `tests/**/*.test.cjs` file — that trips `eslint-rules/no-source-grep.cjs` (bound `local/no-source-grep`, `error` in tests). Text-scanning lives in the `scripts/` pure function; the test `require()`s it and calls it with **inline string literals**, per `tests/issue-498-identity-drift-lint.test.cjs:21-25`. +- It must **not** be modeled on `tests/package-name-single-source.test.cjs`, which only *appears* to satisfy `no-source-grep` because the rule's taint-tracking loses the variable after `.split()` — an evasion, not an exemption. + +*Rejected:* (B) an ESLint `no-restricted-syntax` rule — the repo has exactly one such rule (test-timing hygiene, `eslint.config.mjs:363`) and no single-ownership lint precedent; a `node:test` behavioral contract is the established, proven pattern. (C) outcome-parity only (run two paths, diff outputs, as `tests/phase.test.cjs:6881` `expectParity` does for #3537) — necessary but insufficient: it proves two paths *agree today*, not that only one *implementation* exists, so it cannot catch a third divergent site added tomorrow. + +## Consequences + +**Positive:** +- The recurring #2111/#2114-class bug is root-caused, not point-fixed: one seam owns the parsing, and the Phase-4 guard makes re-divergence a CI failure rather than a future issue number. +- #2114's `roadmap get-phase` / `ui-plan-gate` split is closed by delegation, and the two ROADMAP content matchers collapse to one resolver. +- The config-aware prefix API gives #2104 (and its siblings #1836, #2056) a single correct home the moment #2105 lands. +- Callers gain named, tested parsing functions in place of inline `\b…\b` cleverness that is hard to read and harder to debug (Kernighan's Law). + +**Negative:** +- Touching `normalizePhaseName`'s neighborhood is high-risk (84 affected symbols). Decision 2 (extend-never-mutate) contains the risk but constrains the design — the fixes must be new functions, not tighter versions of the old ones. +- Four sequential PRs plus sub-issues is more process overhead than a single "fix the three bugs" PR — accepted, because a batched fix would re-seed the very divergence this epic removes and violates one-issue-one-PR. +- The Phase-4 guard adds an allowlist that must be curated as #2104/#2105 land; a stale allowlist entry is a small, visible debt rather than a silent gap. + +**Neutral:** +- `phase-id.cts` grows from a normalizer into the full phase-ID surface; it stays pure (no Node built-ins), so the FS-touching resolver deliberately remains `getRoadmapPhaseInternal` in `roadmap-parser.cts`. +- `#2104` remains open and independently tracked; this epic neither closes nor blocks on it. + +## Alternatives considered + +1. **Point-fix each of the three bugs in place.** Rejected: leaves the seam absent, so surface #4 (init.cts, validate.cts) re-diverges under the next issue number — the exact history from #905 → #2111, #1455 → #2114, #2056 → #2104. +2. **One big PR consolidating everything at once.** Rejected: violates `CONTRIBUTING.md` "One issue = one … = one PR"; unreviewable across a CRITICAL-blast-radius symbol; no fail-first regression discipline per bug. +3. **Golden-regex lint (all sites must textually match one pattern).** Rejected: enforces textual sameness, not single ownership, and cannot express the semantic divergence (anchored vs unanchored vs `Phase`-anchored are all syntactically valid). +4. **Second module (`phase-resolver.cts`).** Rejected: a new seam alongside the existing `phase-id.cts` seam deepens, rather than removes, the divergence. + +## Software laws applied + +Cross-referenced via `/skills-from-the-artificer`; the laws that materially shaped the decisions: + +- **Hyrum's Law** — `normalizePhaseName`'s 20 callers depend on its observable behavior ⇒ *extend, never mutate* (Decision 2). +- **Postel's Law** — the current parsers are too liberal (mine any numeral; strip any prefix) ⇒ tighten acceptance to the anchored / config-aware forms (Decisions 3–4). +- **Gall's Law** — `phase-id.cts` is a working simple system; grow it incrementally through a phased epic rather than a big-bang rewrite (Decisions 1, 6). +- **DRY / single-source-of-truth (Generative Fix Divergence)** — one seam, guarded, is the whole point (Decisions 1, 7). +- **Kernighan's Law** — inline `\b…\b` one-liners are hard to debug; naming + centralizing them lowers the debugging cost the bugs were paying (Decision 3). + +## Cross-references + +- Symptom issues: [#2111](https://github.com/open-gsd/gsd-core/issues/2111), [#2114](https://github.com/open-gsd/gsd-core/issues/2114), [#2104](https://github.com/open-gsd/gsd-core/issues/2104) (blocked on [#2105](https://github.com/open-gsd/gsd-core/issues/2105)/#2056). +- Prior art: #1455 (`OPTIONAL_PROJECT_CODE_PREFIX_SOURCE`, `roadmap-parser.cts`); `CLAUDE.md` → "Generative Fix Divergence"; `scripts/lint-package-identity-drift.cjs` + `tests/capability-precedence-parity.test.cjs` (the guard models). +- Owner seam: `src/phase-id.cts`. Impure resolver: `src/roadmap-parser.cts:getRoadmapPhaseInternal`. + +## Amendments + +*(none yet — append-only; amendments extend this ADR with a dated `### # — ` section rather than rewriting the body above.)* diff --git a/docs/adr/README.md b/docs/adr/README.md index 03038a5c9..5b67d9de0 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -65,6 +65,7 @@ See **[CONTRIBUTING.md — "Proposing an ADR or PRD"](../../CONTRIBUTING.md#prop | [1817-state-md-rebuild-derivability-contract.md](1817-state-md-rebuild-derivability-contract.md) | STATE.md rebuild — derivability contract (capstone 11th transition) | Accepted | | [2008-command-exit-zero-gate.md](2008-command-exit-zero-gate.md) | Generic gate-predicate evaluator with a `command-exit-zero` kind (#2008) | Accepted | | [1990-existing-code-onboarding.md](1990-existing-code-onboarding.md) | Existing Code Onboarding Module owns deterministic repo-state detection and onboarding route selection | Proposed | +| [2121-phase-identifier-parsing-consolidation.md](2121-phase-identifier-parsing-consolidation.md) | Phase-identifier parsing consolidation — single canonical owner (phase-id.cts) + anti-divergence guard | Accepted | ## Seam map diff --git a/docs/reference/host-integration-capability-matrix.md b/docs/reference/host-integration-capability-matrix.md index f9fe517e4..8123fc72c 100644 --- a/docs/reference/host-integration-capability-matrix.md +++ b/docs/reference/host-integration-capability-matrix.md @@ -101,6 +101,12 @@ Sources consulted: | dispatch.subagentToolkit | full | https://developers.openai.com/codex/multi-agent | "Subagents inherit the sandbox policy and tool surface from the parent session." | | dispatch.backgroundDispatch | true | https://github.com/openai/codex/blob/main/codex-rs/core/templates/collab/experimental_prompt.md | "Sub-agents have access to the same set of tools as you do so you must tell them if they are allowed to spawn sub-agents themselves or not." The config (codex-rs/config/src/config_toml.rs) exposes an | +**GSD integration status — Phase D dogfood complete (#2088, ADR-1239).** Codex installs through the `declarative` embedding adapter (`createDeclarativeAdapter` → `installRuntimeArtifacts`); the hardcoded `runtime === 'codex'`/`isCodex` projection is folded into descriptor-driven `runtime.hostBehaviors`, and install/uninstall output is byte-parity-gated (`tests/fixtures/golden-install-parity/codex.json`). Three capability upgrades land, each with a test driving the user-reachable surface: + +- **Skill root** — skills install to the canonical `$HOME/.agents/skills` (Codex core-skills `loader.rs` user-scope root), not the deprecated `$CODEX_HOME/skills` fallback. Declared via the skills-kind `home: ".agents"` override; pre-move installs are migrated (stale `~/.codex/skills/gsd-*` cleaned on both install and uninstall). +- **Hook events** — GSD registers all documented `hooks.json` lifecycle events beyond `SessionStart`: `SubagentStart`, `Stop`, `PostToolUse` (#772), plus the six added in #2088 — `PreToolUse`, `PermissionRequest`, `PreCompact`, `PostCompact`, `SubagentStop`, `UserPromptSubmit` — all routed through `gsd-context-monitor.js`. (The descriptor `extendedHookEvents` field reflects the schema-valid cross-runtime subset `SubagentStop`/`Stop`/`PreCompact`; Codex's full event set is codex-hooks-json-native, registered directly in `hooks.json`.) +- **Dispatch tuning** — `[agents] max_depth = 1` is written explicitly into the managed `config.toml` block, pinning the `dispatch.maxDepth: 1` axis instead of relying on codex-cli's implicit default. Because `maxDepth === 1`, `degradationFor` flattens GSD-hosted wave dispatch to single-level even though `dispatch.nested`/`background`/`backgroundDispatch` are all `true`. The block is a bare `[agents]` AgentsToml scalar table (coexisting with the flattened `[agents.gsd-*]` role sub-tables); `validateCodexConfigSchema` permits a known-scalar-only `[agents]` while still rejecting `[[agents]]` and unknown-key forms. + Sources consulted: - https://github.com/openai/codex (repo via gh CLI) - /openai/codex (Context7 library ID) @@ -181,6 +187,11 @@ Sources consulted: - https://cursor.com/docs/enterprise/llm-safety-and-controls - /websites/cursor (Context7) +**GSD integration status — Phase D dogfood complete (#2089, ADR-1239).** Cursor installs through the `imperative` embedding adapter (`createImperativeAdapter` → `installRuntimeArtifacts`); the hardcoded `runtime === 'cursor'` / `isCursor` projection is folded into descriptor-driven `runtime.hostBehaviors`, and install/uninstall output is byte-parity-gated (`tests/fixtures/golden-install-parity/cursor.json`). Two capability upgrades land, each with a test driving the user-reachable surface: + +- **Expanded hook-bus coverage** — GSD registers all 6 managed lifecycle events in `hooks.json` beyond the original `sessionStart`/`postToolUse`: `preToolUse`, `stop`, `subagentStart`, `subagentStop` (AC4a, cite https://cursor.com/docs/hooks). The hook-bus binding is descriptor-driven via `src/host-integration-adapters/imperative-hook-bus.cts` (reads `hostBehaviors.managedHookEvents`), not a hardcoded event pair. +- **Named/background nested subagent dispatch** — `dispatch.background`/`backgroundDispatch`/`nested` are all `true` with `maxDepth: 2`; `shouldFlattenDispatch(cursor)` returns `false` so GSD's wave-based execution drives Cursor's native background + depth-2 nested subagent dispatch instead of flattening to inline sequential calls (AC4b, cite https://cursor.com/docs/subagents + https://cursor.com/docs/sdk/typescript). + --- ## cline diff --git a/eslint-rules/lib/portability-vocab.cjs b/eslint-rules/lib/portability-vocab.cjs index 017068e37..44e44ab98 100644 --- a/eslint-rules/lib/portability-vocab.cjs +++ b/eslint-rules/lib/portability-vocab.cjs @@ -49,6 +49,9 @@ const PATH_RETURNING_FNS = [ 'resolveAgentDir', 'getGlobalConfigDir', 'getGlobalSkillsBase', + // #2088 (ADR-1239 upgrade 3): resolves the on-disk skills-install dir honoring + // a skills-kind `home` override (e.g. Codex → $HOME/.agents/skills). + '_resolveSkillsRootDir', 'getGlobalSkillDir', 'getGlobalSkillDisplayPath', 'resolveSkillsBaseFromDescriptor', diff --git a/gsd-core/bin/lib/capability-registry.cjs b/gsd-core/bin/lib/capability-registry.cjs index 1524c0e31..3a2fac1b3 100644 --- a/gsd-core/bin/lib/capability-registry.cjs +++ b/gsd-core/bin/lib/capability-registry.cjs @@ -814,7 +814,8 @@ const capabilities = { "prefix": "gsd-", "nesting": "flat", "recursive": false, - "converter": "convertClaudeCommandToCodexSkill" + "converter": "convertClaudeCommandToCodexSkill", + "home": ".agents" } ], "local": [ @@ -824,7 +825,8 @@ const capabilities = { "prefix": "gsd-", "nesting": "flat", "recursive": false, - "converter": "convertClaudeCommandToCodexSkill" + "converter": "convertClaudeCommandToCodexSkill", + "home": ".agents" } ] }, @@ -836,7 +838,11 @@ const capabilities = { "installSurface": "codex-toml", "writesSharedSettings": false, "permissionWriter": null, - "extendedHookEvents": [], + "extendedHookEvents": [ + "SubagentStop", + "Stop", + "PreCompact" + ], "hostIntegration": { "embeddingMode": "declarative", "commandSurface": "slash-file", @@ -853,6 +859,13 @@ const capabilities = { "stateIO": "filesystem", "transport": "mcp", "runtime": "node" + }, + "hostBehaviors": { + "reapplyCommand": "$gsd-update --reapply", + "tomlConfigInstall": true, + "cleanupSkillSidecars": true, + "agentTomlFiles": true, + "frontmatterDialect": "codex" } } }, @@ -1046,6 +1059,23 @@ const capabilities = { "stateIO": "filesystem", "transport": "mcp", "runtime": "node" + }, + "hostBehaviors": { + "reapplyCommand": "gsd-update --reapply (mention the skill name)", + "frontmatterDialect": "cursor", + "hooksJsonSurface": true, + "skipSharedHooksInstall": true, + "reportCommandsDir": true, + "skipUpdateBannerCommand": true, + "skipSettingsUi": true, + "managedHookEvents": [ + "sessionStart", + "postToolUse", + "preToolUse", + "stop", + "subagentStart", + "subagentStop" + ] } } }, @@ -4134,7 +4164,8 @@ const runtimes = { "prefix": "gsd-", "nesting": "flat", "recursive": false, - "converter": "convertClaudeCommandToCodexSkill" + "converter": "convertClaudeCommandToCodexSkill", + "home": ".agents" } ], "local": [ @@ -4144,7 +4175,8 @@ const runtimes = { "prefix": "gsd-", "nesting": "flat", "recursive": false, - "converter": "convertClaudeCommandToCodexSkill" + "converter": "convertClaudeCommandToCodexSkill", + "home": ".agents" } ] }, @@ -4156,7 +4188,11 @@ const runtimes = { "installSurface": "codex-toml", "writesSharedSettings": false, "permissionWriter": null, - "extendedHookEvents": [], + "extendedHookEvents": [ + "SubagentStop", + "Stop", + "PreCompact" + ], "hostIntegration": { "embeddingMode": "declarative", "commandSurface": "slash-file", @@ -4173,6 +4209,13 @@ const runtimes = { "stateIO": "filesystem", "transport": "mcp", "runtime": "node" + }, + "hostBehaviors": { + "reapplyCommand": "$gsd-update --reapply", + "tomlConfigInstall": true, + "cleanupSkillSidecars": true, + "agentTomlFiles": true, + "frontmatterDialect": "codex" } } }, @@ -4366,6 +4409,23 @@ const runtimes = { "stateIO": "filesystem", "transport": "mcp", "runtime": "node" + }, + "hostBehaviors": { + "reapplyCommand": "gsd-update --reapply (mention the skill name)", + "frontmatterDialect": "cursor", + "hooksJsonSurface": true, + "skipSharedHooksInstall": true, + "reportCommandsDir": true, + "skipUpdateBannerCommand": true, + "skipSettingsUi": true, + "managedHookEvents": [ + "sessionStart", + "postToolUse", + "preToolUse", + "stop", + "subagentStart", + "subagentStop" + ] } } }, diff --git a/gsd-core/bin/lib/runtime-artifact-install-plan.cjs b/gsd-core/bin/lib/runtime-artifact-install-plan.cjs index e05aad2ec..1f203c95f 100644 --- a/gsd-core/bin/lib/runtime-artifact-install-plan.cjs +++ b/gsd-core/bin/lib/runtime-artifact-install-plan.cjs @@ -111,7 +111,7 @@ function createRuntimeArtifactInstallPlan(args) { items.push({ kind: kind.kind, sourceDir, - destDir: assertDestWithinConfigHome(layout.configDir, kind.destSubpath), + destDir: assertDestWithinConfigHome(kind.home ?? layout.configDir, kind.destSubpath), }); } return { ok: true, plan: { items, cleanupDirs } }; @@ -120,7 +120,7 @@ function createRuntimeArtifactUninstallPlan(layout) { return { items: layout.kinds.map((kind) => ({ kind: kind.kind, - destDir: assertDestWithinConfigHome(layout.configDir, kind.destSubpath), + destDir: assertDestWithinConfigHome(kind.home ?? layout.configDir, kind.destSubpath), })), }; } diff --git a/hooks/gsd-cursor-pre-tool.js b/hooks/gsd-cursor-pre-tool.js new file mode 100644 index 000000000..a608255c0 --- /dev/null +++ b/hooks/gsd-cursor-pre-tool.js @@ -0,0 +1,76 @@ +#!/usr/bin/env node +// gsd-hook-version: {{GSD_VERSION}} +// gsd-cursor-pre-tool.js — Cursor preToolUse hook (ADR-1239 / #2089) +// +// Cursor invokes this script before each tool call executes. +// Protocol: JSON from Cursor on stdin; JSON response on stdout. +// +// Input schema (cursor preToolUse): +// { tool_name, tool_input, conversation_id, generation_id, model, +// hook_event_name, cursor_version, workspace_roots, user_email, +// transcript_path } +// +// Output schema (cursor preToolUse): +// { additional_context?: string, block?: boolean, reason?: string } +// +// Behaviour: +// - If a write-class tool targets .planning/, reminds the agent to keep +// STATE.md current before the write proceeds. +// - Fails open: any error silently exits 0 so a hook bug never wedges Cursor. +// +// Cursor docs: https://cursor.com/docs/hooks + +'use strict'; + +const fs = require('fs'); +const path = require('path'); + +const WRITE_TOOL_RE = /write|edit|replace|create|delete|remove|append|apply|patch|insert|mkdir/i; +const PATH_KEY_RE = /^(path|file|file_?path|filepath|target_?path|target|dir|directory|uri|filename)$/i; +const PLANNING_PATH_RE = /(^|[\\/])\.planning([\\/]|$)/; + +let raw = ''; +const stdinTimeout = setTimeout(() => { + process.exit(0); +}, 10000); + +process.stdin.setEncoding('utf8'); +process.stdin.on('data', (chunk) => { raw += chunk; }); +process.stdin.on('end', () => { + clearTimeout(stdinTimeout); + try { + let input; + try { input = JSON.parse(raw || '{}'); } catch { process.stdout.write(JSON.stringify({})); return; } + + const toolName = String( + input.tool_name || input.toolName || '' + ).toLowerCase(); + + const isWrite = WRITE_TOOL_RE.test(toolName); + if (!isWrite) { process.stdout.write(JSON.stringify({})); return; } + + const paths = []; + const walk = (v, depth) => { + if (depth > 5 || paths.length > 64) return; + if (Array.isArray(v)) { for (const x of v) walk(x, depth + 1); return; } + if (v && typeof v === 'object') { + for (const k of Object.keys(v)) { + const val = v[k]; + if (typeof val === 'string' && PATH_KEY_RE.test(k)) paths.push(val); + else walk(val, depth + 1); + } + } + }; + walk(input.tool_input || input.toolInput || {}, 0); + + if (paths.some((p) => PLANNING_PATH_RE.test(p))) { + process.stdout.write(JSON.stringify({ + additional_context: + 'GSD: .planning/ write detected — ensure STATE.md reflects the latest phase and progress after this change.', + })); + return; + } + } catch { /* fall through to empty response */ } + + process.stdout.write(JSON.stringify({})); +}); diff --git a/hooks/gsd-cursor-stop.js b/hooks/gsd-cursor-stop.js new file mode 100644 index 000000000..4c69bbbaf --- /dev/null +++ b/hooks/gsd-cursor-stop.js @@ -0,0 +1,48 @@ +#!/usr/bin/env node +// gsd-hook-version: {{GSD_VERSION}} +// gsd-cursor-stop.js — Cursor stop hook (ADR-1239 / #2089) +// +// Cursor invokes this script when the agent stops responding. +// Protocol: JSON from Cursor on stdin; JSON response on stdout. +// +// Input schema (cursor stop): +// { conversation_id, generation_id, model, hook_event_name, +// cursor_version, workspace_roots, user_email, transcript_path } +// +// Output schema (cursor stop): +// { additional_context?: string } +// +// Behaviour: +// - Reminds the user to verify work if .planning/ is present. +// - Fails open: any error silently exits 0. +// +// Cursor docs: https://cursor.com/docs/hooks + +'use strict'; + +const fs = require('fs'); +const path = require('path'); + +let raw = ''; +const stdinTimeout = setTimeout(() => { + process.exit(0); +}, 10000); + +process.stdin.setEncoding('utf8'); +process.stdin.on('data', (chunk) => { raw += chunk; }); +process.stdin.on('end', () => { + clearTimeout(stdinTimeout); + try { + const statePath = path.join(process.cwd(), '.planning', 'STATE.md'); + if (fs.existsSync(statePath)) { + process.stdout.write(JSON.stringify({ + additional_context: + 'GSD: Agent stopping — run /gsd:verify-work or /gsd:progress to confirm the phase goal is met before ending the session.', + })); + } else { + process.stdout.write(JSON.stringify({})); + } + } catch { + process.stdout.write(JSON.stringify({})); + } +}); diff --git a/hooks/gsd-cursor-subagent-start.js b/hooks/gsd-cursor-subagent-start.js new file mode 100644 index 000000000..ad1e3ca48 --- /dev/null +++ b/hooks/gsd-cursor-subagent-start.js @@ -0,0 +1,50 @@ +#!/usr/bin/env node +// gsd-hook-version: {{GSD_VERSION}} +// gsd-cursor-subagent-start.js — Cursor subagentStart hook (ADR-1239 / #2089) +// +// Cursor invokes this script when a subagent session starts. +// Protocol: JSON from Cursor on stdin; JSON response on stdout. +// +// Input schema (cursor subagentStart): +// { session_id, is_background_agent, conversation_id, generation_id, +// model, hook_event_name, cursor_version, workspace_roots, +// user_email, transcript_path } +// +// Output schema (cursor subagentStart): +// { additional_context?: string } +// +// Behaviour: +// - Injects a brief GSD state reminder so subagents (planner, executor, +// verifier) have the current phase context. +// - Fails open: any error silently exits 0. +// +// Cursor docs: https://cursor.com/docs/hooks + +'use strict'; + +const fs = require('fs'); +const path = require('path'); + +const MSG_PRESENT = + 'GSD: Subagent session started — review .planning/STATE.md for the current phase and any blockers before acting.'; +const MSG_ABSENT = + 'GSD: Subagent session started — no .planning/ workflow found.'; + +let raw = ''; +const stdinTimeout = setTimeout(() => { + process.exit(0); +}, 10000); + +process.stdin.setEncoding('utf8'); +process.stdin.on('data', (chunk) => { raw += chunk; }); +process.stdin.on('end', () => { + clearTimeout(stdinTimeout); + try { + const statePath = path.join(process.cwd(), '.planning', 'STATE.md'); + const statePresent = fs.existsSync(statePath); + const msg = statePresent ? MSG_PRESENT : MSG_ABSENT; + process.stdout.write(JSON.stringify({ additional_context: msg })); + } catch { + process.stdout.write(JSON.stringify({})); + } +}); diff --git a/hooks/gsd-cursor-subagent-stop.js b/hooks/gsd-cursor-subagent-stop.js new file mode 100644 index 000000000..fa5bb6826 --- /dev/null +++ b/hooks/gsd-cursor-subagent-stop.js @@ -0,0 +1,40 @@ +#!/usr/bin/env node +// gsd-hook-version: {{GSD_VERSION}} +// gsd-cursor-subagent-stop.js — Cursor subagentStop hook (ADR-1239 / #2089) +// +// Cursor invokes this script when a subagent session completes. +// Protocol: JSON from Cursor on stdin; JSON response on stdout. +// +// Input schema (cursor subagentStop): +// { session_id, conversation_id, generation_id, model, hook_event_name, +// cursor_version, workspace_roots, user_email, transcript_path } +// +// Output schema (cursor subagentStop): +// { additional_context?: string } +// +// Behaviour: +// - Reminds the orchestrating agent to check the subagent's output. +// - Fails open: any error silently exits 0. +// +// Cursor docs: https://cursor.com/docs/hooks + +'use strict'; + +let raw = ''; +const stdinTimeout = setTimeout(() => { + process.exit(0); +}, 10000); + +process.stdin.setEncoding('utf8'); +process.stdin.on('data', (chunk) => { raw += chunk; }); +process.stdin.on('end', () => { + clearTimeout(stdinTimeout); + try { + process.stdout.write(JSON.stringify({ + additional_context: + 'GSD: Subagent completed — review its output and update .planning/STATE.md if the phase progressed.', + })); + } catch { + process.stdout.write(JSON.stringify({})); + } +}); diff --git a/hooks/managed-hooks-registry.cjs b/hooks/managed-hooks-registry.cjs index 1d392471d..0a05e5841 100644 --- a/hooks/managed-hooks-registry.cjs +++ b/hooks/managed-hooks-registry.cjs @@ -21,7 +21,11 @@ const MANAGED_HOOKS = [ 'gsd-config-reload.js', 'gsd-context-monitor.js', 'gsd-cursor-post-tool.js', + 'gsd-cursor-pre-tool.js', 'gsd-cursor-session-start.js', + 'gsd-cursor-stop.js', + 'gsd-cursor-subagent-start.js', + 'gsd-cursor-subagent-stop.js', 'gsd-ensure-canonical-path.js', 'gsd-graphify-update.sh', 'gsd-phase-boundary.sh', diff --git a/scripts/build-hooks.js b/scripts/build-hooks.js index a4e220860..9e80d813e 100644 --- a/scripts/build-hooks.js +++ b/scripts/build-hooks.js @@ -37,9 +37,13 @@ const HOOKS_TO_COPY = [ // so require('./managed-hooks-registry.cjs') resolves in the installed hooks/ dir. 'managed-hooks-registry.cjs', 'gsd-context-monitor.js', - // Cursor lifecycle hooks (issue #777): sessionStart context injection + postToolUse monitor + // Cursor lifecycle hooks (#777 + ADR-1239/#2089): 6 managed events 'gsd-cursor-session-start.js', 'gsd-cursor-post-tool.js', + 'gsd-cursor-pre-tool.js', + 'gsd-cursor-stop.js', + 'gsd-cursor-subagent-start.js', + 'gsd-cursor-subagent-stop.js', // Claude Code FileChanged hook (#770) — hot-reloads gsd config when // .planning/config.json changes mid-session. Must ship to dist so the // installer can copy it to the target hooks/ dir and register FileChanged. diff --git a/scripts/run-tests.cjs b/scripts/run-tests.cjs index e52c7fe92..e03ab8a98 100644 --- a/scripts/run-tests.cjs +++ b/scripts/run-tests.cjs @@ -577,6 +577,23 @@ function main() { const MAX_FILES_PER_CHUNK = process.env.RUN_TESTS_MAX_FILES_PER_CHUNK ? Number(process.env.RUN_TESTS_MAX_FILES_PER_CHUNK) : 60; + // #2088: file COUNT is a poor proxy for a chunk's wall-clock — install-heavy + // files (real installs; install-minimal-hooks.test.cjs alone runs ~250 cases + // doing dozens of installs) are ~10× a unit file. When several land in the SAME + // chunk — e.g. a PR touching the whole install surface, whose *targeted* lane is + // unsharded (13 install-heavy files → one chunk) — that chunk blows the 600s + // backstop while unit-only chunks finish in seconds. WEIGHT install-heavy files + // so they fill a chunk's budget faster and therefore SPREAD across chunks + // instead of clustering. Light files keep weight 1, so pure-unit chunking (and + // its harness tests) is byte-for-byte unchanged. `MAX_FILES_PER_CHUNK` is now a + // per-chunk WEIGHT budget (backwards-compatible: it equals the file count when + // every file is light). Tune via RUN_TESTS_HEAVY_FILE_WEIGHT; classify via the + // basename prefix (install*/installer*/codex-* are the real install-heavy suites). + const HEAVY_TEST_RE = /^(?:install|codex-)/; + const HEAVY_FILE_WEIGHT = process.env.RUN_TESTS_HEAVY_FILE_WEIGHT + ? Number(process.env.RUN_TESTS_HEAVY_FILE_WEIGHT) + : 12; + const fileWeight = (f) => (HEAVY_TEST_RE.test(basename(f)) ? HEAVY_FILE_WEIGHT : 1); // node:test does not exit until the event loop drains. A unit test that leaks // an open handle (un-terminated Worker, un-killed child_process, ref'd timer) @@ -595,18 +612,21 @@ function main() { const chunks = []; let current = []; let currentLen = FIXED_OVERHEAD; + let currentWeight = 0; for (const file of selected) { const add = file.length + 1; // +1 for the inter-arg separator if ( current.length > 0 && - (currentLen + add > MAX_CMDLINE_CHARS || current.length >= MAX_FILES_PER_CHUNK) + (currentLen + add > MAX_CMDLINE_CHARS || currentWeight >= MAX_FILES_PER_CHUNK) ) { chunks.push(current); current = []; currentLen = FIXED_OVERHEAD; + currentWeight = 0; } current.push(file); currentLen += add; + currentWeight += fileWeight(file); // heavy install files count for more } if (current.length > 0) chunks.push(current); diff --git a/src/host-integration-adapters/imperative-hook-bus.cts b/src/host-integration-adapters/imperative-hook-bus.cts new file mode 100644 index 000000000..6716bd9a7 --- /dev/null +++ b/src/host-integration-adapters/imperative-hook-bus.cts @@ -0,0 +1,155 @@ +/** + * Imperative hook-bus adapter — descriptor-driven hooks.json binding + * (ADR-1239 Phase D / #2089). + * + * Generalizes the Cursor-specific `writeCursorHooksJson`/`removeCursorHooksJson` + * into a descriptor-driven hook-bus binding that reads the negotiated `hookBus` + * axis + the host's documented hook-event list (from + * `runtime.hostBehaviors.managedHookEvents`), NOT a hardcoded + * `sessionStart`/`postToolUse` pair. + * + * This module is PURE (no I/O): it resolves the event→script mapping and builds + * the hooks.json entry manifest. The actual file I/O (copying scripts, writing + * hooks.json) stays in `runtime-hooks-surface.cts`, which calls into the pure + * functions exported here. This separation makes the binding testable without a + * filesystem. + * + * Cursor hook-event universe (closed vocabulary per ADR-1239, + * https://cursor.com/docs/hooks): + * sessionStart, sessionEnd, preToolUse, postToolUse, subagentStart, + * subagentStop, beforeShellExecution, afterShellExecution, + * afterMCPExecution, afterFileEdit, preCompact, stop, + * beforeTabFileRead, afterTabFileEdit, workspaceOpen + * + * GSD registers for the 6 events in the portable floor + subagent lifecycle + * (AC4a upgrade, #2089): + * sessionStart, postToolUse, preToolUse, stop, subagentStart, subagentStop + */ +'use strict'; + +/** + * The full set of Cursor hook events GSD can register for. + * Frozen closed vocabulary — adding an event requires updating both this set + * and the event→script mapping below. + */ +export const CURSOR_HOOK_EVENTS = Object.freeze([ + 'sessionStart', + 'postToolUse', + 'preToolUse', + 'stop', + 'subagentStart', + 'subagentStop', +] as const); + +export type CursorHookEvent = (typeof CURSOR_HOOK_EVENTS)[number]; + +/** + * Event → hook-script mapping. Each event maps to a standalone `.js` script + * under `hooks/` that Cursor invokes via `hooks.json`. + * + * Convention: `gsd-cursor-.js`. The script files are authored in + * `hooks/` and copied to `/hooks/` during install by + * `runtime-hooks-surface.cts`. + */ +export const CURSOR_EVENT_SCRIPT_MAP: Readonly> = Object.freeze({ + sessionStart: 'gsd-cursor-session-start.js', + postToolUse: 'gsd-cursor-post-tool.js', + preToolUse: 'gsd-cursor-pre-tool.js', + stop: 'gsd-cursor-stop.js', + subagentStart: 'gsd-cursor-subagent-start.js', + subagentStop: 'gsd-cursor-subagent-stop.js', +}); + +/** + * The GSD-managed marker written into each hooks.json entry so the + * reconcile pass can distinguish GSD-owned entries from user-owned ones. + */ +export const GSD_HOOK_MARKER = 'gsd-managed'; + +/** + * Resolve the managed hook events from a runtime descriptor's + * `hostBehaviors.managedHookEvents` list. Falls back to the full + * `CURSOR_HOOK_EVENTS` set when the descriptor does not declare the list + * (backward-compat for descriptors predating #2089). + * + * Pure: no I/O, never throws. Unknown event names are silently filtered + * (fail-closed — an unrecognized event is never registered). Falls back to + * the full CURSOR_HOOK_EVENTS set when the descriptor is absent or all entries + * are unrecognized (ensures the portable-event floor is always covered). + * + * @param managedHookEvents - the descriptor's `hostBehaviors.managedHookEvents` array + * @returns a deduplicated, validated array of event names + */ +export function resolveManagedHookEvents( + managedHookEvents: readonly string[] | null | undefined, +): readonly string[] { + if (!Array.isArray(managedHookEvents) || managedHookEvents.length === 0) { + return CURSOR_HOOK_EVENTS; + } + const valid = new Set(CURSOR_HOOK_EVENTS); + const seen = new Set(); + const result: string[] = []; + for (const ev of managedHookEvents) { + if (typeof ev === 'string' && valid.has(ev) && !seen.has(ev)) { + seen.add(ev); + result.push(ev); + } + } + return result.length > 0 ? result : CURSOR_HOOK_EVENTS; +} + +/** + * Build the list of hook script files that need to be copied for the given + * managed events. Each event maps to a script via `CURSOR_EVENT_SCRIPT_MAP`. + * + * Pure: returns a deduplicated array of script filenames. + * + * @param events - the managed event names (validated by `resolveManagedHookEvents`) + * @returns array of script filenames (e.g. `['gsd-cursor-session-start.js', ...]`) + */ +export function resolveHookScripts( + events: readonly string[], +): readonly string[] { + const scripts: string[] = []; + const seen = new Set(); + for (const ev of events) { + const script = CURSOR_EVENT_SCRIPT_MAP[ev]; + if (script && !seen.has(script)) { + seen.add(script); + scripts.push(script); + } + } + return scripts; +} + +/** + * Build the hooks.json managed-entry manifest for the given events. + * Each entry is `{ type: 'command', command: , [GSD_HOOK_MARKER]: true }`. + * + * The `command` string is built by the caller (it requires platform-specific + * node-runner resolution from `runtime-hooks-surface.cts`). This function + * receives a pre-built `event → command` map and attaches the marker. + * + * Pure: no I/O. + * + * @param events - the managed event names + * @param commands - a map of event → command string (built by the caller) + * @returns a map of event → managed entry, ready for hooks.json reconciliation + */ +export function buildHookBusEntries( + events: readonly string[], + commands: Readonly>, +): Record { + const entries: Record = {}; + for (const ev of events) { + const cmd = commands[ev]; + if (cmd) { + entries[ev] = { + type: 'command', + command: cmd, + [GSD_HOOK_MARKER]: true, + }; + } + } + return entries; +} diff --git a/src/init.cts b/src/init.cts index 196b266e2..ee6e7ffa2 100644 --- a/src/init.cts +++ b/src/init.cts @@ -37,7 +37,7 @@ import { formatGsdSlash, resolveRuntime } from './runtime-slash.cjs'; // eslint-disable-next-line @typescript-eslint/no-require-imports -- commands.cjs is an export= CommonJS module import commandsMod = require('./commands.cjs'); import { validatePath, loadTrustedGlobalRoots } from './security.cjs'; -import { getGlobalSkillDir, getGlobalSkillDisplayPath, getGlobalSkillsBase } from './runtime-homes.cjs'; +import { getGlobalSkillDir, getGlobalSkillDisplayPath, getGlobalSkillsBase, getGlobalConfigDir } from './runtime-homes.cjs'; // eslint-disable-next-line @typescript-eslint/no-require-imports -- frontmatter.cjs is an export= CommonJS module import frontmatterMod = require('./frontmatter.cjs'); // eslint-disable-next-line @typescript-eslint/no-require-imports -- verification.cjs is an export= CommonJS module @@ -2372,11 +2372,24 @@ function buildSkillManifest(cwd: string, skillsDir: string | null = null): Skill kind: 'skills', }, { - root: '~/.codex/skills', + // ADR-1239 upgrade 3 (#2088): Codex's canonical skill root is + // $HOME/.agents/skills (per codex core-skills loader.rs), resolved via + // the skills-kind `home` override in getGlobalSkillsBase. + root: '~/.agents/skills', path: getGlobalSkillsBase('codex') as string, scope: 'global', kind: 'skills', }, + { + // Codex's deprecated fallback skill root ($CODEX_HOME/skills). Kept as a + // discovery-only legacy root so pre-move installs remain inventoried; + // GSD no longer installs here (#2088). + root: '~/.codex/skills', + path: path.join(getGlobalConfigDir('codex'), 'skills'), + scope: 'global', + kind: 'skills', + deprecated: true, + }, { root: '.claude/gsd-core/skills', path: path.join(os.homedir(), '.claude', 'gsd-core', 'skills'), diff --git a/src/install-engine.cts b/src/install-engine.cts index eda2107de..dcfbb6e18 100644 --- a/src/install-engine.cts +++ b/src/install-engine.cts @@ -276,15 +276,21 @@ function _copyStaged(stagedDir: string, destDir: string, kind: any, configDir: s '_copyStaged: configDir (install root) is required to confine writes — refusing to write', ); } + // The install root is normally configDir, but a kind may declare an alternate + // `home` (ADR-1239 upgrade 3 / #2088, e.g. Codex skills -> $HOME/.agents) — in + // that case this defense-in-depth check must confine against the resolved + // alternate root instead, matching the upstream gate's own root selection in + // createRuntimeArtifactInstallPlan. + const installRoot = (kind && typeof kind.home === 'string' && kind.home !== '') ? kind.home : configDir; // Strict-subpath + NUL containment via the canonical gate (shared with the // layout-driven install plan); throws if destDir escapes the install root. - // destDir here is an absolute path; path.resolve(configDir, absoluteDest) returns it unchanged, so the gate's strict-subpath check still correctly confines it to configDir. - const resolvedDest = runtimeArtifactInstallPlan.assertDestWithinConfigHome(configDir, destDir); - // Symlink-escape guard: reject if any path component between configDir and - // destDir is a symlink that would redirect writes outside configDir. - if (hasExistingSymlinkBetween(path.resolve(configDir), resolvedDest)) { + // destDir here is an absolute path; path.resolve(installRoot, absoluteDest) returns it unchanged, so the gate's strict-subpath check still correctly confines it to installRoot. + const resolvedDest = runtimeArtifactInstallPlan.assertDestWithinConfigHome(installRoot, destDir); + // Symlink-escape guard: reject if any path component between the install root and + // destDir is a symlink that would redirect writes outside the install root. + if (hasExistingSymlinkBetween(path.resolve(installRoot), resolvedDest)) { throw new Error( - `_copyStaged: destDir "${destDir}" contains a symlink escaping the install root "${configDir}" — refusing to write`, + `_copyStaged: destDir "${destDir}" contains a symlink escaping the install root "${installRoot}" — refusing to write`, ); } // Use the validated absolute path for the actual writes below. @@ -620,11 +626,17 @@ function installRuntimeArtifacts( if (!kind) throw new Error(`Install plan returned unknown artifact kind: ${item.kind}`); const dest = item.destDir; // Symlink-escape guard: reject before mkdir if dest (or any component - // between configDir and dest) is a symlink pointing outside configDir. - // mkdirSync follows symlinks, so this must run BEFORE the mkdir call. - if (hasExistingSymlinkBetween(path.resolve(configDir), dest)) { + // between the install root and dest) is a symlink pointing outside that + // root. mkdirSync follows symlinks, so this must run BEFORE the mkdir + // call. The install root is normally configDir, but a kind may declare + // an alternate `home` (ADR-1239 upgrade 3 / #2088, e.g. Codex skills -> + // $HOME/.agents) — in that case the guard must check against the + // resolved alternate root instead, matching assertDestWithinConfigHome's + // own root selection in createRuntimeArtifactInstallPlan. + const installRoot = (kind && typeof kind.home === 'string' && kind.home !== '') ? kind.home : configDir; + if (hasExistingSymlinkBetween(path.resolve(installRoot), dest)) { throw new Error( - `installRuntimeArtifacts: destDir "${dest}" contains a symlink escaping the install root "${configDir}" — refusing to create`, + `installRuntimeArtifacts: destDir "${dest}" contains a symlink escaping the install root "${installRoot}" — refusing to create`, ); } fs.mkdirSync(dest, { recursive: true }); diff --git a/src/installer-migration-report.cts b/src/installer-migration-report.cts index b77dccc27..70b74252a 100644 --- a/src/installer-migration-report.cts +++ b/src/installer-migration-report.cts @@ -32,7 +32,11 @@ export const BUNDLED_GSD_HOOK_FILES: ReadonlySet = Object.freeze(new Set 'hooks/gsd-config-reload.js', 'hooks/gsd-context-monitor.js', 'hooks/gsd-cursor-post-tool.js', + 'hooks/gsd-cursor-pre-tool.js', 'hooks/gsd-cursor-session-start.js', + 'hooks/gsd-cursor-stop.js', + 'hooks/gsd-cursor-subagent-start.js', + 'hooks/gsd-cursor-subagent-stop.js', 'hooks/gsd-ensure-canonical-path.js', 'hooks/gsd-graphify-update.sh', 'hooks/gsd-phase-boundary.sh', diff --git a/src/phase-id.cts b/src/phase-id.cts index 07d0799e6..e08a4af43 100644 --- a/src/phase-id.cts +++ b/src/phase-id.cts @@ -255,6 +255,97 @@ function phaseTokenMatches(dirName: string, normalized: string): boolean { return false; } +// ─── #2121 canonical surface (ADR-2121) ────────────────────────────────────── + +/** + * Parse a phase identifier from a STATE.md `Phase:` prose field VALUE — the text + * after the `Phase:` label (e.g. `"3 of 4 (Delta)"`, `"3A — Delta (executing)"`, + * or `"Milestone v0.5 complete"`). + * + * The token is anchored to the START of the value (after an optional literal + * `Phase ` label and an optional project-code prefix) so a phase is only + * returned when the value actually begins with one. This is the #2111 fix: the + * prior unanchored `/\b(\d+[A-Z]?(?:\.\d+)*)\b/i` mined the first numeral + * anywhere, so `"Milestone v0.5 complete"` collapsed to `"5"` (the minor-version + * digit) and `"v1.0"` to `"0"` (a reserved sentinel). Here both yield + * `{ phase: null }` because they do not begin with a phase token. The name + * extraction (parenthetical or em-dash tail, minus status words) is unchanged. + */ +function parsePhaseFromProse(value: string | null): { phase: string | null; name: string | null } { + if (!value) return { phase: null, name: null }; + // Coerce defensively so a non-string caller cannot throw on this canonical + // surface (mirrors the sibling #2121 functions' String(...) handling). + const str = String(value); + const phaseMatch = str.match(/^\s*(?:Phase\s+)?(?:[A-Z][A-Z0-9_]*-)?(\d+[A-Z]?(?:\.\d+)*)\b/i); + // The name-extraction quantifiers are length-bounded so a crafted long + // unterminated run (many `(` or `—`) in an untrusted STATE.md field value + // cannot drive O(n^2) regex backtracking (CPU-exhaustion DoS). A real phase + // name is far shorter than the cap. + const parenName = str.match(/\(([^)]{1,200})\)/); + const dashName = str.match(/—\s*([^(\n]{1,200}?)(?:\s*\(|$)/); + const rawName = parenName?.[1] ?? dashName?.[1] ?? null; + const name = rawName && !/^(?:complete|executing|not started)$/i.test(rawName.trim()) + ? rawName.trim() + : null; + return { + phase: phaseMatch ? phaseMatch[1] : null, + name, + }; +} + +/** + * Config-AWARE project-code prefix strip. Unlike the config-blind + * `stripProjectCodePrefix` (which strips ANY `-` shape), this strips the + * leading `-` ONLY when `` case-insensitively equals the configured + * `projectCode`. A foreign prefix (`MEM-01` when the configured code is `LKML`) + * or an absent/empty `projectCode` is preserved verbatim — this is the #2104 + * fix: a foreign-prefixed id must not collapse to a bare numeric phase and + * collide with a real one. + */ +function stripConfiguredProjectCodePrefix(value: unknown, projectCode: string | null | undefined): string { + const input = String(value); + const configured = typeof projectCode === 'string' ? projectCode.trim() : ''; + if (!configured) return input; + const m = input.match(PROJECT_CODE_PREFIX_CAPTURE_RE_I); + if (!m) return input; + if (m[1].toUpperCase() !== configured.toUpperCase()) return input; + return m[2]; +} + +/** + * True when `phase` carries a project-code prefix that is NOT the configured + * `projectCode` (or when no `projectCode` is configured). The canonical + * predicate the init-command foreign-prefix guard (#2056 / PR #2105) delegates + * to, so every call site shares one foreign-prefix rule. + */ +function isForeignPrefixedPhaseQuery(phase: unknown, projectCode: unknown): boolean { + const m = String(phase).match(PROJECT_CODE_PREFIX_CAPTURE_RE_I); + if (!m) return false; + const configured = typeof projectCode === 'string' ? projectCode.trim() : ''; + return !configured || m[1].toUpperCase() !== configured.toUpperCase(); +} + +/** + * Canonical ROADMAP heading lookup-source list (moved here from + * roadmap-parser.cts so phase-id.cts is the single owner of the ordering). + * Sources are tried in a fixed, deduplicated order: exact (only when the query + * itself is project-code-prefixed) → bare numeric / padding-tolerant → + * prefix-tolerant fallback. The bare numeric source precedes the prefix-tolerant + * form so a canonical heading (`### Phase 117:`) is preferred over a drifted + * prefixed one (`### Phase MANIFOLD-117:`) when both exist in one ROADMAP. + */ +function roadmapPhaseLookupSources(phaseNum: unknown): string[] { + const sources: string[] = []; + const exactSource = phaseMarkdownRegexSourceExact(phaseNum); + if (exactSource) sources.push(exactSource); + + const numericSource = phaseMarkdownRegexSource(phaseNum); + sources.push(numericSource); + sources.push(`${OPTIONAL_PROJECT_CODE_PREFIX_SOURCE}${numericSource}`); + + return [...new Set(sources)]; +} + export = { escapeRegex, OPTIONAL_PROJECT_CODE_PREFIX_SOURCE, @@ -268,4 +359,8 @@ export = { comparePhaseNum, extractPhaseToken, phaseTokenMatches, + parsePhaseFromProse, + stripConfiguredProjectCodePrefix, + isForeignPrefixedPhaseQuery, + roadmapPhaseLookupSources, }; diff --git a/src/roadmap-parser.cts b/src/roadmap-parser.cts index 1dd7c1c99..f2ab3f648 100644 --- a/src/roadmap-parser.cts +++ b/src/roadmap-parser.cts @@ -22,10 +22,11 @@ import phaseIdModule = require('./phase-id.cjs'); const { escapeRegex, phaseMarkdownRegexSource, - phaseMarkdownRegexSourceExact, stripProjectCodePrefix, - OPTIONAL_PROJECT_CODE_PREFIX_SOURCE, OPTIONAL_PHASE_TAG_SOURCE, + // #2121: roadmapPhaseLookupSources now lives in phase-id.cjs (single owner of + // the lookup-source ordering); imported here rather than defined locally. + roadmapPhaseLookupSources, } = phaseIdModule; // eslint-disable-next-line @typescript-eslint/no-require-imports import planningWorkspace = require('./planning-workspace.cjs'); @@ -242,23 +243,6 @@ function findRoadmapPhaseInContent(content: string, phaseNum: unknown, phaseSour }; } -function roadmapPhaseLookupSources(phaseNum: unknown): string[] { - const sources: string[] = []; - const exactSource = phaseMarkdownRegexSourceExact(phaseNum); - if (exactSource) sources.push(exactSource); - - const numericSource = phaseMarkdownRegexSource(phaseNum); - // Source order matters: the bare numeric source is tried before the - // prefix-tolerant form so that a canonical bare heading ("Phase 117:") is - // preferred over a drifted prefixed heading ("Phase MANIFOLD-117:") when - // both exist in the same ROADMAP. The prefix-tolerant form is the fallback - // that handles the drifted-only case. - sources.push(numericSource); - sources.push(`${OPTIONAL_PROJECT_CODE_PREFIX_SOURCE}${numericSource}`); - - return [...new Set(sources)]; -} - function getRoadmapPhaseInternal(cwd: string, phaseNum: unknown): RoadmapPhaseResult | null { if (!phaseNum) return null; const normalizedPhase = stripProjectCodePrefix(phaseNum); diff --git a/src/runtime-artifact-install-plan.cts b/src/runtime-artifact-install-plan.cts index aea0a0218..e2daf8d3e 100644 --- a/src/runtime-artifact-install-plan.cts +++ b/src/runtime-artifact-install-plan.cts @@ -32,6 +32,10 @@ interface ArtifactKind { destSubpath: string; prefix?: string; stage: (resolvedProfile: ResolvedProfile, agentCtx?: AgentCtx) => string; + /** Resolved absolute alternate install root for this kind, if the descriptor + * specifies one (e.g. codex skills → $HOME/.agents). Undefined means the + * kind installs under the runtime's normal configDir. */ + home?: string; } interface Layout { @@ -216,7 +220,7 @@ function createRuntimeArtifactInstallPlan(args: CreateRuntimeArtifactInstallPlan items.push({ kind: kind.kind, sourceDir, - destDir: assertDestWithinConfigHome(layout.configDir, kind.destSubpath), + destDir: assertDestWithinConfigHome(kind.home ?? layout.configDir, kind.destSubpath), }); } @@ -227,7 +231,7 @@ function createRuntimeArtifactUninstallPlan(layout: Layout): UninstallPlan { return { items: layout.kinds.map((kind) => ({ kind: kind.kind, - destDir: assertDestWithinConfigHome(layout.configDir, kind.destSubpath), + destDir: assertDestWithinConfigHome(kind.home ?? layout.configDir, kind.destSubpath), })), }; } diff --git a/src/runtime-artifact-layout.cts b/src/runtime-artifact-layout.cts index 073d670b8..4d8f34c57 100644 --- a/src/runtime-artifact-layout.cts +++ b/src/runtime-artifact-layout.cts @@ -73,6 +73,10 @@ interface ArtifactKind { /** For agents kind with a converter, accepts an optional AgentCtx as the second * arg so cross-cutting can be applied pre-converter (ADR-1235 §1). */ stage: (resolvedProfile: ResolvedProfile, agentCtx?: AgentCtx) => string; + /** Resolved absolute alternate install root for this kind, if the descriptor + * specifies one (e.g. codex skills → $HOME/.agents). Undefined means the + * kind installs under the runtime's normal configDir. */ + home?: string; } interface Layout { @@ -419,6 +423,10 @@ interface ArtifactKindDescriptor { nesting: 'flat' | 'nested'; recursive: boolean; converter: string | null; + /** Optional alternate install home, relative to the user's home directory + * (e.g. ".agents" for codex skills → $HOME/.agents/skills). When absent, + * the kind installs under the runtime's normal configDir. */ + home?: string; } interface ArtifactLayoutDescriptor { @@ -445,18 +453,19 @@ function dispatchKindEntry(entry: ArtifactKindDescriptor, runtime: string, confi const { kind, destSubpath, prefix, nesting, converter } = entry; const nested = nesting === 'nested'; + let result: ArtifactKind; switch (kind) { case 'commands': - if (converter == null) { - return commandsKind(destSubpath, prefix, configDir); - } - return convertedCommandsKind(destSubpath, prefix, converter, configDir); + result = converter == null + ? commandsKind(destSubpath, prefix, configDir) + : convertedCommandsKind(destSubpath, prefix, converter, configDir); + break; case 'agents': - if (converter == null) { - return agentsKind(destSubpath, prefix, configDir); - } - return convertedAgentsKind(destSubpath, prefix, converter, configDir, scope); + result = converter == null + ? agentsKind(destSubpath, prefix, configDir) + : convertedAgentsKind(destSubpath, prefix, converter, configDir, scope); + break; case 'skills': if (converter == null) { @@ -464,16 +473,24 @@ function dispatchKindEntry(entry: ArtifactKindDescriptor, runtime: string, confi `resolveRuntimeArtifactLayout: skills entry for '${runtime}' has converter=null (converter is required for skills)`, ); } - return skillsKind(destSubpath, prefix, converter, runtime, configDir, nested, scope); + result = skillsKind(destSubpath, prefix, converter, runtime, configDir, nested, scope); + break; case 'kimi-agents': - return kimiAgentsKind(destSubpath, prefix, configDir); + result = kimiAgentsKind(destSubpath, prefix, configDir); + break; default: throw new TypeError( `resolveRuntimeArtifactLayout: unknown kind '${kind}' in descriptor for runtime '${runtime}'`, ); } + + if (typeof entry.home === 'string' && entry.home !== '') { + result.home = path.join(os.homedir(), entry.home); + } + + return result; } /** diff --git a/src/runtime-homes.cts b/src/runtime-homes.cts index 0b5e70e18..b935e0511 100644 --- a/src/runtime-homes.cts +++ b/src/runtime-homes.cts @@ -118,6 +118,9 @@ type ConfigHomeDescriptor = interface RuntimeArtifactKindDescriptor { kind: string; destSubpath: string; + // ADR-1239 upgrade 3 (#2088): optional split-home override (relative to + // os.homedir()), e.g. Codex skills → ".agents". Absent for most runtimes. + home?: string; } interface RuntimeDescriptor { @@ -416,6 +419,14 @@ export function getGlobalSkillsBase(runtime: string): string | null { const runtimeEntry = getRegistry().runtimes[runtime]; const descriptor = runtimeEntry?.runtime; const globalSkillsKind = descriptor?.artifactLayout?.global?.find((entry) => entry.kind === 'skills'); + // ADR-1239 upgrade 3 (#2088): honor a skills-kind `home` override (e.g. Codex + // → $HOME/.agents/skills, independent of $CODEX_HOME) so the reported skills + // root matches where the installer actually writes (the artifact layout / + // _resolveSkillsRootDir). Without this, `--skills-root` and the sync-skills + // workflow would look under configHome/skills while skills live under ~/.agents. + if (globalSkillsKind?.home && globalSkillsKind?.destSubpath) { + return path.join(os.homedir(), globalSkillsKind.home, globalSkillsKind.destSubpath); + } if (descriptor?.configHome && globalSkillsKind?.destSubpath) { return resolveSkillsBaseFromDescriptor( descriptor.configHome, diff --git a/src/runtime-hooks-surface.cts b/src/runtime-hooks-surface.cts index cb0151da0..dbd1141ad 100644 --- a/src/runtime-hooks-surface.cts +++ b/src/runtime-hooks-surface.cts @@ -27,6 +27,13 @@ import fs from 'node:fs'; import path from 'node:path'; import os from 'node:os'; +import { + CURSOR_HOOK_EVENTS, + CURSOR_EVENT_SCRIPT_MAP, + resolveManagedHookEvents, + resolveHookScripts, + buildHookBusEntries, +} from './host-integration-adapters/imperative-hook-bus.cjs'; // eslint-disable-next-line @typescript-eslint/no-require-imports import shellCmdProjection = require('./shell-command-projection.cjs'); const { @@ -80,8 +87,20 @@ const GSD_COPILOT_SESSION_HOOK_PWSH = // --------------------------------------------------------------------------- const GSD_CURSOR_SESSION_HOOK_SCRIPT = 'gsd-cursor-session-start.js'; const GSD_CURSOR_POST_TOOL_HOOK_SCRIPT = 'gsd-cursor-post-tool.js'; +const GSD_CURSOR_PRE_TOOL_HOOK_SCRIPT = 'gsd-cursor-pre-tool.js'; +const GSD_CURSOR_STOP_HOOK_SCRIPT = 'gsd-cursor-stop.js'; +const GSD_CURSOR_SUBAGENT_START_HOOK_SCRIPT = 'gsd-cursor-subagent-start.js'; +const GSD_CURSOR_SUBAGENT_STOP_HOOK_SCRIPT = 'gsd-cursor-subagent-stop.js'; const GSD_CURSOR_HOOK_MARKER = 'gsd-managed'; +// The full set of Cursor hook events GSD manages — sourced from the adapter +// (src/host-integration-adapters/imperative-hook-bus.cts) so the vocabulary +// stays closed and first-party. Used by reconcileCursorHooksJson (the +// reconciliation scope is always the full set). The install path +// (writeCursorHooksJson) resolves a descriptor-driven subset via +// resolveManagedHookEvents(opts.managedHookEvents). +const CURSOR_MANAGED_EVENTS = CURSOR_HOOK_EVENTS; + // --------------------------------------------------------------------------- // Cline / AGENTS.md constants // --------------------------------------------------------------------------- @@ -976,9 +995,8 @@ function reconcileCursorHooksJson(hooksJsonPath: string, managedEntries: CursorM const hasNestedHooksObject = parsed['hooks'] && typeof parsed['hooks'] === 'object' && !Array.isArray(parsed['hooks']); if (!hasNestedHooksObject) { - const eventKeys = ['sessionStart', 'postToolUse']; const lifted: Record = {}; - for (const k of eventKeys) { + for (const k of CURSOR_MANAGED_EVENTS) { if (Array.isArray(parsed[k])) { lifted[k] = parsed[k]; delete parsed[k]; @@ -989,10 +1007,9 @@ function reconcileCursorHooksJson(hooksJsonPath: string, managedEntries: CursorM if (!parsed['version']) parsed['version'] = 1; const hookTable = parsed['hooks'] as Record; - const MANAGED_EVENTS = ['sessionStart', 'postToolUse']; const entries = managedEntries || {}; - for (const event of MANAGED_EVENTS) { + for (const event of CURSOR_MANAGED_EVENTS) { const existing = Array.isArray(hookTable[event]) ? (hookTable[event] as unknown[]) : []; const userOwned = existing.filter((e) => !isManagedCursorHookEntry(e)); const newEntry = entries[event] || null; @@ -1020,6 +1037,7 @@ function reconcileCursorHooksJson(hooksJsonPath: string, managedEntries: CursorM interface WriteCursorHooksJsonOpts { absoluteRunner?: string | null; platform?: string; + managedHookEvents?: readonly string[]; } function writeCursorHooksJson(targetDir: string, src: string, opts?: WriteCursorHooksJsonOpts): { hooksJsonPath: string; changed: boolean } { @@ -1027,7 +1045,11 @@ function writeCursorHooksJson(targetDir: string, src: string, opts?: WriteCursor const hooksDir = path.join(targetDir, 'hooks'); fs.mkdirSync(hooksDir, { recursive: true }); - const hookScripts = [GSD_CURSOR_SESSION_HOOK_SCRIPT, GSD_CURSOR_POST_TOOL_HOOK_SCRIPT]; + // Descriptor-driven event resolution (#2089): the managed event set comes + // from the host descriptor's hostBehaviors.managedHookEvents via the pure + // adapter (resolveManagedHookEvents), NOT a hardcoded constant. + const events = resolveManagedHookEvents(opts.managedHookEvents); + const hookScripts = resolveHookScripts(events); const srcHooksDir = path.join(src, 'hooks'); const installedScripts = new Set(); for (const script of hookScripts) { @@ -1043,28 +1065,16 @@ function writeCursorHooksJson(targetDir: string, src: string, opts?: WriteCursor } const hookOpts: BuildHookCommandOpts = { runtime: 'cursor', platform: opts.platform || process.platform }; - const sessionStartCmd = installedScripts.has('gsd-cursor-session-start.js') - ? buildHookCommand(targetDir, 'gsd-cursor-session-start.js', hookOpts) - : null; - const postToolCmd = installedScripts.has('gsd-cursor-post-tool.js') - ? buildHookCommand(targetDir, 'gsd-cursor-post-tool.js', hookOpts) - : null; - - const managedEntries: CursorManagedEntries = {}; - if (sessionStartCmd) { - managedEntries['sessionStart'] = { - type: 'command', - command: sessionStartCmd, - [GSD_CURSOR_HOOK_MARKER]: true, - }; - } - if (postToolCmd) { - managedEntries['postToolUse'] = { - type: 'command', - command: postToolCmd, - [GSD_CURSOR_HOOK_MARKER]: true, - }; + const commands: Record = {}; + for (const ev of events) { + const script = CURSOR_EVENT_SCRIPT_MAP[ev]; + if (script && installedScripts.has(script)) { + commands[ev] = buildHookCommand(targetDir, script, hookOpts); + } else { + commands[ev] = null; + } } + const managedEntries = buildHookBusEntries(events, commands) as CursorManagedEntries; const hooksJsonPath = path.join(targetDir, 'hooks.json'); const result = reconcileCursorHooksJson(hooksJsonPath, managedEntries); @@ -1729,6 +1739,10 @@ export = { removeCursorHooksJson, GSD_CURSOR_SESSION_HOOK_SCRIPT, GSD_CURSOR_POST_TOOL_HOOK_SCRIPT, + GSD_CURSOR_PRE_TOOL_HOOK_SCRIPT, + GSD_CURSOR_STOP_HOOK_SCRIPT, + GSD_CURSOR_SUBAGENT_START_HOOK_SCRIPT, + GSD_CURSOR_SUBAGENT_STOP_HOOK_SCRIPT, GSD_CURSOR_HOOK_MARKER, // Copilot diff --git a/src/state.cts b/src/state.cts index 02cfc8b59..0e8199266 100644 --- a/src/state.cts +++ b/src/state.cts @@ -1118,8 +1118,12 @@ function matchSessionSection(body: string): RegExpMatchArray | null { function parseProsePhaseField(value: string | null): { phase: string | null; name: string | null } { if (!value) return { phase: null, name: null }; const phaseMatch = value.match(/\b(\d+[A-Z]?(?:\.\d+)*)\b/i); - const parenName = value.match(/\(([^)]+)\)/); - const dashName = value.match(/—\s*([^(\n]+?)(?:\s*\(|$)/); + // #2124 review: length-bound the name quantifiers so a crafted long + // unterminated `(` / `—` run in an untrusted STATE.md field cannot drive + // O(n^2) backtracking (CPU DoS). (Phase 2 / #2125 supersedes this function + // by delegating to phase-id.cts:parsePhaseFromProse, which is bounded too.) + const parenName = value.match(/\(([^)]{1,200})\)/); + const dashName = value.match(/—\s*([^(\n]{1,200}?)(?:\s*\(|$)/); const rawName = parenName?.[1] ?? dashName?.[1] ?? null; const name = rawName && !/^(?:complete|executing|not started)$/i.test(rawName.trim()) ? rawName.trim() diff --git a/tests/codex-config.test.cjs b/tests/codex-config.test.cjs index b8f3f9c58..480d78214 100644 --- a/tests/codex-config.test.cjs +++ b/tests/codex-config.test.cjs @@ -60,21 +60,34 @@ const { resolveInstallPlan } = require('../gsd-core/bin/lib/runtime-config-adapt function runCodexInstall(codexHome, cwd = path.join(__dirname, '..')) { const previousCodeHome = process.env.CODEX_HOME; + const previousHome = process.env.HOME; + const previousUserProfile = process.env.USERPROFILE; const previousCwd = process.cwd(); process.env.CODEX_HOME = codexHome; + // #2088: Codex skills now install to the canonical $HOME/.agents/skills root + // (os.homedir()-relative, independent of CODEX_HOME — per codex core-skills + // loader.rs). Sandbox HOME to codexHome so skills land under the temp dir + // (codexHome/.agents/skills) instead of polluting the developer's real home. + process.env.HOME = codexHome; + process.env.USERPROFILE = codexHome; try { process.chdir(cwd); return install(true, 'codex'); } finally { process.chdir(previousCwd); - if (previousCodeHome === undefined) { - delete process.env.CODEX_HOME; - } else { - process.env.CODEX_HOME = previousCodeHome; - } + if (previousCodeHome === undefined) delete process.env.CODEX_HOME; + else process.env.CODEX_HOME = previousCodeHome; + if (previousHome === undefined) delete process.env.HOME; + else process.env.HOME = previousHome; + if (previousUserProfile === undefined) delete process.env.USERPROFILE; + else process.env.USERPROFILE = previousUserProfile; } } +// #2088: the canonical Codex skill-install root, sandboxed under codexHome. +function codexSkillsRoot(codexHome) { + return path.join(codexHome, '.agents', 'skills'); +} function readCodexConfig(codexHome) { return fs.readFileSync(path.join(codexHome, 'config.toml'), 'utf8'); @@ -722,17 +735,19 @@ describe('generateCodexConfigBlock', () => { assert.ok(result.startsWith(GSD_CODEX_MARKER), 'starts with marker'); }); - test('does not include feature flags or agents table header', () => { + test('emits the [agents] max_depth tuning block but no feature flags (#2088)', () => { const result = generateCodexConfigBlock(agents); assert.ok(!result.includes('[features]'), 'no features table'); assert.ok(!result.includes('multi_agent'), 'no multi_agent'); assert.ok(!result.includes('default_mode_request_user_input'), 'no request_user_input'); - // Should not have bare [agents] table header (only [agents.] structs). - assert.ok(!result.match(/^\[agents\]\s*$/m), 'no bare [agents] table'); + // #2088: the managed block DOES pin dispatch depth via a bare [agents] + // AgentsToml scalar table (coexisting with the [agents.] role structs). + assert.match(result, /^\[agents\]$/m, 'emits the [agents] tuning table'); + assert.match(result, /^max_depth = 1$/m, 'pins max_depth = 1'); // Should not emit [[agents]] sequence format (rejected by Codex 0.124.0). assert.ok(!result.includes('[[agents]]'), 'no [[agents]] sequence format'); - assert.ok(!result.includes('max_threads'), 'no max_threads'); - assert.ok(!result.includes('max_depth'), 'no max_depth'); + // Only max_depth is managed — max_threads is intentionally left to the user. + assert.ok(!result.includes('max_threads'), 'no max_threads (only max_depth is GSD-managed)'); }); test('#2727: emits [agents.] struct format (Codex 0.120.0+, replaces #2645 [[agents]])', () => { @@ -2721,7 +2736,7 @@ describe('cleanupCodexSkillMetadataSidecars (#1326)', () => { const codexHome = path.join(tmpDir, 'codex-home'); fs.mkdirSync(codexHome, { recursive: true }); runCodexInstall(codexHome); - const skillsDir = path.join(codexHome, 'skills'); + const skillsDir = codexSkillsRoot(codexHome); assert.ok(fs.existsSync(skillsDir), 'Codex install must create a skills/ directory'); const gsdSkillDirs = fs.readdirSync(skillsDir, { withFileTypes: true }) .filter(e => e.isDirectory() && e.name.startsWith('gsd-') && e.name !== 'gsd-dev-preferences'); @@ -2798,12 +2813,28 @@ before(() => { describe('#2698: CRLF stale gsd-update-check block is removed on Codex reinstall', () => { let tmpDir; + let _previousHome; + let _previousUserProfile; beforeEach(() => { tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-crlf-install-2698-')); + // #2088 (ADR-1239 upgrade 3): Codex's skills-kind `home: ".agents"` override + // applies to BOTH global and local scope and resolves via os.homedir(). This + // describe block calls install(false, 'codex') (local scope) directly — + // without sandboxing HOME/USERPROFILE to tmpDir, that in-process install + // would materialize a full gsd-* skill set into the developer/CI machine's + // REAL $HOME/.agents/skills instead of the temp dir. + _previousHome = process.env.HOME; + _previousUserProfile = process.env.USERPROFILE; + process.env.HOME = tmpDir; + process.env.USERPROFILE = tmpDir; }); afterEach(() => { + if (_previousHome === undefined) delete process.env.HOME; + else process.env.HOME = _previousHome; + if (_previousUserProfile === undefined) delete process.env.USERPROFILE; + else process.env.USERPROFILE = _previousUserProfile; // Use the shared 5s Windows-EBUSY retry budget instead of inline 1s. cleanup(tmpDir); }); @@ -2993,7 +3024,16 @@ if (previousGsdTestMode === undefined) { function runCodexInstall(codexHome, cwd = path.join(__dirname, '..')) { const previousCodeHome = process.env.CODEX_HOME; const previousCwd = process.cwd(); + // #2088 (ADR-1239 upgrade 3): Codex skills now install to the canonical + // $HOME/.agents/skills root (os.homedir()-relative, independent of + // CODEX_HOME). Sandbox HOME (and USERPROFILE) to codexHome so this + // in-process install never materializes skills under the developer/CI + // machine's real home directory. + const previousHome = process.env.HOME; + const previousUserProfile = process.env.USERPROFILE; process.env.CODEX_HOME = codexHome; + process.env.HOME = codexHome; + process.env.USERPROFILE = codexHome; try { process.chdir(cwd); return install(true, 'codex'); @@ -3004,6 +3044,10 @@ function runCodexInstall(codexHome, cwd = path.join(__dirname, '..')) { } else { process.env.CODEX_HOME = previousCodeHome; } + if (previousHome === undefined) delete process.env.HOME; + else process.env.HOME = previousHome; + if (previousUserProfile === undefined) delete process.env.USERPROFILE; + else process.env.USERPROFILE = previousUserProfile; } } @@ -4583,7 +4627,16 @@ before(() => { function runCodexInstall(codexHome) { const previousCodexHome = process.env.CODEX_HOME; const previousCwd = process.cwd(); + // #2088 (ADR-1239 upgrade 3): Codex skills now install to the canonical + // $HOME/.agents/skills root (os.homedir()-relative, independent of + // CODEX_HOME). Sandbox HOME (and USERPROFILE) to codexHome so this + // in-process install never materializes skills under the developer/CI + // machine's real home directory. + const previousHome = process.env.HOME; + const previousUserProfile = process.env.USERPROFILE; process.env.CODEX_HOME = codexHome; + process.env.HOME = codexHome; + process.env.USERPROFILE = codexHome; try { process.chdir(path.join(__dirname, '..')); return install(true, 'codex'); @@ -4594,6 +4647,10 @@ function runCodexInstall(codexHome) { } else { process.env.CODEX_HOME = previousCodexHome; } + if (previousHome === undefined) delete process.env.HOME; + else process.env.HOME = previousHome; + if (previousUserProfile === undefined) delete process.env.USERPROFILE; + else process.env.USERPROFILE = previousUserProfile; } } @@ -4908,7 +4965,7 @@ describe('#3245 — idempotent rollback reverts skills/, agents/, and VERSION', assert.strictEqual(threw, true, 'install must throw when validation fails'); // skills/ — GSD writes gsd-* subdirs here. All must be absent after rollback. - const skillsDir = path.join(codexHome, 'skills'); + const skillsDir = codexSkillsRoot(codexHome); if (fs.existsSync(skillsDir)) { const gsdSkills = fs.readdirSync(skillsDir, { withFileTypes: true }) .filter(e => e.isDirectory() && e.name.startsWith('gsd-')); @@ -4963,7 +5020,7 @@ describe('#3245 — idempotent rollback reverts skills/, agents/, and VERSION', assert.strictEqual(threw, true, 'install must throw when validation fails (very early failure)'); // Rollback removes all gsd-* skill dirs it wrote. Even if skills/ was // created during the install, no gsd-* dirs should survive after rollback. - const skillsDir = path.join(codexHome, 'skills'); + const skillsDir = codexSkillsRoot(codexHome); const remainingGsdSkills = fs.existsSync(skillsDir) ? fs.readdirSync(skillsDir, { withFileTypes: true }) .filter((e) => e.isDirectory() && e.name.startsWith('gsd-')) @@ -4978,7 +5035,7 @@ describe('#3245 — idempotent rollback reverts skills/, agents/, and VERSION', test('rollback does not remove pre-existing user skills that GSD did not write', () => { // If the user has a custom skill dir (not gsd-*) it must survive rollback. - const skillsDir = path.join(codexHome, 'skills'); + const skillsDir = codexSkillsRoot(codexHome); const userSkill = path.join(skillsDir, 'my-custom-skill'); fs.mkdirSync(userSkill, { recursive: true }); fs.writeFileSync(path.join(userSkill, 'SKILL.md'), '# Custom\n', 'utf8'); @@ -5248,7 +5305,16 @@ describe('#3285 — install succeeds when config.toml contains hooks.state entri function runCodexInstall() { const previousCodexHome = process.env.CODEX_HOME; const previousCwd = process.cwd(); + // #2088 (ADR-1239 upgrade 3): Codex skills now install to the canonical + // $HOME/.agents/skills root (os.homedir()-relative, independent of + // CODEX_HOME). Sandbox HOME (and USERPROFILE) to tmpDir so this + // in-process install never materializes skills under the developer/CI + // machine's real home directory. + const previousHome = process.env.HOME; + const previousUserProfile = process.env.USERPROFILE; process.env.CODEX_HOME = codexHome; + process.env.HOME = tmpDir; + process.env.USERPROFILE = tmpDir; try { process.chdir(path.join(__dirname, '..')); return install(true, 'codex'); @@ -5259,6 +5325,10 @@ describe('#3285 — install succeeds when config.toml contains hooks.state entri } else { process.env.CODEX_HOME = previousCodexHome; } + if (previousHome === undefined) delete process.env.HOME; + else process.env.HOME = previousHome; + if (previousUserProfile === undefined) delete process.env.USERPROFILE; + else process.env.USERPROFILE = previousUserProfile; } } @@ -5930,11 +6000,24 @@ describe('#3426 uninstall: gsd-check-update.cmd is removed from hooks dir on uni function withCodexHome(dir, fn) { const prev = process.env.CODEX_HOME; + // #2088 (ADR-1239 upgrade 3): Codex skills now resolve an alternate install + // home rooted at the REAL os.homedir() ($HOME/.agents), independent of + // CODEX_HOME. Fake $HOME (and $USERPROFILE) too so this in-process install + // never touches the developer/CI machine's real home directory — confined + // entirely to `dir`, which the caller cleans up. + const prevHome = process.env.HOME; + const prevUserProfile = process.env.USERPROFILE; process.env.CODEX_HOME = dir; + process.env.HOME = dir; + process.env.USERPROFILE = dir; try { return fn(); } finally { if (prev == null) delete process.env.CODEX_HOME; else process.env.CODEX_HOME = prev; + if (prevHome == null) delete process.env.HOME; + else process.env.HOME = prevHome; + if (prevUserProfile == null) delete process.env.USERPROFILE; + else process.env.USERPROFILE = prevUserProfile; } } @@ -6050,12 +6133,27 @@ const BUILD_HOOKS_SCRIPT = path.join(__dirname, '..', 'scripts', 'build-hooks.js function withCodexHome(codexHome, fn) { const prev = process.env.CODEX_HOME; + // #2088 (ADR-1239 upgrade 3): Codex skills now resolve an alternate install + // home rooted at the REAL os.homedir() ($HOME/.agents), independent of + // CODEX_HOME. Fake $HOME (and $USERPROFILE) too — using the sandbox root + // (codexHome's parent, since codexHome is conventionally `/.codex` + // in this file) — so this in-process install never touches the developer/CI + // machine's real home directory. tmpRoot is reclaimed by the caller's afterEach. + const prevHome = process.env.HOME; + const prevUserProfile = process.env.USERPROFILE; + const fakeHome = path.dirname(codexHome); process.env.CODEX_HOME = codexHome; + process.env.HOME = fakeHome; + process.env.USERPROFILE = fakeHome; try { return fn(); } finally { if (prev == null) delete process.env.CODEX_HOME; else process.env.CODEX_HOME = prev; + if (prevHome == null) delete process.env.HOME; + else process.env.HOME = prevHome; + if (prevUserProfile == null) delete process.env.USERPROFILE; + else process.env.USERPROFILE = prevUserProfile; } } @@ -6107,22 +6205,32 @@ describe('#3427 + #3433 — Codex installer avoids duplicate skills and mixed ho withCodexHome(codexHome, () => install(true, 'codex')); - const skillsDir = path.join(codexHome, 'skills'); - const entries = fs.existsSync(skillsDir) - ? fs.readdirSync(skillsDir, { withFileTypes: true }).filter((e) => e.isDirectory()).map((e) => e.name) + // #2088: the managed gsd-* skill surface now regenerates at the + // canonical $HOME/.agents/skills root (fakeHome === tmpRoot here — see + // withCodexHome above), not under the legacy $CODEX_HOME/skills. + const newSkillsDir = codexSkillsRoot(tmpRoot); + const newEntries = fs.existsSync(newSkillsDir) + ? fs.readdirSync(newSkillsDir, { withFileTypes: true }).filter((e) => e.isDirectory()).map((e) => e.name) : []; - // #3562: $gsd-* commands are discoverable only when skills/gsd-*/SKILL.md - // exists. The installer must regenerate (not remove) the managed gsd-* - // directories. - assert.equal(entries.includes('gsd-help'), true); - const refreshedBody = fs.readFileSync(path.join(skillsDir, 'gsd-help', 'SKILL.md'), 'utf8'); + // #3562: $gsd-* commands are discoverable only when + // .agents/skills/gsd-*/SKILL.md exists. The installer must regenerate + // (not remove) the managed gsd-* directories. + assert.equal(newEntries.includes('gsd-help'), true); + const refreshedBody = fs.readFileSync(path.join(newSkillsDir, 'gsd-help', 'SKILL.md'), 'utf8'); assert.notEqual(refreshedBody, legacySkillBody, 'stale legacy body must be overwritten'); const frontmatter = parseFrontmatter(refreshedBody); assert.equal(frontmatter.name, 'gsd-help', 'refreshed SKILL.md frontmatter must declare name: gsd-help'); - // Unrelated user skills are preserved — the regen scope is `gsd-*` only. - assert.equal(entries.includes('custom-user-skill'), true); + // #2088 migration: the installer cleans stale gsd-* dirs out of the old + // $CODEX_HOME/skills location on a pre-move install. + const legacyHelpDir = path.join(codexHome, 'skills', 'gsd-help'); + assert.equal(fs.existsSync(legacyHelpDir), false, 'migration must remove the stale legacy gsd-help skill dir from $CODEX_HOME/skills'); + + // Unrelated user skills are preserved in place — migration only removes + // `gsd-*` dirs from the old location; non-gsd-* user dirs are untouched. + const userSkill = path.join(codexHome, 'skills', 'custom-user-skill', 'SKILL.md'); + assert.equal(fs.existsSync(userSkill), true, 'unrelated user skill must survive the #2088 migration'); }); test('stores managed SessionStart update hook in hooks.json and removes inline gsd hook from config.toml', () => { @@ -6239,12 +6347,27 @@ const BUILD_HOOKS_SCRIPT = path.join(__dirname, '..', 'scripts', 'build-hooks.js function withCodexHome(codexHome, fn) { const prev = process.env.CODEX_HOME; + // #2088 (ADR-1239 upgrade 3): Codex skills now resolve an alternate install + // home rooted at the REAL os.homedir() ($HOME/.agents), independent of + // CODEX_HOME. Fake $HOME (and $USERPROFILE) too — using the sandbox root + // (codexHome's parent, since codexHome is conventionally `/.codex` + // in this file) — so this in-process install never touches the developer/CI + // machine's real home directory. tmpRoot is reclaimed by the caller's afterEach. + const prevHome = process.env.HOME; + const prevUserProfile = process.env.USERPROFILE; + const fakeHome = path.dirname(codexHome); process.env.CODEX_HOME = codexHome; + process.env.HOME = fakeHome; + process.env.USERPROFILE = fakeHome; try { return fn(); } finally { if (prev == null) delete process.env.CODEX_HOME; else process.env.CODEX_HOME = prev; + if (prevHome == null) delete process.env.HOME; + else process.env.HOME = prevHome; + if (prevUserProfile == null) delete process.env.USERPROFILE; + else process.env.USERPROFILE = prevUserProfile; } } @@ -6268,7 +6391,9 @@ describe('#3562 — Codex install produces discoverable $gsd-* skill surface', { test('global install creates skills/gsd-help/SKILL.md', () => { withCodexHome(codexHome, () => install(true, 'codex')); - const skillPath = path.join(codexHome, 'skills', 'gsd-help', 'SKILL.md'); + // #2088: skills now install to the canonical $HOME/.agents/skills root. + // withCodexHome fakes $HOME to tmpRoot (codexHome's parent) above. + const skillPath = path.join(codexSkillsRoot(tmpRoot), 'gsd-help', 'SKILL.md'); assert.ok( fs.existsSync(skillPath), `Codex install must create ${skillPath} so $gsd-help is discoverable. ` + @@ -6279,7 +6404,7 @@ describe('#3562 — Codex install produces discoverable $gsd-* skill surface', { test('SKILL.md content has frontmatter expected by Codex skill discovery', () => { withCodexHome(codexHome, () => install(true, 'codex')); - const skillPath = path.join(codexHome, 'skills', 'gsd-help', 'SKILL.md'); + const skillPath = path.join(codexSkillsRoot(tmpRoot), 'gsd-help', 'SKILL.md'); assert.ok(fs.existsSync(skillPath), 'precondition: SKILL.md exists'); const content = fs.readFileSync(skillPath, 'utf8'); @@ -6290,7 +6415,7 @@ describe('#3562 — Codex install produces discoverable $gsd-* skill surface', { test('multiple core $gsd-* skills are produced (not just gsd-help)', () => { withCodexHome(codexHome, () => install(true, 'codex')); - const skillsDir = path.join(codexHome, 'skills'); + const skillsDir = codexSkillsRoot(tmpRoot); assert.ok(fs.existsSync(skillsDir), 'skills/ directory must exist after install'); const gsdSkills = fs @@ -6384,12 +6509,27 @@ const BUILD_HOOKS_SCRIPT = path.join(__dirname, '..', 'scripts', 'build-hooks.js function withCodexHome(codexHome, fn) { const prev = process.env.CODEX_HOME; + // #2088 (ADR-1239 upgrade 3): Codex skills now resolve an alternate install + // home rooted at the REAL os.homedir() ($HOME/.agents), independent of + // CODEX_HOME. Fake $HOME (and $USERPROFILE) too — using the sandbox root + // (codexHome's parent, since codexHome is conventionally `/.codex` + // in this file) — so this in-process install never touches the developer/CI + // machine's real home directory. tmpRoot is reclaimed by the caller's afterEach. + const prevHome = process.env.HOME; + const prevUserProfile = process.env.USERPROFILE; + const fakeHome = path.dirname(codexHome); process.env.CODEX_HOME = codexHome; + process.env.HOME = fakeHome; + process.env.USERPROFILE = fakeHome; try { return fn(); } finally { if (prev == null) delete process.env.CODEX_HOME; else process.env.CODEX_HOME = prev; + if (prevHome == null) delete process.env.HOME; + else process.env.HOME = prevHome; + if (prevUserProfile == null) delete process.env.USERPROFILE; + else process.env.USERPROFILE = prevUserProfile; } } @@ -6659,7 +6799,16 @@ function runCodexInstallCaptured() { const previousCodexHome = process.env.CODEX_HOME; const previousCwd = process.cwd(); + // #2088 (ADR-1239 upgrade 3): Codex skills now resolve an alternate install + // home rooted at os.homedir() ($HOME/.agents), independent of CODEX_HOME. + // Sandbox $HOME (and $USERPROFILE) to codexHome too — otherwise this + // in-process install would materialize skills under the developer/CI + // machine's REAL home directory instead of the temp dir. + const previousHome = process.env.HOME; + const previousUserProfile = process.env.USERPROFILE; process.env.CODEX_HOME = codexHome; + process.env.HOME = codexHome; + process.env.USERPROFILE = codexHome; process.env.GSD_TEST_MODE = '1'; try { process.chdir(ROOT); @@ -6679,6 +6828,16 @@ function runCodexInstallCaptured() { } else { process.env.CODEX_HOME = previousCodexHome; } + 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 (previousGsdTestMode === undefined) { delete process.env.GSD_TEST_MODE; } else { @@ -6703,7 +6862,7 @@ describe('bug-3582: Codex global install materializes the skill surface', { conc }); test('writes the exact expected set of gsd-*/SKILL.md skills (deepEqual on name set)', () => { - const skillsDir = path.join(installRun.codexHome, 'skills'); + const skillsDir = codexSkillsRoot(installRun.codexHome); assert.ok( fs.existsSync(skillsDir), `Codex install must create ${skillsDir} (the 1.42.2 regression skipped this entirely)`, @@ -6735,7 +6894,7 @@ describe('bug-3582: Codex global install materializes the skill surface', { conc }); test('SKILL.md frontmatter declares hyphen-form name matching the directory', () => { - const skillsDir = path.join(installRun.codexHome, 'skills'); + const skillsDir = codexSkillsRoot(installRun.codexHome); const skillDirs = fs.readdirSync(skillsDir, { withFileTypes: true }) .filter(e => e.isDirectory() && e.name.startsWith('gsd-')) .map(e => e.name); @@ -6767,7 +6926,7 @@ describe('bug-3582: Codex global install materializes the skill surface', { conc // review); the file on disk must contain its full output verbatim // (open tag, body, closing ``). A truncated, // empty, or missing-closing-tag adapter cannot satisfy this assertion. - const skillsDir = path.join(installRun.codexHome, 'skills'); + const skillsDir = codexSkillsRoot(installRun.codexHome); const skillDirs = fs.readdirSync(skillsDir, { withFileTypes: true }) .filter(e => e.isDirectory() && e.name.startsWith('gsd-')) .map(e => e.name); @@ -6807,7 +6966,7 @@ describe('bug-3582: Codex global install materializes the skill surface', { conc 'gsd-new-project', 'gsd-health', ]; - const skillsDir = path.join(installRun.codexHome, 'skills'); + const skillsDir = codexSkillsRoot(installRun.codexHome); for (const name of representative) { const skillMd = path.join(skillsDir, name, 'SKILL.md'); assert.ok( @@ -6818,7 +6977,7 @@ describe('bug-3582: Codex global install materializes the skill surface', { conc }); test('installed Codex skills do not ask agents to run bare gsd-tools commands', () => { - const skillsDir = path.join(installRun.codexHome, 'skills'); + const skillsDir = codexSkillsRoot(installRun.codexHome); const skillDirs = fs.readdirSync(skillsDir, { withFileTypes: true }) .filter(e => e.isDirectory() && e.name.startsWith('gsd-')) .map(e => e.name); @@ -7075,12 +7234,27 @@ const BUILD_HOOKS_SCRIPT = path.join(__dirname, '..', 'scripts', 'build-hooks.js function withCodexHome(codexHome, fn) { const prev = process.env.CODEX_HOME; + // #2088 (ADR-1239 upgrade 3): Codex skills now resolve an alternate install + // home rooted at the REAL os.homedir() ($HOME/.agents), independent of + // CODEX_HOME. Fake $HOME (and $USERPROFILE) too — using the sandbox root + // (codexHome's parent, since codexHome is conventionally `/.codex` + // in this file) — so this in-process install never touches the developer/CI + // machine's real home directory. tmpRoot is reclaimed by the caller's afterEach. + const prevHome = process.env.HOME; + const prevUserProfile = process.env.USERPROFILE; + const fakeHome = path.dirname(codexHome); process.env.CODEX_HOME = codexHome; + process.env.HOME = fakeHome; + process.env.USERPROFILE = fakeHome; try { return fn(); } finally { if (prev == null) delete process.env.CODEX_HOME; else process.env.CODEX_HOME = prev; + if (prevHome == null) delete process.env.HOME; + else process.env.HOME = prevHome; + if (prevUserProfile == null) delete process.env.USERPROFILE; + else process.env.USERPROFILE = prevUserProfile; } } diff --git a/tests/codex-declarative-reference.test.cjs b/tests/codex-declarative-reference.test.cjs new file mode 100644 index 000000000..378ace06a --- /dev/null +++ b/tests/codex-declarative-reference.test.cjs @@ -0,0 +1,289 @@ +// allow-test-rule: AC2 requires asserting no `runtime === 'codex'` string-equality and no positive `isCodex` branch remain in bin/install.js/src — the descriptor-migration contract is a property of the source text, so a source-grep is the only faithful check (#2088) +'use strict'; + +/** + * codex declarative reference host — ADR-1239 Phase D / #2088 (EoS/codex). + * + * Proves Codex is driven through the PUBLIC Host-Integration Interface (the + * declarative embedding mode), that its negotiated axes classify + negotiate + * correctly (including the documented `maxDepth === 1 → flat` dispatch + * degradation), that negotiation fails CLOSED on a corrupted descriptor, that + * the three Context7-verified UPGRADES land on the user-reachable surface + * (skill-root → $HOME/.agents/skills, the 6 new hooks.json lifecycle events, and + * explicit `[agents] max_depth` dispatch tuning), and that the migration retired + * the hardcoded `runtime === 'codex'` / positive-`isCodex` projection (folded + * into descriptor-driven `runtime.hostBehaviors`). + */ + +const { test } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const os = require('node:os'); +const path = require('node:path'); + +process.env.GSD_TEST_MODE = '1'; + +const { createImperativeAdapter } = require('../gsd-core/bin/lib/adapter-imperative.cjs'); +const { createDeclarativeAdapter } = require('../gsd-core/bin/lib/adapter-declarative.cjs'); +const { + profileOf, + negotiateHostCapabilities, + degradationFor, + PROFILE_BASELINES, + UNDOCUMENTED, +} = require('../gsd-core/bin/lib/host-integration.cjs'); + +const install = require('../bin/install.js'); +const { cleanup } = require('./helpers.cjs'); + +const CODEX_CAP = JSON.parse( + fs.readFileSync(path.join(__dirname, '..', 'capabilities', 'codex', 'capability.json'), 'utf8'), +); +const CODEX_AXES = CODEX_CAP.runtime.hostIntegration; + +// -- AC2: driven through the public interface (declarative embedding mode) ---- + +test('codex axes classify as the declarative-cli reference profile', () => { + assert.equal(profileOf(CODEX_AXES), 'declarative-cli'); +}); + +test('createDeclarativeAdapter classifies codex as declarative + delegates install to the engine', () => { + const adapter = createDeclarativeAdapter({ runtime: 'codex' }); + assert.equal(adapter.kind, 'declarative'); + assert.equal(adapter.runtime, 'codex'); + assert.equal(typeof adapter.install, 'function'); + assert.equal(typeof adapter.uninstall, 'function'); +}); + +test('the composed-registry install adapter drives codex install/uninstall', () => { + const adapter = createImperativeAdapter({ runtime: 'codex' }); + assert.equal(adapter.kind, 'imperative'); + assert.equal(adapter.runtime, 'codex'); + assert.ok(adapter.registry && typeof adapter.registry === 'object'); + assert.equal(typeof adapter.install, 'function'); + assert.equal(typeof adapter.uninstall, 'function'); +}); + +// -- AC3: every negotiated axis populated (no undocumented sentinel) ---------- + +test('every codex hostIntegration axis is populated (zero undocumented sentinels)', () => { + const axisVals = [ + CODEX_AXES.embeddingMode, CODEX_AXES.commandSurface, CODEX_AXES.modelMode, + CODEX_AXES.hookBus, CODEX_AXES.stateIO, CODEX_AXES.transport, CODEX_AXES.runtime, + ]; + for (const v of axisVals) { + assert.notEqual(v, UNDOCUMENTED, `axis must be documented, got ${v}`); + assert.ok(typeof v === 'string' && v.length > 0); + } + const d = CODEX_AXES.dispatch; + for (const k of ['namedDispatch', 'nested', 'maxDepth', 'subagentToolkit', 'background', 'backgroundDispatch']) { + assert.notEqual(d[k], UNDOCUMENTED, `dispatch.${k} must be documented`); + assert.notEqual(d[k], undefined, `dispatch.${k} must be present`); + } + assert.equal(d.embeddingMode, undefined); // sanity: dispatch has no stray keys +}); + +// -- AC5: dispatch degrades to flat because maxDepth === 1 ------------------- + +test('dispatch degrades to FLAT for codex — maxDepth===1 even though nested/background are all true', () => { + assert.equal(CODEX_AXES.dispatch.nested, true); + assert.equal(CODEX_AXES.dispatch.background, true); + assert.equal(CODEX_AXES.dispatch.backgroundDispatch, true); + assert.equal(CODEX_AXES.dispatch.maxDepth, 1); + + const flat = degradationFor('dispatch', CODEX_AXES); + assert.equal(flat.level, 'degraded', 'maxDepth===1 must degrade dispatch, not grant full nesting'); + assert.match(flat.fallback, /flat dispatch/, 'the documented fallback is flat/inline waves'); + + // Prove maxDepth is the cause: at depth 2 the same axes grant full dispatch. + const deeper = { ...CODEX_AXES, dispatch: { ...CODEX_AXES.dispatch, maxDepth: 2 } }; + assert.equal(degradationFor('dispatch', deeper).level, 'full'); +}); + +// -- AC5: negotiation fails CLOSED on a corrupted descriptor ------------------ + +test('negotiateHostCapabilities never throws for codex, even fully corrupted', () => { + assert.doesNotThrow(() => negotiateHostCapabilities({})); + assert.doesNotThrow(() => negotiateHostCapabilities({ ...CODEX_AXES, embeddingMode: UNDOCUMENTED })); + assert.doesNotThrow(() => negotiateHostCapabilities({ ...CODEX_AXES, embeddingMode: 'future-unknown' })); +}); + +test('a partial/empty codex descriptor degrades to the safe floor, not the declarative-cli baseline', () => { + const result = negotiateHostCapabilities({}); + assert.equal(result.effective.embeddingMode, 'declarative', 'omitted embeddingMode degrades closed'); + assert.equal(result.effective.hookBus, 'none'); + assert.notDeepEqual(result.effective, PROFILE_BASELINES['declarative-cli']); + assert.ok(result.warnings.length > 0); +}); + +// -- AC2: the hardcoded projection is retired -------------------------------- + +test('codex descriptor declares runtime.hostBehaviors (the folded-in behaviors)', () => { + const hb = CODEX_CAP.runtime.hostBehaviors; + assert.ok(hb && typeof hb === 'object'); + assert.equal(hb.tomlConfigInstall, true, 'config.toml + agent-toml + hooks.json install runs through the descriptor gate'); + assert.equal(hb.cleanupSkillSidecars, true); + assert.equal(hb.agentTomlFiles, true); + assert.equal(hb.frontmatterDialect, 'codex'); + assert.equal(hb.reapplyCommand, '$gsd-update --reapply'); +}); + +test('no `runtime === "codex"` string-equality and no positive `isCodex` gate remain in the install source (AC2)', () => { + const strip = (src) => src + .replace(/\/\*[\s\S]*?\*\//g, '') + .replace(/\/\/[^\r\n]*/g, '') + .replace(/`[^`]*`/g, ''); + for (const rel of ['bin/install.js', 'src/install-engine.cts', 'src/runtime-artifact-conversion.cts']) { + const raw = fs.readFileSync(path.join(__dirname, '..', rel), 'utf8'); + const src = strip(raw); + + const stringEq = src.match(/runtime\s*[!=]==\s*'codex'/g) || []; + assert.deepEqual(stringEq, [], `AC2: no hardcoded runtime==='codex' branch may remain in ${rel}; found: ${stringEq.join(', ')}`); + + // Every `isCodex` reference must be either a runtimeFlags(...) destructure + // line or a NEGATED occurrence inside a shared multi-runtime roster + // (`!isCodex && !isCopilot && ...`). A positive `isCodex` gate is forbidden. + for (const line of src.split(/\r?\n/)) { + if (!/\bisCodex\b/.test(line)) continue; + if (/runtimeFlags\s*\(/.test(line)) continue; // destructure declaration + const positive = line.replace(/!\s*isCodex\b/g, '').match(/\bisCodex\b/); + assert.equal(positive, null, `AC2: positive isCodex gate forbidden in ${rel}: ${line.trim()}`); + } + } +}); + +// -- AC4 upgrade 3: skill root is the canonical $HOME/.agents/skills --------- + +test('upgrade 3 — codex skills resolve to $HOME/.agents/skills, not the deprecated $CODEX_HOME/skills', () => { + const codexHome = path.join(os.homedir(), '.codex'); + const root = install._resolveSkillsRootDir('codex', codexHome, 'global'); + assert.equal(root, path.join(os.homedir(), '.agents', 'skills'), + 'skills must install to the canonical ~/.agents/skills root'); + assert.ok(!root.startsWith(codexHome), 'skills must NOT land under the deprecated $CODEX_HOME/skills'); +}); + +test('upgrade 3 — the pre-move location is migrated (stale ~/.codex/skills/gsd-* cleaned; user content preserved)', () => { + const codexHome = path.join(os.homedir(), '.codex'); + const oldDir = install._resolveMovedSkillsOldDir('codex', codexHome, 'global'); + assert.equal(oldDir, path.join(codexHome, 'skills'), 'the pre-move location is $CODEX_HOME/skills'); + // A runtime with no home override yields null (no false migration). + assert.equal(install._resolveMovedSkillsOldDir('kimi', path.join(os.homedir(), '.kimi'), 'global'), null); + + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'codex-migrate-')); + const skillsDir = path.join(tmp, 'skills'); + fs.mkdirSync(path.join(skillsDir, 'gsd-plan'), { recursive: true }); + fs.mkdirSync(path.join(skillsDir, 'gsd-dev-preferences'), { recursive: true }); // user-owned, preserved + fs.mkdirSync(path.join(skillsDir, 'my-own-skill'), { recursive: true }); // non-gsd, preserved + try { + const removed = install.cleanupMovedSkillsOldLocation(skillsDir, 'gsd-'); + assert.equal(removed, 1, 'only the managed gsd-plan dir is removed'); + assert.ok(!fs.existsSync(path.join(skillsDir, 'gsd-plan')), 'stale managed skill removed'); + assert.ok(fs.existsSync(path.join(skillsDir, 'gsd-dev-preferences')), 'user-owned gsd-dev-preferences preserved'); + assert.ok(fs.existsSync(path.join(skillsDir, 'my-own-skill')), 'non-gsd dir preserved'); + } finally { + cleanup(tmp); + } +}); + +// -- AC4 upgrade 1: the 6 new hooks.json lifecycle events are registered ------ + +test('upgrade 1 — codex registers all documented hooks.json lifecycle events, incl. the 6 new in #2088', () => { + const expected = [ + 'SubagentStart', 'Stop', 'PostToolUse', // #772 + 'PreToolUse', 'PermissionRequest', 'PreCompact', 'PostCompact', 'SubagentStop', 'UserPromptSubmit', // #2088 + ]; + assert.deepEqual(install.CODEX_EXTENDED_HOOK_EVENTS, expected, + 'the shared install/uninstall event list must contain the 3 original + 6 new events'); + + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'codex-hooks-')); + try { + fs.mkdirSync(path.join(tmp, 'hooks'), { recursive: true }); + fs.writeFileSync(path.join(tmp, 'hooks', 'gsd-context-monitor.js'), '// stub'); + fs.writeFileSync(path.join(tmp, 'hooks', 'gsd-check-update.js'), '// stub'); + for (const ev of install.CODEX_EXTENDED_HOOK_EVENTS) { + install.ensureCodexHooksJsonEvent(tmp, ev, { absoluteRunner: '/usr/bin/node', platform: 'linux' }); + } + install.ensureCodexHooksJsonSessionStart(tmp, { absoluteRunner: '/usr/bin/node', platform: 'linux' }); + const hooksJson = JSON.parse(fs.readFileSync(path.join(tmp, 'hooks.json'), 'utf8')); + const registered = Object.keys(hooksJson.hooks || hooksJson || {}); + for (const ev of ['PreToolUse', 'PermissionRequest', 'PreCompact', 'PostCompact', 'SubagentStop', 'UserPromptSubmit']) { + assert.ok(registered.includes(ev), `#2088 must register the ${ev} hook in hooks.json`); + } + assert.ok(registered.includes('SessionStart'), 'the SessionStart baseline stays registered'); + } finally { + cleanup(tmp); + } +}); + +test('upgrade 1 — extendedHookEvents descriptor reconciled to the schema-valid wired subset (no longer [])', () => { + assert.deepEqual(CODEX_CAP.runtime.extendedHookEvents, ['SubagentStop', 'Stop', 'PreCompact'], + 'reconciled from [] to the wired extended-lifecycle events expressible in the cross-runtime vocabulary'); +}); + +// -- AC4 upgrade 2: explicit [agents] max_depth dispatch tuning --------------- + +test('upgrade 2 — the managed config block writes [agents] max_depth = 1', () => { + const block = install.generateCodexConfigBlock( + [{ name: 'gsd-foo', description: 'Foo' }, { name: 'gsd-bar', description: 'Bar' }], + path.join(os.homedir(), '.codex'), + ); + assert.match(block, /\[agents\]\nmax_depth = 1\n/, 'the block pins max_depth = 1 on a bare [agents] table'); + // The bare [agents] scalar table coexists with the [agents.gsd-*] role tables. + assert.match(block, /\[agents\.gsd-foo\]/); +}); + +test('upgrade 2 — validateCodexConfigSchema accepts the managed [agents] block but still rejects break-forms', () => { + const block = install.generateCodexConfigBlock([{ name: 'gsd-foo', description: 'Foo' }], path.join(os.homedir(), '.codex')); + assert.equal(install.validateCodexConfigSchema(block).ok, true, 'known-scalar [agents] + role tables must validate'); + + // Still rejects the actual #2760 break-forms. + assert.equal(install.validateCodexConfigSchema('[[agents]]\nname = "x"\n').ok, false, '[[agents]] sequence still rejected'); + assert.equal(install.validateCodexConfigSchema('[agents]\ndefault = "x"\n').ok, false, 'bare [agents] with an unknown key still rejected'); + + // A user's own AgentsToml scalar table is now accepted (no longer over-rejected). + assert.equal(install.validateCodexConfigSchema('[agents]\nmax_threads = 4\n').ok, true, 'user known-scalar [agents] accepted'); + assert.equal(install.codexBareAgentsHasOnlyKnownScalars('max_depth = 1\n'), true); + assert.equal(install.codexBareAgentsHasOnlyKnownScalars('default = "x"\n'), false); +}); + +test('upgrade 2 — uninstall removes the managed [agents] max_depth block', () => { + const block = install.generateCodexConfigBlock([{ name: 'gsd-foo', description: 'Foo' }], path.join(os.homedir(), '.codex')); + const stripped = install.stripGsdFromCodexConfig('model = "gpt-5"\n\n' + block); + assert.ok(stripped === null || !/\[agents\]/.test(stripped), `uninstall must remove the managed [agents] block; got: ${JSON.stringify(stripped)}`); +}); + +// Regression (#2088 review): install must NOT silently drop a user's own +// AgentsToml scalar tuning when it purges the bare [agents] table to add max_depth. +test('upgrade 2 — install preserves the user\'s own AgentsToml scalars (max_threads etc.), GSD-manages max_depth', () => { + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'codex-agents-merge-')); + try { + const cfgPath = path.join(tmp, 'config.toml'); + fs.writeFileSync(cfgPath, '[agents]\nmax_threads = 4\nmax_depth = 9\ninterrupt_message = false\n\n[model]\nname = "o3"\n'); + + // extract helper: user scalars except the GSD-managed max_depth. + const scalars = install.extractCodexUserAgentsScalars(fs.readFileSync(cfgPath, 'utf8')); + assert.ok(scalars.includes('max_threads = 4'), 'max_threads is a preserved user scalar'); + assert.ok(scalars.includes('interrupt_message = false'), 'interrupt_message is a preserved user scalar'); + assert.ok(!scalars.some((s) => s.startsWith('max_depth')), 'max_depth is GSD-managed, not preserved from the user'); + + install.mergeCodexConfig(cfgPath, install.generateCodexConfigBlock([{ name: 'gsd-foo', description: 'Foo' }], tmp)); + const merged = fs.readFileSync(cfgPath, 'utf8'); + + assert.match(merged, /max_threads = 4/, 'user max_threads must survive install'); + assert.match(merged, /interrupt_message = false/, 'user interrupt_message must survive install'); + assert.match(merged, /max_depth = 1/, 'GSD pins max_depth = 1'); + assert.doesNotMatch(merged, /max_depth = 9/, 'user max_depth is overridden by the GSD-managed value'); + assert.equal((merged.match(/^\[agents\]$/mg) || []).length, 1, 'exactly one managed [agents] table (no duplicate)'); + assert.equal(install.validateCodexConfigSchema(merged).ok, true, 'the merged config still validates'); + + // Symmetric: uninstall restores the user's scalars and drops GSD's max_depth. + const uninstalled = install.stripGsdFromCodexConfig(merged); + assert.match(uninstalled, /max_threads = 4/, 'uninstall restores the user\'s max_threads'); + assert.match(uninstalled, /interrupt_message = false/, 'uninstall restores interrupt_message'); + assert.doesNotMatch(uninstalled, /max_depth/, 'uninstall drops the GSD-managed max_depth'); + assert.doesNotMatch(uninstalled, /gsd-foo/, 'uninstall removes the gsd role table'); + assert.equal(install.validateCodexConfigSchema(uninstalled).ok, true, 'the restored config validates'); + } finally { + cleanup(tmp); + } +}); diff --git a/tests/cursor-dispatch-upgrade.test.cjs b/tests/cursor-dispatch-upgrade.test.cjs new file mode 100644 index 000000000..f99d8d608 --- /dev/null +++ b/tests/cursor-dispatch-upgrade.test.cjs @@ -0,0 +1,71 @@ +'use strict'; + +/** + * cursor dispatch UPGRADE — ADR-1239 / #2089 AC4b. + * + * Proves GSD's wave-based execution drives Cursor's native named/background + * nested subagent dispatch (background:true, backgroundDispatch:true, + * nested:true, maxDepth:2) instead of flattening to inline sequential calls. + * + * Cite: + * https://cursor.com/docs/subagents — named + background dispatch + * https://cursor.com/docs/sdk/typescript — nested subagent depth-2 constraint + */ + +const { test } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); + +const { shouldFlattenDispatch } = require('../gsd-core/bin/lib/host-integration.cjs'); + +const CUR_CAP = JSON.parse( + fs.readFileSync(path.join(__dirname, '..', 'capabilities', 'cursor', 'capability.json'), 'utf8'), +); +const CUR_DISPATCH = CUR_CAP.runtime.hostIntegration.dispatch; + +// -- AC4b: cursor dispatch axes ---------------------------------------------- + +test('cursor dispatch declares namedDispatch + nested + background + backgroundDispatch', () => { + assert.equal(CUR_DISPATCH.namedDispatch, true, + 'cite https://cursor.com/docs/subagents — named subagent invocation'); + assert.equal(CUR_DISPATCH.nested, true, + 'cite https://cursor.com/docs/sdk/typescript — nested subagents'); + assert.equal(CUR_DISPATCH.background, true, + 'cite https://cursor.com/docs/subagents — background dispatch'); + assert.equal(CUR_DISPATCH.backgroundDispatch, true, + 'cite https://cursor.com/docs/subagents FAQ — subagents can launch child subagents'); +}); + +test('cursor dispatch respects maxDepth: 2 (the documented constraint)', () => { + assert.equal(CUR_DISPATCH.maxDepth, 2, + 'cite https://cursor.com/docs/sdk/typescript — "a subagent launched by another subagent can\'t launch further"'); +}); + +// -- AC4b: shouldFlattenDispatch returns false (NOT force-flattened) ---------- + +test('shouldFlattenDispatch(cursor) is false — GSD uses native background dispatch', () => { + assert.equal(shouldFlattenDispatch(CUR_DISPATCH), false, + 'cursor has background:true + backgroundDispatch:true → GSD must NOT force-flatten'); +}); + +test('pre-upgrade cursor axes (background:false) DID force-flatten', () => { + const preUpgrade = { ...CUR_DISPATCH, background: false, backgroundDispatch: 'undocumented' }; + assert.equal(shouldFlattenDispatch(preUpgrade), true, + 'pre-upgrade cursor (no background dispatch) was force-flattened — the behavioral change #2089 lands'); +}); + +test('shouldFlattenDispatch is true when backgroundDispatch is false (depth-2 but no bg dispatch)', () => { + const noBgDispatch = { ...CUR_DISPATCH, backgroundDispatch: false }; + assert.equal(shouldFlattenDispatch(noBgDispatch), true, + 'background without backgroundDispatch still flattens (the #853 rule)'); +}); + +// -- AC4b: boundary — maxDepth 2 is the discriminator vs unbounded ----------- + +test('maxDepth 2 is the documented constraint (not -1 unbounded)', () => { + assert.notEqual(CUR_DISPATCH.maxDepth, -1, + 'cursor is NOT unbounded — depth-2 is the documented hard limit'); + assert.ok(CUR_DISPATCH.maxDepth > 0 && CUR_DISPATCH.maxDepth <= 2, + 'maxDepth must be a positive integer ≤ 2 per cursor docs'); +}); diff --git a/tests/cursor-hook-bus-upgrade.test.cjs b/tests/cursor-hook-bus-upgrade.test.cjs new file mode 100644 index 000000000..881bb1a6e --- /dev/null +++ b/tests/cursor-hook-bus-upgrade.test.cjs @@ -0,0 +1,173 @@ +'use strict'; + +/** + * cursor hook-bus UPGRADE — ADR-1239 / #2089 AC4a. + * + * Proves the expanded hook-bus coverage: GSD registers for subagentStart, + * subagentStop, preToolUse, and stop IN ADDITION to the baseline sessionStart + * and postToolUse. Cite: https://cursor.com/docs/hooks + * + * Tests the descriptor-driven adapter module (pure) + the reconcile behavior + * (all 6 managed events in the generated hooks.json). + */ + +const { test } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); +const os = require('node:os'); + +const { + CURSOR_HOOK_EVENTS, + CURSOR_EVENT_SCRIPT_MAP, + resolveManagedHookEvents, + resolveHookScripts, +} = require('../gsd-core/bin/lib/host-integration-adapters/imperative-hook-bus.cjs'); + +const { + reconcileCursorHooksJson, + GSD_CURSOR_HOOK_MARKER, +} = require('../bin/install.js'); + +const { cleanup } = require('./helpers.cjs'); + +const CUR_CAP = JSON.parse( + fs.readFileSync(path.join(__dirname, '..', 'capabilities', 'cursor', 'capability.json'), 'utf8'), +); + +const EXPECTED_EVENTS = [ + 'sessionStart', + 'postToolUse', + 'preToolUse', + 'stop', + 'subagentStart', + 'subagentStop', +]; + +// -- AC4a: the adapter declares all 6 managed events ------------------------- + +test('CURSOR_HOOK_EVENTS contains all 6 managed events', () => { + for (const ev of EXPECTED_EVENTS) { + assert.ok(CURSOR_HOOK_EVENTS.includes(ev), + `CURSOR_HOOK_EVENTS must include ${ev}`); + } + assert.equal(CURSOR_HOOK_EVENTS.length, 6, + 'exactly 6 managed events (no extras)'); +}); + +test('CURSOR_EVENT_SCRIPT_MAP maps every event to a script', () => { + for (const ev of EXPECTED_EVENTS) { + const script = CURSOR_EVENT_SCRIPT_MAP[ev]; + assert.ok(typeof script === 'string' && script.endsWith('.js'), + `${ev} must map to a .js script, got: ${script}`); + } +}); + +test('descriptor managedHookEvents matches the adapter event list', () => { + const declared = CUR_CAP.runtime.hostBehaviors.managedHookEvents; + assert.deepEqual(declared.sort(), [...EXPECTED_EVENTS].sort(), + 'descriptor managedHookEvents must match the 6-event managed set'); +}); + +// -- AC4a: resolveManagedHookEvents + resolveHookScripts --------------------- + +test('resolveManagedHookEvents returns all 6 from the descriptor list', () => { + const resolved = resolveManagedHookEvents(CUR_CAP.runtime.hostBehaviors.managedHookEvents); + assert.equal(resolved.length, 6); + for (const ev of EXPECTED_EVENTS) { + assert.ok(resolved.includes(ev), `resolveManagedHookEvents must include ${ev}`); + } +}); + +test('resolveManagedHookEvents filters unknown events (fail-closed)', () => { + const resolved = resolveManagedHookEvents(['sessionStart', 'bogusEvent', 'stop']); + assert.deepEqual([...resolved].sort(), ['sessionStart', 'stop']); +}); + +test('resolveManagedHookEvents falls back to full set when descriptor is empty', () => { + const resolved = resolveManagedHookEvents(null); + assert.equal(resolved.length, 6); +}); + +test('resolveHookScripts returns a script for every managed event', () => { + const scripts = resolveHookScripts(EXPECTED_EVENTS); + assert.equal(scripts.length, 6); + for (const s of scripts) { + assert.ok(s.startsWith('gsd-cursor-') && s.endsWith('.js'), + `script must follow gsd-cursor-*.js convention: ${s}`); + } +}); + +// -- AC4a: hook scripts exist on disk --------------------------------------- + +test('all 6 hook scripts exist under hooks/', () => { + for (const ev of EXPECTED_EVENTS) { + const script = CURSOR_EVENT_SCRIPT_MAP[ev]; + const scriptPath = path.join(__dirname, '..', 'hooks', script); + assert.ok(fs.existsSync(scriptPath), + `hook script must exist: hooks/${script} (event: ${ev})`); + } +}); + +// -- AC4a: reconcile generates hooks.json with all 6 events ------------------ + +test('reconcileCursorHooksJson writes all 6 managed events into hooks.json', (t) => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-cursor-hook-bus-')); + t.after(() => cleanup(tmpDir)); + const hooksJsonPath = path.join(tmpDir, 'hooks.json'); + const managedEntries = {}; + for (const ev of EXPECTED_EVENTS) { + managedEntries[ev] = { + type: 'command', + command: `node /fake/${ev}.js`, + [GSD_CURSOR_HOOK_MARKER]: true, + }; + } + const result = reconcileCursorHooksJson(hooksJsonPath, managedEntries); + assert.ok(result.changed, 'first write must report changed=true'); + + const written = JSON.parse(fs.readFileSync(hooksJsonPath, 'utf8')); + const hookTable = written.hooks; + assert.ok(hookTable && typeof hookTable === 'object'); + for (const ev of EXPECTED_EVENTS) { + assert.ok(Array.isArray(hookTable[ev]), + `hooks.json must have a ${ev} array`); + assert.equal(hookTable[ev].length, 1, + `${ev} must have exactly 1 managed entry`); + assert.equal(hookTable[ev][0][GSD_CURSOR_HOOK_MARKER], true, + `${ev} entry must carry the GSD managed marker`); + } +}); + +test('reconcileCursorHooksJson preserves user entries across all 6 events', (t) => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-cursor-hook-bus-')); + t.after(() => cleanup(tmpDir)); + const hooksJsonPath = path.join(tmpDir, 'hooks.json'); + // Seed with user-owned entries in two events. + const seed = { + version: 1, + hooks: { + sessionStart: [{ type: 'command', command: 'user-start.sh' }], + preToolUse: [{ type: 'command', command: 'user-pre.sh' }], + }, + }; + fs.writeFileSync(hooksJsonPath, JSON.stringify(seed, null, 2) + '\n'); + + const managedEntries = {}; + for (const ev of EXPECTED_EVENTS) { + managedEntries[ev] = { + type: 'command', + command: `node /gsd/${ev}.js`, + [GSD_CURSOR_HOOK_MARKER]: true, + }; + } + reconcileCursorHooksJson(hooksJsonPath, managedEntries); + + const written = JSON.parse(fs.readFileSync(hooksJsonPath, 'utf8')); + // sessionStart: 1 user + 1 managed + assert.equal(written.hooks.sessionStart.length, 2); + // preToolUse: 1 user + 1 managed + assert.equal(written.hooks.preToolUse.length, 2); + // postToolUse: 1 managed only + assert.equal(written.hooks.postToolUse.length, 1); +}); diff --git a/tests/cursor-imperative-reference.test.cjs b/tests/cursor-imperative-reference.test.cjs new file mode 100644 index 000000000..f3e0209a8 --- /dev/null +++ b/tests/cursor-imperative-reference.test.cjs @@ -0,0 +1,126 @@ +// allow-test-rule: AC2 requires asserting no `runtime === 'cursor'` string-equality branch remains in bin/install.js/src — the descriptor-migration contract is a property of the source text, so a source-grep is the only faithful check (#2089) +'use strict'; + +/** + * cursor imperative reference host — ADR-1239 Phase D / #2089 (EoS/cursor). + * + * Proves cursor is driven through the PUBLIC Host-Integration Interface (the + * imperative adapter), that its negotiated axes classify + negotiate correctly, + * that negotiation fails CLOSED on a corrupted descriptor, that the Context7- + * verified dispatch UPGRADE (named/background nested subagents) changes + * `shouldFlattenDispatch`, and that the migration retired the hardcoded + * `runtime === 'cursor'` / `isCursor` branches (folded into descriptor-driven + * `runtime.hostBehaviors`). + */ + +const { test } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); + +const { createImperativeAdapter } = require('../gsd-core/bin/lib/adapter-imperative.cjs'); +const { + profileOf, + negotiateHostCapabilities, + shouldFlattenDispatch, + PROFILE_BASELINES, + UNDOCUMENTED, +} = require('../gsd-core/bin/lib/host-integration.cjs'); + +const CUR_CAP = JSON.parse( + fs.readFileSync(path.join(__dirname, '..', 'capabilities', 'cursor', 'capability.json'), 'utf8'), +); +const CUR_AXES = CUR_CAP.runtime.hostIntegration; + +// -- AC2: driven through the public interface (imperative adapter) ----------- + +test('createImperativeAdapter classifies cursor as imperative + composes the registry', () => { + const adapter = createImperativeAdapter({ runtime: 'cursor' }); + assert.equal(adapter.kind, 'imperative'); + assert.equal(adapter.runtime, 'cursor'); + assert.ok(adapter.registry && typeof adapter.registry === 'object'); + assert.equal(typeof adapter.install, 'function'); + assert.equal(typeof adapter.uninstall, 'function'); +}); + +test('cursor axes classify as the programmatic-cli reference profile', () => { + assert.equal(profileOf(CUR_AXES), 'programmatic-cli'); +}); + +// -- AC3: all axes populated + validated ------------------------------------- + +test('cursor descriptor declares all 8 axes + 6 dispatch sub-axes (no undocumented)', () => { + assert.equal(CUR_AXES.embeddingMode, 'imperative'); + assert.equal(CUR_AXES.commandSurface, 'slash-file'); + assert.equal(CUR_AXES.modelMode, 'passive'); + assert.equal(CUR_AXES.hookBus, 'host'); + assert.equal(CUR_AXES.stateIO, 'filesystem'); + assert.equal(CUR_AXES.transport, 'mcp'); + assert.equal(CUR_AXES.runtime, 'node'); + const d = CUR_AXES.dispatch; + assert.equal(d.namedDispatch, true); + assert.equal(d.nested, true); + assert.equal(d.maxDepth, 2); + assert.equal(d.background, true); + assert.equal(d.subagentToolkit, 'full'); + assert.equal(d.backgroundDispatch, true); +}); + +// -- AC4b: the Context7-verified dispatch UPGRADE (named/background nested) --- + +test('cursor descriptor declares background dispatch true/true + nested + maxDepth 2', () => { + assert.equal(CUR_AXES.dispatch.background, true); + assert.equal(CUR_AXES.dispatch.backgroundDispatch, true); + assert.equal(CUR_AXES.dispatch.nested, true); + assert.equal(CUR_AXES.dispatch.maxDepth, 2, 'cite https://cursor.com/docs/sdk/typescript'); +}); + +test('dispatch UPGRADE changes shouldFlattenDispatch: false now (may background), true for pre-upgrade axes', () => { + assert.equal(shouldFlattenDispatch(CUR_AXES.dispatch), false, + 'with background:true+backgroundDispatch:true, GSD must NOT force-flatten cursor dispatch'); + const preUpgrade = { ...CUR_AXES.dispatch, background: false, backgroundDispatch: 'undocumented' }; + assert.equal(shouldFlattenDispatch(preUpgrade), true, + 'pre-upgrade (background:false) cursor was force-flattened — this is the behavioral change #2089 lands'); +}); + +// -- AC5: negotiation fails CLOSED on a corrupted descriptor ------------------ + +test('negotiateHostCapabilities never throws for cursor, even fully corrupted', () => { + assert.doesNotThrow(() => negotiateHostCapabilities({})); + assert.doesNotThrow(() => negotiateHostCapabilities({ ...CUR_AXES, embeddingMode: UNDOCUMENTED })); + assert.doesNotThrow(() => negotiateHostCapabilities({ ...CUR_AXES, embeddingMode: 'future-unknown' })); +}); + +test('a partial/empty cursor descriptor degrades to the safe floor, not the programmatic-cli baseline', () => { + const result = negotiateHostCapabilities({}); + assert.equal(result.effective.embeddingMode, 'declarative', 'omitted embeddingMode degrades closed'); + assert.equal(result.effective.hookBus, 'none'); + assert.notDeepEqual(result.effective, PROFILE_BASELINES['programmatic-cli']); + assert.ok(result.warnings.length > 0); +}); + +// -- AC2: the hardcoded branches are retired --------------------------------- + +test('cursor descriptor declares runtime.hostBehaviors (the folded-in behaviors)', () => { + const hb = CUR_CAP.runtime.hostBehaviors; + assert.ok(hb && typeof hb === 'object'); + assert.equal(hb.reapplyCommand, 'gsd-update --reapply (mention the skill name)'); + assert.equal(hb.frontmatterDialect, 'cursor'); + assert.equal(hb.hooksJsonSurface, true); + assert.equal(hb.skipSharedHooksInstall, true); + assert.equal(hb.reportCommandsDir, true); + assert.ok(Array.isArray(hb.managedHookEvents) && hb.managedHookEvents.length >= 6, + 'managedHookEvents must list at least 6 events (AC4a)'); +}); + +test('no `runtime === "cursor"` string-equality branch remains in the install source (AC2)', () => { + const strip = (src) => src + .replace(/\/\*[\s\S]*?\*\//g, '') + .replace(/\/\/[^\r\n]*/g, '') + .replace(/`[^`]*`/g, ''); + for (const rel of ['bin/install.js', 'src/install-engine.cts', 'src/runtime-artifact-conversion.cts', 'src/runtime-hooks-surface.cts']) { + const src = fs.readFileSync(path.join(__dirname, '..', rel), 'utf8'); + const offenders = strip(src).match(/runtime\s*[!=]==\s*'cursor'/g) || []; + assert.deepEqual(offenders, [], `AC2: no hardcoded runtime==='cursor' branch may remain in ${rel}; found: ${offenders.join(', ')}`); + } +}); diff --git a/tests/fixtures/golden-install-parity/antigravity.json b/tests/fixtures/golden-install-parity/antigravity.json index 4f674f008..0594525e6 100644 --- a/tests/fixtures/golden-install-parity/antigravity.json +++ b/tests/fixtures/golden-install-parity/antigravity.json @@ -315,7 +315,11 @@ "hooks/gsd-config-reload.js": "96546e0e8bb47904", "hooks/gsd-context-monitor.js": "6d81d7326e5b2710", "hooks/gsd-cursor-post-tool.js": "9168e0a09de1972a", + "hooks/gsd-cursor-pre-tool.js": "873998b25e308c29", "hooks/gsd-cursor-session-start.js": "9b2e6f4f0c405375", + "hooks/gsd-cursor-stop.js": "bfaaf60f419e3238", + "hooks/gsd-cursor-subagent-start.js": "06d77fde5c1372b6", + "hooks/gsd-cursor-subagent-stop.js": "4bbf22917da4d389", "hooks/gsd-ensure-canonical-path.js": "64d092d7e4a01211", "hooks/gsd-graphify-update.sh": "e4c6e14fe6ad64ff", "hooks/gsd-phase-boundary.sh": "32739d5fbe0d0a1c", @@ -330,7 +334,7 @@ "hooks/gsd-worktree-path-guard.js": "838498aa91619740", "hooks/lib/git-cmd.js": "268ba15992ca0b23", "hooks/lib/gsd-graphify-rebuild.sh": "66af89601074d2a9", - "hooks/managed-hooks-registry.cjs": "45b2431992d3d7d2", + "hooks/managed-hooks-registry.cjs": "721d696556b7509f", "package.json": "dbf8353f77358bc1", "scripts/changeset/README.md": "86ff89331dfd94b2", "scripts/changeset/cli.cjs": "68f92a344b199271", diff --git a/tests/fixtures/golden-install-parity/augment.json b/tests/fixtures/golden-install-parity/augment.json index 2742f85c7..7e232323d 100644 --- a/tests/fixtures/golden-install-parity/augment.json +++ b/tests/fixtures/golden-install-parity/augment.json @@ -386,7 +386,11 @@ "hooks/gsd-config-reload.js": "96546e0e8bb47904", "hooks/gsd-context-monitor.js": "44ff1bbf292747af", "hooks/gsd-cursor-post-tool.js": "9168e0a09de1972a", + "hooks/gsd-cursor-pre-tool.js": "873998b25e308c29", "hooks/gsd-cursor-session-start.js": "9b2e6f4f0c405375", + "hooks/gsd-cursor-stop.js": "bfaaf60f419e3238", + "hooks/gsd-cursor-subagent-start.js": "06d77fde5c1372b6", + "hooks/gsd-cursor-subagent-stop.js": "4bbf22917da4d389", "hooks/gsd-ensure-canonical-path.js": "d569f5f3578e93e5", "hooks/gsd-graphify-update.sh": "e4c6e14fe6ad64ff", "hooks/gsd-phase-boundary.sh": "32739d5fbe0d0a1c", @@ -401,7 +405,7 @@ "hooks/gsd-worktree-path-guard.js": "65b934c3a1709e89", "hooks/lib/git-cmd.js": "268ba15992ca0b23", "hooks/lib/gsd-graphify-rebuild.sh": "66af89601074d2a9", - "hooks/managed-hooks-registry.cjs": "f46a329fcfefa465", + "hooks/managed-hooks-registry.cjs": "e61da0f7a3037c35", "package.json": "dbf8353f77358bc1", "scripts/changeset/README.md": "86ff89331dfd94b2", "scripts/changeset/cli.cjs": "68f92a344b199271", diff --git a/tests/fixtures/golden-install-parity/claude-local.json b/tests/fixtures/golden-install-parity/claude-local.json index 4f1ce96ee..28a4f3e27 100644 --- a/tests/fixtures/golden-install-parity/claude-local.json +++ b/tests/fixtures/golden-install-parity/claude-local.json @@ -385,7 +385,11 @@ "hooks/gsd-config-reload.js": "96546e0e8bb47904", "hooks/gsd-context-monitor.js": "ecbe9747e4a442e0", "hooks/gsd-cursor-post-tool.js": "8a8a249c0642cc71", + "hooks/gsd-cursor-pre-tool.js": "8cb8e8f895edaec9", "hooks/gsd-cursor-session-start.js": "05a14e903c5edafa", + "hooks/gsd-cursor-stop.js": "d33be8ac96f4081d", + "hooks/gsd-cursor-subagent-start.js": "d773df8caa605de2", + "hooks/gsd-cursor-subagent-stop.js": "8ee488d826bf3c37", "hooks/gsd-ensure-canonical-path.js": "b4b3b88a0e493b16", "hooks/gsd-graphify-update.sh": "e4c6e14fe6ad64ff", "hooks/gsd-phase-boundary.sh": "32739d5fbe0d0a1c", @@ -400,7 +404,7 @@ "hooks/gsd-worktree-path-guard.js": "02be1bb504b22eb5", "hooks/lib/git-cmd.js": "268ba15992ca0b23", "hooks/lib/gsd-graphify-rebuild.sh": "66af89601074d2a9", - "hooks/managed-hooks-registry.cjs": "ea876b1ec185173e", + "hooks/managed-hooks-registry.cjs": "f2e325aa9ba31647", "package.json": "dbf8353f77358bc1", "scripts/changeset/README.md": "86ff89331dfd94b2", "scripts/changeset/cli.cjs": "68f92a344b199271", diff --git a/tests/fixtures/golden-install-parity/claude.json b/tests/fixtures/golden-install-parity/claude.json index 2a35df78e..0b53e630a 100644 --- a/tests/fixtures/golden-install-parity/claude.json +++ b/tests/fixtures/golden-install-parity/claude.json @@ -314,7 +314,11 @@ "hooks/gsd-config-reload.js": "96546e0e8bb47904", "hooks/gsd-context-monitor.js": "ecbe9747e4a442e0", "hooks/gsd-cursor-post-tool.js": "8a8a249c0642cc71", + "hooks/gsd-cursor-pre-tool.js": "8cb8e8f895edaec9", "hooks/gsd-cursor-session-start.js": "05a14e903c5edafa", + "hooks/gsd-cursor-stop.js": "d33be8ac96f4081d", + "hooks/gsd-cursor-subagent-start.js": "d773df8caa605de2", + "hooks/gsd-cursor-subagent-stop.js": "8ee488d826bf3c37", "hooks/gsd-ensure-canonical-path.js": "b4b3b88a0e493b16", "hooks/gsd-graphify-update.sh": "e4c6e14fe6ad64ff", "hooks/gsd-phase-boundary.sh": "32739d5fbe0d0a1c", @@ -329,7 +333,7 @@ "hooks/gsd-worktree-path-guard.js": "02be1bb504b22eb5", "hooks/lib/git-cmd.js": "268ba15992ca0b23", "hooks/lib/gsd-graphify-rebuild.sh": "66af89601074d2a9", - "hooks/managed-hooks-registry.cjs": "ea876b1ec185173e", + "hooks/managed-hooks-registry.cjs": "f2e325aa9ba31647", "package.json": "dbf8353f77358bc1", "scripts/changeset/README.md": "86ff89331dfd94b2", "scripts/changeset/cli.cjs": "68f92a344b199271", diff --git a/tests/fixtures/golden-install-parity/codebuddy.json b/tests/fixtures/golden-install-parity/codebuddy.json index 3fd1f7110..acc74ecd4 100644 --- a/tests/fixtures/golden-install-parity/codebuddy.json +++ b/tests/fixtures/golden-install-parity/codebuddy.json @@ -386,7 +386,11 @@ "hooks/gsd-config-reload.js": "96546e0e8bb47904", "hooks/gsd-context-monitor.js": "f372804867cabe40", "hooks/gsd-cursor-post-tool.js": "9168e0a09de1972a", + "hooks/gsd-cursor-pre-tool.js": "873998b25e308c29", "hooks/gsd-cursor-session-start.js": "9b2e6f4f0c405375", + "hooks/gsd-cursor-stop.js": "bfaaf60f419e3238", + "hooks/gsd-cursor-subagent-start.js": "06d77fde5c1372b6", + "hooks/gsd-cursor-subagent-stop.js": "4bbf22917da4d389", "hooks/gsd-ensure-canonical-path.js": "434887487ae63ec5", "hooks/gsd-graphify-update.sh": "e4c6e14fe6ad64ff", "hooks/gsd-phase-boundary.sh": "32739d5fbe0d0a1c", @@ -401,7 +405,7 @@ "hooks/gsd-worktree-path-guard.js": "548fc57131a04fa7", "hooks/lib/git-cmd.js": "268ba15992ca0b23", "hooks/lib/gsd-graphify-rebuild.sh": "66af89601074d2a9", - "hooks/managed-hooks-registry.cjs": "0368fd4ac7bb3d1d", + "hooks/managed-hooks-registry.cjs": "0e7a61bde8688e11", "package.json": "dbf8353f77358bc1", "scripts/changeset/README.md": "86ff89331dfd94b2", "scripts/changeset/cli.cjs": "68f92a344b199271", diff --git a/tests/fixtures/golden-install-parity/codex.json b/tests/fixtures/golden-install-parity/codex.json index b7f1dd231..3e059529b 100644 --- a/tests/fixtures/golden-install-parity/codex.json +++ b/tests/fixtures/golden-install-parity/codex.json @@ -1,4 +1,75 @@ { + ".agents/skills/gsd-add-tests/SKILL.md": "f3c1594a059de3ee", + ".agents/skills/gsd-ai-integration-phase/SKILL.md": "ba4b5f406db6de96", + ".agents/skills/gsd-audit-fix/SKILL.md": "9cea2764b92aa352", + ".agents/skills/gsd-audit-milestone/SKILL.md": "0528ef8a5834b5df", + ".agents/skills/gsd-audit-uat/SKILL.md": "6c2aa0c2b62c5233", + ".agents/skills/gsd-autonomous/SKILL.md": "d56dc692750f4926", + ".agents/skills/gsd-capture/SKILL.md": "211d601d122b5dd6", + ".agents/skills/gsd-cleanup/SKILL.md": "18cba17463c6ef97", + ".agents/skills/gsd-code-review/SKILL.md": "74749a1aff4224d7", + ".agents/skills/gsd-complete-milestone/SKILL.md": "77bee8741f8cb381", + ".agents/skills/gsd-config/SKILL.md": "5fde92e30619edcb", + ".agents/skills/gsd-debug/SKILL.md": "7e56964e14fd0a10", + ".agents/skills/gsd-discuss-phase/SKILL.md": "837d46c55cc2424e", + ".agents/skills/gsd-docs-update/SKILL.md": "5028dc5dd44bee7d", + ".agents/skills/gsd-eval-review/SKILL.md": "a300e78a27ba74d6", + ".agents/skills/gsd-execute-phase/SKILL.md": "e77bee13068600d3", + ".agents/skills/gsd-explore/SKILL.md": "d660cde0e6f31ebf", + ".agents/skills/gsd-extract-learnings/SKILL.md": "e32388999da384e4", + ".agents/skills/gsd-fast/SKILL.md": "14c407a76a40e115", + ".agents/skills/gsd-forensics/SKILL.md": "6d496a16f5ca74b3", + ".agents/skills/gsd-graphify/SKILL.md": "1bbd96129fbacc5c", + ".agents/skills/gsd-health/SKILL.md": "656c9c63852e3787", + ".agents/skills/gsd-help/SKILL.md": "28eb8d0b487239f8", + ".agents/skills/gsd-import/SKILL.md": "4d15ee09e531342f", + ".agents/skills/gsd-inbox/SKILL.md": "1315033595643485", + ".agents/skills/gsd-ingest-docs/SKILL.md": "5bd2838bf0b6dc1b", + ".agents/skills/gsd-manager/SKILL.md": "cb1cf56f5d3f66d6", + ".agents/skills/gsd-map-codebase/SKILL.md": "cd27dc028718426b", + ".agents/skills/gsd-mempalace-capture/SKILL.md": "1a7fac4d53f607ab", + ".agents/skills/gsd-mempalace-recall/SKILL.md": "7205b02250e89f25", + ".agents/skills/gsd-milestone-summary/SKILL.md": "af84ecb400f23556", + ".agents/skills/gsd-mvp-phase/SKILL.md": "72b65ae927b280fe", + ".agents/skills/gsd-new-milestone/SKILL.md": "587574d2d475bb10", + ".agents/skills/gsd-new-project/SKILL.md": "129ac16e6a1f04ba", + ".agents/skills/gsd-next/SKILL.md": "7855fd70e387087e", + ".agents/skills/gsd-ns-context/SKILL.md": "9b496da79789b3f9", + ".agents/skills/gsd-ns-ideate/SKILL.md": "84ca1cde06110981", + ".agents/skills/gsd-ns-manage/SKILL.md": "909dafa0cd19ba5a", + ".agents/skills/gsd-ns-project/SKILL.md": "936b6998d42520ac", + ".agents/skills/gsd-ns-review/SKILL.md": "022253010db0e072", + ".agents/skills/gsd-ns-workflow/SKILL.md": "cba075bc819b539e", + ".agents/skills/gsd-onboard/SKILL.md": "f42fc2edebeda671", + ".agents/skills/gsd-pause-work/SKILL.md": "b379469eed78a196", + ".agents/skills/gsd-phase/SKILL.md": "25477edc97a90c91", + ".agents/skills/gsd-plan-phase/SKILL.md": "19ead1acb151a868", + ".agents/skills/gsd-plan-review-convergence/SKILL.md": "f654089c11024027", + ".agents/skills/gsd-pr-branch/SKILL.md": "6901da15e321913e", + ".agents/skills/gsd-profile-user/SKILL.md": "6259fabfb6afe7be", + ".agents/skills/gsd-progress/SKILL.md": "85d76286162b9189", + ".agents/skills/gsd-quick/SKILL.md": "b4f4e711ba664aa0", + ".agents/skills/gsd-resume-work/SKILL.md": "04f6c2e5b579e8c4", + ".agents/skills/gsd-review-backlog/SKILL.md": "469696b944c4e7b5", + ".agents/skills/gsd-review/SKILL.md": "a45ab2fdca06793b", + ".agents/skills/gsd-secure-phase/SKILL.md": "e64ad269c1e6e319", + ".agents/skills/gsd-settings/SKILL.md": "fcdda8dd545622ae", + ".agents/skills/gsd-ship/SKILL.md": "9615cc4c8f6de060", + ".agents/skills/gsd-sketch/SKILL.md": "90c219b843db7ec7", + ".agents/skills/gsd-spec-phase/SKILL.md": "56a5cbd606db9cba", + ".agents/skills/gsd-spike/SKILL.md": "77209fef7a04c11a", + ".agents/skills/gsd-stats/SKILL.md": "566024444ef71f44", + ".agents/skills/gsd-surface/SKILL.md": "492cda98187a969b", + ".agents/skills/gsd-thread/SKILL.md": "85326c97c83a02d1", + ".agents/skills/gsd-ui-phase/SKILL.md": "a0c8fbcbe5e3c2b9", + ".agents/skills/gsd-ui-review/SKILL.md": "081a3292a2d357f5", + ".agents/skills/gsd-ultraplan-phase/SKILL.md": "06fb3d76eb785f94", + ".agents/skills/gsd-undo/SKILL.md": "007d3b307e027af9", + ".agents/skills/gsd-update/SKILL.md": "e6e49e117c7c8f35", + ".agents/skills/gsd-validate-phase/SKILL.md": "373059517a94a194", + ".agents/skills/gsd-verify-work/SKILL.md": "4272275698e9a3fe", + ".agents/skills/gsd-workspace/SKILL.md": "06d6400d68318361", + ".agents/skills/gsd-workstreams/SKILL.md": "2f77bb94db1be1a4", ".gsd-profile": "0e716a5fef4e6dc1", ".gsd/defaults.json": "560664b045e645cb", "agents/gsd-advisor-researcher.md": "eea6d1604aaf305c", @@ -69,7 +140,7 @@ "agents/gsd-user-profiler.toml": "b9c244bb8fbf8140", "agents/gsd-verifier.md": "4ac4b860e2504374", "agents/gsd-verifier.toml": "8ed9fb961409e894", - "config.toml": "a34316b9ac61a620", + "config.toml": "fa48d84b92174890", "gsd-core/VERSION": "ef0deccd81a6723c", "gsd-core/bin/check-latest-version.cjs": "e4a224058c8f4d74", "gsd-core/bin/ensure-runtime-build.cjs": "51bc64467ab30f62", @@ -359,76 +430,5 @@ "scripts/gen-capability-registry.cjs": "c52201ff4d1c2cd7", "scripts/gen-loop-host-contract.cjs": "c7f15237234811a0", "scripts/lib/allowlist-ratchet.cjs": "ffaceaac3efc2660", - "scripts/lib/cli-exit.cjs": "612d0c372c75b7e7", - "skills/gsd-add-tests/SKILL.md": "f3c1594a059de3ee", - "skills/gsd-ai-integration-phase/SKILL.md": "ba4b5f406db6de96", - "skills/gsd-audit-fix/SKILL.md": "9cea2764b92aa352", - "skills/gsd-audit-milestone/SKILL.md": "0528ef8a5834b5df", - "skills/gsd-audit-uat/SKILL.md": "6c2aa0c2b62c5233", - "skills/gsd-autonomous/SKILL.md": "d56dc692750f4926", - "skills/gsd-capture/SKILL.md": "211d601d122b5dd6", - "skills/gsd-cleanup/SKILL.md": "18cba17463c6ef97", - "skills/gsd-code-review/SKILL.md": "74749a1aff4224d7", - "skills/gsd-complete-milestone/SKILL.md": "77bee8741f8cb381", - "skills/gsd-config/SKILL.md": "5fde92e30619edcb", - "skills/gsd-debug/SKILL.md": "7e56964e14fd0a10", - "skills/gsd-discuss-phase/SKILL.md": "837d46c55cc2424e", - "skills/gsd-docs-update/SKILL.md": "5028dc5dd44bee7d", - "skills/gsd-eval-review/SKILL.md": "a300e78a27ba74d6", - "skills/gsd-execute-phase/SKILL.md": "e77bee13068600d3", - "skills/gsd-explore/SKILL.md": "d660cde0e6f31ebf", - "skills/gsd-extract-learnings/SKILL.md": "e32388999da384e4", - "skills/gsd-fast/SKILL.md": "14c407a76a40e115", - "skills/gsd-forensics/SKILL.md": "6d496a16f5ca74b3", - "skills/gsd-graphify/SKILL.md": "1bbd96129fbacc5c", - "skills/gsd-health/SKILL.md": "656c9c63852e3787", - "skills/gsd-help/SKILL.md": "28eb8d0b487239f8", - "skills/gsd-import/SKILL.md": "4d15ee09e531342f", - "skills/gsd-inbox/SKILL.md": "1315033595643485", - "skills/gsd-ingest-docs/SKILL.md": "5bd2838bf0b6dc1b", - "skills/gsd-manager/SKILL.md": "cb1cf56f5d3f66d6", - "skills/gsd-map-codebase/SKILL.md": "cd27dc028718426b", - "skills/gsd-mempalace-capture/SKILL.md": "1a7fac4d53f607ab", - "skills/gsd-mempalace-recall/SKILL.md": "7205b02250e89f25", - "skills/gsd-milestone-summary/SKILL.md": "af84ecb400f23556", - "skills/gsd-mvp-phase/SKILL.md": "72b65ae927b280fe", - "skills/gsd-new-milestone/SKILL.md": "587574d2d475bb10", - "skills/gsd-new-project/SKILL.md": "129ac16e6a1f04ba", - "skills/gsd-next/SKILL.md": "7855fd70e387087e", - "skills/gsd-ns-context/SKILL.md": "9b496da79789b3f9", - "skills/gsd-ns-ideate/SKILL.md": "84ca1cde06110981", - "skills/gsd-ns-manage/SKILL.md": "909dafa0cd19ba5a", - "skills/gsd-ns-project/SKILL.md": "936b6998d42520ac", - "skills/gsd-ns-review/SKILL.md": "022253010db0e072", - "skills/gsd-ns-workflow/SKILL.md": "cba075bc819b539e", - "skills/gsd-onboard/SKILL.md": "f42fc2edebeda671", - "skills/gsd-pause-work/SKILL.md": "b379469eed78a196", - "skills/gsd-phase/SKILL.md": "25477edc97a90c91", - "skills/gsd-plan-phase/SKILL.md": "19ead1acb151a868", - "skills/gsd-plan-review-convergence/SKILL.md": "f654089c11024027", - "skills/gsd-pr-branch/SKILL.md": "6901da15e321913e", - "skills/gsd-profile-user/SKILL.md": "6259fabfb6afe7be", - "skills/gsd-progress/SKILL.md": "85d76286162b9189", - "skills/gsd-quick/SKILL.md": "b4f4e711ba664aa0", - "skills/gsd-resume-work/SKILL.md": "04f6c2e5b579e8c4", - "skills/gsd-review-backlog/SKILL.md": "469696b944c4e7b5", - "skills/gsd-review/SKILL.md": "a45ab2fdca06793b", - "skills/gsd-secure-phase/SKILL.md": "e64ad269c1e6e319", - "skills/gsd-settings/SKILL.md": "fcdda8dd545622ae", - "skills/gsd-ship/SKILL.md": "9615cc4c8f6de060", - "skills/gsd-sketch/SKILL.md": "90c219b843db7ec7", - "skills/gsd-spec-phase/SKILL.md": "56a5cbd606db9cba", - "skills/gsd-spike/SKILL.md": "77209fef7a04c11a", - "skills/gsd-stats/SKILL.md": "566024444ef71f44", - "skills/gsd-surface/SKILL.md": "492cda98187a969b", - "skills/gsd-thread/SKILL.md": "85326c97c83a02d1", - "skills/gsd-ui-phase/SKILL.md": "a0c8fbcbe5e3c2b9", - "skills/gsd-ui-review/SKILL.md": "081a3292a2d357f5", - "skills/gsd-ultraplan-phase/SKILL.md": "06fb3d76eb785f94", - "skills/gsd-undo/SKILL.md": "007d3b307e027af9", - "skills/gsd-update/SKILL.md": "e6e49e117c7c8f35", - "skills/gsd-validate-phase/SKILL.md": "373059517a94a194", - "skills/gsd-verify-work/SKILL.md": "4272275698e9a3fe", - "skills/gsd-workspace/SKILL.md": "06d6400d68318361", - "skills/gsd-workstreams/SKILL.md": "2f77bb94db1be1a4" + "scripts/lib/cli-exit.cjs": "612d0c372c75b7e7" } diff --git a/tests/fixtures/golden-install-parity/cursor.json b/tests/fixtures/golden-install-parity/cursor.json index 992990115..9ac2844fc 100644 --- a/tests/fixtures/golden-install-parity/cursor.json +++ b/tests/fixtures/golden-install-parity/cursor.json @@ -382,7 +382,11 @@ "gsd-core/workflows/verify-phase.md": "e0957e153788a222", "gsd-core/workflows/verify-work.md": "e7e7e900c4874490", "hooks/gsd-cursor-post-tool.js": "019d503aee8b4a3f", + "hooks/gsd-cursor-pre-tool.js": "fe274720781fcbb5", "hooks/gsd-cursor-session-start.js": "c6e04ed597ea7020", + "hooks/gsd-cursor-stop.js": "902a005c49e7660b", + "hooks/gsd-cursor-subagent-start.js": "693d38d298d2252c", + "hooks/gsd-cursor-subagent-stop.js": "8da03aad6bb05d3f", "scripts/changeset/README.md": "86ff89331dfd94b2", "scripts/changeset/cli.cjs": "68f92a344b199271", "scripts/changeset/github-release-notes.cjs": "795677f0c009b132", diff --git a/tests/fixtures/golden-install-parity/hermes.json b/tests/fixtures/golden-install-parity/hermes.json index 4dc7cd405..80e728340 100644 --- a/tests/fixtures/golden-install-parity/hermes.json +++ b/tests/fixtures/golden-install-parity/hermes.json @@ -315,7 +315,11 @@ "hooks/gsd-config-reload.js": "880b696458e85e9b", "hooks/gsd-context-monitor.js": "41d28e0db7b20968", "hooks/gsd-cursor-post-tool.js": "8a8a249c0642cc71", + "hooks/gsd-cursor-pre-tool.js": "8cb8e8f895edaec9", "hooks/gsd-cursor-session-start.js": "05a14e903c5edafa", + "hooks/gsd-cursor-stop.js": "d33be8ac96f4081d", + "hooks/gsd-cursor-subagent-start.js": "d773df8caa605de2", + "hooks/gsd-cursor-subagent-stop.js": "8ee488d826bf3c37", "hooks/gsd-ensure-canonical-path.js": "7d116d7d65c50b4b", "hooks/gsd-graphify-update.sh": "e4c6e14fe6ad64ff", "hooks/gsd-phase-boundary.sh": "32739d5fbe0d0a1c", @@ -330,7 +334,7 @@ "hooks/gsd-worktree-path-guard.js": "108ab88ccbafc5d8", "hooks/lib/git-cmd.js": "268ba15992ca0b23", "hooks/lib/gsd-graphify-rebuild.sh": "66af89601074d2a9", - "hooks/managed-hooks-registry.cjs": "2218a41c279720c2", + "hooks/managed-hooks-registry.cjs": "a494d1a70ed87690", "package.json": "dbf8353f77358bc1", "scripts/changeset/README.md": "86ff89331dfd94b2", "scripts/changeset/cli.cjs": "68f92a344b199271", diff --git a/tests/fixtures/golden-install-parity/opencode.json b/tests/fixtures/golden-install-parity/opencode.json index 543721560..759b24f55 100644 --- a/tests/fixtures/golden-install-parity/opencode.json +++ b/tests/fixtures/golden-install-parity/opencode.json @@ -386,7 +386,11 @@ "hooks/gsd-config-reload.js": "96546e0e8bb47904", "hooks/gsd-context-monitor.js": "7a9787868a39b76d", "hooks/gsd-cursor-post-tool.js": "9168e0a09de1972a", + "hooks/gsd-cursor-pre-tool.js": "873998b25e308c29", "hooks/gsd-cursor-session-start.js": "9b2e6f4f0c405375", + "hooks/gsd-cursor-stop.js": "bfaaf60f419e3238", + "hooks/gsd-cursor-subagent-start.js": "06d77fde5c1372b6", + "hooks/gsd-cursor-subagent-stop.js": "4bbf22917da4d389", "hooks/gsd-ensure-canonical-path.js": "2801ae3fef9579bf", "hooks/gsd-graphify-update.sh": "e4c6e14fe6ad64ff", "hooks/gsd-phase-boundary.sh": "32739d5fbe0d0a1c", @@ -401,7 +405,7 @@ "hooks/gsd-worktree-path-guard.js": "726fb9afefda5d42", "hooks/lib/git-cmd.js": "268ba15992ca0b23", "hooks/lib/gsd-graphify-rebuild.sh": "66af89601074d2a9", - "hooks/managed-hooks-registry.cjs": "763730ef31e5fd1c", + "hooks/managed-hooks-registry.cjs": "9163e096b74ec4b3", "opencode.json": "2c12c446a88f2f36", "package.json": "dbf8353f77358bc1", "plugins/gsd-core.js": "931ca839dc9eb7f1", diff --git a/tests/fixtures/golden-install-parity/qwen.json b/tests/fixtures/golden-install-parity/qwen.json index 0db2d73fb..b1536043b 100644 --- a/tests/fixtures/golden-install-parity/qwen.json +++ b/tests/fixtures/golden-install-parity/qwen.json @@ -315,7 +315,11 @@ "hooks/gsd-config-reload.js": "4f52b8a0120bb1b8", "hooks/gsd-context-monitor.js": "437a33e6e3058640", "hooks/gsd-cursor-post-tool.js": "8a8a249c0642cc71", + "hooks/gsd-cursor-pre-tool.js": "8cb8e8f895edaec9", "hooks/gsd-cursor-session-start.js": "05a14e903c5edafa", + "hooks/gsd-cursor-stop.js": "d33be8ac96f4081d", + "hooks/gsd-cursor-subagent-start.js": "d773df8caa605de2", + "hooks/gsd-cursor-subagent-stop.js": "8ee488d826bf3c37", "hooks/gsd-ensure-canonical-path.js": "2df5e295b36c3334", "hooks/gsd-graphify-update.sh": "e4c6e14fe6ad64ff", "hooks/gsd-phase-boundary.sh": "32739d5fbe0d0a1c", @@ -330,7 +334,7 @@ "hooks/gsd-worktree-path-guard.js": "8389e4c9175b2613", "hooks/lib/git-cmd.js": "268ba15992ca0b23", "hooks/lib/gsd-graphify-rebuild.sh": "66af89601074d2a9", - "hooks/managed-hooks-registry.cjs": "a57697c1ae4ac163", + "hooks/managed-hooks-registry.cjs": "a5a93d50c4ea7a0c", "package.json": "dbf8353f77358bc1", "scripts/changeset/README.md": "86ff89331dfd94b2", "scripts/changeset/cli.cjs": "68f92a344b199271", diff --git a/tests/helpers/install-shared.cjs b/tests/helpers/install-shared.cjs index 825be4ffb..060571854 100644 --- a/tests/helpers/install-shared.cjs +++ b/tests/helpers/install-shared.cjs @@ -12,6 +12,10 @@ const os = require('node:os'); const { spawnSync } = require('node:child_process'); const assert = require('node:assert/strict'); +const { + resolveRuntimeArtifactLayout, +} = require('../../gsd-core/bin/lib/runtime-artifact-layout.cjs'); + const INSTALL_SCRIPT = path.join(__dirname, '..', '..', 'bin', 'install.js'); const MANIFEST_NAME = 'gsd-file-manifest.json'; @@ -177,9 +181,27 @@ function manifestAgentCount(manifest) { return Object.keys(manifest.files).filter((k) => k.startsWith('agents/')).length; } -function collectSkillBasenamesOnDisk(configDir) { +/** + * Collect gsd-* skill/command basenames actually present on disk under configDir. + * + * @param {string} configDir + * @param {string} [runtime] - when provided, the skills-kind destination is + * resolved via resolveRuntimeArtifactLayout so a skills-kind `home` override + * (Codex only, ADR-1239 upgrade 3 / #2088: skills -> $HOME/.agents/skills + * instead of configDir/skills) is honored. Omitted callers keep the prior + * configDir/skills default. + * @param {string} [scope='global'] + */ +function collectSkillBasenamesOnDisk(configDir, runtime, scope = 'global') { const out = new Set(); - const skillsDir = path.join(configDir, 'skills'); + let skillsDir = path.join(configDir, 'skills'); + if (runtime) { + try { + const layout = resolveRuntimeArtifactLayout(runtime, configDir, scope); + const skillsKind = layout.kinds.find((k) => k.kind === 'skills'); + if (skillsKind) skillsDir = path.join(skillsKind.home || configDir, skillsKind.destSubpath); + } catch { /* fall back to configDir/skills */ } + } if (fs.existsSync(skillsDir)) { for (const entry of fs.readdirSync(skillsDir, { withFileTypes: true })) { if (entry.isDirectory() && entry.name.startsWith('gsd-')) { diff --git a/tests/install-minimal-hooks.test.cjs b/tests/install-minimal-hooks.test.cjs index a94ec8d6c..72297e7b7 100644 --- a/tests/install-minimal-hooks.test.cjs +++ b/tests/install-minimal-hooks.test.cjs @@ -63,6 +63,31 @@ const { collectSkillBasenamesOnDisk, } = require('./helpers/install-shared.cjs'); +/** + * collectSkillBasenamesOnDisk(configDir, runtime, scope) re-resolves the + * runtime's skills-kind layout via os.homedir(). runMinimalInstall() already + * sandboxes HOME/USERPROFILE to `root` for the spawned install subprocess, + * but that sandboxing does not persist into this (parent) process — without + * re-sandboxing here, Codex's skills-kind `home: ".agents"` override + * (ADR-1239 upgrade 3, #2088) would resolve against the developer's REAL + * $HOME/.agents/skills instead of the sandboxed install root. Sandbox + * HOME/USERPROFILE to `root` for the synchronous duration of the on-disk scan. + */ +function collectSkillBasenamesOnDiskSandboxed(configDir, runtime, scope, root) { + const savedHome = process.env.HOME; + const savedUserProfile = process.env.USERPROFILE; + process.env.HOME = root; + process.env.USERPROFILE = root; + try { + return collectSkillBasenamesOnDisk(configDir, runtime, scope); + } 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; + } +} + // ─── Section 9: install-profiles — MINIMAL_SKILL_ALLOWLIST ─────────────────── describe('install-profiles: MINIMAL_SKILL_ALLOWLIST', () => { @@ -391,7 +416,7 @@ describe('install: on-disk skill files match manifest for --minimal', () => { }); try { assert.ok(manifest); - const onDisk = collectSkillBasenamesOnDisk(configDir); + const onDisk = collectSkillBasenamesOnDiskSandboxed(configDir, runtime, scope, root); const inManifest = manifestSkillSet(manifest); assert.deepStrictEqual([...onDisk].sort(), [...inManifest].sort()); // Not the shared listAgentFiles() helper: asserts on the INSTALLED @@ -542,10 +567,15 @@ describe('install: Codex full → minimal downgrade cleans stale agent state', ( ].join('\n'); fs.writeFileSync(path.join(targetDir, 'config.toml'), codexConfig); + // Sandbox HOME/USERPROFILE to targetDir: Codex's skills-kind `home: ".agents"` + // override (ADR-1239 upgrade 3, #2088) resolves via os.homedir(), so an + // unsandboxed spawn here would write gsd-* skill dirs into the developer's + // real $HOME/.agents/skills. This test only asserts on agents/ and + // config.toml (both under targetDir), so the sandbox has no effect on intent. const result = spawnSync( process.execPath, [INSTALL_SCRIPT, '--codex', '--global', '--config-dir', targetDir, '--minimal'], - { encoding: 'utf8', env: installerEnv() }, + { encoding: 'utf8', env: installerEnv({ HOME: targetDir, USERPROFILE: targetDir }) }, ); assert.ok(result.stdout || result.stderr); diff --git a/tests/install-nested-layout.test.cjs b/tests/install-nested-layout.test.cjs index de6ff5acf..1790692d0 100644 --- a/tests/install-nested-layout.test.cjs +++ b/tests/install-nested-layout.test.cjs @@ -18,6 +18,10 @@ const { installRuntimeArtifacts, } = require('../gsd-core/bin/lib/install-engine.cjs'); +const { + resolveRuntimeArtifactLayout, +} = require('../gsd-core/bin/lib/runtime-artifact-layout.cjs'); + const { cleanup } = require('./helpers.cjs'); const { @@ -58,16 +62,55 @@ const FLAT = [ // Helpers // --------------------------------------------------------------------------- +/** + * Codex resolves its skills-kind destination via os.homedir() + a 'skills'-kind + * 'home: ".agents"' layout override (ADR-1239 EoS upgrade 3, #2088), NOT + * tmpDir/skills. Sandbox HOME/USERPROFILE to tmpDir for the duration of the + * synchronous callback so codex's resolved skills dir becomes + * tmpDir/.agents/skills instead of the developer's real ~/.agents/skills. + */ +function withSandboxedHome(tmpDir, fn) { + const savedHome = process.env.HOME; + const savedUserProfile = process.env.USERPROFILE; + process.env.HOME = tmpDir; + process.env.USERPROFILE = tmpDir; + try { + return fn(); + } 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; + } +} + /** * Create a fresh temp dir, run installRuntimeArtifacts into it, and return - * the tmpDir path. Caller must cleanup in finally. + * the tmpDir path. Caller must cleanup in finally. HOME is sandboxed to + * tmpDir for the install call so codex never writes to the real ~/.agents/skills. */ function runInstall(runtime, scope, resolved) { const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), `gsd-nest-test-${runtime}-`)); - installRuntimeArtifacts(runtime, tmpDir, scope, resolved); + withSandboxedHome(tmpDir, () => installRuntimeArtifacts(runtime, tmpDir, scope, resolved)); return tmpDir; } +/** + * Resolve the skills-kind destination directory for a runtime, honoring the + * skills-kind 'home' override (codex only — resolves under tmpDir/.agents + * once HOME is sandboxed). All other runtimes have no 'home' override, so + * this falls back to tmpDir/, matching the static skillsSub + * tables above. + */ +function resolveSkillsDir(runtime, tmpDir, scope) { + return withSandboxedHome(tmpDir, () => { + const layout = resolveRuntimeArtifactLayout(runtime, tmpDir, scope); + const skillsKind = layout.kinds.find((k) => k.kind === 'skills'); + assert.ok(skillsKind, `${runtime} must have skills kind`); + return path.join(skillsKind.home || tmpDir, skillsKind.destSubpath); + }); +} + // Resolve the full profile once (shared by all installs) const MANIFEST = loadSkillsManifest(COMMANDS_GSD); const RESOLVED_FULL = resolveProfile({ modes: ['full'], manifest: MANIFEST }); @@ -267,7 +310,7 @@ describe('claude: total top-level gsd- entries >= 60 (flat layout, #924)', () => // FLAT runtimes: concrete skills stay top-level, no nesting // --------------------------------------------------------------------------- -for (const { runtime, scope, skillsSub } of FLAT) { +for (const { runtime, scope } of FLAT) { describe(`${runtime} (flat layout)`, () => { let tmpDir; @@ -282,7 +325,7 @@ for (const { runtime, scope, skillsSub } of FLAT) { }); test(`${runtime}: stays flat — concrete skills remain top-level, no nesting`, () => { - const skillsDir = path.join(tmpDir, skillsSub); + const skillsDir = resolveSkillsDir(runtime, tmpDir, scope); assert.ok(fs.existsSync(skillsDir), `skillsDir must exist: ${skillsDir}`); const topLevel = fs.readdirSync(skillsDir); diff --git a/tests/install-runtime-artifacts.test.cjs b/tests/install-runtime-artifacts.test.cjs index 6e9e86076..96a96afc4 100644 --- a/tests/install-runtime-artifacts.test.cjs +++ b/tests/install-runtime-artifacts.test.cjs @@ -187,11 +187,31 @@ function readAllSkillMd(dir) { return out.join('\n'); } +// Codex resolves its skills-kind destination via os.homedir() + a 'skills'-kind +// 'home: ".agents"' layout override (ADR-1239 EoS upgrade 3, #2088), NOT +// configDir/skills. Without sandboxing HOME, an in-process codex install would +// write to (and an uninstall would mutate) the developer's REAL ~/.agents/skills. +// Sandbox HOME/USERPROFILE to configDir before resolving the layout or invoking +// install/uninstall so codex's resolved skills dir is configDir/.agents/skills. +function sandboxHome(t, dir) { + const savedHome = process.env.HOME; + const savedUserProfile = process.env.USERPROFILE; + process.env.HOME = dir; + process.env.USERPROFILE = dir; + t.after(() => { + if (savedHome === undefined) delete process.env.HOME; + else process.env.HOME = savedHome; + if (savedUserProfile === undefined) delete process.env.USERPROFILE; + else process.env.USERPROFILE = savedUserProfile; + }); +} + describe('installRuntimeArtifacts — skills runtimes write gsd-prefixed skill dirs', () => { for (const runtime of SKILLS_RUNTIMES_LAYOUT) { test(`${runtime}: gsd-prefixed skill dirs in skills/`, (t) => { const configDir = createTempDir(`gsd-ial-${runtime}-`); t.after(() => cleanup(configDir)); + sandboxHome(t, configDir); assert.strictEqual(typeof installRuntimeArtifacts, 'function'); installRuntimeArtifacts(runtime, configDir, 'global', RESOLVED_CORE); @@ -200,7 +220,7 @@ describe('installRuntimeArtifacts — skills runtimes write gsd-prefixed skill d const skillsKind = layout.kinds.find(k => k.kind === 'skills'); assert.ok(skillsKind, `${runtime} must have skills kind`); - const destDir = path.join(configDir, skillsKind.destSubpath); + const destDir = path.join(skillsKind.home || configDir, skillsKind.destSubpath); assert.ok(fs.existsSync(destDir)); assert.ok( fs.existsSync(path.join(destDir, `${skillsKind.prefix}help`, 'SKILL.md')), @@ -481,6 +501,7 @@ describe('uninstallRuntimeArtifacts — removes gsd-owned entries, preserves for test(`${runtime}: gsd entries removed, foreign preserved`, (t) => { const configDir = createTempDir(`gsd-ual-${runtime}-`); t.after(() => cleanup(configDir)); + sandboxHome(t, configDir); const { uninstallRuntimeArtifacts } = require('../bin/install.js'); assert.strictEqual(typeof uninstallRuntimeArtifacts, 'function'); @@ -519,7 +540,7 @@ describe('uninstallRuntimeArtifacts — removes gsd-owned entries, preserves for } for (const kind of layout.kinds) { - const destDir = path.join(configDir, kind.destSubpath); + const destDir = path.join(kind.home || configDir, kind.destSubpath); fs.mkdirSync(destDir, { recursive: true }); if (kind.kind === 'skills') { writeSkillEntry(destDir, kind.prefix, 'help'); @@ -545,7 +566,7 @@ describe('uninstallRuntimeArtifacts — removes gsd-owned entries, preserves for uninstallRuntimeArtifacts(runtime, configDir, 'global'); for (const kind of layout.kinds) { - const destDir = path.join(configDir, kind.destSubpath); + const destDir = path.join(kind.home || configDir, kind.destSubpath); if (kind.kind === 'skills') { assert.ok(!fs.existsSync(path.join(destDir, `${kind.prefix}help`))); assert.ok(!fs.existsSync(path.join(destDir, `${kind.prefix}phase`))); diff --git a/tests/install.test.cjs b/tests/install.test.cjs index c9560061b..acf762dc5 100644 --- a/tests/install.test.cjs +++ b/tests/install.test.cjs @@ -9957,7 +9957,9 @@ function readWorkflow() { describe('install.js --skills-root', () => { const CASES = [ { runtime: 'claude', expected: path.join(os.homedir(), '.claude', 'skills') }, - { runtime: 'codex', expected: path.join(os.homedir(), '.codex', 'skills') }, + // #2088 (ADR-1239 upgrade 3): Codex skills resolve to the canonical + // $HOME/.agents/skills root (skills-kind home override), not $CODEX_HOME/skills. + { runtime: 'codex', expected: path.join(os.homedir(), '.agents', 'skills') }, { runtime: 'copilot', expected: path.join(os.homedir(), '.copilot', 'skills') }, { runtime: 'cursor', expected: path.join(os.homedir(), '.cursor', 'skills') }, { runtime: 'trae', expected: path.join(os.homedir(), '.trae', 'skills') }, @@ -10182,10 +10184,15 @@ const INSTALL = path.join(__dirname, '..', 'bin', 'install.js'); */ function installAndRead(runtime) { const dir = fs.mkdtempSync(path.join(os.tmpdir(), `gsd-inst-${runtime}-`)); + // Sandbox HOME/USERPROFILE to `dir`: Codex's skills-kind `home: ".agents"` + // override (ADR-1239 upgrade 3, #2088) resolves via os.homedir(), so an + // unsandboxed real install here would write a full (non-minimal) gsd-* skill + // set into the developer/CI machine's real $HOME/.agents/skills. Harmless + // no-op for cursor/claude, which have no skills-kind home override. const res = spawnSync( process.execPath, [INSTALL, `--${runtime}`, '--global', '--config-dir', dir], - { encoding: 'utf8', timeout: 120000 }, + { encoding: 'utf8', timeout: 120000, env: { ...process.env, HOME: dir, USERPROFILE: dir } }, ); assert.strictEqual(res.status, 0, `install --${runtime} failed: ${res.stderr || res.stdout}`); const wf = path.join(dir, 'gsd-core', 'workflows', 'execute-phase.md'); diff --git a/tests/installer-migration-install.integration.test.cjs b/tests/installer-migration-install.integration.test.cjs index 220cd63b1..b03c655f1 100644 --- a/tests/installer-migration-install.integration.test.cjs +++ b/tests/installer-migration-install.integration.test.cjs @@ -81,6 +81,19 @@ function withEnv(key, value, fn) { } } +// #2088: Codex CLI skills install to `$HOME/.agents/skills` (resolved via +// os.homedir()), not `$CODEX_HOME/skills`. In-process codex installs must +// sandbox HOME (and USERPROFILE, for Windows os.homedir() resolution) to the +// test's codexHome dir, or skills get materialized into the real developer +// home directory. withEnv saves/restores a single key, so nesting is safe. +function withCodexEnv(codexHome, fn) { + return withEnv('CODEX_HOME', codexHome, () => + withEnv('HOME', codexHome, () => + withEnv('USERPROFILE', codexHome, fn) + ) + ); +} + function captureConsole(fn) { const originalLog = console.log; const originalWarn = console.warn; @@ -216,11 +229,20 @@ function assertFreshInstallContract(runtime, targetDir) { } if (contract.surface === 'flat-skills') { - // Pre-#3562: codex was special-cased to expect zero gsd-* skill dirs - // (assumption: Codex auto-discovers from workflows). That assumption - // does not hold for Codex CLI 0.130.0 — fresh installs now materialize - // the same flat-skills surface as the other runtimes. - assertHasGsdDirectory(targetDir, 'skills'); + if (runtime === 'codex') { + // #2088: Codex CLI skills install to the canonical `$HOME/.agents/skills` + // root (resolved via os.homedir()), NOT `/skills`. Here + // runInstallerCli sandboxes HOME to /home, so assert + // the skill dir under that sandboxed home instead of under targetDir. + const codexSandboxHome = path.join(path.dirname(targetDir), 'home'); + assertHasGsdDirectory(path.join(codexSandboxHome, '.agents'), 'skills'); + } else { + // Pre-#3562: codex was special-cased to expect zero gsd-* skill dirs + // (assumption: Codex auto-discovers from workflows). That assumption + // does not hold for Codex CLI 0.130.0 — fresh installs now materialize + // the same flat-skills surface as the other runtimes. + assertHasGsdDirectory(targetDir, 'skills'); + } } else if (contract.surface === 'hermes-skills') { // Hermes layout uses prefix: '' — skill dirs have bare stem names (no gsd- prefix). // Assert that the category dir contains at least one skill dir with SKILL.md. @@ -335,7 +357,7 @@ describe('installer migration install integration', { concurrency: false }, () = }); const { output } = captureConsole(() => - withEnv('CODEX_HOME', codexHome, () => install(true, 'codex')) + withCodexEnv(codexHome, () => install(true, 'codex')) ); const plainOutput = stripAnsi(output); @@ -353,13 +375,17 @@ describe('installer migration install integration', { concurrency: false }, () = assert.throws( () => captureConsole(() => - withEnv('CODEX_HOME', codexHome, () => install(true, 'codex')) + withCodexEnv(codexHome, () => install(true, 'codex')) ), /installer migration blocked/ ); assert.equal(fs.readFileSync(path.join(codexHome, 'hooks/gsd-retired-hook.txt'), 'utf8'), 'old gsd hook\n'); assert.equal(fs.existsSync(path.join(codexHome, 'skills')), false); + // #2088: with HOME sandboxed to codexHome via withCodexEnv, Codex's + // canonical skill root ($HOME/.agents/skills) resolves under codexHome + // too — assert nothing was materialized there when the install is blocked. + assert.equal(fs.existsSync(path.join(codexHome, '.agents', 'skills')), false); assert.equal(fs.existsSync(path.join(codexHome, 'gsd-core', 'VERSION')), false); }); @@ -431,7 +457,7 @@ describe('installer migration install integration', { concurrency: false }, () = assert.throws( () => captureConsole(() => withEnv('CLAUDE_CONFIG_DIR', claudeHome, () => - withEnv('CODEX_HOME', codexHome, () => + withCodexEnv(codexHome, () => withWriteFailure(path.join(codexHome, 'gsd-core', 'VERSION'), () => installModule.installAllRuntimes(['claude', 'codex'], true, false) ) diff --git a/tests/installer-migrations.test.cjs b/tests/installer-migrations.test.cjs index e64822c9c..3c01bc2f9 100644 --- a/tests/installer-migrations.test.cjs +++ b/tests/installer-migrations.test.cjs @@ -1618,12 +1618,24 @@ const BUILD_HOOKS_SCRIPT = path.join(__dirname, '..', 'scripts', 'build-hooks.js function withCodexHome(codexHome, fn) { const previousCodexHome = process.env.CODEX_HOME; + // #2088 (ADR-1239 upgrade 3): Codex skills now install to $HOME/.agents/skills + // (os.homedir()-relative, independent of CODEX_HOME). Sandbox HOME (and + // USERPROFILE) to codexHome so in-process installs never write to the + // developer/CI machine's real home directory. + const previousHome = process.env.HOME; + const previousUserProfile = process.env.USERPROFILE; process.env.CODEX_HOME = codexHome; + process.env.HOME = codexHome; + process.env.USERPROFILE = codexHome; try { return fn(); } finally { if (previousCodexHome == null) delete process.env.CODEX_HOME; else process.env.CODEX_HOME = previousCodexHome; + if (previousHome == null) delete process.env.HOME; + else process.env.HOME = previousHome; + if (previousUserProfile == null) delete process.env.USERPROFILE; + else process.env.USERPROFILE = previousUserProfile; } } diff --git a/tests/phase-id.test.cjs b/tests/phase-id.test.cjs index 94746235b..286cf7a0c 100644 --- a/tests/phase-id.test.cjs +++ b/tests/phase-id.test.cjs @@ -22,6 +22,7 @@ const { test, describe } = require('node:test'); const assert = require('node:assert/strict'); const phaseId = require('../gsd-core/bin/lib/phase-id.cjs'); +const fc = require('fast-check'); // ─── escapeRegex ───────────────────────────────────────────────────────────── @@ -450,3 +451,169 @@ describe('getPhaseDirFromPhaseId', () => { assert.ok(!result.endsWith('-')); }); }); + +// ─── parsePhaseFromProse (#2121, anchored — fixes #2111) ───────────────────── + +describe('parsePhaseFromProse', () => { + test('null / empty input yields null phase and name', () => { + assert.deepEqual(phaseId.parsePhaseFromProse(null), { phase: null, name: null }); + assert.deepEqual(phaseId.parsePhaseFromProse(''), { phase: null, name: null }); + }); + + test('#2111: a milestone-completion string carries no phase', () => { + assert.equal(phaseId.parsePhaseFromProse('Milestone v0.5 complete').phase, null); + assert.equal(phaseId.parsePhaseFromProse('Milestone v1.0 complete').phase, null); + assert.equal(phaseId.parsePhaseFromProse('Milestone v2.10 complete').phase, null); + }); + + test('#2111: a bare version token or stray numeral is not a phase', () => { + assert.equal(phaseId.parsePhaseFromProse('v0.5').phase, null); + assert.equal(phaseId.parsePhaseFromProse('v1.0').phase, null); + assert.equal(phaseId.parsePhaseFromProse('Fixed 12 bugs in v2.3').phase, null); + }); + + test('a genuine phase value (starting with the token) is parsed', () => { + assert.deepEqual(phaseId.parsePhaseFromProse('3 of 4 (Delta)'), { phase: '3', name: 'Delta' }); + assert.deepEqual(phaseId.parsePhaseFromProse('3A — Delta'), { phase: '3A', name: 'Delta' }); + assert.equal(phaseId.parsePhaseFromProse('12.1: Setup').phase, '12.1'); + assert.equal(phaseId.parsePhaseFromProse('29 of 30').phase, '29'); + assert.equal(phaseId.parsePhaseFromProse('029').phase, '029'); + }); + + test('a leading project-code prefix is tolerated but not captured (bare token)', () => { + assert.equal(phaseId.parsePhaseFromProse('MEM-01 — Foo').phase, '01'); + assert.equal(phaseId.parsePhaseFromProse('AB-29 of 30').phase, '29'); + }); + + test('an optional leading "Phase" label is tolerated', () => { + assert.equal(phaseId.parsePhaseFromProse('Phase 3A — Delta').phase, '3A'); + }); + + test('a status-word parenthetical is filtered from the name (preserved behavior)', () => { + // parenName wins over the em-dash tail; "executing" is a status word → name null. + assert.deepEqual(phaseId.parsePhaseFromProse('3A — Delta (executing)'), { phase: '3A', name: null }); + assert.equal(phaseId.parsePhaseFromProse('3 (complete)').name, null); + }); + + test('#2124 review: name quantifiers are length-bounded (ReDoS guard)', () => { + // A parenthetical within the bound extracts; one longer than the bound is + // NOT matched — the cap is what prevents O(n^2) backtracking on a crafted + // untrusted value. Removing the bound would extract the long name → fail. + assert.equal(phaseId.parsePhaseFromProse('3 (Delta)').name, 'Delta'); + assert.equal(phaseId.parsePhaseFromProse(`3 (${'x'.repeat(201)})`).name, null); + // A long unterminated "(" run yields no name and still parses the phase. + assert.deepEqual(phaseId.parsePhaseFromProse(`3 ${'('.repeat(5000)}`), { phase: '3', name: null }); + }); + + test('#2124 review: non-string input is coerced, never throws', () => { + assert.doesNotThrow(() => phaseId.parsePhaseFromProse(3)); + assert.equal(phaseId.parsePhaseFromProse(3).phase, '3'); + assert.deepEqual(phaseId.parsePhaseFromProse(true), { phase: null, name: null }); + }); +}); + +// ─── stripConfiguredProjectCodePrefix (#2121 / #2104, config-aware) ─────────── + +describe('stripConfiguredProjectCodePrefix', () => { + test('#2104: a foreign prefix is preserved (not collapsed to a bare phase)', () => { + assert.equal(phaseId.stripConfiguredProjectCodePrefix('MEM-01', 'LKML'), 'MEM-01'); + }); + + test('the configured prefix is stripped (case-insensitive)', () => { + assert.equal(phaseId.stripConfiguredProjectCodePrefix('CK-01', 'CK'), '01'); + assert.equal(phaseId.stripConfiguredProjectCodePrefix('LKML-29', 'lkml'), '29'); + assert.equal(phaseId.stripConfiguredProjectCodePrefix('AB-29', 'AB'), '29'); + }); + + test('a value with no prefix is returned unchanged', () => { + assert.equal(phaseId.stripConfiguredProjectCodePrefix('01', 'CK'), '01'); + assert.equal(phaseId.stripConfiguredProjectCodePrefix('029', 'CK'), '029'); + }); + + test('an absent/empty projectCode preserves the value verbatim', () => { + assert.equal(phaseId.stripConfiguredProjectCodePrefix('MEM-01', ''), 'MEM-01'); + assert.equal(phaseId.stripConfiguredProjectCodePrefix('MEM-01', null), 'MEM-01'); + assert.equal(phaseId.stripConfiguredProjectCodePrefix('MEM-01', undefined), 'MEM-01'); + }); +}); + +// ─── isForeignPrefixedPhaseQuery (#2121 / #2056) ───────────────────────────── + +describe('isForeignPrefixedPhaseQuery', () => { + test('a prefix that is not the configured code is foreign', () => { + assert.equal(phaseId.isForeignPrefixedPhaseQuery('MEM-01', 'LKML'), true); + }); + + test('the configured prefix is not foreign (case-insensitive)', () => { + assert.equal(phaseId.isForeignPrefixedPhaseQuery('CK-01', 'CK'), false); + assert.equal(phaseId.isForeignPrefixedPhaseQuery('ck-01', 'CK'), false); + }); + + test('a value with no prefix is never foreign', () => { + assert.equal(phaseId.isForeignPrefixedPhaseQuery('01', 'CK'), false); + assert.equal(phaseId.isForeignPrefixedPhaseQuery('29', 'AB'), false); + }); + + test('a prefixed query with no configured code is foreign; a bare one is not', () => { + assert.equal(phaseId.isForeignPrefixedPhaseQuery('MEM-01', ''), true); + assert.equal(phaseId.isForeignPrefixedPhaseQuery('MEM-01', null), true); + assert.equal(phaseId.isForeignPrefixedPhaseQuery('01', ''), false); + }); +}); + +// ─── roadmapPhaseLookupSources (#2121, owned here after the move) ───────────── + +describe('roadmapPhaseLookupSources', () => { + const PREFIX_TOLERANT = `${phaseId.OPTIONAL_PROJECT_CODE_PREFIX_SOURCE}0*29`; + + test('a bare numeric query yields the numeric then prefix-tolerant sources', () => { + const sources = phaseId.roadmapPhaseLookupSources('29'); + assert.deepEqual(sources, ['0*29', PREFIX_TOLERANT]); + }); + + test('the bare numeric source precedes the prefix-tolerant fallback', () => { + const sources = phaseId.roadmapPhaseLookupSources('29'); + assert.ok(sources.indexOf('0*29') < sources.indexOf(PREFIX_TOLERANT)); + }); + + test('a project-code-prefixed query adds the exact source first (3 sources)', () => { + const sources = phaseId.roadmapPhaseLookupSources('AB-29'); + assert.equal(sources.length, 3); + assert.equal(sources[0], 'AB-29'); + assert.ok(sources.includes('0*29')); + assert.ok(sources.includes(PREFIX_TOLERANT)); + }); + + test('zero-padding is tolerated: 029 resolves the same sources as 29', () => { + assert.deepEqual(phaseId.roadmapPhaseLookupSources('029'), phaseId.roadmapPhaseLookupSources('29')); + }); + + test('sources are deduplicated', () => { + const sources = phaseId.roadmapPhaseLookupSources('29'); + assert.equal(sources.length, new Set(sources).size); + }); +}); + +// ─── #2121 property tests (fast-check) ─────────────────────────────────────── + +describe('phase-id canonical surface — properties', () => { + test('#2111 invariant: a "Milestone vX.Y complete" string never yields a phase', () => { + fc.assert( + fc.property(fc.nat(999), fc.nat(999), (major, minor) => { + return phaseId.parsePhaseFromProse(`Milestone v${major}.${minor} complete`).phase === null; + }), + ); + }); + + test('parse↔normalize: a "N of M" prose value extracts N, and it normalizes stably', () => { + fc.assert( + fc.property(fc.integer({ min: 1, max: 9999 }), fc.integer({ min: 1, max: 9999 }), (n, m) => { + const parsed = phaseId.parsePhaseFromProse(`${n} of ${m}`); + return ( + parsed.phase === String(n) && + phaseId.normalizePhaseName(parsed.phase) === phaseId.normalizePhaseName(String(n)) + ); + }), + ); + }); +}); diff --git a/tests/portability-vocab-drift.test.cjs b/tests/portability-vocab-drift.test.cjs index d95872352..0fdd87711 100644 --- a/tests/portability-vocab-drift.test.cjs +++ b/tests/portability-vocab-drift.test.cjs @@ -47,6 +47,9 @@ const INSTALL_JS_PATH_HELPERS = [ 'resolveOpencodeConfigPath', 'computePathPrefix', 'normalizeInstallRelativePath', + // #2088 (ADR-1239 upgrade 3): resolves the skills-install dir honoring a + // skills-kind `home` override (e.g. Codex → $HOME/.agents/skills). + '_resolveSkillsRootDir', ]; describe('portability-vocab drift guard', () => { diff --git a/tests/profile-output.test.cjs b/tests/profile-output.test.cjs index c848e126b..cc40ba566 100644 --- a/tests/profile-output.test.cjs +++ b/tests/profile-output.test.cjs @@ -325,11 +325,14 @@ describe('generate-dev-preferences command', () => { const result = runGsdTools( ['generate-dev-preferences', '--analysis', analysisPath, '--raw'], tmpDir, - { CODEX_HOME: codexHome, GSD_RUNTIME: 'codex' } + // #2088 (ADR-1239 upgrade 3): Codex skills resolve to $HOME/.agents/skills + // (HOME-relative), so sandbox HOME to keep the dev-preferences write inside + // the temp dir rather than the developer's real ~/.agents/skills. + { CODEX_HOME: codexHome, GSD_RUNTIME: 'codex', HOME: codexHome, USERPROFILE: codexHome } ); assert.ok(result.success, `Failed: ${result.error}`); const out = JSON.parse(result.output); - assert.strictEqual(out.command_path, path.join(codexHome, 'skills', 'gsd-dev-preferences', 'SKILL.md')); + assert.strictEqual(out.command_path, path.join(codexHome, '.agents', 'skills', 'gsd-dev-preferences', 'SKILL.md')); assert.ok(fs.existsSync(out.command_path), 'runtime-aware output should be written'); }); @@ -347,11 +350,12 @@ describe('generate-dev-preferences command', () => { const result = runGsdTools( ['generate-dev-preferences', '--analysis', analysisPath, '--raw'], tmpDir, - { CODEX_HOME: codexHome, GSD_RUNTIME: 'codex-app' } + // #2088: codex-app alias canonicalizes to codex → $HOME/.agents/skills. + { CODEX_HOME: codexHome, GSD_RUNTIME: 'codex-app', HOME: codexHome, USERPROFILE: codexHome } ); assert.ok(result.success, `Failed: ${result.error}`); const out = JSON.parse(result.output); - assert.strictEqual(out.command_path, path.join(codexHome, 'skills', 'gsd-dev-preferences', 'SKILL.md')); + assert.strictEqual(out.command_path, path.join(codexHome, '.agents', 'skills', 'gsd-dev-preferences', 'SKILL.md')); }); test('uses runtime-aware skills dir for cline by default (#782)', () => { diff --git a/tests/read-injection-scanner.property.test.cjs b/tests/read-injection-scanner.property.test.cjs index 92cf925f9..eac847542 100644 --- a/tests/read-injection-scanner.property.test.cjs +++ b/tests/read-injection-scanner.property.test.cjs @@ -12,28 +12,63 @@ * * Invoked as a subprocess (the hook reads a JSON payload on stdin and has no * exported surface), so this exercises the real shipped hook end-to-end. + * + * F.I.R.S.T. design: + * Fast — spawnSync is synchronous; scanner exits in <100ms for any input. + * Isolated — each invocation is a fresh subprocess; no shared state. + * Repeatable — no wall-clock assertion; the 30s safety-net timeout is 6x the + * scanner's own internal 5s timer and is never tested against. + * Tests assert on the scanner's RESULT (exit code + output shape), + * never on timing. + * Self-Val — assertions check exit===0 and output is empty or valid JSON. + * Timely — written alongside the scanner (#1577); hardened for #2089. */ const { describe, test } = require('node:test'); const assert = require('node:assert/strict'); -const { execFileSync } = require('node:child_process'); +const { spawnSync } = require('node:child_process'); const path = require('node:path'); const fc = require('./helpers/fast-check-setup.cjs'); const HOOK_PATH = path.join(__dirname, '..', 'hooks', 'gsd-read-injection-scanner.js'); +/** + * Run the scanner hook with a payload and return its result. + * + * Uses spawnSync (not execFileSync) so non-zero exits return a result object + * rather than throwing — cleaner for property tests that assert on exit code. + * + * Non-serializable payloads (BigInt, circular refs, Symbol, undefined) are + * SKIPPED: the scanner receives JSON via stdin, so these values can never + * reach it. JSON.stringify throwing is a test-harness artifact (fc.anything() + * generates values outside the JSON domain), not a scanner defect. + * + * The 30s safety-net timeout is NOT a test assertion. The scanner exits in + * <100ms for any input; its own internal setTimeout(5000) guarantees exit + * even if stdin never closes (impossible here — spawnSync's `input:` pipes + * and closes stdin). The ceiling only catches a genuinely hung process (a + * real defect) without racing the scanner's internal timer. + */ function runHook(payload) { + let input; try { - const stdout = execFileSync(process.execPath, [HOOK_PATH], { - input: JSON.stringify(payload), - encoding: 'utf-8', - timeout: 5000, - stdio: ['pipe', 'pipe', 'pipe'], - }); - return { exitCode: 0, stdout: stdout.trim() }; - } catch (err) { - return { exitCode: err.status ?? 1, stdout: (err.stdout || '').toString().trim() }; + input = JSON.stringify(payload); + } catch { + return { exitCode: 0, stdout: '', skipped: true }; } + + const result = spawnSync(process.execPath, [HOOK_PATH], { + input, + encoding: 'utf-8', + timeout: 30000, + stdio: ['pipe', 'pipe', 'pipe'], + }); + + return { + exitCode: result.status ?? 1, + stdout: (result.stdout || '').trim(), + signal: result.signal, + }; } // Injection-shaped fragments so the regex-matching path is exercised, not just clean text. diff --git a/tests/run-tests-harness.test.cjs b/tests/run-tests-harness.test.cjs index 6a1310676..7310ff42a 100644 --- a/tests/run-tests-harness.test.cjs +++ b/tests/run-tests-harness.test.cjs @@ -389,6 +389,41 @@ test('ambient GSD workstream vars are stripped by the runner', () => { `expected final file-count chunking marker in stderr; STDERR:\n${r.stderr}`, ); }); + + // #2088: install-heavy files (real installs) are weighted so they never all + // land in one chunk — otherwise the unsharded targeted lane packs the whole + // install surface into a single chunk that blows the 600s per-chunk backstop + // on the slow Windows runner. + test('install-heavy files carry more weight so they SPREAD across chunks (#2088)', () => { + // 4 install-* files at weight 3 = 12 against a 6-weight cap → 2 chunks + // (2 heavy each). The whole install load is never in a single chunk. + const heavy = Array.from({ length: 4 }, (_, i) => `install-weighttest-${i}.test.cjs`); + seed(tmpDir, heavy); + const rh = runHarness(tmpDir, [], { + RUN_TESTS_MAX_CMDLINE_CHARS: '100000', + RUN_TESTS_MAX_FILES_PER_CHUNK: '6', + RUN_TESTS_HEAVY_FILE_WEIGHT: '3', + }); + assert.strictEqual(rh.status, 0, `heavy: expected zero exit; STDERR:\n${rh.stderr}`); + assert.match(rh.stderr, /run-tests: chunk 1\/2 — 2 files/, `install-heavy files must split into 2 chunks; STDERR:\n${rh.stderr}`); + assert.match(rh.stderr, /run-tests: chunk 2\/2 — 2 files/, `STDERR:\n${rh.stderr}`); + }); + + test('light files of the same count stay in ONE chunk — split is weight-driven, not count-driven (#2088)', () => { + // Same file COUNT (4) but LIGHT (weight 1 each): 4 < 6 cap → a single + // chunk. Proves the split above is driven by install-heavy WEIGHT. + const light = Array.from({ length: 4 }, (_, i) => `lightweighttest-${i}.test.cjs`); + seed(tmpDir, light); + const rl = runHarness(tmpDir, [], { + RUN_TESTS_MAX_CMDLINE_CHARS: '100000', + RUN_TESTS_MAX_FILES_PER_CHUNK: '6', + RUN_TESTS_HEAVY_FILE_WEIGHT: '3', + }); + assert.strictEqual(rl.status, 0, `light: expected zero exit; STDERR:\n${rl.stderr}`); + // A single chunk emits NO `chunk N/M` split marker (it only prints when + // chunks.length > 1), so its absence proves the 4 light files stayed together. + assert.doesNotMatch(rl.stderr, /run-tests: chunk \d+\/\d+ — /, `4 light files must stay in one chunk (no split marker); STDERR:\n${rl.stderr}`); + }); }); describe('shard partitioning CLI (#1212)', () => {