mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-05 16:34:00 +01:00
fix(context-bill): --exact names a missing tree before any request
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
1 parent
8799d9f5c9
commit
eea832b85b
3 files changed
+27
-6
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 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.
|
||||
- **`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. `--exact` on a path that does not exist now stops with `No such tree` before any request (it used to send its one-character envelope probe first).
|
||||
|
||||
### 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 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, a skill whose `SKILL.md` is first walked under another name (its frontmatter still measured), and a container skill whose nested skill links to the container's own files (both skills still bill them). 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, where a walker that expands every link would return 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, a skill whose `SKILL.md` is first walked under another name (its frontmatter still measured), a container skill whose nested skill links to the container's own files (both skills still bill them), and `--exact` on a missing tree (named, with no request sent). The installed-layout and escape fixtures fail on main (the first finds no skills, the second bills the outside skill), the missing-tree case fails on main (one request sent), and each later case failed on the commit before its fix.
|
||||
|
||||
### 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.
|
||||
|
||||
+8
-3
@@ -177,9 +177,13 @@ function sumTokens(entries: { tokens: number }[]): number {
|
||||
return entries.reduce((n, e) => n + e.tokens, 0);
|
||||
}
|
||||
|
||||
/** Cache key for a SKILL.md's frontmatter block, which is a slice, not a whole file. */
|
||||
function frontmatterKey(skillMdPath: string): string {
|
||||
return `${skillMdPath}#frontmatter`;
|
||||
/**
|
||||
* Cache key for a .md file's frontmatter block, which is a slice, not a whole
|
||||
* file. The bill looks it up through a SKILL.md path; --exact measures it for
|
||||
* every walked file, so a SKILL.md first walked under another name still has one.
|
||||
*/
|
||||
function frontmatterKey(mdPath: string): string {
|
||||
return `${mdPath}#frontmatter`;
|
||||
}
|
||||
|
||||
function parseFrontmatter(text: string): { bytes: number; keys: string[]; block: string } {
|
||||
@@ -1034,6 +1038,7 @@ export async function contextBillMain(argv: string[], options: MainOptions = {})
|
||||
onDone?: () => void;
|
||||
}> => {
|
||||
if (!flags.exact) return {};
|
||||
if (!fs.existsSync(tree)) throw new Error(`No such tree: ${path.resolve(tree)}`);
|
||||
const walked = walkMd(tree);
|
||||
const treeReal = fs.realpathSync(tree);
|
||||
const outside = walked.filter((f) => !encloses(treeReal, fs.realpathSync(f))).length;
|
||||
|
||||
@@ -157,7 +157,7 @@ describe("upstream fix a: root-as-container + walkMd exclusions", () => {
|
||||
// Regression pin for the v1.63 double-count: totalMd on a skill dir that
|
||||
// CONTAINS other skill dirs (the gstack root wraps the whole tree) used to
|
||||
// swallow the children's .md bytes too, so the TOTAL line billed every
|
||||
// nested skill twice. A revert of the topSeg/SKILL.md skip in totalMd
|
||||
// nested skill twice. Dropping skipNestedSkills from totalMd's walkMd call
|
||||
// (lib/context-bill.ts) must fail here.
|
||||
const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "context-bill-nested-total-"));
|
||||
fs.writeFileSync(path.join(tmp, "SKILL.md"), "---\nname: parent\ndescription: p\n---\n# Parent\n");
|
||||
@@ -899,6 +899,22 @@ describe("--exact (opt-in measurement; offline here via an injected fetch)", ()
|
||||
fs.rmSync(tmp, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
it("names a missing tree, without a raw ENOENT or any request", async () => {
|
||||
const out = capture();
|
||||
const calls: { body: string }[] = [];
|
||||
const code = await contextBillMain([path.join(os.tmpdir(), "context-bill-no-such-tree"), "--exact"], {
|
||||
stdout: out.stream,
|
||||
stderr: out.stream,
|
||||
apiKey: "sk-test",
|
||||
fetchImpl: fakeFetch(calls),
|
||||
egressHome,
|
||||
});
|
||||
expect(code).toBe(1);
|
||||
expect(out.text()).toContain("No such tree:");
|
||||
expect(out.text()).not.toContain("ENOENT");
|
||||
expect(calls.length).toBe(0);
|
||||
});
|
||||
|
||||
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-"));
|
||||
|
||||
Reference in new issue
Block a user