diff --git a/.changeset/fix-legacy-upgrade-agents-ownership.md b/.changeset/fix-legacy-upgrade-agents-ownership.md new file mode 100644 index 0000000000..085a4a4ebf --- /dev/null +++ b/.changeset/fix-legacy-upgrade-agents-ownership.md @@ -0,0 +1,7 @@ +--- +"@fission-ai/openspec": patch +--- + +### Bug Fixes + +- **Don't let a legacy Codex upgrade hijack the vendor-neutral `agents` target** — `openspec update` no longer overwrites an existing `.agents` skills tree (and its ownership marker) when Codex is detected only from leftover global `~/.codex/prompts`. Because Codex and the vendor-neutral `agents` target share `.agents/skills`, a project that used the `agents` target could have its generic skills silently rewritten with Codex-specific syntax and its target flipped to Codex on the next `update --force`. The legacy-upgrade path now respects the established owner of a shared skills directory, matching the one-writer rule `openspec init` already applies. When an upgrade is skipped this way, that tool's repo-local legacy files (e.g. `.codex/prompts/openspec-*.md`) are also preserved rather than cleaned up, since no replacement was written to take their place. A genuine first-time Codex upgrade (no `.agents` tree yet) is unaffected. diff --git a/docs/supported-tools.md b/docs/supported-tools.md index 756a80e878..37d583b71d 100644 --- a/docs/supported-tools.md +++ b/docs/supported-tools.md @@ -180,6 +180,13 @@ For pre-marker projects, OpenSpec infers ownership from managed skill references generic canonical tree alongside legacy `.codex/skills` is treated as an older dual-target install and consolidated into the compatible shared tree. +`openspec update` honors this ownership too. If a project owns `.agents` as the +vendor-neutral target and a leftover Codex install is detected only from stray +prompt files, the update leaves the established `agents` tree in place instead of +rewriting it with Codex syntax, and preserves those legacy prompt files rather +than deleting them. To hand the shared tree to Codex, run `openspec init --tools +codex` explicitly. + ## Non-Interactive Setup For CI/CD or scripted setup, use `--tools` (and optionally `--profile`): diff --git a/src/core/legacy-cleanup.ts b/src/core/legacy-cleanup.ts index ccc47df160..3db9651f28 100644 --- a/src/core/legacy-cleanup.ts +++ b/src/core/legacy-cleanup.ts @@ -801,35 +801,14 @@ export function formatDeferredGlobalPromptSummary(detection: LegacyDetectionResu export function getToolsFromLegacyArtifacts(detection: LegacyDetectionResult): string[] { const tools = new Set(); - // Match directories to tool IDs for (const dir of detection.slashCommandDirs) { - for (const [toolId, pattern] of Object.entries(LEGACY_SLASH_COMMAND_PATHS)) { - if (pattern.type === 'directory' && pattern.path === dir) { - tools.add(toolId); - break; - } - } + const toolId = legacyToolIdForDir(dir); + if (toolId) tools.add(toolId); } - // Match files to tool IDs using glob patterns for (const file of detection.slashCommandFiles) { - // Normalize file path to use forward slashes for consistent matching (Windows compatibility) - const normalizedFile = normalizePathForMatch(file); - for (const [toolId, pattern] of Object.entries(LEGACY_SLASH_COMMAND_PATHS)) { - if (pattern.type === 'files' && pattern.pattern) { - const patterns = Array.isArray(pattern.pattern) ? pattern.pattern : [pattern.pattern]; - let matched = false; - for (const p of patterns) { - const regex = globToRegex(p); - if (regex.test(normalizedFile)) { - tools.add(toolId); - matched = true; - break; - } - } - if (matched) break; - } - } + const toolId = legacyToolIdForFile(file); + if (toolId) tools.add(toolId); } for (const prompt of getLegacyGlobalPromptMatches(detection)) { @@ -839,6 +818,26 @@ export function getToolsFromLegacyArtifacts(detection: LegacyDetectionResult): s return Array.from(tools); } +/** The tool that owns a repo-local legacy slash-command directory, if any. */ +function legacyToolIdForDir(dir: string): string | undefined { + for (const [toolId, pattern] of Object.entries(LEGACY_SLASH_COMMAND_PATHS)) { + if (pattern.type === 'directory' && pattern.path === dir) return toolId; + } + return undefined; +} + +/** The tool that owns a repo-local legacy slash-command file, if any. */ +function legacyToolIdForFile(file: string): string | undefined { + // Normalize to forward slashes so the glob patterns match on Windows too. + const normalizedFile = normalizePathForMatch(file); + for (const [toolId, pattern] of Object.entries(LEGACY_SLASH_COMMAND_PATHS)) { + if (pattern.type !== 'files' || !pattern.pattern) continue; + const patterns = Array.isArray(pattern.pattern) ? pattern.pattern : [pattern.pattern]; + if (patterns.some((p) => globToRegex(p).test(normalizedFile))) return toolId; + } + return undefined; +} + /** * Normalizes global Codex prompt matches so callers can rely on workflow-aware * metadata even when older detection results only carry file paths. @@ -902,6 +901,34 @@ export function omitGlobalLegacyPromptFiles(detection: LegacyDetectionResult): L return nextDetection; } +/** + * Returns a detection snapshot with the repo-local slash-command artifacts of + * the given tools removed. The legacy-upgrade path uses this to skip cleaning a + * tool's legacy files when its replacement was deliberately NOT written — e.g. a + * Codex upgrade suppressed because the shared `.agents` root is already owned by + * another tool. Deleting the legacy prompt without writing its replacement would + * violate the cleanup contract ("remove X because replacement Y now exists") and + * strip the tool's only OpenSpec integration. + */ +export function omitToolLegacyArtifacts( + detection: LegacyDetectionResult, + toolIds: readonly string[] +): LegacyDetectionResult { + if (toolIds.length === 0) return detection; + const skip = new Set(toolIds); + const nextDetection: LegacyDetectionResult = { + ...detection, + slashCommandDirs: detection.slashCommandDirs.filter( + (dir) => !skip.has(legacyToolIdForDir(dir) ?? '') + ), + slashCommandFiles: detection.slashCommandFiles.filter( + (file) => !skip.has(legacyToolIdForFile(file) ?? '') + ), + }; + nextDetection.hasLegacyArtifacts = hasLegacyArtifacts(nextDetection); + return nextDetection; +} + /** * Builds a detection snapshot containing only the selected global Codex prompt * matches for replacement-gated cleanup. diff --git a/src/core/shared-skill-target.ts b/src/core/shared-skill-target.ts index e3f214442e..a17c862447 100644 --- a/src/core/shared-skill-target.ts +++ b/src/core/shared-skill-target.ts @@ -158,6 +158,38 @@ export function isSharedSkillTargetActive(projectPath: string, toolId: string): .some((candidate) => candidate.value === toolId); } +/** + * The tool that already owns `toolId`'s shared skills root, when a DIFFERENT + * one does. Returns the owner's tool id only when the root already carries an + * ownership signal (a marker or generated skills) AND reconciliation resolves + * it to another tool. An empty or unclaimed root returns undefined, so a + * genuine first-time legacy upgrade — e.g. a Codex-only user with no `.agents` + * yet — is never reported as owned. + */ +export function sharedSkillRootOwner(projectPath: string, toolId: string): string | undefined { + const tool = AI_TOOLS.find((candidate) => candidate.value === toolId); + if (!tool?.skillsDir) return undefined; + const sharingRoot = AI_TOOLS.filter((candidate) => candidate.skillsDir === tool.skillsDir); + if (sharingRoot.length < 2) return undefined; + + const hasOwnerSignal = + readSharedSkillTarget(projectPath, tool.skillsDir) !== undefined || + hasCurrentSkills(projectPath, tool.skillsDir); + if (!hasOwnerSignal) return undefined; + + const owner = reconcileSharedSkillTargets(projectPath, sharingRoot)[0]?.value; + return owner && owner !== toolId ? owner : undefined; +} + +/** + * Whether generating `toolId` into its shared skills root would clobber a tree + * a DIFFERENT tool already owns — the guard the legacy-upgrade path uses before + * writing skills. See {@link sharedSkillRootOwner} for the ownership rules. + */ +export function sharedSkillRootOwnedByOther(projectPath: string, toolId: string): boolean { + return sharedSkillRootOwner(projectPath, toolId) !== undefined; +} + export function writeSharedSkillTarget(projectPath: string, toolId: string): void { const tool = AI_TOOLS.find((candidate) => candidate.value === toolId); if (!tool?.skillsDir) return; diff --git a/src/core/update.ts b/src/core/update.ts index 3ac02feb44..69fa4bafe4 100644 --- a/src/core/update.ts +++ b/src/core/update.ts @@ -38,6 +38,7 @@ import { getLegacyWorkflowIdsForTool, getToolsFromLegacyArtifacts, omitGlobalLegacyPromptFiles, + omitToolLegacyArtifacts, pickGlobalLegacyPromptFiles, type LegacyDetectionResult, } from './legacy-cleanup.js'; @@ -70,7 +71,7 @@ import { shouldReconcileCommandFilesForTool, shouldRemoveSkillsForTool, } from './command-surface.js'; -import { writeSharedSkillTarget } from './shared-skill-target.js'; +import { writeSharedSkillTarget, sharedSkillRootOwner } from './shared-skill-target.js'; import { includesGitHubCopilot, writeCopilotCloudFiles, removeCopilotCloudFiles, isCopilotCloudEnabled, readCopilotCloudOptIn, findUnmanagedCloudFiles } from './github-copilot/cloud-agent.js'; const require = createRequire(import.meta.url); @@ -84,6 +85,12 @@ type LegacyUpgradeResult = { newlyConfiguredTools: string[]; workflowOverrides: Partial>; deferredGlobalCleanup?: LegacyDetectionResult; + /** + * Tools whose skill generation was skipped because another tool already owns + * their shared skills root. Their repo-local legacy artifacts must be exempt + * from immediate cleanup — no replacement was written to justify deleting them. + */ + skippedSharedSkillTools?: string[]; }; /** @@ -886,7 +893,11 @@ export class UpdateCommand { desiredWorkflows, delivery ); - await this.performImmediateLegacyCleanup(projectPath, detection); + await this.performImmediateLegacyCleanup( + projectPath, + detection, + legacyUpgrade.skippedSharedSkillTools + ); return { ...legacyUpgrade, deferredGlobalCleanup: pickGlobalLegacyPromptFiles( @@ -919,7 +930,11 @@ export class UpdateCommand { desiredWorkflows, delivery ); - await this.performImmediateLegacyCleanup(projectPath, detection); + await this.performImmediateLegacyCleanup( + projectPath, + detection, + legacyUpgrade.skippedSharedSkillTools + ); return { ...legacyUpgrade, deferredGlobalCleanup: pickGlobalLegacyPromptFiles( @@ -939,9 +954,15 @@ export class UpdateCommand { */ private async performImmediateLegacyCleanup( projectPath: string, - detection: LegacyDetectionResult + detection: LegacyDetectionResult, + skippedSharedSkillTools: readonly string[] = [] ): Promise { - const immediateDetection = omitGlobalLegacyPromptFiles(detection); + // Tools whose upgrade was skipped (shared root owned by another) had no + // replacement written, so their repo-local legacy files must be preserved. + const immediateDetection = omitToolLegacyArtifacts( + omitGlobalLegacyPromptFiles(detection), + skippedSharedSkillTools + ); if (immediateDetection.hasLegacyArtifacts) { await this.performLegacyCleanup(projectPath, immediateDetection); } @@ -1083,6 +1104,7 @@ export class UpdateCommand { // Create skills/commands for selected tools using effective profile+delivery. const newlyConfigured: string[] = []; + const skippedSharedSkillTools: string[] = []; const workflowOverrides: LegacyUpgradeResult['workflowOverrides'] = {}; for (const toolId of selectedTools) { @@ -1107,6 +1129,34 @@ export class UpdateCommand { const skillTemplates = getSkillTemplates(toolWorkflows); const commandContents = getCommandContents(toolWorkflows); + // A shared skills root (e.g. `.agents`) already owned by another tool + // must not be overwritten by a tool inferred from legacy artifacts: a + // Codex install detected only from global `~/.codex/prompts` would + // otherwise rewrite an existing vendor-neutral `agents` tree with + // Codex-specific syntax and flip its ownership marker `agents → codex`. + // Leave the established owner in place. (init applies the same + // one-writer rule up front when both targets are selected.) + // + // Skipping here means the tool is never recorded as configured, so a + // persistent legacy signal re-offers it on later runs. Because no + // replacement is written, this tool is also exempted from immediate + // legacy cleanup (see skippedSharedSkillTools) — otherwise a repo-local + // `.codex/prompts` would be deleted with nothing put in its place. That + // repeat is idempotent and harmless — the alternative is the silent + // hijack this prevents. + const sharedOwner = shouldGenerateSkills + ? sharedSkillRootOwner(projectPath, tool.value) + : undefined; + if (sharedOwner) { + const ownerName = + AI_TOOLS.find((candidate) => candidate.value === sharedOwner)?.name ?? sharedOwner; + spinner.info( + `Skipped ${tool.name}: ${tool.skillsDir}/skills is already managed by another tool (${ownerName}).` + ); + skippedSharedSkillTools.push(tool.value); + continue; + } + // Create skill files when delivery includes skills if (shouldGenerateSkills) { for (const { template, dirName } of skillTemplates) { @@ -1164,6 +1214,6 @@ export class UpdateCommand { console.log(); } - return { newlyConfiguredTools: newlyConfigured, workflowOverrides }; + return { newlyConfiguredTools: newlyConfigured, workflowOverrides, skippedSharedSkillTools }; } } diff --git a/test/core/legacy-cleanup.test.ts b/test/core/legacy-cleanup.test.ts index 55c72d5f85..f357052649 100644 --- a/test/core/legacy-cleanup.test.ts +++ b/test/core/legacy-cleanup.test.ts @@ -17,6 +17,7 @@ import { formatDetectionSummary, formatProjectMdMigrationHint, getToolsFromLegacyArtifacts, + omitToolLegacyArtifacts, LEGACY_CONFIG_FILES, LEGACY_GLOBAL_SLASH_COMMAND_PATHS, LEGACY_SLASH_COMMAND_PATHS, @@ -1479,4 +1480,55 @@ ${OPENSPEC_MARKERS.end}`); expect(tools).toHaveLength(0); }); }); + + describe('omitToolLegacyArtifacts', () => { + const baseDetection = () => ({ + configFiles: [], + configFilesToUpdate: [], + slashCommandDirs: ['.claude/commands/openspec'], + slashCommandFiles: ['.codex/prompts/openspec-explore.md', '.cursor/commands/openspec-apply.md'], + globalSlashCommandFiles: [], + hasOpenspecAgents: false, + hasProjectMd: false, + hasRootAgentsWithMarkers: false, + hasLegacyArtifacts: true, + }); + + it('removes only the named tool\'s repo-local artifacts', () => { + const result = omitToolLegacyArtifacts(baseDetection(), ['codex']); + expect(result.slashCommandFiles).toEqual(['.cursor/commands/openspec-apply.md']); + // Other tools' files and directories are untouched. + expect(result.slashCommandDirs).toEqual(['.claude/commands/openspec']); + expect(result.hasLegacyArtifacts).toBe(true); + }); + + it('recomputes hasLegacyArtifacts to false when nothing is left', () => { + const detection = { + ...baseDetection(), + slashCommandDirs: [], + slashCommandFiles: ['.codex/prompts/openspec-explore.md'], + }; + const result = omitToolLegacyArtifacts(detection, ['codex']); + expect(result.slashCommandFiles).toEqual([]); + expect(result.hasLegacyArtifacts).toBe(false); + }); + + it('returns the detection unchanged when no tools are skipped', () => { + const detection = baseDetection(); + expect(omitToolLegacyArtifacts(detection, [])).toBe(detection); + }); + + it('omits backslash-delimited paths for the skipped tool (Windows)', () => { + // Defensive: `legacyToolIdForFile` normalizes separators, so a + // Windows-style path must map to `codex` and be filtered too. + const detection = { + ...baseDetection(), + slashCommandDirs: [], + slashCommandFiles: ['.codex\\prompts\\openspec-explore.md'], + }; + const result = omitToolLegacyArtifacts(detection, ['codex']); + expect(result.slashCommandFiles).toEqual([]); + expect(result.hasLegacyArtifacts).toBe(false); + }); + }); }); diff --git a/test/core/shared-skill-target.test.ts b/test/core/shared-skill-target.test.ts new file mode 100644 index 0000000000..37e7fd90ad --- /dev/null +++ b/test/core/shared-skill-target.test.ts @@ -0,0 +1,89 @@ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { promises as fs } from 'fs'; +import os from 'os'; +import path from 'path'; +import { + sharedSkillRootOwnedByOther, + sharedSkillRootOwner, +} from '../../src/core/shared-skill-target.js'; + +/** + * `.agents` is shared by the vendor-neutral `agents` target and Codex. When a + * legacy Codex install is detected only from global `~/.codex/prompts`, the + * update path must not rewrite an existing `agents`-owned `.agents` tree. This + * guards the predicate that decides that. + */ +describe('sharedSkillRootOwnedByOther', () => { + let projectPath: string; + + const writeAgentsSkill = async (marker?: string) => { + const skillsRoot = path.join(projectPath, '.agents', 'skills'); + const skillDir = path.join(skillsRoot, 'openspec-propose'); + await fs.mkdir(skillDir, { recursive: true }); + // Generic invocation syntax => inferred owner is `agents` (not `$openspec-`). + await fs.writeFile( + path.join(skillDir, 'SKILL.md'), + '# openspec-propose\n\nRun /openspec-propose to start.\n' + ); + if (marker !== undefined) { + await fs.writeFile(path.join(skillsRoot, '.openspec-target'), `${marker}\n`); + } + }; + + beforeEach(async () => { + projectPath = await fs.mkdtemp(path.join(os.tmpdir(), 'openspec-shared-target-')); + }); + + afterEach(async () => { + await fs.rm(projectPath, { recursive: true, force: true }); + }); + + it('reports the .agents root as owned by another tool when agents holds it (marker + generic tree)', async () => { + await writeAgentsSkill('agents'); + // Codex, inferred only from global prompts, must not clobber this tree. + expect(sharedSkillRootOwnedByOther(projectPath, 'codex')).toBe(true); + // The owner itself is never "owned by another". + expect(sharedSkillRootOwnedByOther(projectPath, 'agents')).toBe(false); + }); + + it('infers agents ownership from a generic tree even without a marker', async () => { + await writeAgentsSkill(); // no marker; content is generic `/openspec-` + expect(sharedSkillRootOwnedByOther(projectPath, 'codex')).toBe(true); + }); + + it('does NOT block Codex when the marker names Codex', async () => { + await writeAgentsSkill('codex'); + expect(sharedSkillRootOwnedByOther(projectPath, 'codex')).toBe(false); + }); + + it('does NOT block a first-time legacy upgrade with no .agents tree yet', async () => { + // Codex-only user with global prompts and no `.agents`: nothing to clobber. + expect(sharedSkillRootOwnedByOther(projectPath, 'codex')).toBe(false); + expect(sharedSkillRootOwnedByOther(projectPath, 'agents')).toBe(false); + }); + + it('returns false for a tool that does not share its skills root', async () => { + await writeAgentsSkill('agents'); + // Claude writes to its own `.claude` root, never `.agents`. + expect(sharedSkillRootOwnedByOther(projectPath, 'claude')).toBe(false); + }); + + it('treats an existing tree with no marker and no inferable syntax as agents-owned', async () => { + // Neither `$openspec-` nor `/openspec-` in the content and no marker: + // ownership can't be inferred, so reconciliation keeps the established + // `agents` target rather than letting Codex claim the existing tree. + const skillDir = path.join(projectPath, '.agents', 'skills', 'openspec-propose'); + await fs.mkdir(skillDir, { recursive: true }); + await fs.writeFile(path.join(skillDir, 'SKILL.md'), '# openspec-propose\n\nNo invocation syntax here.\n'); + expect(sharedSkillRootOwnedByOther(projectPath, 'codex')).toBe(true); + // The established `agents` target is the resolved owner of the ambiguous tree. + expect(sharedSkillRootOwner(projectPath, 'codex')).toBe('agents'); + }); + + it('names the owning tool via sharedSkillRootOwner', async () => { + await writeAgentsSkill('agents'); + expect(sharedSkillRootOwner(projectPath, 'codex')).toBe('agents'); + // The owner is never "owned by another"; an unclaimed root has no owner. + expect(sharedSkillRootOwner(projectPath, 'agents')).toBeUndefined(); + }); +}); diff --git a/test/core/update.test.ts b/test/core/update.test.ts index 3a40b7e97c..c5751da7ce 100644 --- a/test/core/update.test.ts +++ b/test/core/update.test.ts @@ -1,6 +1,7 @@ import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; import { UpdateCommand, scanInstalledWorkflows } from '../../src/core/update.js'; import { InitCommand } from '../../src/core/init.js'; +import { getConfiguredToolsForProfileSync } from '../../src/core/profile-sync-drift.js'; import { FileSystemUtils } from '../../src/utils/file-system.js'; import { OPENSPEC_MARKERS } from '../../src/core/config.js'; import type { GlobalConfig } from '../../src/core/global-config.js'; @@ -613,6 +614,109 @@ metadata: ).toContain('User edit'); }); + it('does not let a legacy Codex global prompt hijack an established agents target', async () => { + // Regression for the hijack this PR fixes: the guard must actually be + // invoked by the update flow, not merely be correct in isolation. + setMockConfig({ featureFlags: {}, profile: 'core', delivery: 'skills' }); + // The vendor-neutral `agents` target owns `.agents` (marker + generic skills). + await new InitCommand({ tools: 'agents', force: true }).execute(testDir); + // A leftover global Codex install, detected only from `~/.codex/prompts`. + const promptDir = path.join(process.env.CODEX_HOME!, 'prompts'); + const globalPrompt = path.join(promptDir, 'opsx-explore.md'); + await fs.mkdir(promptDir, { recursive: true }); + await fs.writeFile(globalPrompt, 'legacy explore prompt'); + + // The skip message is emitted via an ora spinner, which writes to the + // process streams rather than through console.log. Restore the spies in a + // finally so a throw can never swallow stdout for the rest of the suite. + let streamOutput = ''; + const capture = (chunk: unknown) => { + streamOutput += String(chunk); + return true; + }; + const stdoutSpy = vi.spyOn(process.stdout, 'write').mockImplementation(capture as never); + const stderrSpy = vi.spyOn(process.stderr, 'write').mockImplementation(capture as never); + try { + await new UpdateCommand({ force: true }).execute(testDir); + } finally { + stdoutSpy.mockRestore(); + stderrSpy.mockRestore(); + } + + const skillsDir = path.join(testDir, '.agents', 'skills'); + // Ownership marker is not flipped to codex... + expect(await fs.readFile(path.join(skillsDir, '.openspec-target'), 'utf-8')).toBe('agents\n'); + // ...and the tree keeps generic `/openspec-` syntax, never Codex `$openspec-`. + const propose = await fs.readFile( + path.join(skillsDir, 'openspec-propose', 'SKILL.md'), + 'utf-8' + ); + expect(propose).not.toContain('$openspec-'); + expect(propose).toContain('/openspec-'); + // Generation AND configuration are skipped: Codex is never recorded as a + // configured tool, so a stray global prompt cannot flip ownership later. + const configured = getConfiguredToolsForProfileSync(testDir); + expect(configured).toContain('agents'); + expect(configured).not.toContain('codex'); + // The skip names the established owner so the user understands why. + expect(streamOutput).toMatch(/Skipped Codex/); + expect(streamOutput).toMatch(/managed by another tool \(Shared \.agents skills\)/); + // The legacy signal must survive: because Codex was skipped, no + // replacement skill exists, so the deferred global-prompt cleanup must + // preserve `~/.codex/prompts` untouched (byte-for-byte) rather than + // delete it — otherwise the skip could never re-offer Codex later. + expect(await FileSystemUtils.fileExists(globalPrompt)).toBe(true); + expect(await fs.readFile(globalPrompt, 'utf-8')).toBe('legacy explore prompt'); + }); + + it('lets a first-time legacy Codex upgrade claim an unowned agents root', async () => { + // Inverse of the hijack guard: with no `.agents` tree yet, nothing is + // owned, so the real update path must still generate Codex skills and + // stamp the `codex` marker — proving the guard is not over-broad. + setMockConfig({ featureFlags: {}, profile: 'core', delivery: 'skills' }); + const promptDir = path.join(process.env.CODEX_HOME!, 'prompts'); + await fs.mkdir(promptDir, { recursive: true }); + await fs.writeFile(path.join(promptDir, 'opsx-explore.md'), 'legacy explore prompt'); + + await new UpdateCommand({ force: true }).execute(testDir); + + const skillsDir = path.join(testDir, '.agents', 'skills'); + // The codex marker is written (writeSharedSkillTarget on the non-owned path). + expect(await fs.readFile(path.join(skillsDir, '.openspec-target'), 'utf-8')).toBe('codex\n'); + // A single opsx-explore prompt infers only the `explore` workflow, and the + // generated skill carries Codex `$openspec-` syntax. + const explore = await fs.readFile( + path.join(skillsDir, 'openspec-explore', 'SKILL.md'), + 'utf-8' + ); + expect(explore).toContain('$openspec-'); + // Codex is now recorded as configured (mirrors the negative check above). + expect(getConfiguredToolsForProfileSync(testDir)).toContain('codex'); + }); + + it('preserves a skipped tool\'s repo-local legacy prompts instead of deleting them', async () => { + // When the guard skips Codex (agents owns `.agents`), no replacement skill + // is written — so Codex's repo-local `.codex/prompts` must NOT be cleaned + // up. Deleting them would strip the legacy signal with nothing in its place. + setMockConfig({ featureFlags: {}, profile: 'core', delivery: 'skills' }); + await new InitCommand({ tools: 'agents', force: true }).execute(testDir); + const legacyPrompts = path.join(testDir, '.codex', 'prompts'); + await fs.mkdir(legacyPrompts, { recursive: true }); + await fs.writeFile(path.join(legacyPrompts, 'openspec-explore.md'), 'legacy repo-local prompt'); + + await new UpdateCommand({ force: true }).execute(testDir); + + // agents tree preserved, and the repo-local legacy prompt survives + // byte-for-byte — asserting content, not mere existence, distinguishes + // "left untouched" from "deleted then rewritten". + expect( + await fs.readFile(path.join(testDir, '.agents', 'skills', '.openspec-target'), 'utf-8') + ).toBe('agents\n'); + const preservedPrompt = path.join(legacyPrompts, 'openspec-explore.md'); + expect(await FileSystemUtils.fileExists(preservedPrompt)).toBe(true); + expect(await fs.readFile(preservedPrompt, 'utf-8')).toBe('legacy repo-local prompt'); + }); + it('should let an explicit Codex init take ownership of an agents tree', async () => { await new InitCommand({ tools: 'agents', force: true }).execute(testDir);