mirror of
https://github.com/software-mansion/argent.git
synced 2026-09-14 19:27:14 +08:00
filip131311
c56cfa47f6
Fixes #701. ## The problem `copyRulesAndAgents` copied the bundled trees onto each managed directory with one recursive `fs.cpSync`. When the destination is a symlink at a directory elsewhere, `fs.cp` refuses to overwrite it: ``` Could not copy agents to /repo/.claude/agents: Error: Cannot overwrite non-directory /repo/.claude/agents with directory .../@swmansion/argent/agents ``` Everything else installs, so it reads as a success apart from one warning line — but `argent-environment-inspector` never lands, while `rules/argent.md` keeps telling the agent to use it. The layout is the point of the report: keep one canonical, harness-neutral copy of the definitions and symlink each vendor path at it, so there's one definition to edit and no drift. The host tools already support that — Claude Code documents symlinks for `.claude/rules/`, and loads argent's skills through exactly such a link today. `rules/` had the same defect; the reporter just hadn't symlinked it. ## It is worse than the report on Node 20 `fs.cp`'s handling of a symlinked destination depends on which runtime you are on — Node 20 and Node 22 ship different implementations, and they disagree exactly here. Measured on both: | destination | Node 20 (JS impl) | Node 22 (C++ impl) | |---|---|---| | top-level symlinked dir | refuses (the reported bug) | refuses (the reported bug) | | nested symlinked dir | refuses | writes through | | symlinked **file** | **replaces the link, canonical left stale** | writes through | | dangling / unwritable dir | catchable throw | **aborts the process, exit 134** | So on Node 20 — which is what this repo's own CI runs — released 0.18.1 silently leaves a symlinked agent file stale on every `update`, with no warning at all. Verified against the released CLI: ``` released 0.18.1, Node 20: symlinked file: canon=stale link=intact (0 warnings) nested symlinked dir: files-in-canonical=0 ``` ## The fix The copy walks the tree itself, resolving each destination as it goes, and copies leaves with `fs.copyFileSync`. Nothing is delegated to `fs.cp`: that is what makes the behaviour the same on every supported runtime, and `copyFileSync` reports each failure — dangling link, loop, unwritable directory, a link pointing at a directory — as a plain catchable errno. The fix lives in `copyDir` rather than in a new helper because that function was already an exact, unused copy of the same buggy shape. Routing `copyRulesAndAgents` through it covers rules and agents, init and update, and removes the latent duplicate. That also settles the case that kills the install outright on Node 22: `fs.cp` does not throw when it cannot create a directory, it aborts through an uncatchable C++ `std::filesystem` exception. Two reachable shapes, both verified against released 0.18.1: - a broken `.claude/agents/references` link (the bundled agents tree really does have that subdirectory) — `argent init` dies at exit 134 mid-run; - **a read-only `.claude/agents`** — same abort, and this one needs no symlink at all, so it hits users of today's release. Both are now ordinary reported warnings. Resolution follows a link chain to its end, resolves each hop against the directory the link really lives in (not the lexical parent — they differ when a parent is itself a link into another subtree), and completes a dangling link only into a directory that already exists, so a link can't conjure a tree at an arbitrary path. Where the bytes land can now differ from the configured path, so the output names both — in the same shortened form the rest of the managed-content output uses: `Copied agents to .claude/agents -> .agents/agents`. A destination outside the workspace stays absolute, so an escape stands out. Nothing changes for a layout with no symlinks. ## Verification Reproduced against released 0.18.1 and a local build, then re-run against the fix. Every row below was executed end-to-end through the real `argent init`: | scenario | before | after | |---|---|---| | `.claude/agents -> ../.agents/agents` (the report) | warning, no files | both files land, link intact | | single symlinked `.md` file inside a real `.claude/agents` | worked | still works (regression test added) | | symlink chain to a real directory | warning, no files | resolves, files land | | link created before its target directory | warning, no files | target completed, files land | | dangling `.claude/agents/references` | **abort, exit 134** | files land, no warning | | **read-only `.claude/agents`, no symlinks** | **abort, exit 134** | `EACCES` warning, install completes | | link at a file / dangling into a missing tree | warning | warning, victim untouched, nothing created | | destination outside the project | writes there | writes there, disclosed absolutely | | `.gemini` / `.opencode` / `.cursor` targets, global `~/.claude` scope | refused | land correctly | | uninstall over a symlinked layout | — | removes only argent's files; user's files, link and directory kept | | re-run over a populated layout (`update`) | — | clean, no warnings | | symlinked file, **Node 20** | **silently stale, no warning** | canonical updated, link intact | | nested symlinked dir, **Node 20** | refused | writes through, link intact | | no symlinks anywhere | works | unchanged, no redirect in output | Every row was run under **both Node 20 and Node 22**, with identical outcomes on each. 15 new unit tests; 552 green on both runtimes. Build, eslint, prettier and the test typecheck all pass. Symlink tests skip on Windows, matching `update.test.ts`. `argent update` is not run here because it acts on the real global npm install, but its call site is byte-identical to init's and the same function was exercised directly in both scopes. ## Deliberately not in this PR - **The summary still prints a green "Rules & agents copied" when a copy failed** (`copiedRules: copyResults.length > 0` in `init.ts`). The issue calls this out as why the failure was easy to overlook. Fixing it honestly means giving `copyRulesAndAgents` a structured return and deciding whether a failed copy should make `argent update` exit non-zero — a separate change, and the general case isn't symlink-specific. Happy to follow up. - **Uninstall leaves both halves of a symlinked layout.** It removes argent's files through the link correctly and never touches the user's link or their other files; it just can't prune the emptied canonical directory (`rmdir` on a symlink fails `ENOTDIR`, swallowed). Not a regression, and arguably right. - **Writing through a link can leave the workspace.** That is pre-existing — `fs.cp` already wrote through symlinked files and nested symlinked directories before this change — and confining it would break the very layout the issue asks for. What this PR does add is bounded: a dangling link is completed only where its parent already exists.