From eff3500d4dd639616488110202a41fc1d5fcc180 Mon Sep 17 00:00:00 2001 From: meganemura Date: Sun, 2 Aug 2026 00:35:10 +0900 Subject: [PATCH] fix(installer): stop swapping symlinked config files for detached copies (shared AGENTS.md setups) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit atomicWriteFileSync landed its temp file on the destination via renameSync, which replaces a symlink itself rather than its target. Setups that symlink one shared instructions file into each agent's expected location (~/.claude/CLAUDE.md -> ~/AGENTS.md, ~/.codex/AGENTS.md -> ~/AGENTS.md, ...), and dotfiles-managed configs, silently lost the link: the file became a detached regular-file copy, and later edits to the shared source never reached the file the agent actually reads. Resolve the symlink chain first (manually — realpathSync throws on dangling links, and creating a dangling link's target must keep working) and run the temp-file-plus-rename against the real target, keeping the write atomic on the target's own filesystem. When several selected agents point at the same shared file, the marker-based upsert dedupes across them: the guidance block is written exactly once. Co-Authored-By: Claude Fable 5 --- CHANGELOG.md | 1 + __tests__/installer-targets.test.ts | 154 ++++++++++++++++++++++++++++ src/installer/targets/shared.ts | 35 ++++++- 3 files changed, 189 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 62952b961..c6a5f64f7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -42,6 +42,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - When a `codegraph_explore` answer shows a file as excerpts, a large excerpt that no longer fit was dropped entirely instead of being shortened. If the file's first excerpt happened to be a trivial one — an import block, a one-line helper next to the code you asked about — the excerpt carrying the actual answer was the one thrown away, and the file came back with a quarter of the room it had been given. On real projects that meant the top-ranked file delivered a fraction of its share while a far less relevant file took the rest. Excerpts are now shortened to fit, whole method by whole method, and only dropped when what is left is too small to hold anything readable. - When you name a symbol in a `codegraph_explore` query, its definition now actually comes back. Two cases previously lost it. If the symbols you named don't call one another — sibling functions inside the same factory or module are the everyday example — CodeGraph stopped treating them as symbols you had asked for, and answered with whatever sat at the top of their file instead; on one 1,400-line file that meant a same-stem `QueuedMessage` interface on line 70 came back while the `queueMessage` function on line 1087 did not. And when an answer had to be trimmed to fit, it was trimmed from the bottom of the file down, so a symbol near the end of a long file was always the first thing cut. Trimming now protects the definitions you named wherever they sit in the file. - The blast-radius section of `codegraph_explore` flagged "no covering tests found" whenever no test called a symbol directly — falsely branding helpers that tests exercise through their callers as untested (about 40% of flagged symbols in a measured sample). The check now follows caller chains up to 3 hops and reports indirect coverage as "tested via callers"; when nothing is found it states exactly what was checked instead of an unconditional warning. Thanks @inth3shadows for measuring the false-positive rate. (#1475) +- `codegraph install` no longer replaces symlinked config files with regular files — writes now follow the symlink and update its real target. Setups that share one `AGENTS.md` across agents via symlinks (for example `~/.claude/CLAUDE.md` and `~/.codex/AGENTS.md` both pointing at one file), and dotfiles-managed configs, keep receiving edits; installing several agents wired to the same shared file writes its guidance block exactly once. If a previous install already turned your symlink into a regular file, restore the link once and future runs will preserve it. Thanks @0x1306a94 for first diagnosing this and proposing a fix in #433. ## [1.5.0] - 2026-07-21 diff --git a/__tests__/installer-targets.test.ts b/__tests__/installer-targets.test.ts index 0d185a9d8..f6cae8bfc 100644 --- a/__tests__/installer-targets.test.ts +++ b/__tests__/installer-targets.test.ts @@ -23,6 +23,13 @@ import { ALL_TARGETS, getTarget, resolveTargetFlag } from '../src/installer/targ import { uninstallTargets, refreshTargets } from '../src/installer'; import { upsertTomlTable, removeTomlTable, buildTomlTable } from '../src/installer/targets/toml'; import { cleanupLegacyHooks, writePromptHookEntry, removePromptHookEntry } from '../src/installer/targets/claude'; +import { + atomicWriteFileSync, + writeJsonFile, + upsertInstructionsEntry, + removeMarkedSection, +} from '../src/installer/targets/shared'; +import { CODEGRAPH_SECTION_START, CODEGRAPH_SECTION_END } from '../src/installer/instructions-template'; function mkTmpDir(label: string): string { return fs.mkdtempSync(path.join(os.tmpdir(), `cg-targets-${label}-`)); @@ -2445,3 +2452,150 @@ describe('Installer targets — Copilot family', () => { expect(jetbrains.detect('global').alreadyConfigured).toBe(true); }); }); + +// --------------------------------------------------------------------------- +// symlink preservation (shared write helpers) +// +// `atomicWriteFileSync` lands a tmp file on `filePath` via `renameSync` — +// but rename replaces the destination *link itself*, not what it points to. +// Left unhandled, installing into a dotfiles-managed symlink (e.g. +// `~/.claude/CLAUDE.md` -> `~/dotfiles/claude.md`) would silently detach the +// link and leave a plain file behind, so future dotfiles edits stop +// reaching the file the agent actually reads. These tests pin the fix: +// every write must follow the link to its real target, the same way a +// plain `writeFileSync` would. +// +// POSIX-only: symlink creation needs elevated privileges on Windows (see +// the repo's Windows-gated-tests convention). +// --------------------------------------------------------------------------- +describe('symlink preservation (shared write helpers)', () => { + let tmpDir: string; + + beforeEach(() => { + tmpDir = mkTmpDir('symlink'); + }); + + afterEach(() => { + fs.rmSync(tmpDir, { recursive: true, force: true }); + }); + + it.runIf(process.platform !== 'win32')('atomicWriteFileSync writes through a symlink, preserving the link', () => { + const realDir = path.join(tmpDir, 'real'); + const linkDir = path.join(tmpDir, 'link'); + fs.mkdirSync(realDir, { recursive: true }); + fs.mkdirSync(linkDir, { recursive: true }); + const realPath = path.join(realDir, 'config.md'); + const linkPath = path.join(linkDir, 'config.md'); + fs.writeFileSync(realPath, 'old'); + fs.symlinkSync(realPath, linkPath); + + atomicWriteFileSync(linkPath, 'new'); + + expect(fs.lstatSync(linkPath).isSymbolicLink()).toBe(true); + expect(fs.readFileSync(realPath, 'utf-8')).toBe('new'); + // No leftover tmp files in either directory. + expect(fs.readdirSync(linkDir).some((f) => f.includes('.tmp.'))).toBe(false); + expect(fs.readdirSync(realDir).some((f) => f.includes('.tmp.'))).toBe(false); + }); + + it.runIf(process.platform !== 'win32')('atomicWriteFileSync follows a symlink chain to the real target', () => { + const realPath = path.join(tmpDir, 'real.md'); + const bPath = path.join(tmpDir, 'b'); + const aPath = path.join(tmpDir, 'a'); + fs.writeFileSync(realPath, 'old'); + fs.symlinkSync(realPath, bPath); + fs.symlinkSync(bPath, aPath); + + atomicWriteFileSync(aPath, 'chained'); + + expect(fs.lstatSync(aPath).isSymbolicLink()).toBe(true); + expect(fs.lstatSync(bPath).isSymbolicLink()).toBe(true); + expect(fs.readFileSync(realPath, 'utf-8')).toBe('chained'); + }); + + it.runIf(process.platform !== 'win32')('atomicWriteFileSync creates the target of a dangling symlink', () => { + const realDir = path.join(tmpDir, 'real'); + const realPath = path.join(realDir, 'notyet.md'); + const linkPath = path.join(tmpDir, 'link.md'); + // realDir doesn't exist yet — the symlink target is unreachable. + fs.symlinkSync(realPath, linkPath); + expect(fs.existsSync(realDir)).toBe(false); + + atomicWriteFileSync(linkPath, 'created through dangling link'); + + expect(fs.lstatSync(linkPath).isSymbolicLink()).toBe(true); + expect(fs.readFileSync(realPath, 'utf-8')).toBe('created through dangling link'); + }); + + it.runIf(process.platform !== 'win32')('writeJsonFile writes through a symlink, preserving the link', () => { + const realPath = path.join(tmpDir, 'real.json'); + const linkPath = path.join(tmpDir, 'link.json'); + fs.writeFileSync(realPath, '{}\n'); + fs.symlinkSync(realPath, linkPath); + + writeJsonFile(linkPath, { foo: 'bar' }); + + expect(fs.lstatSync(linkPath).isSymbolicLink()).toBe(true); + expect(JSON.parse(fs.readFileSync(realPath, 'utf-8'))).toEqual({ foo: 'bar' }); + }); + + it.runIf(process.platform !== 'win32')( + 'reproduces the reported case: a dotfiles-managed CLAUDE.md symlink keeps user content across install/uninstall', + () => { + const dotfilesDir = path.join(tmpDir, 'dotfiles'); + fs.mkdirSync(dotfilesDir, { recursive: true }); + const realPath = path.join(dotfilesDir, 'claude.md'); + const linkPath = path.join(tmpDir, 'CLAUDE.md'); + const userContent = '# My CLAUDE.md\n\nSome personal notes I keep in dotfiles.'; + fs.writeFileSync(realPath, userContent + '\n'); + fs.symlinkSync(realPath, linkPath); + + const first = upsertInstructionsEntry(linkPath); + expect(first.action).toBe('updated'); + expect(fs.lstatSync(linkPath).isSymbolicLink()).toBe(true); + const afterFirst = fs.readFileSync(realPath, 'utf-8'); + expect(afterFirst).toContain(CODEGRAPH_SECTION_START); + expect(afterFirst).toContain(userContent); + + const second = upsertInstructionsEntry(linkPath); + expect(second.action).toBe('unchanged'); + expect(fs.lstatSync(linkPath).isSymbolicLink()).toBe(true); + + const removeResult = removeMarkedSection(linkPath, CODEGRAPH_SECTION_START, CODEGRAPH_SECTION_END); + expect(removeResult).toBe('removed'); + expect(fs.lstatSync(linkPath).isSymbolicLink()).toBe(true); + const afterRemove = fs.readFileSync(realPath, 'utf-8'); + expect(afterRemove).not.toContain(CODEGRAPH_SECTION_START); + expect(afterRemove).toContain(userContent); + }, + ); + + it.runIf(process.platform !== 'win32')( + 'two agents symlinked to one shared AGENTS.md get exactly one block: the second upsert is unchanged', + () => { + // Multi-select install: several targets' instructions files are + // symlinks to one shared AGENTS.md. Now that writes resolve to the + // shared target, the marker-based upsert must dedupe across + // targets — same guarantee gemini.ts documents for Gemini + + // Antigravity sharing GEMINI.md, extended through symlinks. + const sharedPath = path.join(tmpDir, 'AGENTS.md'); + const userContent = '# Shared agent instructions'; + fs.writeFileSync(sharedPath, userContent + '\n'); + const claudeLink = path.join(tmpDir, 'CLAUDE.md'); + const codexLink = path.join(tmpDir, 'codex-AGENTS.md'); + fs.symlinkSync(sharedPath, claudeLink); + fs.symlinkSync(sharedPath, codexLink); + + const first = upsertInstructionsEntry(claudeLink); + expect(first.action).toBe('updated'); + const second = upsertInstructionsEntry(codexLink); + expect(second.action).toBe('unchanged'); + + const content = fs.readFileSync(sharedPath, 'utf-8'); + expect(content.split(CODEGRAPH_SECTION_START).length - 1).toBe(1); + expect(content).toContain(userContent); + expect(fs.lstatSync(claudeLink).isSymbolicLink()).toBe(true); + expect(fs.lstatSync(codexLink).isSymbolicLink()).toBe(true); + }, + ); +}); diff --git a/src/installer/targets/shared.ts b/src/installer/targets/shared.ts index 364f40427..af1cc2ac4 100644 --- a/src/installer/targets/shared.ts +++ b/src/installer/targets/shared.ts @@ -72,13 +72,46 @@ export function readJsonFile(filePath: string): Record { } } +/** + * Follow a symlink chain to the path a write should land on. + * + * `renameSync` replaces the destination *link itself* rather than its + * target, so an atomic write aimed at a symlinked config (e.g. a + * dotfiles-managed CLAUDE.md) would silently swap the link for a + * regular file and detach it from the user's dotfiles. Resolving + * first gives the temp-file-plus-rename the same follow-the-link + * semantics a plain `writeFileSync` has. + * + * `fs.realpathSync` alone can't do this: it throws on dangling links, + * and writing through a dangling link (creating its target) must keep + * working. Hence the manual walk. The 32-hop cap mirrors typical + * kernel ELOOP limits; on a loop we just write to the last path seen. + */ +function resolveWriteTarget(filePath: string): string { + let target = filePath; + for (let i = 0; i < 32; i++) { + let st: fs.Stats; + try { + st = fs.lstatSync(target); + } catch { + return target; // end of chain — target doesn't exist yet + } + if (!st.isSymbolicLink()) return target; + target = path.resolve(path.dirname(target), fs.readlinkSync(target)); + } + return target; +} + /** * Write a file atomically: write to `.tmp.`, then rename. * * Prevents corruption if the process crashes mid-write. The temp - * file is cleaned up on rename failure. + * file is cleaned up on rename failure. Follows symlinks: the write + * lands on the link's target, like plain `writeFileSync`, instead of + * replacing the link itself. */ export function atomicWriteFileSync(filePath: string, content: string): void { + filePath = resolveWriteTarget(filePath); const dir = path.dirname(filePath); if (!fs.existsSync(dir)) { fs.mkdirSync(dir, { recursive: true });