fix(test): adversarial findings — stable root key, symlink-alias dedupe, fixture-shape guard

Adversarial review (Claude subagent) verified the fixture's root-skill key was
the capture machine's checkout dirname: any non-gstack-named clone (every
Conductor worktree) failed the free suite, and the documented re-run-the-capture
recovery baked the local dirname into the committed fixture — silent corruption
through the tool's own protocol. The root skill is now pinned to ROOT_SKILL_KEY
('gstack', its frontmatter name). Symlink aliases are realpath-deduped (census
precedent): connect-chrome no longer gets its own ceiling, so Windows checkouts
that materialize the symlink as a plain file can't fail the stale-ceiling
set-equality test. New guards: fixture-shape validation (a string alwaysOnTotal
can no longer silently disable the ceiling), a mutation pin that the filter
shrinks the always-on ledger vs the raw bill, an alwaysOnTotal violation test
(the branch was load-bearing with only under-budget coverage), and an atomic
temp+rename fixture write. Fixture regenerated: 59 ceilings, alwaysOnTotal 6344.
Deferred with a TODO: anchoring transformFrontmatter's denylist strip to the
frontmatter block (latent, zero live collisions, pre-existing path).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

TODOS.md

@@ -691,6 +691,32 @@ or asserts dashboard/report text, then decide skeleton-vs-section placement per
 **Depends on / blocked by:** Token-reduction program Phases 1-4 landing (carve
 machinery churn would conflict).
 
+### P3: Anchor transformFrontmatter's denylist strip to the frontmatter block
+
+**What:** `transformFrontmatter` (scripts/gen-skill-docs.ts:525-530, denylist branch)
+deletes the FIRST line matching `^<field>:` anywhere in the file, not just inside
+the frontmatter block, and would orphan continuation lines of a block-style YAML
+value. Slice the frontmatter, strip within it, reassemble.
+
+**Why:** Latent mis-strip class: a skill body line beginning `interactive:` or
+`benefits-from:` (e.g. a skill documenting the frontmatter contract) would be
+silently deleted from the render. Zero live collisions today (verified across all
+tracked SKILL.md bodies during the v1.69.x token-reduction Phase 0 review), but
+each new stripFields entry widens the exposure.
+
+**Pros:** Kills the whole latent class; makes stripFields safe to grow.
+**Cons:** Touches the generator hot path — needs a full regen + the per-host
+golden fixtures re-checked; deserves its own small PR, not a rider.
+
+**Context:** Found by the Phase 0 adversarial review on branch
+`prompt-token-load-reduction` (finding ADV4). The gen-side parser reads only
+inline `[...]` array form (gen-skill-docs.ts:751), so block-form YAML for these
+keys fails silently twice — worth a validation error at the same time.
+
+**Effort estimate:** S (human team) → S (CC+gstack)
+**Priority:** P3
+**Depends on / blocked by:** none.
+
 ### P3: Revisit plan-ceo-review doctrine carve after the preamble program lands
 
 **What:** Re-evaluate carving plan-ceo-review's ~13KB of always-loaded doctrine

test/context-bill.test.ts

@@ -419,6 +419,18 @@ describe("--budget", () => {
     expect(violations).toHaveLength(1);
     expect(violations[0].ceiling).toBe("eagerPerInvocation.ghost");
   });
