fix(update): don't hijack the agents target on legacy Codex upgrade

.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.

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`):

src/core/legacy-cleanup.ts

@@ -801,35 +801,14 @@ export function formatDeferredGlobalPromptSummary(detection: LegacyDetectionResu
 export function getToolsFromLegacyArtifacts(detection: LegacyDetectionResult): string[] {
   const tools = new Set<string>();
 
-  // 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.

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;

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<Record<string, readonly (typeof ALL_WORKFLOWS)[number][]>>;
   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<void> {
-    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 };
   }
 }

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);
+    });
+  });
 });