diff --git a/CHANGELOG.md b/CHANGELOG.md index 9a0d075b3..79fc35ce2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,10 +5,10 @@ **`gstack-context-bill` now bills a real install.** On `~/.claude/skills` it reported "2 skills, ~134 tok" always-on, because setup installs each skill as a real directory whose `SKILL.md` and `sections/` are symlinks into the gstack checkout, and the walker only counted regular files. The tool built to answer the token-cost complaint gave a falsely tiny answer on the layout users actually have. On a fresh install it now reports 58 skills, ~6.8K tokens always-on, and 3.6 MB on disk (it was 37 KB across 2 skills). Billing the checkout itself is unchanged (66 skills, ~7.2K tokens). ### Fixed -- **`lib/context-bill.ts`**: `findSkillDirs`, `walkMd` and the foreign-file check follow symlinked files and directories. Every path the bill reports, and every file `--exact` keys on, stays the lexical path under the root you passed. A link to a directory that contains the tree being walked (`..`, `/`, `$HOME`) is never followed, so a link cannot climb out of the root into its parent or your home directory. The markdown walk expands each real directory once and counts each real file once per skill, under the first path reached in sorted order, which is how a skill loads it: loops end, links that fan out into each other cannot multiply the walk, and a tree with more than 200,000 entries is refused with an error instead of being cut short. `--exact` sends each real file once and prices every link to it with that one measurement. A link named `*.md` counts only when its target is a `.md` file, so `--exact` never sends a non-markdown file through a markdown-named link. `walkMd` skips a child repo checkout (the `gstack/` link beside the installed skills), as `findSkillDirs` already did, so `--exact` does not send the whole checkout. +- **`lib/context-bill.ts`**: `findSkillDirs`, `walkMd` and the foreign-file check follow symlinked files and directories. Every path the bill reports, and every file `--exact` keys on, stays the lexical path under the root you passed. A link to a directory that contains the tree being billed or the skill being walked (`..`, `/`, `$HOME`) is never followed, so a link cannot climb out of the root into its parent or your home directory, even from a symlinked skill that lives elsewhere. The markdown walk expands each real directory once and counts each real file once per skill, under the first path reached in sorted order, which is how a skill loads it: loops end, links that fan out into each other cannot multiply the walk, and a tree with more than 200,000 entries is refused with an error instead of being cut short. `--exact` sends each real file once and prices every link to it with that one measurement, and its notice says how many of the files it sends sit outside the tree behind symlinks (on an install, the checkout's skill files). A link named `*.md` counts only when its target is a `.md` file, so `--exact` never sends a non-markdown file through a markdown-named link. `walkMd` skips a child repo checkout (the `gstack/` link beside the installed skills), as `findSkillDirs` already did, so `--exact` does not send the whole checkout. ### Tests -- `test/context-bill.test.ts`: an installed-layout fixture (a checkout outside the skills root, real skill directories with symlinked `SKILL.md` and `sections/`, and a `gstack/` link to the checkout) bills both skills with the target's exact frontmatter, `SKILL.md` and section bytes, and keeps every walked path under the root. Negative controls: a dangling `SKILL.md` link and a `SKILL.md` link to a directory are not skills. A second fixture holds a self link, a parent link that reaches a skill outside the root, a loop back into the skill and a link up to the temp root; only the one real skill and its own files are billed. Further cases cover a loop to a directory below the walk root, a root whose name starts with `..`, a symlinked `agents.md` (with a dangling link and a directory of a foreign-file name as controls), `--exact` on the installed layout (every billed file measured; nothing behind `gstack/` or a `.md`-named link to a non-markdown file is sent), a 20-level fan-out of links that reaches one file by 2^20 paths (walked once; hangs on the previous walker), and two skills linking one checkout file and directory (each real file sent once, both skills priced from it). The installed-layout and escape fixtures fail on main (the first finds no skills, the second bills the outside skill). +- `test/context-bill.test.ts`: an installed-layout fixture (a checkout outside the skills root, real skill directories with symlinked `SKILL.md` and `sections/`, and a `gstack/` link to the checkout) bills both skills with the target's exact frontmatter, `SKILL.md` and section bytes, and keeps every walked path under the root. Negative controls: a dangling `SKILL.md` link and a `SKILL.md` link to a directory are not skills. A second fixture holds a self link, a parent link that reaches a skill outside the root, a loop back into the skill and a link up to the temp root; only the one real skill and its own files are billed. Further cases cover a loop to a directory below the walk root, a root whose name starts with `..`, a symlinked `agents.md` (with a dangling link and a directory of a foreign-file name as controls), `--exact` on the installed layout (every billed file measured; nothing behind `gstack/` or a `.md`-named link to a non-markdown file is sent), a 12-level fan-out of links that reaches one file by 2^12 paths (walked once; the previous walker returned all 4,096), two skills linking one checkout file and directory (each real file sent once, both skills priced from it), a symlinked skill outside the root with a link up to the root's parent, the `--exact` notice's outside-the-tree count, and a skill whose `SKILL.md` is first walked under another name (its frontmatter still measured). The installed-layout and escape fixtures fail on main (the first finds no skills, the second bills the outside skill). ### Gate integrity - No threshold, budget, timeout, trial count, case kind or assertion changed. The bill of this repo's own tree, which the context-budget ratchet reads, is byte-for-byte unchanged. diff --git a/lib/context-bill.ts b/lib/context-bill.ts index 65185f81f..1489ed060 100644 --- a/lib/context-bill.ts +++ b/lib/context-bill.ts @@ -238,16 +238,18 @@ function encloses(outer: string, inner: string): boolean { * Each real directory is expanded once and each real file returned once per * walk, under the first lexical path reached with entries in sorted order: a * skill loads a file once however many links reach it, and links that fan out - * into each other cannot multiply the walk. A directory that encloses `dir` is - * never entered. A link named *.md counts only when its target is a .md file - * too, so --exact never sends a non-markdown file through a markdown-named - * link. More than WALK_ENTRY_CAP entries is an error, never a silent cut. + * into each other cannot multiply the walk. A directory that encloses `dir`, + * or `billedRoot` (the tree a skill is billed under), is never entered. A link + * named *.md counts only when its target is a .md file too, so --exact never + * sends a non-markdown file through a markdown-named link. More than WALK_ENTRY_CAP entries is an error, never a silent cut. */ -export function walkMd(dir: string): string[] { +export function walkMd(dir: string, billedRoot: string = dir): string[] { const out: string[] = []; let rootReal: string; + let billedReal: string; try { rootReal = fs.realpathSync(dir); + billedReal = fs.realpathSync(billedRoot); } catch { return out; } @@ -271,7 +273,11 @@ export function walkMd(dir: string): string[] { if (e.name.startsWith(".") || e.name === "node_modules") continue; const p = path.join(current, e.name); const kind = entryKind(p, e); - if (kind === "file" ? !e.name.endsWith(".md") : kind !== "dir" || fs.existsSync(path.join(p, ".git"))) continue; + if (kind === "file") { + if (!e.name.endsWith(".md")) continue; + } else if (kind !== "dir" || fs.existsSync(path.join(p, ".git"))) { + continue; + } let real: string; try { real = fs.realpathSync(p); @@ -285,7 +291,7 @@ export function walkMd(dir: string): string[] { out.push(p); continue; } - if (encloses(real, rootReal)) continue; + if (encloses(real, rootReal) || encloses(real, billedReal)) continue; seen.add(real); walk(p); } @@ -294,10 +300,10 @@ export function walkMd(dir: string): string[] { return out; } -function totalMd(dir: string, tokensOf: TokensOf): { bytes: number; tokens: number } { +function totalMd(dir: string, tokensOf: TokensOf, billedRoot: string): { bytes: number; tokens: number } { let bytes = 0; let tokens = 0; - for (const p of walkMd(dir)) { + for (const p of walkMd(dir, billedRoot)) { // A skill dir that CONTAINS other skill dirs (the gstack root skill wraps // the whole tree) must not swallow its children's files: each nested // skill reports its own totalMd, and the grand total sums per-skill @@ -319,7 +325,12 @@ function totalMd(dir: string, tokensOf: TokensOf): { bytes: number; tokens: numb return { bytes, tokens }; } -export function parseSkill(skillDir: string, name: string, tokensOf: TokensOf = estimateTokensOf): SkillBill { +export function parseSkill( + skillDir: string, + name: string, + tokensOf: TokensOf = estimateTokensOf, + billedRoot: string = skillDir, +): SkillBill { const skillMdPath = path.join(skillDir, "SKILL.md"); const text = fs.readFileSync(skillMdPath, "utf8"); const skillMdBytes = bytesOf(skillMdPath) ?? 0; @@ -358,7 +369,7 @@ export function parseSkill(skillDir: string, name: string, tokensOf: TokensOf = } } - const total = totalMd(skillDir, tokensOf); + const total = totalMd(skillDir, tokensOf, billedRoot); const eagerBytes = skillMdBytes + sumBytes(forcedRefs); const eagerTokens = skillMdTokens + sumTokens(forcedRefs); return { @@ -496,7 +507,7 @@ export function buildBill( const resolved = path.resolve(root); if (!fs.existsSync(resolved)) throw new Error(`No such tree: ${resolved}`); const skills = findSkillDirs(resolved).map((dir) => - parseSkill(dir, path.relative(resolved, dir) || path.basename(resolved), tokensOf), + parseSkill(dir, path.relative(resolved, dir) || path.basename(resolved), tokensOf, resolved), ); const total = skills.reduce((n, s) => n + s.totalMdBytes, 0); const totalTokens = skills.reduce((n, s) => n + s.totalMdTokens, 0); @@ -819,10 +830,8 @@ export async function measureExactTokens( const text = fs.readFileSync(file, "utf8"); texts.set(file, text); lexicalOf.set(fs.realpathSync(file), file); - if (path.basename(file) === "SKILL.md") { - const fm = parseFrontmatter(text); - if (fm.block) texts.set(frontmatterKey(file), fm.block); - } + const fm = parseFrontmatter(text); + if (fm.block) texts.set(frontmatterKey(file), fm.block); } // Receipt-before-send. Content-free: file count + total bytes only. @@ -1025,10 +1034,13 @@ export async function contextBillMain(argv: string[], options: MainOptions = {}) onDone?: () => void; }> => { if (!flags.exact) return {}; - const files = walkMd(tree).length; + const walked = walkMd(tree); + const treeReal = fs.realpathSync(tree); + const outside = walked.filter((f) => !encloses(treeReal, fs.realpathSync(f))).length; stderr.write( - `--exact: sending the content of ${files} .md file(s) under ${tree} to ` + - `api.anthropic.com for count_tokens (${flags.exactModel}). No other data leaves this machine.\n`, + `--exact: sending the content of ${walked.length} .md file(s) under ${tree}` + + (outside ? ` (${outside} of them through symlinks to files outside it)` : "") + + ` to api.anthropic.com for count_tokens (${flags.exactModel}). No other data leaves this machine.\n`, ); try { const measured = await measureExactTokens(tree, { diff --git a/test/context-bill.test.ts b/test/context-bill.test.ts index 75a6a6f5e..f041abf48 100644 --- a/test/context-bill.test.ts +++ b/test/context-bill.test.ts @@ -334,9 +334,9 @@ describe("upstream fix b: installed-tree layout (repo-checkout subdir skip)", () it("walks a fan-out of links once per real directory and file", () => { if (process.platform === "win32") return; const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "context-bill-fanout-")); - // L0..L19 sit outside the root; each level holds two links to the next, so - // a walk that expands every link reaches leaf.md by 2^20 paths. - const depth = 20; + // L0..L11 sit outside the root; each level holds two links to the next, so + // a walk that expands every link reaches leaf.md by 2^12 paths. + const depth = 12; for (let d = 0; d <= depth; d++) fs.mkdirSync(path.join(tmp, `L${d}`)); fs.writeFileSync(path.join(tmp, `L${depth}`, "leaf.md"), "l".repeat(300)); for (let d = 0; d < depth; d++) { @@ -354,6 +354,25 @@ describe("upstream fix b: installed-tree layout (repo-checkout subdir skip)", () fs.rmSync(tmp, { recursive: true, force: true }); }); + it("keeps a symlinked skill that lives outside the root from climbing to the root's parent", () => { + if (process.platform === "win32") return; + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "context-bill-fence-")); + const home = path.join(tmp, "home"); + const root = path.join(home, "skills"); + fs.mkdirSync(path.join(root, "real"), { recursive: true }); + fs.writeFileSync(path.join(root, "real", "SKILL.md"), "---\nname: real\ndescription: d\n---\n# real\n"); + fs.writeFileSync(path.join(home, "private.md"), "p".repeat(1_000)); + const ext = path.join(tmp, "opt", "ext"); + fs.mkdirSync(ext, { recursive: true }); + fs.writeFileSync(path.join(ext, "SKILL.md"), "---\nname: ext\ndescription: d\n---\n# ext\n"); + fs.symlinkSync(home, path.join(ext, "up")); + fs.symlinkSync(ext, path.join(root, "ext")); + + const ext_ = buildBill(root).skills.find((s) => s.name === "ext")!; + expect(ext_.totalMdBytes).toBe(fileBytes(ext, "SKILL.md")); + fs.rmSync(tmp, { recursive: true, force: true }); + }); + it("refuses a parent link when the root's own name starts with '..'", () => { if (process.platform === "win32") return; const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "context-bill-dotdot-")); @@ -837,6 +856,46 @@ describe("--exact (opt-in measurement; offline here via an injected fetch)", () expect(bill.calibration.rows.length).toBeGreaterThan(0); }); + it("says how many of the files it sends sit outside the tree behind symlinks", async () => { + if (process.platform === "win32") return; + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "context-bill-notice-")); + fs.mkdirSync(path.join(tmp, "checkout", "qa", "sections"), { recursive: true }); + fs.writeFileSync(path.join(tmp, "checkout", "qa", "SKILL.md"), "---\nname: qa\ndescription: d\n---\n# qa\n"); + fs.writeFileSync(path.join(tmp, "checkout", "qa", "sections", "flow.md"), "f"); + const skills = path.join(tmp, "skills"); + fs.mkdirSync(path.join(skills, "qa"), { recursive: true }); + fs.symlinkSync(path.join(tmp, "checkout", "qa", "SKILL.md"), path.join(skills, "qa", "SKILL.md")); + fs.symlinkSync(path.join(tmp, "checkout", "qa", "sections"), path.join(skills, "qa", "sections")); + fs.writeFileSync(path.join(skills, "qa", "notes.md"), "n"); + const stderr = capture(); + const code = await contextBillMain([skills, "--exact", "--json"], { + stdout: capture().stream, + stderr: stderr.stream, + apiKey: "sk-test", + fetchImpl: fakeFetch(), + egressHome, + }); + expect(code).toBe(0); + expect(stderr.text()).toContain("sending the content of 3 .md file(s)"); + expect(stderr.text()).toContain("(2 of them through symlinks to files outside it)"); + fs.rmSync(tmp, { recursive: true, force: true }); + }); + + it("measures a skill's frontmatter even when its file was first walked under another name", async () => { + if (process.platform === "win32") return; + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "context-bill-fm-alias-")); + const skills = path.join(tmp, "skills"); + fs.mkdirSync(path.join(skills, "aaa"), { recursive: true }); + fs.mkdirSync(path.join(skills, "zskill"), { recursive: true }); + fs.writeFileSync(path.join(skills, "aaa", "notes.md"), "---\nname: zskill\ndescription: d\n---\n# z\n"); + fs.symlinkSync(path.join(skills, "aaa", "notes.md"), path.join(skills, "zskill", "SKILL.md")); + const measured = await measureExactTokens(skills, { model: "m", apiKey: "sk-test", fetchImpl: fakeFetch(), egressHome }); + const bill = buildBill(skills, { tokensOf: measured.tokensOf, tokenSource: measured.tokenSource }); + expect(bill.skills.map((s) => s.name)).toEqual(["zskill"]); + expect([...measured.missedKeys]).toEqual([]); + fs.rmSync(tmp, { recursive: true, force: true }); + }); + it("degrades to the estimate when exact mode is unavailable, naming the code", async () => { const stdout = capture(); const stderr = capture();