+
+  // The context-budget ratchet (test/context-budget-ratchet.test.ts) made
+  // this branch load-bearing in CI; it previously had only under-budget
+  // coverage.
+  it("checkBudget flags an alwaysOnTotal violation with every skill's frontmatter listed", () => {
+    const bill = buildBill(TREE_A);
+    const violations = checkBudget(bill, { alwaysOnTotal: 0 });
+    expect(violations).toHaveLength(1);
+    expect(violations[0].ceiling).toBe("alwaysOnTotal");
+    expect(violations[0].actual).toBe(Math.round(bill.totals.alwaysOnTokens));
+    expect(violations[0].files.length).toBe(bill.skills.length);
+  });
 });
 
 describe("--exact (opt-in measurement; offline here via an injected fetch)", () => {

test/context-budget-ratchet.test.ts

@@ -27,7 +27,7 @@
 import { describe, test, expect } from 'bun:test';
 import * as fs from 'fs';
 import * as path from 'path';
-import { checkBudget } from '../lib/context-bill';
+import { buildBill, checkBudget } from '../lib/context-bill';
 import {
   buildRatchetBill,
   captureContextBudget,
@@ -48,6 +48,28 @@ const budget: ContextBudget = JSON.parse(fs.readFileSync(BUDGET_FIXTURE_PATH, 'u
 const bill = buildRatchetBill();
 
 describe('context-budget ratchet', () => {
+  // checkBudget only enforces alwaysOnTotal when it is typeof number — a
+  // string or missing value from a hand edit or bad merge would silently
+  // turn the always-on ceiling OFF while every test stays green. Validate
+  // the fixture shape so the guard cannot be disabled by a typo.
+  test('fixture shape is valid (a malformed fixture must not silently disable ceilings)', () => {
+    expect(typeof budget.alwaysOnTotal).toBe('number');
+    expect(Number.isFinite(budget.alwaysOnTotal)).toBe(true);
+    const bad = Object.entries(budget.eagerPerInvocation).filter(
+      ([, v]) => typeof v !== 'number' || !Number.isFinite(v),
+    );
+    expect(bad, `Non-numeric ceilings: ${bad.map(([k]) => k).join(', ')}. Re-run the capture.`).toEqual([]);
+  });
+
+  // Mutation pin: the fixture-skill filter must actually shrink the
+  // always-on sum vs the raw bill (deleting the totals recompute would leak
+  // fixture tokens under the headroom and never fail a ceiling).
+  test('filtering fixture skills shrinks the always-on ledger vs the raw bill', () => {
+    const raw = buildBill(path.join(import.meta.dir, '..'));
+    expect(bill.skills.some((s) => s.name.startsWith('test/'))).toBe(false);
+    expect(bill.totals.skillCount).toBeLessThan(raw.totals.skillCount);
+    expect(bill.totals.alwaysOnTokens).toBeLessThan(raw.totals.alwaysOnTokens);
+  });
   test('always-on + eager ledgers stay under the fixture ceilings', () => {
     // actual === null means "fixture names a skill missing from the tree" —
     // the dedicated stale-fixture test below owns that case with a clearer

test/fixtures/context-budget.json

@@ -1,6 +1,6 @@
 {
   "_comment": "Context-budget ratchet ceilings (~tokens). Regenerate: bun test/helpers/capture-context-budget.ts. Headroom: alwaysOnTotal x1.05, eagerPerInvocation x1.1. Graded by test/context-budget-ratchet.test.ts via lib/context-bill.ts checkBudget.",
-  "alwaysOnTotal": 6423,
+  "alwaysOnTotal": 6344,
   "eagerPerInvocation": {
     "autoplan": 27445,
     "benchmark": 10114,
@@ -10,7 +10,6 @@
     "canary": 15492,
     "careful": 919,
     "codex": 26760,
-    "connect-chrome": 9687,
     "context-restore": 14506,
     "context-save": 15123,
     "cso": 20096,
@@ -33,7 +32,7 @@
     "ios-fix": 13835,
     "ios-qa": 16132,
     "ios-sync": 14005,
-    "land-and-deploy": 28990,
+    "land-and-deploy": 29370,
     "landing-report": 14416,
     "learn": 14086,
     "make-pdf": 10214,
@@ -56,7 +55,7 @@
     "scrape": 9196,
     "setup-browser-cookies": 8377,
     "setup-deploy": 15246,
-    "setup-gbrain": 25164,
+    "setup-gbrain": 25250,
     "ship": 23847,
     "skillify": 17092,
     "spec": 22662,

test/helpers/capture-context-budget.ts

@@ -56,17 +56,50 @@ export interface ContextBudget {
   eagerPerInvocation: Record<string, number>;
 }
 
+/**
+ * The root SKILL.md's bill name falls back to the checkout directory's
+ * basename (path.relative gives '' at the root), which is machine-specific:
+ * a Conductor worktree named anything but "gstack" would mismatch the fixture
+ * key, and the documented "re-run the capture" recovery would then bake the
+ * local dirname INTO the committed fixture. Pin it to the skill's frontmatter
+ * name instead — stable across every clone.
+ */
+export const ROOT_SKILL_KEY = 'gstack';
+
 /**
  * The bill the ratchet grades: repo tree minus test-fixture skill dirs, with
- * POSIX-normalized names and ALL totals rebuilt from the filtered list (a
- * partially-updated totals object would hand fixture-polluted numbers to any
- * future consumer of the perInvocation/totalMd fields).
+ * POSIX-normalized names, the root skill pinned to ROOT_SKILL_KEY, symlink
+ * aliases deduped by realpath (connect-chrome -> open-gstack-browser; on
+ * Windows checkouts the symlink materializes as a plain file and the alias
+ * dir vanishes, so budgeting it would make the stale-ceiling test
+ * platform-dependent — same dedupe the skill census uses), and ALL totals
+ * rebuilt from the filtered list (a partially-updated totals object would
+ * hand fixture-polluted numbers to any future consumer of the
+ * perInvocation/totalMd fields).
  */
 export function buildRatchetBill(root: string = REPO_ROOT): Bill {
   const bill = buildBill(root);
-  const skills = bill.skills
-    .map((s) => ({ ...s, name: toPosixName(s.name) }))
+  const candidates = bill.skills
+    .map((s) => ({
+      ...s,
+      name: s.dir === bill.root ? ROOT_SKILL_KEY : toPosixName(s.name),
+    }))
     .filter((s) => !isFixtureSkill(s.name));
+  // One ceiling per PHYSICAL skill: group by realpath, prefer the entry whose
+  // dir IS the realpath (the real dir) over symlink aliases.
+  const byReal = new Map<string, (typeof candidates)[number]>();
+  for (const s of candidates) {
+    let real: string;
+    try {
+      real = fs.realpathSync(s.dir);
+    } catch {
+      real = s.dir;
+    }
+    const cur = byReal.get(real);
+    if (!cur || (s.dir === real && cur.dir !== real)) byReal.set(real, s);
+  }
+  const kept = new Set(byReal.values());
+  const skills = candidates.filter((s) => kept.has(s));
   return {
     ...bill,
     skills,
@@ -102,10 +135,13 @@ export function captureContextBudget(root: string = REPO_ROOT): ContextBudget {
   };
 }
 
-// CLI: write the fixture.
+// CLI: write the fixture atomically (temp + rename) — an interrupted capture
+// must never leave truncated JSON that breaks the suite at module load.
 if (import.meta.main) {
   const budget = captureContextBudget();
-  fs.writeFileSync(BUDGET_FIXTURE_PATH, JSON.stringify(budget, null, 2) + '\n');
+  const tmp = `${BUDGET_FIXTURE_PATH}.tmp-${process.pid}`;
+  fs.writeFileSync(tmp, JSON.stringify(budget, null, 2) + '\n');
+  fs.renameSync(tmp, BUDGET_FIXTURE_PATH);
   const n = Object.keys(budget.eagerPerInvocation).length;
   console.log(
     `Wrote ${path.relative(REPO_ROOT, BUDGET_FIXTURE_PATH)}: alwaysOnTotal=${budget.alwaysOnTotal} tok, ${n} eager ceilings`,