feat(skills): install the Forest plugin on Claude Code / Codex, copy the skills for the rest - #814
Open
Gawtier wants to merge 14 commits into
Open
feat(skills): install the Forest plugin on Claude Code / Codex, copy the skills for the rest#814Gawtier wants to merge 14 commits into
Gawtier wants to merge 14 commits into
Conversation
… repo Project-scoped, marketplace-independent onboarding for the client's coding agent (Claude Code + Codex). Fetches the public ForestAdmin/ai-marketplace, drops all 8 skill bundles (forest/* + forest-code + forest-legacy) into .claude/skills/ and .agents/skills/, merges a Forest block into CLAUDE.md/AGENTS.md, and wires the forest-docs MCP (https://docs.forest.app/mcp, no secret) from the marketplace. Includes 11 unit tests (skills-manager) and conforms to repo conventions (array methods, declared `tar` dep, no jest hooks). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
On the no-force skip path, installSkills reported the bundle-derived file list as managed even though nothing was written. A user-authored skill dir that pre-existed the first skills:init (e.g. their own .claude/skills/layout/SKILL.md) was thus recorded in the manifest, and a later refresh — once the skill left the upstream bundle — would prune it, deleting user data. installSkills now takes the previous manifest's file list (null on a first run) and, on the skip path, only carries over the intersection of the bundle-derived list and that previous manifest: files a past run provably wrote. First run over a pre-existing dir claims nothing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… write installDocsMcp runs last (after skill installs, CLAUDE.md/AGENTS.md merges and stale pruning), so its deliberate throw on an unparseable local .mcp.json — or a raw ENOENT on a bundle missing forest-docs/.mcp.json — left a half-applied install with no manifest rewrite. skills:init now fails fast: validateLocalMcp() runs before any disk mutation, and validateMarketplaceBundle() checks the fetched bundle right after the fetch, before the first write. Both are exported from skills-manager (and unit-tested) so skills:update can reuse them. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…i-drift) Refresh installed skills from the marketplace: overlay the current bundle, prune Forest-managed files that left it (manifest-scoped, user files kept), refresh the CLAUDE.md/AGENTS.md block + docs MCP, bump the manifest. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Two review fixes on the update command, mirroring skills:init: - Atomicity: run validateLocalMcp() before any disk mutation and validateMarketplaceBundle() right after the fetch, so a broken local .mcp.json or a bundle without the docs MCP config aborts BEFORE the skills/context files are touched — no more half-applied refresh with a manifest that lies about it. - Ref pinning: an install pinned via --ref (tag/SHA) used to jump silently to main on the first plain `skills:update`. The update still targets the requested ref (default main), but the transition is now logged with the exact --ref flag to pass to stay pinned. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Command-level tests (testCli harness, network fetch mocked to a fake extracted bundle) for the update pipeline: - no manifest → clear error, non-zero exit, marketplace never fetched; - unparsable local .mcp.json → fail fast before any fetch/mutation; - full refresh: managed files overwritten from the bundle, files gone upstream pruned, freshly installed files spared (argument order of removeStaleSkillFiles), manifest rewritten with the effective ref; - ref transition logged when the manifest was pinned to another ref; - --ref passthrough to the fetch and the rewritten manifest. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
`skills:init` re-implemented by hand what both agents now do natively. The two distribution families are no longer the same: - Claude Code and Codex have a plugin system with a non-interactive CLI. Driving it (`claude plugin install --scope project`, `codex plugin add`) gives us versioning, auto-update, the docs MCP and the plugin's slash commands (`/forest:start`) for free, and copies nothing into the client's repo. Both read the same `.claude-plugin/marketplace.json`, so one catalog serves both (verified against claude-code 2.1.100 and codex-cli 0.147.0). - Cursor, OpenCode and the other SKILL.md-compatible agents have no such mechanism, so file copy stays — but only for them. What that removes: the `.mcp.json` merge (a Claude Code file, and Claude Code now gets the docs MCP from the `forest-docs` plugin) along with its two preflight validators, and the duplicate `.claude/skills/` copy. One skills dir remains, `.agents/skills/`, which Cursor, OpenCode and Codex all read. The command now asks which agent(s) the user works with — multi-select, since Claude Code + Cursor is a common pair — pre-checked from detection, and skippable with a repeatable `--agent` flag required in non-interactive runs (npx, CI). Also fixes a data-loss bug on the refresh path: `skills:update` overwrote a user-authored SKILL.md that collided with a bundle path, even though `skills:init` had correctly refused to claim it. `previousFiles` now bounds what may be overwritten, not just what may be claimed, and the kept files are reported. Reproduced end-to-end before and after. Tests: `skills:init` gains the command-level suite it never had (both routes, their combination, --ref passthrough, missing CLI, partial plugin failure); the service suite covers the plugin-CLI argv per agent, ref pinning, detection and the new overwrite guard. 48 tests. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ction detectAgents() treated "the binary answers --version" as equal evidence to a mark left in the repo. On a machine with several agent CLIs installed — which is the norm for anyone evaluating them — Claude Code and Codex came pre-checked in every repo, whatever that repo actually uses. Confirming the prompt then installed plugins the user never picked, which reads as the multi-select not letting you choose at all. Repo signals (.claude/, CLAUDE.md, .cursor/, .cursorrules, .opencode/, opencode.json, .codex/) now decide on their own when there is at least one. The PATH is consulted only when the repo says nothing — the fresh-project case (`npx create-forest`), where what you have installed is the only signal there is. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
inquirer appends its own "(Press <space> to select…)" to the FIRST render only: it vanishes on the first keypress, which is exactly when the user needs it — arrow keys move the cursor without selecting anything, so a user who starts by pressing down or enter has no way left to learn that space is the toggle. Moving the hint into the question text keeps it on screen for every frame, since inquirer re-renders the message each time. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The comment justified the exclusion by its install-time credential prompt, which reads as "an opt-in we could add later". That is not the reason. forest-mcp is a data-access server for querying a live project's records — not developer help — so it does not belong in a command whose job is to teach the agent Forest, whatever its auth policy. No behaviour change: it was already absent from both routes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…plugin route The copy route followed a hand-picked list of skill names while the plugin route installs whole plugins. The two disagreed, and the disagreement was not cosmetic: the list dropped `deploy-heroku`, but `onboard` — which IS copied — hands the production step to it in four places, as do `management`, `boot-standalone-agent` and the `/forest:deploy` command. Every Cursor and OpenCode user therefore had an `onboard` skill pointing at a skill that was never installed, at the most important step of the flow. The list also mislabelled `deploy-heroku` as internal/test-only. It is the user-facing production deploy, and the marketplace treats it as such. `SKILL_SOURCES` becomes `SKILL_PLUGINS`: the plugins to take skills from, all of their skills discovered from the bundle. What ships is now decided in one place, the marketplace, by what a plugin contains — a second definition here could only drift. Copy route goes from 7 skills to 8, identical to the plugin route. The two remaining differences are inherent to the formats, not choices: slash commands and the docs MCP exist only as plugin components, and the copy-route agents read `skills/` alone. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The per-skill filter went in the previous commit; two plugin lists remained, `SKILL_PLUGINS` for the copy route and `FOREST_PLUGINS` for the plugin route. Two lists that must agree are one list waiting to disagree — the exact shape of the bug just fixed, one level up. `FOREST_PLUGINS` is now the single decision this command makes, applied to both routes. A plugin without a `skills/` directory is skipped rather than rejected, because `forest-docs` legitimately carries only an MCP config; the fail-loud guard moves to what actually signals a broken bundle — copying no skill at all. What is left is the one filter worth keeping: WHICH plugins ship. That is a product decision (forest-mcp is data access, not developer help) and plugins are self-contained, so excluding one breaks nothing. Filtering INSIDE a plugin is what breaks things: its skills cross-reference each other. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Aug 18, 2026
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
- installSkills destroyed a user file sitting at `.agents/skills/<skill>`: it was rm -rf'd to make room for the directory, on a first non-`--force` install too. A skill is always a directory, so a regular file there is the user's — it is now preserved and reported. - `--force` overwrote colliding files with no previous manifest, because the overwrite predicate short-circuited to true when `previousFiles` was null. The flag re-writes what a previous run wrote; it does not license destroying files we never wrote. `isManaged` is now the only gate. (New files are unaffected — the guard only ever sees paths that already exist on disk.) - AGENTS.md is written by Codex (plugin route) AND Cursor/OpenCode (copy route). One block per agent meant the second merge replaced the first — same delimiters — so the file described only whichever route ran last. `forestBlock` now takes the agents sharing a context file and covers every route feeding it, and `contextFileGroups` guarantees one merge per file. - A plugin agent whose CLI is missing was skipped, yet still got its context file and a manifest entry, claiming an install that never happened and sending `skills:update` after a plugin that is not there. Only agents actually set up are recorded now. Also: a manifest with no `agents` predates the field, so it can only be a copy-route install. Reading it as "no agents" refreshed nothing AND rewrote the manifest without its files, orphaning every skill on disk. `installSkills` split into a per-skill helper to stay under the complexity rule. 60 tests, one per finding. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
I only read the first four comments on #814; there were nine. The five missed ones are all valid, and three are escapes my own guards were written to prevent but checked one level too shallow. - copyDir checked the destination for a symlink but never its ANCESTORS, and `mkdirSync(..., { recursive: true })` follows them: a symlinked `.agents` or `.agents/skills` sent every copied file outside the project. Checked once before any write now, so the run aborts whole rather than half-applied. - listSkillDirs filtered child entries but not the `<plugin>/skills` root itself. copyDir's source-symlink check only ever saw real children, so a crafted marketplace could symlink that root at any local directory and have its contents copied in. - isWithinSkillDirs required the real parent to sit under the PROJECT root, which a skill subdirectory symlinked at `src/` satisfies — so a crafted manifest entry pruned `src/index.ts`. Containment is now against the real skills dir, which must itself resolve inside the project. - removeStaleSkillFiles normalized separators for the comparison only: existsSync and rmSync still received `.agents\skills\…`, which resolves to nothing on Unix, so a manifest written on Windows pruned nothing. It acts on the normalized path now (Node accepts forward slashes on Windows too). - readManifest let a non-array `agents` ({}, a string) through to callers' `.filter`. It is normalized to undefined instead of rejecting the manifest, since a manifest with no `agents` is a legitimate pre-field copy install. 65 tests, one per finding — including the near-miss the project-root check used to authorise, and the symlinked-ancestor abort verified end-to-end. Co-Authored-By: Claude Opus 5 (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
2 participants
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.
What
forest skills:init/forest skills:update— give a client's coding agent the Forest skills. Two routes, because coding agents split in two families:.claude/settings.json(12 lines, committable) — nothing elseSKILL.mdfiles.agents/skills/(8 skills)The command asks which agent(s) you use — multi-select, since Claude Code + Cursor is a common pair — pre-checked from repo detection, skippable with a repeatable
--agentflag that is required in non-interactive runs (npx, CI).Why not copy files for everyone (what #804 did)
Claude Code and Codex both have a plugin system with a non-interactive CLI, and both read the same
.claude-plugin/marketplace.json— so our existing catalog serves both with no second manifest. Driving it gives us versioning, auto-update, the docs MCP and the plugin's slash commands (/forest:start,/forest:deploy) for free, and copies nothing into the client's repo.That deletes the whole class of machinery #804 had to hand-roll: the
.mcp.jsonmerge and its two preflight validators (.mcp.jsonis a Claude Code file — Cursor reads.cursor/mcp.json, OpenCode readsopencode.json— and Claude Code now gets the docs MCP from theforest-docsplugin), plus the duplicate.claude/skills/copy. One skills dir remains,.agents/skills/, which Cursor, OpenCode and Codex all read.Verified against
claude-code 2.1.100andcodex-cli 0.147.0.Two bugs fixed on the way
Data loss on refresh.
skills:updateoverwrote a user-authoredSKILL.mdthat collided with a bundle path, even thoughskills:initcorrectly refused to claim it.previousFilesnow bounds what may be overwritten, not just what may be claimed, and kept files are reported. Reproduced end-to-end before and after.Dangling skill reference, already shipped. The copy route followed a hand-picked list of skill names while the plugin route installs whole plugins. The list dropped
deploy-herokuas "internal/test-only" — it is the user-facing production deploy, referenced byonboard(4×),management,boot-standalone-agentand the/forest:deploycommand. Every Cursor/OpenCode user had anonboardskill pointing at a skill that was never installed, at the most important step of the flow. There is now one list,FOREST_PLUGINS, read by both routes, and nothing is filtered below the plugin: a plugin is the unit of distribution and is coherent only as a whole.forest-mcpstays out — it is data access, not developer help.Detection and the prompt
Repo signals (
.claude/,CLAUDE.md,.cursor/,.opencode/, …) decide on their own when there is at least one; the PATH is consulted only when the repo is silent — the fresh-project case, where what you have installed is the only signal. Otherwise anyone with several agent CLIs installed gets them all pre-checked in every repo, and confirming the prompt installs plugins they never picked. The "press space" hint lives in the question text, because inquirer's own hint is appended to the first render only and vanishes on the first keypress — exactly when it is needed, since arrow keys move without selecting.Tested
skills:initgains the command-level suite it never had: both routes, their combination,--refpassthrough, missing CLI, partial plugin failure. The service suite covers the plugin-CLI argv per agent, ref pinning per CLI syntax, detection precedence, and the overwrite guard.main.)forest,forest-code,forest-docson both agents and writes only.claude/settings.json; copy route lands 8 skills in.agents/skills/;skills:updaterefreshes both; and the clobber regression is reproduced green.Note for the reviewer
@PMerlet — your two
skills:initfixes and theskills:updatehardening are all still in here and still load-bearing: the manifest-claiming guard is what the new overwrite guard extends, and the ref-transition warning is unchanged. What went away is only what the plugin route makes redundant. Sorry for the moving target on the design — the Codex plugin CLI turned out to read our Claude marketplace, which is what made this shape possible.Note
Add
forest skills:initandforest skills:updatecommands to install Forest skills for coding agentsskills:initinstalls Forest skills via native plugin CLIs for Claude Code and Codex, and copies skill files into.agents/skills/for Cursor, OpenCode, and others.skills:updatere-runs the appropriate install route using a persisted manifest at.forest/skills-manifest.json, overwriting previously managed files and pruning those removed upstream.CLAUDE.mdorAGENTS.mdwithout clobbering user content, and detect installed agents automatically when no--agentflag is provided.--ref(defaultmain) and extracted using the newtardependency.spawnSyncagainst agent CLIs; missing or incompatible CLIs silently skip that agent rather than aborting.Macroscope summarized 3dbc585.