mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-11 19:20:26 +01:00
fix(context-bill): a .md-named link counts only when its target is markdown
walkMd checked only the link's own name, so a link such as notes.md -> a non-markdown file was walked and, under --exact, its content was sent to count_tokens. Require the resolved target to end in .md as well. Adds tests for --exact on the installed layout (every billed file is measured; nothing behind gstack/ or a .md-named link to another file is sent) and for a symlinked foreign-host file (found by /ship review). Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
1 parent
acb59268a7
commit
0b734d3494
3 files changed
+59
-4
No files matched your search
+2
-2
@@ -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 directory already on the current walk (a loop) is never re-entered, and a link to a directory that contains the tree being walked (`..`, `/`, `$HOME`) is never followed, so a link cannot pull a sibling skill or your home directory into the bill. Two links to one directory are both walked, as two links to one skill already were, so the result never depends on directory listing order. `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 directory already on the current walk (a loop) is never re-entered, and a link to a directory that contains the tree being walked (`..`, `/`, `$HOME`) is never followed, so a link cannot pull a sibling skill or your home directory into the bill. 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. Two links to one directory are both walked, as two links to one skill already were, so the result never depends on directory listing order. `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. Both cases 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), and `--exact` on the installed layout (every billed file measured; nothing behind `gstack/` or a `.md`-named link to a non-markdown file is sent). 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.
|
||||
|
||||
+12
-2
@@ -235,7 +235,9 @@ function encloses(outer: string, inner: string): boolean {
|
||||
* the lexical path under `dir`, so buildBill and --exact key the same files.
|
||||
* Two links to one directory are both walked (each is real scanner load), so
|
||||
* the result never depends on readdir order. A directory that is already on
|
||||
* the current walk (a loop) or that encloses `dir` is never entered.
|
||||
* the current walk (a loop) or 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.
|
||||
*/
|
||||
export function walkMd(dir: string): string[] {
|
||||
const out: string[] = [];
|
||||
@@ -257,7 +259,15 @@ export function walkMd(dir: string): string[] {
|
||||
const p = path.join(current, e.name);
|
||||
const kind = entryKind(p, e);
|
||||
if (kind === "file") {
|
||||
if (e.name.endsWith(".md")) out.push(p);
|
||||
if (!e.name.endsWith(".md")) continue;
|
||||
if (e.isSymbolicLink()) {
|
||||
try {
|
||||
if (!fs.realpathSync(p).endsWith(".md")) continue;
|
||||
} catch {
|
||||
continue;
|
||||
}
|
||||
}
|
||||
out.push(p);
|
||||
continue;
|
||||
}
|
||||
if (kind !== "dir" || fs.existsSync(path.join(p, ".git"))) continue;
|
||||
|
||||
@@ -284,6 +284,25 @@ describe("upstream fix b: installed-tree layout (repo-checkout subdir skip)", ()
|
||||
fs.rmSync(tmp, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
it("flags a symlinked foreign-host file, but not a dangling link or a directory with that name", () => {
|
||||
if (process.platform === "win32") return;
|
||||
const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "context-bill-foreign-"));
|
||||
const checkout = path.join(tmp, "checkout", "qa");
|
||||
fs.mkdirSync(checkout, { recursive: true });
|
||||
fs.writeFileSync(path.join(checkout, "SKILL.md"), "---\nname: qa\ndescription: d\n---\n# qa\n");
|
||||
fs.writeFileSync(path.join(checkout, "agents.md"), "a".repeat(100));
|
||||
const skills = path.join(tmp, "skills");
|
||||
fs.mkdirSync(path.join(skills, "qa"), { recursive: true });
|
||||
fs.symlinkSync(path.join(checkout, "SKILL.md"), path.join(skills, "qa", "SKILL.md"));
|
||||
fs.symlinkSync(path.join(checkout, "agents.md"), path.join(skills, "qa", "agents.md"));
|
||||
fs.symlinkSync(path.join(tmp, "missing"), path.join(skills, "qa", ".cursorrules"));
|
||||
fs.mkdirSync(path.join(skills, "qa", "skill.json"));
|
||||
|
||||
const qa = buildBill(skills).skills.find((s) => s.name === "qa")!;
|
||||
expect(qa.foreignFiles.map((f) => [f.path, f.bytes])).toEqual([["agents.md", 100]]);
|
||||
fs.rmSync(tmp, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
it("never follows a link that loops or climbs out of the root", () => {
|
||||
if (process.platform === "win32") return;
|
||||
const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "context-bill-escape-"));
|
||||
@@ -649,6 +668,32 @@ describe("--exact (opt-in measurement; offline here via an injected fetch)", ()
|
||||
expect(measured.missedKeys.size).toBe(0);
|
||||
});
|
||||
|
||||
it("on an installed layout, measures every billed file and sends nothing behind gstack/ or a .md-named link to another file", async () => {
|
||||
if (process.platform === "win32") return;
|
||||
const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "context-bill-exact-installed-"));
|
||||
const checkout = path.join(tmp, "checkout");
|
||||
fs.mkdirSync(path.join(checkout, ".git"), { recursive: true });
|
||||
fs.mkdirSync(path.join(checkout, "qa"), { recursive: true });
|
||||
fs.writeFileSync(path.join(checkout, "qa", "SKILL.md"), "---\nname: qa\ndescription: d\n---\n# qa\n");
|
||||
fs.writeFileSync(path.join(checkout, "README.md"), "checkout only");
|
||||
fs.writeFileSync(path.join(tmp, "notes.txt"), "not markdown");
|
||||
const skills = path.join(tmp, "skills");
|
||||
fs.mkdirSync(path.join(skills, "qa"), { recursive: true });
|
||||
fs.symlinkSync(path.join(checkout, "qa", "SKILL.md"), path.join(skills, "qa", "SKILL.md"));
|
||||
fs.symlinkSync(path.join(tmp, "notes.txt"), path.join(skills, "qa", "notes.md"));
|
||||
fs.symlinkSync(checkout, path.join(skills, "gstack"));
|
||||
|
||||
const calls: { body: string }[] = [];
|
||||
const measured = await measureExactTokens(skills, { model: "m", apiKey: "sk-test", fetchImpl: fakeFetch(calls), egressHome });
|
||||
const sent = calls.map((c) => JSON.parse(c.body).messages[0].content);
|
||||
expect(sent).not.toContain("checkout only");
|
||||
expect(sent).not.toContain("not markdown");
|
||||
const bill = buildBill(skills, { tokensOf: measured.tokensOf, tokenSource: measured.tokenSource });
|
||||
expect(bill.skills.map((s) => s.name)).toEqual(["qa"]);
|
||||
expect([...measured.missedKeys]).toEqual([]);
|
||||
fs.rmSync(tmp, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
it("counts anything it could not measure instead of passing it off as measured", async () => {
|
||||
const measured = await measureExactTokens(TREE_A, {
|
||||
model: "m", apiKey: "sk-test", fetchImpl: fakeFetch(), egressHome,
|
||||
|
||||
Reference in new issue
Block a user