diff --git a/.changeset/2088-eos-codex-declarative-adapter.md b/.changeset/2088-eos-codex-declarative-adapter.md new file mode 100644 index 000000000..6099d33d5 --- /dev/null +++ b/.changeset/2088-eos-codex-declarative-adapter.md @@ -0,0 +1,4 @@ +--- +type: Changed +--- +**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/bin/install.js b/bin/install.js index b8e9fa279..78af35018 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 @@ -357,6 +396,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 +3202,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 +3288,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 +3312,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 +3380,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 +3417,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 +3430,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 +3443,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 +3458,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 +4933,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 +4988,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 +5278,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 +6817,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 +6873,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++; @@ -7514,11 +7784,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,9 +8245,7 @@ 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' + : runtime === 'cursor' ? 'gsd-update --reapply (mention the skill name)' : runtime === 'kimi' ? '/skill:gsd-update --reapply' @@ -8209,8 +8482,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 +8536,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 +8732,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 +8761,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 +8849,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; @@ -8809,7 +9104,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 +9122,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,7 +9178,7 @@ 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); @@ -9197,7 +9492,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 +9635,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 +9746,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 +9886,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 +9914,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 ──────────────────────────────────── } @@ -9694,7 +9988,7 @@ function install(isGlobal, runtime = DEFAULT_RUNTIME, options = {}) { 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 +10006,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 +11303,13 @@ module.exports = { generateCodexAgentToml, cleanupCodexSkillMetadataSidecars, cleanupWindsurfLegacyDevinSkills, + cleanupMovedSkillsOldLocation, + _resolveMovedSkillsOldDir, + _resolveSkillsRootDir, + codexBareAgentsHasOnlyKnownScalars, + extractCodexUserAgentsScalars, + spliceCodexAgentsScalars, + CODEX_EXTENDED_HOOK_EVENTS, generateCodexConfigBlock, stripGsdFromCodexConfig, migrateCodexHooksMapFormat, 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/docs/reference/host-integration-capability-matrix.md b/docs/reference/host-integration-capability-matrix.md index f9fe517e4..45e94be43 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) 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..dea955a3b 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" } } }, @@ -4134,7 +4147,8 @@ const runtimes = { "prefix": "gsd-", "nesting": "flat", "recursive": false, - "converter": "convertClaudeCommandToCodexSkill" + "converter": "convertClaudeCommandToCodexSkill", + "home": ".agents" } ], "local": [ @@ -4144,7 +4158,8 @@ const runtimes = { "prefix": "gsd-", "nesting": "flat", "recursive": false, - "converter": "convertClaudeCommandToCodexSkill" + "converter": "convertClaudeCommandToCodexSkill", + "home": ".agents" } ] }, @@ -4156,7 +4171,11 @@ const runtimes = { "installSurface": "codex-toml", "writesSharedSettings": false, "permissionWriter": null, - "extendedHookEvents": [], + "extendedHookEvents": [ + "SubagentStop", + "Stop", + "PreCompact" + ], "hostIntegration": { "embeddingMode": "declarative", "commandSurface": "slash-file", @@ -4173,6 +4192,13 @@ const runtimes = { "stateIO": "filesystem", "transport": "mcp", "runtime": "node" + }, + "hostBehaviors": { + "reapplyCommand": "$gsd-update --reapply", + "tomlConfigInstall": true, + "cleanupSkillSidecars": true, + "agentTomlFiles": true, + "frontmatterDialect": "codex" } } }, 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/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/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/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/fixtures/golden-install-parity/codex.json b/tests/fixtures/golden-install-parity/codex.json index a1a0bb73c..efead535f 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/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/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)', () => {