refactor(install): prefer native plugin install across targets#609
Merged
refactor(install): prefer native plugin install across targets#609
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 595bbd1ce1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
tmchow
added a commit
that referenced
this pull request
Apr 20, 2026
- Namespace OpenCode managed state per plugin. resolveOpenCodePaths now threads the sanitized plugin name through managedDir and the install manifest path, matching the Codex namespacing in 0829357. Multiple plugins installed into the same OpenCode root no longer clobber each other's manifests. - Restrict Codex legacy-skill detection to the explicit historical allow-list in EXTRA_LEGACY_ARTIFACTS_BY_PLUGIN. getLegacyCodexArtifacts no longer seeds candidates from bundle.skillDirs/generatedSkills/agents, so a flat ~/.codex/skills/<name>/ that collides with a current CE skill name (e.g. ce-debug) is left untouched on first install.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ff0af3f73a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
tmchow
added a commit
that referenced
this pull request
Apr 20, 2026
Mirror the OpenCode namespacing fix across the other writers that support multi-plugin install roots: - resolveGeminiPaths and resolvePiPaths now thread the sanitized plugin name through the managed-dir segment so each plugin gets its own install-manifest.json under <root>/<plugin> (or <root>/.<target>/<plugin>). - compound-engineering kept as the fallback for bundles without a pluginName. - Extracted a small resolveManagedSegment helper into src/targets/managed-artifacts.ts so the fallback contract lives in one place across OpenCode, Gemini, and Pi. - Added multi-plugin regression tests to tests/gemini-writer.test.ts and tests/pi-writer.test.ts.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f0a6eefae9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
tmchow
added a commit
that referenced
this pull request
Apr 20, 2026
cleanupQwen previously flattened colon-separated legacy command names (e.g. compound:plan) to `compound-plan.md` via sanitizePathName and only probed the flat path. The old Bun Qwen writer used resolveCommandPath semantics, which produced nested `commands/compound/plan.md`. Those stale files survived migration and kept shadowing native plugin commands. Now probe both shapes: the flat sanitized form and, for colon-separated names, the nested `<prefix>/<suffix>.md` form. Each candidate that exists under <qwenRoot>/commands/ is moved to legacy-backup. Test updated to seed both flat and nested stale files and assert both get backed up.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5577036880
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
tmchow
added a commit
that referenced
this pull request
Apr 20, 2026
cleanupDroid previously seeded command/skill/droid candidates from the current bundle, so a user-authored ~/.factory/commands/<name> that happens to share a name with a current CE command would be swept into backup even though it was never installed by this plugin (the Droid writer was removed earlier in this PR). Restricted candidate selection to the historical allow-list in EXTRA_LEGACY_ARTIFACTS_BY_PLUGIN, mirroring the prior Codex fix. Known-legacy entries still get backed up; user-authored files with colliding names are left alone.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2c8ff633a9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
tmchow
added a commit
that referenced
this pull request
Apr 20, 2026
… cleanup (#609) Two follow-ups to the earlier namespacing and legacy-detection fixes: - Upgrade path for pre-namespacing installs. readManagedInstallManifest was only probing the new plugin-scoped path, so a user with a prior install under <root>/compound-engineering/install-manifest.json would skip stale-artifact cleanup on first reinstall after upgrade. Added readManagedInstallManifestWithLegacyFallback in managed-artifacts.ts that falls back to the legacy shared path and reuses the existing pluginName validation (so a legacy manifest owned by a different plugin is left alone for that plugin's own migration). Wired into opencode, gemini, and pi. After the new manifest is written, archiveLegacyInstallManifestIfOwned renames the legacy file into legacy-backup/<timestamp>/ so it can't mislead future installs. - Windsurf cleanup restricted to the historical allow-list, mirroring the prior Codex and Droid fixes. getLegacyWindsurfArtifacts no longer seeds candidates from bundle.skills/agents/commands, so a user-authored skills/ce-debug/ survives cleanup. Windsurf cleanup exists solely to back up stale files from past installs of the removed writer; current-bundle names were never a valid candidate source.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 526e1d981c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
tmchow
added a commit
that referenced
this pull request
Apr 20, 2026
Previously, Codex agents were written as `<sanitizePathName(agent.name)>.toml` with no collision check, so two source agents whose names normalize to the same filename (e.g. case or punctuation variants) would silently overwrite one another and drop one agent from the install without warning. Added assertNoCodexAgentFilenameCollisions, invoked before any agent writes. On collision, throws with both source names in the message so the operator sees the conflict explicitly. Chose a throw rather than numeric-suffix dedup because the TOML filename has to match the name used in `Task(subagent_type: ...)` invocations; a `-2` suffix would make the agent unreachable. Current CE agent names don't collide today, so this is preventive.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 317eecdcf3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
tmchow
added a commit
that referenced
this pull request
Apr 20, 2026
…ot allow-list (#609) Two independent gaps in src/commands/cleanup.ts: - Gemini cleanup's default root diverged from the install default. install/convert write to <cwd>/.gemini when --output is unset (resolveTargetOutputRoot uses process.cwd() for Gemini), but cleanup hardcoded ~/.gemini, so the common "install then cleanup" flow silently skipped the actual workspace artifacts. Default Gemini cleanup now iterates both <cwd>/.gemini (matches install) and ~/.gemini (safety net for older installs). Explicit --gemini-home / --output still override. Scanned the rest of cleanup.ts for the same asymmetry: Gemini was the only outlier. - Copilot cleanup candidates now come exclusively from the historical allow-list in EXTRA_LEGACY_ARTIFACTS_BY_PLUGIN, mirroring the prior Codex/Droid/Windsurf fixes. Dropped bundle.skillDirs/generatedSkills/agents/commands seeding. The Copilot writer was removed earlier in this PR, so user-authored files at names matching current CE artifacts (e.g. .github/skills/ce-debug/) are now preserved; only names on the explicit historical list get backed up.
Install generated Codex skills under a plugin-specific directory, expose them through ~/.agents/skills via symlink, and track managed artifacts with a manifest so removed skills and prompts are cleaned up on reinstall. Also move known legacy reproduce-bug Codex artifacts into a backup path and update tests and docs to match the new install layout.
Drop the personal Claude home sync command and remove custom writer support for native or deprecated targets. Keep install auto-detection for supported custom targets, document native marketplace paths, and add legacy cleanup coverage so stale CE artifacts from old installs can be backed up safely.
Clarify that compound-engineering currently ships skills and agents, not Claude source commands. Keep command handling documented as legacy/source-plugin compatibility and add a parser regression against the real plugin inventory.
Stop emitting deprecated tools config for OpenCode permission modes. Update the OpenCode schema notes and add a regression that the current CE plugin outputs skills plus subagents, not command files.
Remove the custom Qwen converter/writer path now that Qwen can install the Claude-compatible plugin natively, and keep cleanup support for old Bun-installed Qwen artifacts. Move the install strategy into compounded integration docs so future install decisions are discoverable with the rest of the integration learnings.
Convert Claude agents into Codex custom-agent TOML files under
~/.codex/agents/compound-engineering/<name>.toml instead of generating
them as skills. Agent names preserve the source category (e.g.
review-ce-correctness-reviewer), and Task/agent references in prompts
and skills are rewritten to spawn the custom agents. The empirical
test on 2026-04-19 confirmed Codex discovers nested agent TOML files
and accepts hyphenated names.
Add kiro as a cleanup target so historical CE artifacts under
.kiro/{skills,agents,agents/prompts} are backed up on every install
and via `cleanup --target kiro`.
Retire the custom Cursor output target; users install via the native
Cursor Plugin Marketplace (/add-plugin compound-engineering). The
resolve-output cursor branch is removed along with its tests.
Expand known-legacy coverage for Codex, Pi, Kiro, OpenCode, Gemini,
Droid, and Qwen so renamed/removed CE skills, prompts, and commands
(including nested forms like compound:plan and resolve_pr_parallel)
are swept into compound-engineering/legacy-backup on install and
manual cleanup.
- Namespace OpenCode managed state per plugin. resolveOpenCodePaths now threads the sanitized plugin name through managedDir and the install manifest path, matching the Codex namespacing in 0829357. Multiple plugins installed into the same OpenCode root no longer clobber each other's manifests. - Restrict Codex legacy-skill detection to the explicit historical allow-list in EXTRA_LEGACY_ARTIFACTS_BY_PLUGIN. getLegacyCodexArtifacts no longer seeds candidates from bundle.skillDirs/generatedSkills/agents, so a flat ~/.codex/skills/<name>/ that collides with a current CE skill name (e.g. ce-debug) is left untouched on first install.
Mirror the OpenCode namespacing fix across the other writers that support multi-plugin install roots: - resolveGeminiPaths and resolvePiPaths now thread the sanitized plugin name through the managed-dir segment so each plugin gets its own install-manifest.json under <root>/<plugin> (or <root>/.<target>/<plugin>). - compound-engineering kept as the fallback for bundles without a pluginName. - Extracted a small resolveManagedSegment helper into src/targets/managed-artifacts.ts so the fallback contract lives in one place across OpenCode, Gemini, and Pi. - Added multi-plugin regression tests to tests/gemini-writer.test.ts and tests/pi-writer.test.ts.
cleanupQwen previously flattened colon-separated legacy command names (e.g. compound:plan) to `compound-plan.md` via sanitizePathName and only probed the flat path. The old Bun Qwen writer used resolveCommandPath semantics, which produced nested `commands/compound/plan.md`. Those stale files survived migration and kept shadowing native plugin commands. Now probe both shapes: the flat sanitized form and, for colon-separated names, the nested `<prefix>/<suffix>.md` form. Each candidate that exists under <qwenRoot>/commands/ is moved to legacy-backup. Test updated to seed both flat and nested stale files and assert both get backed up.
cleanupDroid previously seeded command/skill/droid candidates from the current bundle, so a user-authored ~/.factory/commands/<name> that happens to share a name with a current CE command would be swept into backup even though it was never installed by this plugin (the Droid writer was removed earlier in this PR). Restricted candidate selection to the historical allow-list in EXTRA_LEGACY_ARTIFACTS_BY_PLUGIN, mirroring the prior Codex fix. Known-legacy entries still get backed up; user-authored files with colliding names are left alone.
… cleanup (#609) Two follow-ups to the earlier namespacing and legacy-detection fixes: - Upgrade path for pre-namespacing installs. readManagedInstallManifest was only probing the new plugin-scoped path, so a user with a prior install under <root>/compound-engineering/install-manifest.json would skip stale-artifact cleanup on first reinstall after upgrade. Added readManagedInstallManifestWithLegacyFallback in managed-artifacts.ts that falls back to the legacy shared path and reuses the existing pluginName validation (so a legacy manifest owned by a different plugin is left alone for that plugin's own migration). Wired into opencode, gemini, and pi. After the new manifest is written, archiveLegacyInstallManifestIfOwned renames the legacy file into legacy-backup/<timestamp>/ so it can't mislead future installs. - Windsurf cleanup restricted to the historical allow-list, mirroring the prior Codex and Droid fixes. getLegacyWindsurfArtifacts no longer seeds candidates from bundle.skills/agents/commands, so a user-authored skills/ce-debug/ survives cleanup. Windsurf cleanup exists solely to back up stale files from past installs of the removed writer; current-bundle names were never a valid candidate source.
Previously, Codex agents were written as `<sanitizePathName(agent.name)>.toml` with no collision check, so two source agents whose names normalize to the same filename (e.g. case or punctuation variants) would silently overwrite one another and drop one agent from the install without warning. Added assertNoCodexAgentFilenameCollisions, invoked before any agent writes. On collision, throws with both source names in the message so the operator sees the conflict explicitly. Chose a throw rather than numeric-suffix dedup because the TOML filename has to match the name used in `Task(subagent_type: ...)` invocations; a `-2` suffix would make the agent unreachable. Current CE agent names don't collide today, so this is preventive.
…ot allow-list (#609) Two independent gaps in src/commands/cleanup.ts: - Gemini cleanup's default root diverged from the install default. install/convert write to <cwd>/.gemini when --output is unset (resolveTargetOutputRoot uses process.cwd() for Gemini), but cleanup hardcoded ~/.gemini, so the common "install then cleanup" flow silently skipped the actual workspace artifacts. Default Gemini cleanup now iterates both <cwd>/.gemini (matches install) and ~/.gemini (safety net for older installs). Explicit --gemini-home / --output still override. Scanned the rest of cleanup.ts for the same asymmetry: Gemini was the only outlier. - Copilot cleanup candidates now come exclusively from the historical allow-list in EXTRA_LEGACY_ARTIFACTS_BY_PLUGIN, mirroring the prior Codex/Droid/Windsurf fixes. Dropped bundle.skillDirs/generatedSkills/agents/commands seeding. The Copilot writer was removed earlier in this PR, so user-authored files at names matching current CE artifacts (e.g. .github/skills/ce-debug/) are now preserved; only names on the explicit historical list get backed up.
tmchow
force-pushed
the
tmchow/installer-cleanup-targets
branch
from
f129f1c to
5d523f0
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5d523f00b7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…609) install/convert previously hardcoded the OpenCode global config path to ~/.config/opencode, ignoring OPENCODE_CONFIG_DIR. That caused stale files for users on NixOS, in Docker, or with a custom XDG_CONFIG_HOME, since OpenCode itself reads from OPENCODE_CONFIG_DIR but we wrote to the XDG default. - resolveOutputRoot in src/commands/install.ts now checks OPENCODE_CONFIG_DIR first and falls back to ~/.config/opencode. No behavior change when the env var is unset. - When the output root is the OpenCode global config tree (either the XDG default or OPENCODE_CONFIG_DIR), install now passes scope="global" to writeOpenCodeBundle so resolveOpenCodePaths picks the flat layout regardless of the basename. Without this, an OPENCODE_CONFIG_DIR with a non-conventional basename (e.g. /run/user/1000/opencode-xyz) would fall through to the nested ".opencode/" project layout. - Added a writer regression test with a non-"opencode"/".opencode" basename and scope="global" to lock in the flat layout for OPENCODE_CONFIG_DIR-derived roots. Closes #573. Original fix by @cavanaug against the pre-cleanup codebase; the sync/registry.ts half of that PR is obsolete (the sync command was removed in this PR), and resolveOpenCodePaths now takes a pluginName for per-plugin namespacing, so the env-var + scope-threading substance is carried forward on the new code path. Co-Authored-By: John Cavanaugh <cavanaug@users.noreply.github.com>
) Two more cleanup-correctness issues in src/commands/cleanup.ts, continuing the pattern of fixes this PR has been working through. - Dedup parallel cleanup roots by canonical path before Promise.all. When cwd == $HOME, cleanupGemini's workspaceGemini (<cwd>/.gemini) and geminiHome (~/.gemini) resolve to the same directory, so the two concurrent passes race to move the same artifact and one fails with ENOENT. dedupeRoots canonicalizes via fs.realpath (falling back to path.normalize when the dir doesn't exist yet) and drops dupes. realpath is required, not just path.resolve — on macOS `$HOME` and `process.cwd()` can be string-different but point at the same inode. Applied defensively to the Copilot (copilotHome + workspace .github + agentsHome) and Windsurf (windsurfHome + workspace .windsurf) parallel cases too, so user overrides can't re-introduce the race. - Restrict cleanupQwen to the historical allow-list. Mirrors the Codex/Droid/Windsurf/Copilot fixes from earlier rounds: dropped plugin.skills/agents/commands seeding; candidates come exclusively from EXTRA_LEGACY_ARTIFACTS_BY_PLUGIN. User-authored files at current-bundle names (e.g. ~/.qwen/skills/ce-debug/) now survive. The namespaced-command dual-probe from the earlier Qwen fix (both flat `compound-plan.md` and nested `compound/plan.md`) is preserved and still fires on allow-list entries containing `:`.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8889556c51
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…low (#609) Follow-up to the OPENCODE_CONFIG_DIR port. That fix introduced two coherence bugs: the OpenCode-specific default in resolveOutputRoot bled into other primary targets via the --also flow's path.join(outputRoot, extra) construction, and cleanup still hardcoded ~/.config/opencode so install and cleanup disagreed on where OPENCODE_CONFIG_DIR installations live. - Extracted resolveOpenCodeGlobalRoot into src/utils/opencode-config.ts. Install and cleanup now read OPENCODE_CONFIG_DIR through the same helper, so the env-var-vs-XDG fallback lives in exactly one place. - Simplified resolveOutputRoot in install.ts and convert.ts to plain cwd-or-explicit. The OpenCode global-config decision moved into resolveTargetOutputRoot alongside codex and pi, where all other per-target path computation already lives. - Removed the path.join(outputRoot, extra) mangling in the --also loops. Each extra target's root comes from resolveTargetOutputRoot directly. Incidentally repairs a pre-existing latent bug where --output X --also gemini produced <X>/gemini/.gemini instead of <X>/.gemini (and similarly for kiro). - Write-scope selection for OpenCode centralized in resolveOpenCodeWriteScope, used by --to, --to all, and --also in both install.ts and convert.ts. - cleanup --target opencode now respects OPENCODE_CONFIG_DIR through the shared helper. --opencode-home arg description updated to reflect env precedence. Regression tests cover: --to codex --also opencode (OpenCode lands at global root, not nested under it); --to opencode --also codex (symmetry); --to opencode honoring OPENCODE_CONFIG_DIR; cleanup honoring OPENCODE_CONFIG_DIR; resolve-output unit tests for OPENCODE_CONFIG_DIR precedence and scope resolution.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 20dead0897
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
detect-tools was the third place OpenCode's default root needs to be resolved consistently; install and cleanup already route through the shared resolveOpenCodeGlobalRoot helper after the earlier fixes. Previously detection only probed ~/.config/opencode and <cwd>/.opencode, so users on NixOS/Docker/custom XDG with OPENCODE_CONFIG_DIR set would see --to all silently skip OpenCode. Detection now calls resolveOpenCodeGlobalRoot so the env-var-vs-XDG fallback lives in exactly one place. Workspace <cwd>/.opencode probe unchanged. Tests cover the env-var-present and env-var-absent paths.
…een CE versions cleanup --target codex previously only checked names on the historical allow-list (EXTRA_LEGACY_ARTIFACTS_BY_PLUGIN). That catches renamed and removed skills/prompts but misses artifacts whose *emission format* changed between CE versions. The concrete case: CE previously emitted agents as generated skills under skills/<plugin>/<agent-name>/ and is now emitting them as TOML custom agents under agents/<plugin>/<name>.toml. Those stale agent-as-skill directories weren't on the historical allow- list (the names are CURRENT agent names, just in the wrong location) so cleanup left them in place. A subsequent install would self-clean via the managed-install manifest, but cleanup run alone left skills double-registered. Fix: cleanupCodex now reads the Codex install manifest and migrates any manifest-listed skills/prompts/agents that are not in the current bundle to legacy-backup. This gives cleanup parity with install-time cleanup — running cleanup alone completes the migration, no followup install required. readInstallManifest in src/targets/codex.ts was private. Exported as readCodexInstallManifest for the cleanup path. Regression test: a fake prior install with three current-named agents installed as namespaced skills + a stale prompt + a survivor skill, verified cleanup migrates the stale three + prompt while preserving the current-named skill that's still in the bundle. Surfaced during Unit 7 empirical verification of #616 (Codex native plugin install). Without this fix, users migrating from a pre-TOML- agents CE version would silently accumulate duplicate skill registrations unless they happened to run `install` after `cleanup`.
tmchow
added a commit
that referenced
this pull request
Apr 21, 2026
Adds a ### Codex section between Cursor and the experimental-Bun list, matching the native-install pattern used by Claude Code and Cursor. Documents: - codex plugin marketplace add + codex plugin install commands for skills registration - The followup bunx ... --to codex step required for custom agents, with a clear explanation of why (Codex plugin format does not register custom agents or slash commands per spec) - cleanup --target codex pointer for users migrating from the Bun-only install - coding-tutor-specific note that its slash commands still require the Bun converter The existing "OpenCode, Codex, ... (experimental)" section still mentions Codex; that list gets pruned in a followup when PR #609 lands, since #609 restructures the experimental list and removes Codex from it.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 62d8395db3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…ns (#609) Adds isSafeManagedPath helper and applies it at both read time (on managed and Codex install manifests) and as defense-in-depth inside cleanup helpers. Rejects absolute paths, `..` segments, and any entry that resolves outside its target root, preventing a poisoned manifest from directing fs.rm at arbitrary filesystem locations.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6108e787d0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- OpenCode cleanup now honors --output so workspace-scoped installs (install --to opencode --output <workspace>) migrate from the matching <workspace>/.opencode tree instead of silently scanning the global root. resolveOpenCodeWorkspaceRoot follows the same explicit-home > explicit-output > default precedence already used by Gemini cleanup. - Pi manifest cleanup now rejects unsafe paths via the shared isSafeManagedPath helper at read time and again as defense-in-depth inside cleanupRemovedSkills/Prompts/Extensions, scoped per-group against each cleanup root since Pi's three trees are siblings rather than nested. Mirrors the protection added for shared managed manifests and Codex.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1170c7cf7f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
cleanupCodexSharedAgents previously moved any matching entry under ~/.agents/skills into legacy-backup based on name alone, which could back up user-created skills whose names collide with CE's legacy list. Now it only touches entries that are symlinks whose resolved target lives inside a CE-managed Codex root, matching the pattern used by cleanupLegacyAgentsSkillSymlinks. Plain files and symlinks pointing elsewhere are left alone. Extracted the ownership check and managed- root builder into shared helpers in src/targets/codex.ts so install and cleanup share one gate.
tmchow
added a commit
that referenced
this pull request
Apr 21, 2026
Adds a ### Codex section between Cursor and the experimental-Bun list, matching the native-install pattern used by Claude Code and Cursor. Documents: - codex plugin marketplace add + codex plugin install commands for skills registration - The followup bunx ... --to codex step required for custom agents, with a clear explanation of why (Codex plugin format does not register custom agents or slash commands per spec) - cleanup --target codex pointer for users migrating from the Bun-only install - coding-tutor-specific note that its slash commands still require the Bun converter The existing "OpenCode, Codex, ... (experimental)" section still mentions Codex; that list gets pruned in a followup when PR #609 lands, since #609 restructures the experimental list and removes Codex from it.
tmchow
added a commit
that referenced
this pull request
Apr 22, 2026
Add an invariant test that fails when a name in STALE_SKILL_DIRS or in EXTRA_LEGACY_ARTIFACTS_BY_PLUGIN["compound-engineering"].skills matches a current directory under plugins/compound-engineering/skills/. If a retired skill is ever re-added to the plugin, the stale registry entry would make cleanupStaleSkillDirs() fingerprint-match the freshly-installed skill and delete it on every install. The test caught a pre-existing collision: ce-update was erroneously seeded into the legacy artifacts list in #609 even though it has been a live skill since #538. Remove it from the registry and flip the assertions in plugin-legacy-artifacts.test.ts and cli.test.ts that were locking in the buggy behavior (the CLI test was effectively asserting that cleanup would move a user's live ce-update install into legacy-backup). Export STALE_SKILL_DIRS so the invariant test can import it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Claude Code, Cursor, GitHub Copilot CLI, Factory Droid, and Qwen Code now install compound-engineering directly from the plugin marketplace. Custom Bun writers were only worth maintaining when the target platform couldn't read the Claude plugin format itself. Five can now, so the custom Copilot, Droid, Qwen, Windsurf, and OpenClaw paths are gone along with the
synccommand. The converter CLI now focuses on the five targets that still need translation: OpenCode, Codex, Pi, Gemini CLI, and Kiro.Migrating users don't have to hunt down stale artifacts. A new
cleanupcommand walks a catalog of known legacy CE files across every supported target and moves them tocompound-engineering/legacy-backup, so previous installs don't shadow the native plugin.Install surface after this PR
/plugin install compound-engineering/add-plugin compound-engineering(marketplace)copilot plugin install compound-engineering@compound-engineering-plugindroid plugin install compound-engineering@compound-engineering-pluginqwen extensions install EveryInc/compound-engineering-plugin:compound-engineeringbunx @every-env/compound-plugin install compound-engineering --to <target>Windsurf and OpenClaw are no longer supported.
Key design decisions
Codex agents emit as TOML custom agents, not skills. Agents land at
~/.codex/agents/compound-engineering/<category>-ce-<name>.toml.TaskandAgentreferences in prompts/skills are rewritten to spawn these custom agents. The 2026-04-19 empirical test confirmed Codex walks nested agent TOML directories and accepts hyphenated filenames, so the category prefix (e.g.review-ce-correctness-reviewer) survives the round trip.Managed Codex skills are namespaced and manifest-tracked. Generated skills install under a plugin-specific directory and are exposed through
~/.agents/skillsvia symlink. A manifest records what was written so a reinstall cleans up skills that have since been removed from the plugin, rather than leaving orphans.cleanup --target <t>is safe to re-run. It never deletes — it moves known-legacy artifacts intocompound-engineering/legacy-backup. The catalog insrc/data/plugin-legacy-artifacts.tscovers renamed and removed CE skills/prompts/commands (including nested forms likecompound:planandresolve_pr_parallel) across Codex, Pi, Kiro, OpenCode, Gemini, Droid, and Qwen.OpenCode output now matches the current permissions API. The deprecated
toolsconfig block was dropped; permission modes emit the current shape. Writer tests updated alongside.Testing
bun testpasses. The sync/* test suite and target-specific writer tests for copilot, droid, qwen, windsurf, and openclaw are deleted along with the code they covered.tests/cli.test.tsgained ~570 lines of coverage for the new install/cleanup flows, andtests/plugin-legacy-artifacts.test.tslocks down the legacy catalog.Notes for reviewers
src/data/plugin-legacy-artifacts.ts.compound-engineeringtogether. That's intentional, not a misconfiguration.