From 0c6919ada95fcabd6d73c20d5400a879b1efc103 Mon Sep 17 00:00:00 2001 From: joshwheelock Date: Wed, 8 Jul 2026 20:10:49 +0200 Subject: [PATCH] Align Codex init file strategy --- CHANGELOG.md | 8 + docs/getting-started/what-jumbo-creates.md | 2 + docs/reference/commands/maintenance.md | 2 +- docs/reference/commands/project.md | 2 +- docs/reference/project-initialization.md | 4 +- .../context/project/init/AgentFileProtocol.ts | 23 +- .../context/project/init/CodexConfigurer.ts | 374 +++++++++++++----- .../evolve/EvolveController.test.ts | 9 + .../project/init/AgentFileProtocol.test.ts | 78 +++- .../project/init/CodexConfigurer.test.ts | 154 +++++++- 10 files changed, 539 insertions(+), 117 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a2432bca..5932008c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed + +- **Codex initialization**: `jumbo init`, repair, and `jumbo evolve --yes` now install Codex-managed skills to the documented repository skill location `.agents/skills` while keeping `.codex` limited to Codex hooks/configuration. + +### Fixed + +- **Codex managed artifact preservation**: Obsolete Jumbo-managed Codex skill copies under `.codex/skills` are removed only when byte-identical to the current managed templates, preserving customized skill content, user files, hook comments, unknown hook settings, and unrelated `.codex` configuration. + ## [3.13.0] - 2026-07-07 ### Removed diff --git a/docs/getting-started/what-jumbo-creates.md b/docs/getting-started/what-jumbo-creates.md index 506535df..f0860812 100644 --- a/docs/getting-started/what-jumbo-creates.md +++ b/docs/getting-started/what-jumbo-creates.md @@ -69,6 +69,8 @@ These hooks load the session router when an agent session begins and preserve wo Jumbo copies workflow and maintenance skills from `assets/skills` into the selected agents' skill directories, including bootstrap/session use, lifecycle hooks, command discovery, context maintenance, and correction capture. Additive initialization does not overwrite existing managed skill directories; repair refreshes Jumbo-managed skills from assets while preserving user-created skills. +Codex skills are installed to `.agents/skills`. Jumbo may remove obsolete Codex skill copies from `.codex/skills` during repair or evolve, but only when the obsolete directory is byte-identical to the current managed template and contains no extra user files. + --- ## How the event store works diff --git a/docs/reference/commands/maintenance.md b/docs/reference/commands/maintenance.md index 286bf74d..a0a44b75 100644 --- a/docs/reference/commands/maintenance.md +++ b/docs/reference/commands/maintenance.md @@ -76,7 +76,7 @@ Runs the complete installation update workflow in sequence: 7. Refresh managed harness and hook configuration 8. Rebuild database projections from the event store -The command is designed to be idempotent. Re-running it after a successful evolve should not create duplicate events or duplicate relations. Existing `.jumbo/settings.jsonc` files are upgraded additively: missing defaults are inserted, explicit values and unknown entries are preserved, and invalid JSONC fails instead of being partially rewritten. +The command is designed to be idempotent. Re-running it after a successful evolve should not create duplicate events or duplicate relations. Existing `.jumbo/settings.jsonc` files are upgraded additively: missing defaults are inserted, explicit values and unknown entries are preserved, and invalid JSONC fails instead of being partially rewritten. Codex skills are refreshed in `.agents/skills`; obsolete Jumbo-managed copies under `.codex/skills` are removed only when they exactly match the current managed templates. ### Examples diff --git a/docs/reference/commands/project.md b/docs/reference/commands/project.md index 6302de69..7bc5a686 100644 --- a/docs/reference/commands/project.md +++ b/docs/reference/commands/project.md @@ -42,7 +42,7 @@ jumbo project init --non-interactive --name [options] 4. Always creates bootstrap-only `JUMBO.md` and reference-only `AGENTS.md` 5. In non-interactive mode, configures all supported agents -Managed agent markdown and JSON hook/settings fragments are loaded from `assets/agent-files`. Managed skills are copied from `assets/skills` to the selected agents' skill directories. +Managed agent markdown and JSON hook/settings fragments are loaded from `assets/agent-files`. Managed skills are copied from `assets/skills` to the selected agents' skill directories. Codex skills are installed to `.agents/skills`; `.codex` is used for Codex hooks/configuration. ### Examples diff --git a/docs/reference/project-initialization.md b/docs/reference/project-initialization.md index 0697b81b..729fc57a 100644 --- a/docs/reference/project-initialization.md +++ b/docs/reference/project-initialization.md @@ -87,7 +87,7 @@ Jumbo configures hooks for popular AI coding assistants: | Agent | Configuration | |-------|---------------| | **Claude Code** | `CLAUDE.md`, `.claude/settings.json`, and `.claude/skills` | -| **Codex** | `.codex/hooks.json` and `.codex/skills` | +| **Codex** | `.codex/hooks.json` and `.agents/skills` | | **GitHub Copilot** | `.github/copilot-instructions.md`, `.github/hooks/hooks.json`, and `.agents/skills` | | **Gemini CLI** | `GEMINI.md`, `.gemini/settings.json`, and `.gemini/skills` | | **Cursor** | `.cursor/rules/jumbo.mdc` and `.cursor/hooks.json` | @@ -100,6 +100,8 @@ Managed instruction files are bootstrap-only. `JUMBO.md` tells agents to follow Jumbo-owned markdown files and JSON hook/settings fragments are loaded from `assets/agent-files`. Managed skills are copied from `assets/skills`, with additive initialization preserving existing user-created skills and repair refreshing Jumbo-managed skill directories. +For Codex, Jumbo uses the repository skill directory `.agents/skills` and keeps `.codex` for documented Codex hooks/configuration. During repair and evolve, obsolete Jumbo-managed skill copies under `.codex/skills` are removed only when they exactly match the current managed templates; customized skills and extra user files are preserved. + --- ## Update project settings diff --git a/src/infrastructure/context/project/init/AgentFileProtocol.ts b/src/infrastructure/context/project/init/AgentFileProtocol.ts index ace0a051..b4cbdfbd 100644 --- a/src/infrastructure/context/project/init/AgentFileProtocol.ts +++ b/src/infrastructure/context/project/init/AgentFileProtocol.ts @@ -42,18 +42,19 @@ const DEFAULT_TEMPLATE_SKILLS_ROOT = path.resolve( "assets", "skills" ); - export class AgentFileProtocol implements IAgentFileProtocol { - private readonly configurers: IConfigurer[] = [ - new ClaudeConfigurer(), - new GeminiConfigurer(), - new CopilotConfigurer(), - new VibeConfigurer(), - new CodexConfigurer(), - new CursorConfigurer(), - ]; - - constructor(private readonly templateSkillsRoot: string = DEFAULT_TEMPLATE_SKILLS_ROOT) {} + private readonly configurers: IConfigurer[]; + + constructor(private readonly templateSkillsRoot: string = DEFAULT_TEMPLATE_SKILLS_ROOT) { + this.configurers = [ + new ClaudeConfigurer(), + new GeminiConfigurer(), + new CopilotConfigurer(), + new VibeConfigurer(), + new CodexConfigurer(templateSkillsRoot), + new CursorConfigurer(), + ]; + } /** * Ensure JUMBO.md exists with full Jumbo instructions diff --git a/src/infrastructure/context/project/init/CodexConfigurer.ts b/src/infrastructure/context/project/init/CodexConfigurer.ts index 9fd351e9..551b7299 100644 --- a/src/infrastructure/context/project/init/CodexConfigurer.ts +++ b/src/infrastructure/context/project/init/CodexConfigurer.ts @@ -3,9 +3,12 @@ * * Encapsulates all knowledge about OpenAI Codex configuration: * - .codex/hooks.json with Codex lifecycle hooks - * - Skills distributed to .codex/skills + * - Skills distributed to .agents/skills + * - Cleanup of obsolete Jumbo-managed skill copies under .codex/skills * * Codex reads AGENTS.md natively, so no instruction file is needed. + * Codex reads repo skills from .agents/skills and repo hooks from + * the trusted project .codex configuration layer. * Codex hooks parse JSON stdout as hook-control envelopes, so Jumbo * lifecycle commands use text output to avoid parser conflicts. * @@ -14,6 +17,7 @@ import path from "path"; import fs from "fs-extra"; +import { applyEdits, modify, parse, type FormattingOptions, type ParseError } from "jsonc-parser"; import { IConfigurer } from "./IConfigurer.js"; import { PlannedFileChange } from "../../../../application/context/project/init/PlannedFileChange.js"; import { AgentFileAssetContent } from "../../../../domain/project/AgentFileAssetContent.js"; @@ -37,13 +41,27 @@ interface CodexHooksDocument { hooks?: Record; } +const JSON_FORMATTING_OPTIONS: FormattingOptions = { + insertSpaces: true, + tabSize: 2, + eol: "\n", +}; +const STALE_JUMBO_COMMAND_REPLACEMENTS: Readonly> = { + "jumbo session start": "jumbo session start --format text", + "jumbo work resume": "jumbo work resume --format text", + "jumbo work pause": "jumbo work pause --format text", +}; +const OBSOLETE_CODEX_SKILLS_RELATIVE_PATH = ".codex/skills"; + export class CodexConfigurer implements IConfigurer { readonly agent = { id: "codex", name: "Codex", } as const; - readonly skillPlatforms = [".codex/skills"] as const; + readonly skillPlatforms = [".agents/skills"] as const; + + constructor(private readonly templateSkillsRoot: string) {} /** * Configure all OpenAI Codex requirements for Jumbo. @@ -54,13 +72,16 @@ export class CodexConfigurer implements IConfigurer { */ async configure(projectRoot: string): Promise { await this.ensureCodexHooks(projectRoot); + await this.removeObsoleteManagedSkillCopies(projectRoot); } /** - * Repair Codex configuration by replacing stale Jumbo hook commands. + * Repair Codex configuration by replacing stale Jumbo hook commands + * and cleaning obsolete Jumbo-managed skill copies. */ async repair(projectRoot: string): Promise { await this.ensureCodexHooks(projectRoot); + await this.removeObsoleteManagedSkillCopies(projectRoot); } private async ensureCodexHooks(projectRoot: string): Promise { @@ -75,20 +96,18 @@ export class CodexConfigurer implements IConfigurer { } const existingContent = await fs.readFile(hooksPath, "utf-8"); - let existingHooks = {}; + const parseErrors: ParseError[] = []; + const existingHooks = existingContent.trim() + ? (parse(existingContent, parseErrors, { allowTrailingComma: true }) as unknown) + : {}; - if (existingContent.trim()) { - try { - existingHooks = JSON.parse(existingContent); - } catch { - await fs.writeFile(hooksPath, JSON.stringify(this.getJumboHooks(), null, 2) + "\n", "utf-8"); - return; - } + if (parseErrors.length > 0 || !this.isJsonObject(existingHooks)) { + await fs.writeFile(hooksPath, JSON.stringify(this.getJumboHooks(), null, 2) + "\n", "utf-8"); + return; } - const normalizedExistingHooks = this.removeStaleJumboHooks(existingHooks); - const mergedHooks = this.mergeHooks(normalizedExistingHooks, this.getJumboHooks()); - await fs.writeFile(hooksPath, JSON.stringify(mergedHooks, null, 2) + "\n", "utf-8"); + const mergedContent = this.mergeJumboHooks(existingContent, existingHooks); + await fs.writeFile(hooksPath, this.ensureTrailingNewline(mergedContent), "utf-8"); } catch (error) { console.warn( `Warning: Failed to configure Codex hooks: ${error instanceof Error ? error.message : String(error)}` @@ -96,128 +115,297 @@ export class CodexConfigurer implements IConfigurer { } } + private mergeJumboHooks(existingContent: string, existingHooks: Record): string { + if (!existingContent.trim()) { + return JSON.stringify(this.getJumboHooks(), null, 2); + } + + const updatedContent = this.replaceStaleJumboCommandValues(existingContent, existingHooks); + const updatedHooks = this.parseCodexHooksDocument(updatedContent); + return this.addMissingJumboHooks(updatedContent, updatedHooks ?? {}); + } + + private ensureTrailingNewline(content: string): string { + return content.endsWith("\n") ? content : `${content}\n`; + } + private getJumboHooks(): CodexHooksDocument { return AgentFileAssetContent.readJson("codex-hooks.fragment.json"); } - private mergeHooks(existing: CodexHooksDocument, jumbo: CodexHooksDocument): CodexHooksDocument { - const result: CodexHooksDocument = { ...existing }; - const mergedHooks: Record = { ...(result.hooks ?? {}) }; + private isJsonObject(value: unknown): value is Record { + return Boolean(value) && typeof value === "object" && !Array.isArray(value); + } + + private replaceStaleJumboCommandValues( + existingContent: string, + existingHooks: Record + ): string { + const rawHooks = existingHooks.hooks; + if (!this.isJsonObject(rawHooks)) { + return existingContent; + } + + let updatedContent = existingContent; + for (const [eventName, matcherGroups] of Object.entries(rawHooks)) { + if (!Array.isArray(matcherGroups)) { + continue; + } + + matcherGroups.forEach((matcherGroup, matcherGroupIndex) => { + if (!this.isJsonObject(matcherGroup) || !Array.isArray(matcherGroup.hooks)) { + return; + } - for (const [eventName, matcherGroups] of Object.entries(jumbo.hooks ?? {})) { - const existingGroups = mergedHooks[eventName] ?? []; - mergedHooks[eventName] = this.mergeMatcherGroups(existingGroups, matcherGroups); + matcherGroup.hooks.forEach((hook, hookIndex) => { + if (!this.isJsonObject(hook) || typeof hook.command !== "string") { + return; + } + + const replacement = STALE_JUMBO_COMMAND_REPLACEMENTS[hook.command]; + if (!replacement) { + return; + } + + updatedContent = this.applyJsoncEdit(updatedContent, [ + "hooks", + eventName, + matcherGroupIndex, + "hooks", + hookIndex, + "command", + ], replacement); + }); + }); } - result.hooks = mergedHooks; - return result; + return updatedContent; } - private mergeMatcherGroups( - existing: CodexMatcherGroup[], - additions: CodexMatcherGroup[] - ): CodexMatcherGroup[] { - const merged = [...existing]; + private addMissingJumboHooks(existingContent: string, existingHooks: CodexHooksDocument): string { + const jumboHooks = this.getJumboHooks(); + if (!this.isJsonObject(existingHooks.hooks)) { + return this.applyJsoncEdit(existingContent, ["hooks"], jumboHooks.hooks ?? {}); + } - for (const addition of additions) { - const existingIndex = merged.findIndex((entry) => entry?.matcher === addition.matcher); + let updatedContent = existingContent; + for (const [eventName, jumboMatcherGroups] of Object.entries(jumboHooks.hooks ?? {})) { + const currentHooks = this.parseCodexHooksDocument(updatedContent); + const currentMatcherGroups = currentHooks?.hooks?.[eventName]; + if (!Array.isArray(currentMatcherGroups)) { + updatedContent = this.applyJsoncEdit(updatedContent, ["hooks", eventName], jumboMatcherGroups); + continue; + } - if (existingIndex >= 0) { - const current = merged[existingIndex]; - merged[existingIndex] = { - ...current, - hooks: this.mergeHookArray(current.hooks ?? [], addition.hooks ?? []), - }; - } else { - merged.push(addition); + for (const jumboMatcherGroup of jumboMatcherGroups) { + updatedContent = this.addMissingJumboMatcherGroupHook( + updatedContent, + eventName, + jumboMatcherGroup + ); } } - return merged; + return updatedContent; } - private mergeHookArray( - existing: CodexHookCommand[], - additions: CodexHookCommand[] - ): CodexHookCommand[] { - const merged = [...existing]; - const existingKeys = new Set(existing.map((hook) => JSON.stringify(hook))); + private addMissingJumboMatcherGroupHook( + existingContent: string, + eventName: string, + jumboMatcherGroup: CodexMatcherGroup + ): string { + const currentHooks = this.parseCodexHooksDocument(existingContent); + const currentMatcherGroups = currentHooks?.hooks?.[eventName]; + if (!Array.isArray(currentMatcherGroups)) { + return existingContent; + } + + const matcherGroupIndex = currentMatcherGroups.findIndex( + (matcherGroup) => matcherGroup.matcher === jumboMatcherGroup.matcher + ); + if (matcherGroupIndex === -1) { + return this.applyJsoncEdit(existingContent, ["hooks", eventName, -1], jumboMatcherGroup); + } + + const currentMatcherGroup = currentMatcherGroups[matcherGroupIndex]; + if (!Array.isArray(currentMatcherGroup.hooks)) { + return this.applyJsoncEdit( + existingContent, + ["hooks", eventName, matcherGroupIndex, "hooks"], + jumboMatcherGroup.hooks ?? [] + ); + } - for (const addition of additions) { - const additionKey = JSON.stringify(addition); - if (!existingKeys.has(additionKey)) { - merged.push(addition); - existingKeys.add(additionKey); + let updatedContent = existingContent; + for (const jumboHook of jumboMatcherGroup.hooks ?? []) { + const refreshedHooks = this.parseCodexHooksDocument(updatedContent); + const refreshedHookArray = refreshedHooks?.hooks?.[eventName]?.[matcherGroupIndex]?.hooks; + if (refreshedHookArray?.some((hook) => this.hasSameHookCommand(hook, jumboHook))) { + continue; } + + updatedContent = this.applyJsoncEdit( + updatedContent, + ["hooks", eventName, matcherGroupIndex, "hooks", -1], + jumboHook + ); } - return merged; + return updatedContent; } - private removeStaleJumboHooks(existing: unknown): CodexHooksDocument { - if (!existing || typeof existing !== "object") { - return {}; + private hasSameHookCommand(existingHook: CodexHookCommand, jumboHook: CodexHookCommand): boolean { + return existingHook.type === jumboHook.type && existingHook.command === jumboHook.command; + } + + private parseCodexHooksDocument(content: string): CodexHooksDocument | null { + const parseErrors: ParseError[] = []; + const parsed = parse(content, parseErrors, { allowTrailingComma: true }) as unknown; + if (parseErrors.length > 0 || !this.isJsonObject(parsed)) { + return null; } - const source = existing as Record; - const result = { ...source } as CodexHooksDocument; - const rawHooks = source.hooks; - if (!rawHooks || typeof rawHooks !== "object") { - return result; + return parsed as CodexHooksDocument; + } + + private applyJsoncEdit(content: string, pathSegments: (string | number)[], value: unknown): string { + const edits = modify(content, pathSegments, value, { + formattingOptions: JSON_FORMATTING_OPTIONS, + }); + return applyEdits(content, edits); + } + + /** + * Remove Jumbo-managed skill copies from the obsolete .codex/skills + * location now that Codex reads repo skills from .agents/skills. + */ + private async removeObsoleteManagedSkillCopies(projectRoot: string): Promise { + const obsoleteCodexSkillsRoot = path.join(projectRoot, OBSOLETE_CODEX_SKILLS_RELATIVE_PATH); + if (!(await fs.pathExists(obsoleteCodexSkillsRoot))) { + return; } - const normalizedHooks: Record = {}; - for (const [eventName, matcherGroups] of Object.entries(rawHooks)) { - if (!Array.isArray(matcherGroups)) { - continue; + for (const skillName of await this.getTemplateSkillNames()) { + const templateSkillDirectory = path.join(this.templateSkillsRoot, skillName); + const obsoleteSkillDirectory = path.join(obsoleteCodexSkillsRoot, skillName); + if (await this.isObsoleteManagedSkillCopy(obsoleteSkillDirectory, templateSkillDirectory)) { + try { + await fs.remove(obsoleteSkillDirectory); + } catch (error) { + console.warn( + `Warning: Failed to remove obsolete Codex managed skill '${skillName}': ${ + error instanceof Error ? error.message : String(error) + }` + ); + } } + } - const normalizedGroups = matcherGroups - .map((matcherGroup) => this.removeStaleJumboHooksFromMatcher(matcherGroup)) - .filter((matcherGroup): matcherGroup is CodexMatcherGroup => matcherGroup !== null); + await this.removeDirectoryIfEmpty(obsoleteCodexSkillsRoot); + } - if (normalizedGroups.length > 0) { - normalizedHooks[eventName] = normalizedGroups; - } + private async getTemplateSkillNames(): Promise { + if (!(await fs.pathExists(this.templateSkillsRoot))) { + return []; } - result.hooks = normalizedHooks; - return result; + const entries = await fs.readdir(this.templateSkillsRoot, { withFileTypes: true }); + return entries.filter((entry) => entry.isDirectory()).map((entry) => entry.name); } - private removeStaleJumboHooksFromMatcher(matcherGroup: unknown): CodexMatcherGroup | null { - if (!matcherGroup || typeof matcherGroup !== "object") { - return null; + /** + * A .codex skill copy is only considered Jumbo-managed (and safe to delete) + * when its entire content is byte-identical to the current managed template: + * every file matches and no extra entries exist. Any deviation — customized + * bodies, extra user files, or unknown entry types — marks it user-owned. + */ + private async isObsoleteManagedSkillCopy( + skillDirectory: string, + templateSkillDirectory: string + ): Promise { + try { + if (!(await this.isDirectory(skillDirectory)) || !(await this.isDirectory(templateSkillDirectory))) { + return false; + } + + return await this.directoryContentMatchesManagedTemplate(skillDirectory, templateSkillDirectory); + } catch { + return false; } + } - const candidate = matcherGroup as Record; - const hooks: CodexHookCommand[] = Array.isArray(candidate.hooks) ? candidate.hooks : []; - const nonJumboHooks = hooks.filter((hook) => !this.isJumboLifecycleHook(hook)); + private async directoryContentMatchesManagedTemplate( + directory: string, + templateDirectory: string + ): Promise { + const entries = await fs.readdir(directory, { withFileTypes: true }); + if (entries.length === 0) { + return false; + } - if (nonJumboHooks.length === 0) { - return null; + for (const entry of entries) { + const entryPath = path.join(directory, entry.name); + const templateEntryPath = path.join(templateDirectory, entry.name); + + if (entry.isDirectory()) { + if ( + !(await this.isDirectory(templateEntryPath)) || + !(await this.directoryContentMatchesManagedTemplate(entryPath, templateEntryPath)) + ) { + return false; + } + } else if (entry.isFile()) { + if (!(await this.fileContentMatchesManagedTemplate(entryPath, templateEntryPath))) { + return false; + } + } else { + return false; + } } - return { - ...candidate, - hooks: nonJumboHooks, - } as CodexMatcherGroup; + return true; } - private isJumboLifecycleHook(hook: unknown): boolean { - if (!hook || typeof hook !== "object") { + private async fileContentMatchesManagedTemplate( + filePath: string, + templateFilePath: string + ): Promise { + if (!(await this.isFile(templateFilePath))) { return false; } - const command = (hook as Record).command; - return ( - command === "jumbo session start" || - command === "jumbo session start --format text" || - command === "jumbo work resume" || - command === "jumbo work resume --format text" || - command === "jumbo work pause" || - command === "jumbo work pause --format text" - ); + const [fileContent, templateFileContent] = await Promise.all([ + fs.readFile(filePath), + fs.readFile(templateFilePath), + ]); + return fileContent.equals(templateFileContent); + } + + private async isDirectory(directoryPath: string): Promise { + try { + return (await fs.stat(directoryPath)).isDirectory(); + } catch { + return false; + } + } + + private async isFile(filePath: string): Promise { + try { + return (await fs.stat(filePath)).isFile(); + } catch { + return false; + } + } + + private async removeDirectoryIfEmpty(directoryPath: string): Promise { + try { + const entries = await fs.readdir(directoryPath); + if (entries.length === 0) { + await fs.remove(directoryPath); + } + } catch { + return; + } } /** diff --git a/tests/application/evolve/EvolveController.test.ts b/tests/application/evolve/EvolveController.test.ts index 9faaea0f..99be95c3 100644 --- a/tests/application/evolve/EvolveController.test.ts +++ b/tests/application/evolve/EvolveController.test.ts @@ -116,6 +116,15 @@ describe("EvolveController", () => { expect(response.steps.every((step) => step.status === "repaired")).toBe(true); }); + it("routes repeated evolve runs through agent configuration repair for idempotent managed skill migration", async () => { + await controller.handle(); + await controller.handle(); + + expect(agentFileProtocol.repairAgentConfigurations).toHaveBeenCalledTimes(2); + expect(agentFileProtocol.repairAgentConfigurations).toHaveBeenNthCalledWith(1, "C:/repo"); + expect(agentFileProtocol.repairAgentConfigurations).toHaveBeenNthCalledWith(2, "C:/repo"); + }); + it("skips database migrations and rebuild when schema migrations fail", async () => { runSchemaMigrations.mockRejectedValue(new Error("DDL failed")); diff --git a/tests/infrastructure/context/project/init/AgentFileProtocol.test.ts b/tests/infrastructure/context/project/init/AgentFileProtocol.test.ts index fc7909d6..a4b6c03a 100644 --- a/tests/infrastructure/context/project/init/AgentFileProtocol.test.ts +++ b/tests/infrastructure/context/project/init/AgentFileProtocol.test.ts @@ -18,7 +18,7 @@ jest.setTimeout(30_000); describe("AgentFileProtocol", () => { let tmpDir: string; let protocol: AgentFileProtocol; - const skillPlatforms = [".agents/skills", ".claude/skills", ".codex/skills", ".gemini/skills", ".vibe/skills"]; + const skillPlatforms = [".agents/skills", ".claude/skills", ".gemini/skills", ".vibe/skills"]; beforeEach(async () => { tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "test-agent-files-")); @@ -226,7 +226,7 @@ describe("AgentFileProtocol", () => { } }); - it("should distribute fine-grained managed skills from packaged assets", async () => { + it("should distribute fine-grained Codex managed skills to the documented repo skill location", async () => { protocol = new AgentFileProtocol(); await protocol.ensureAgentConfigurations(tmpDir, ["codex"]); @@ -239,10 +239,37 @@ describe("AgentFileProtocol", () => { ]; for (const skillName of expectedSkillNames) { - expect(await fs.pathExists(path.join(tmpDir, ".codex", "skills", skillName, "SKILL.md"))).toBe(true); + expect(await fs.pathExists(path.join(tmpDir, ".agents", "skills", skillName, "SKILL.md"))).toBe(true); + expect(await fs.pathExists(path.join(tmpDir, ".codex", "skills", skillName, "SKILL.md"))).toBe(false); } }); + it("should remove obsolete Jumbo-managed Codex skill copies while preserving user-owned Codex skills", async () => { + const managedSkillContent = + "---\nname: managed-skill\ndescription: Managed.\n---\n\n# Managed\n\nCurrent.\n"; + const templateSkillPath = path.join(tmpDir, "assets", "skills", "managed-skill"); + await fs.ensureDir(templateSkillPath); + await fs.writeFile(path.join(templateSkillPath, "SKILL.md"), managedSkillContent, "utf-8"); + + const obsoleteCodexSkillPath = path.join(tmpDir, ".codex", "skills", "managed-skill"); + await fs.ensureDir(obsoleteCodexSkillPath); + await fs.writeFile(path.join(obsoleteCodexSkillPath, "SKILL.md"), managedSkillContent, "utf-8"); + + const userCodexSkillPath = path.join(tmpDir, ".codex", "skills", "my-custom-skill"); + await fs.ensureDir(userCodexSkillPath); + await fs.writeFile( + path.join(userCodexSkillPath, "SKILL.md"), + "---\nname: my-custom-skill\ndescription: User owned.\n---\n\n# User\n\nKeep me.\n", + "utf-8" + ); + + await protocol.ensureAgentConfigurations(tmpDir, ["codex"]); + + expect(await fs.pathExists(path.join(tmpDir, ".agents", "skills", "managed-skill", "SKILL.md"))).toBe(true); + expect(await fs.pathExists(obsoleteCodexSkillPath)).toBe(false); + expect(await fs.readFile(path.join(userCodexSkillPath, "SKILL.md"), "utf-8")).toContain("Keep me."); + }); + it("should keep user-created skills and avoid overwriting managed skills during additive install", async () => { const templateSkillPath = path.join(tmpDir, "assets", "skills", "managed-skill"); await fs.ensureDir(templateSkillPath); @@ -567,6 +594,47 @@ describe("AgentFileProtocol", () => { expect(preservedUserContent).toContain("Keep me."); }); + it("should remove obsolete Codex skill copies during repeated repair while preserving user-owned Codex skills", async () => { + const managedSkillContent = + "---\nname: managed-skill\ndescription: Managed.\n---\n\n# Managed\n\nTemplate current version.\n"; + const templateSkillPath = path.join(tmpDir, "assets", "skills", "managed-skill"); + await fs.ensureDir(templateSkillPath); + await fs.writeFile(path.join(templateSkillPath, "SKILL.md"), managedSkillContent, "utf-8"); + + const userSameNameTemplatePath = path.join(tmpDir, "assets", "skills", "user-owned-same-name"); + await fs.ensureDir(userSameNameTemplatePath); + await fs.writeFile( + path.join(userSameNameTemplatePath, "SKILL.md"), + "---\nname: user-owned-same-name\ndescription: Managed.\n---\n\n# Managed\n\nTemplate current version.\n", + "utf-8" + ); + + const obsoleteCodexSkillPath = path.join(tmpDir, ".codex", "skills", "managed-skill"); + await fs.ensureDir(obsoleteCodexSkillPath); + await fs.writeFile(path.join(obsoleteCodexSkillPath, "SKILL.md"), managedSkillContent, "utf-8"); + + const userSameNameCodexSkillPath = path.join(tmpDir, ".codex", "skills", "user-owned-same-name"); + await fs.ensureDir(userSameNameCodexSkillPath); + await fs.writeFile( + path.join(userSameNameCodexSkillPath, "SKILL.md"), + "---\nname: user-owned-same-name\ndescription: User owned.\n---\n\n# User\n\nKeep same-name user skill.\n", + "utf-8" + ); + + await protocol.repairAgentConfigurations(tmpDir, ["codex"]); + await protocol.repairAgentConfigurations(tmpDir, ["codex"]); + + const repairedManagedContent = await fs.readFile( + path.join(tmpDir, ".agents", "skills", "managed-skill", "SKILL.md"), + "utf-8" + ); + expect(repairedManagedContent).toContain("Template current version."); + expect(await fs.pathExists(obsoleteCodexSkillPath)).toBe(false); + expect(await fs.readFile(path.join(userSameNameCodexSkillPath, "SKILL.md"), "utf-8")).toContain( + "Keep same-name user skill." + ); + }); + it("should call repair on each configurer", async () => { await protocol.repairAgentConfigurations(tmpDir); @@ -744,10 +812,6 @@ describe("AgentFileProtocol", () => { path: ".claude/skills/my-skill", description: expect.stringContaining("assets/skills"), }), - expect.objectContaining({ - path: ".codex/skills/my-skill", - description: expect.stringContaining("assets/skills"), - }), expect.objectContaining({ path: ".gemini/skills/my-skill", description: expect.stringContaining("assets/skills"), diff --git a/tests/infrastructure/context/project/init/CodexConfigurer.test.ts b/tests/infrastructure/context/project/init/CodexConfigurer.test.ts index b1ed6991..581f3207 100644 --- a/tests/infrastructure/context/project/init/CodexConfigurer.test.ts +++ b/tests/infrastructure/context/project/init/CodexConfigurer.test.ts @@ -5,6 +5,7 @@ import fs from "fs-extra"; import * as path from "path"; import os from "os"; +import { parse } from "jsonc-parser"; import { CodexConfigurer } from "../../../../../src/infrastructure/context/project/init/CodexConfigurer"; import { jest } from "@jest/globals"; @@ -12,10 +13,28 @@ jest.setTimeout(30_000); describe("CodexConfigurer", () => { let tmpDir: string; - const configurer = new CodexConfigurer(); + let templateSkillsRoot: string; + let configurer: CodexConfigurer; + + interface ParsedCodexHooks { + readonly hooks?: { + readonly SessionStart?: readonly { + readonly matcher?: string; + readonly hooks?: readonly { + readonly type?: string; + readonly command?: string; + }[]; + }[]; + }; + readonly userSettings?: { + readonly enabled?: boolean; + }; + } beforeEach(async () => { tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "test-codex-configurer-")); + templateSkillsRoot = path.join(tmpDir, "assets", "skills"); + configurer = new CodexConfigurer(templateSkillsRoot); }); afterEach(async () => { @@ -29,8 +48,8 @@ describe("CodexConfigurer", () => { }); describe("skillPlatforms", () => { - it("should declare .codex/skills as the skill platform", () => { - expect(configurer.skillPlatforms).toEqual([".codex/skills"]); + it("should declare .agents/skills as the documented repo skill platform", () => { + expect(configurer.skillPlatforms).toEqual([".agents/skills"]); }); }); @@ -158,6 +177,52 @@ describe("CodexConfigurer", () => { expect(hooks.hooks.Stop[0].hooks[0].command).toBe("echo stop"); }); + it("should preserve nested comments and unknown settings in existing Codex hooks configuration", async () => { + const hooksPath = path.join(tmpDir, ".codex", "hooks.json"); + await fs.ensureDir(path.join(tmpDir, ".codex")); + await fs.writeFile( + hooksPath, + [ + "{", + " // user comment", + ' "userSettings": {', + ' "enabled": true', + " },", + ' "hooks": {', + ' "SessionStart": [', + " {", + " // keep matcher comment", + ' "matcher": "startup",', + ' "hooks": [', + " // keep user hook comment", + ' { "type": "command", "command": "echo user startup" },', + " // keep Jumbo hook comment", + ' { "type": "command", "command": "jumbo session start" }', + " ]", + " }", + " ]", + " }", + "}", + "", + ].join("\n"), + "utf-8" + ); + + await configurer.configure(tmpDir); + + const content = await fs.readFile(hooksPath, "utf-8"); + const hooks = parse(content) as ParsedCodexHooks; + expect(content).toContain("// user comment"); + expect(content).toContain("// keep matcher comment"); + expect(content).toContain("// keep user hook comment"); + expect(content).toContain("// keep Jumbo hook comment"); + expect(hooks.userSettings?.enabled).toBe(true); + expect(hooks.hooks?.SessionStart?.[0].hooks).toEqual([ + { type: "command", command: "echo user startup" }, + { type: "command", command: "jumbo session start --format text" }, + ]); + }); + it("should not duplicate Jumbo hooks across repeated configuration", async () => { await configurer.configure(tmpDir); await configurer.configure(tmpDir); @@ -206,6 +271,89 @@ describe("CodexConfigurer", () => { }); }); + describe("obsolete .codex/skills cleanup", () => { + const managedSkillContent = + "---\nname: managed-skill\ndescription: Managed.\n---\n\n# Managed\n\nCurrent.\n"; + + async function writeTemplateSkill(skillName: string, content: string): Promise { + const templateSkillPath = path.join(templateSkillsRoot, skillName); + await fs.ensureDir(templateSkillPath); + await fs.writeFile(path.join(templateSkillPath, "SKILL.md"), content, "utf-8"); + } + + async function writeCodexSkill(skillName: string, content: string): Promise { + const codexSkillPath = path.join(tmpDir, ".codex", "skills", skillName); + await fs.ensureDir(codexSkillPath); + await fs.writeFile(path.join(codexSkillPath, "SKILL.md"), content, "utf-8"); + return codexSkillPath; + } + + it("should remove a .codex skill copy that is byte-identical to the managed template", async () => { + await writeTemplateSkill("managed-skill", managedSkillContent); + const obsoleteSkillPath = await writeCodexSkill("managed-skill", managedSkillContent); + + await configurer.configure(tmpDir); + + expect(await fs.pathExists(obsoleteSkillPath)).toBe(false); + expect(await fs.pathExists(path.join(tmpDir, ".codex", "skills"))).toBe(false); + }); + + it("should preserve a user-customized skill whose SKILL.md metadata matches a managed template", async () => { + await writeTemplateSkill("managed-skill", managedSkillContent); + const customizedSkillPath = await writeCodexSkill( + "managed-skill", + "---\nname: managed-skill\ndescription: Managed.\n---\n\n# Managed\n\nUser customized body.\n" + ); + + await configurer.configure(tmpDir); + + expect(await fs.readFile(path.join(customizedSkillPath, "SKILL.md"), "utf-8")).toContain( + "User customized body." + ); + }); + + it("should preserve user files inside a former managed skill directory", async () => { + await writeTemplateSkill("managed-skill", managedSkillContent); + const skillWithUserFilesPath = await writeCodexSkill("managed-skill", managedSkillContent); + await fs.writeFile(path.join(skillWithUserFilesPath, "user-notes.md"), "# My notes\n", "utf-8"); + + await configurer.configure(tmpDir); + + expect(await fs.pathExists(path.join(skillWithUserFilesPath, "SKILL.md"))).toBe(true); + expect(await fs.readFile(path.join(skillWithUserFilesPath, "user-notes.md"), "utf-8")).toContain( + "My notes" + ); + }); + + it("should preserve user-created skills that have no managed template counterpart", async () => { + await writeTemplateSkill("managed-skill", managedSkillContent); + const userSkillPath = await writeCodexSkill( + "my-custom-skill", + "---\nname: my-custom-skill\ndescription: User owned.\n---\n\n# User\n\nKeep me.\n" + ); + + await configurer.configure(tmpDir); + + expect(await fs.readFile(path.join(userSkillPath, "SKILL.md"), "utf-8")).toContain("Keep me."); + expect(await fs.pathExists(path.join(tmpDir, ".codex", "skills"))).toBe(true); + }); + + it("should be idempotent across repeated repair runs", async () => { + await writeTemplateSkill("managed-skill", managedSkillContent); + const obsoleteSkillPath = await writeCodexSkill("managed-skill", managedSkillContent); + const userSkillPath = await writeCodexSkill( + "my-custom-skill", + "---\nname: my-custom-skill\ndescription: User owned.\n---\n\n# User\n\nKeep me.\n" + ); + + await configurer.repair!(tmpDir); + await configurer.repair!(tmpDir); + + expect(await fs.pathExists(obsoleteSkillPath)).toBe(false); + expect(await fs.readFile(path.join(userSkillPath, "SKILL.md"), "utf-8")).toContain("Keep me."); + }); + }); + describe("getPlannedFileChanges()", () => { it("should return planned changes for the Codex hooks file", async () => { const changes = await configurer.getPlannedFileChanges(tmpDir);