fix(test): review findings — Windows path normalization, full totals rebuild, ratchet coverage

Pre-landing review (5 specialists) found one critical: the ratchet test runs
in the curated Windows lane, where path.relative yields backslash skill names
that miss the test/ filter and mismatch every POSIX fixture key. Names are now
normalized once in buildRatchetBill (toPosixName) and the fixture filter is
tightened to test/fixtures/. All eight Bill.totals fields are rebuilt from the
filtered list (no fixture-polluted perInvocation/totalMd numbers for future
consumers). New coverage: Windows-separator normalization pins, a
captureContextBudget round-trip against tree-a (headroom math exact), a
stripFields regression pin (interactive/benefits-from absent from renders,
hooks/gbrain preserved), and the ceilings test no longer double-reports
stale-fixture entries.

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

test/context-budget-ratchet.test.ts

@@ -26,10 +26,16 @@
  */
 import { describe, test, expect } from 'bun:test';
 import * as fs from 'fs';
+import * as path from 'path';
 import { checkBudget } from '../lib/context-bill';
 import {
   buildRatchetBill,
+  captureContextBudget,
+  isFixtureSkill,
+  toPosixName,
   BUDGET_FIXTURE_PATH,
+  ALWAYS_ON_HEADROOM,
+  EAGER_HEADROOM,
   type ContextBudget,
 } from './helpers/capture-context-budget';
 
@@ -43,10 +49,13 @@ const bill = buildRatchetBill();
 
 describe('context-budget ratchet', () => {
   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
+    // message; filtering here keeps one failure from producing two reports.
     const violations = checkBudget(bill, {
       alwaysOnTotal: budget.alwaysOnTotal,
       eagerPerInvocation: budget.eagerPerInvocation,
-    });
+    }).filter((v) => v.actual !== null);
     const detail = violations
       .map((v) => `  ${v.ceiling}: ${v.actual} tok > limit ${v.limit}\n    ${v.files.join('\n    ')}`)
       .join('\n');
@@ -75,4 +84,30 @@ describe('context-budget ratchet', () => {
       `Fixture carries ceilings for removed skills: ${stale.join(', ')}. Re-run the capture.`,
     ).toEqual([]);
   });
+
+  // Windows lane: skill names arrive backslash-separated from path.relative;
+  // the normalization must make the filter and the POSIX fixture keys agree.
+  test('name normalization handles Windows separators', () => {
+    expect(toPosixName(['test', 'fixtures', 'context-bill', 'tree-a', 'alpha'].join(path.sep))).toBe(
+      'test/fixtures/context-bill/tree-a/alpha',
+    );
+    expect(isFixtureSkill(['test', 'fixtures', 'x'].join(path.sep))).toBe(true);
+    expect(isFixtureSkill('test/fixtures/context-bill/tree-a/alpha')).toBe(true);
+    expect(isFixtureSkill('openclaw/skills/gstack-openclaw-retro')).toBe(false);
+    expect(bill.skills.every((s) => !s.name.includes('\\'))).toBe(true);
+  });
+
+  // Round-trip: a fresh capture must pass its own ratchet, and the headroom
+  // math must be exactly ceil(actual x headroom) — the recovery protocol is
+  // "re-run the capture", so a corrupt write side poisons every future fixture.
+  test('captureContextBudget round-trips against its own bill', () => {
+    const TREE_A = path.join(import.meta.dir, 'fixtures', 'context-bill', 'tree-a');
+    const capture = captureContextBudget(TREE_A);
+    const treeBill = buildRatchetBill(TREE_A);
+    expect(checkBudget(treeBill, capture)).toEqual([]);
+    for (const s of treeBill.skills) {
+      expect(capture.eagerPerInvocation[s.name]).toBe(Math.ceil(s.eagerTokens * EAGER_HEADROOM));
+    }
+    expect(capture.alwaysOnTotal).toBe(Math.ceil(treeBill.totals.alwaysOnTokens * ALWAYS_ON_HEADROOM));
+  });
 });

test/gen-skill-docs.test.ts

@@ -3295,6 +3295,29 @@ describe('voice-triggers processing', () => {
     const frontmatter = content.slice(0, fmEnd);
     expect(frontmatter).not.toContain('voice-triggers:');
   });
+
+  // Gen-time-only keys: interactive + benefits-from are read from the .tmpl by
+  // buildContext; the generated copy has no reader (the host reads name/
+  // description/allowed-tools/hooks; gbrain: is runtime-read and NOT stripped).
+  // Pin the strip so a stripFields refactor can't silently re-add the always-on
+  // frontmatter weight — mirrors the voice-triggers pins above.
+  test('generated SKILL.md strips gen-time-only keys the .tmpl still declares', () => {
+    const tmpl = fs.readFileSync(path.join(ROOT, 'plan-ceo-review', 'SKILL.md.tmpl'), 'utf-8');
+    const tmplFm = tmpl.slice(0, tmpl.indexOf('\n---', 4));
+    expect(tmplFm).toContain('interactive:');
+    expect(tmplFm).toContain('benefits-from:');
+
+    const generated = fs.readFileSync(path.join(ROOT, 'plan-ceo-review', 'SKILL.md'), 'utf-8');
+    const genFm = generated.slice(0, generated.indexOf('\n---', 4));
+    expect(genFm).not.toContain('interactive:');
+    expect(genFm).not.toContain('benefits-from:');
+
+    // The runtime-read and host-read keys survive the strip.
+    const investigate = fs.readFileSync(path.join(ROOT, 'investigate', 'SKILL.md'), 'utf-8');
+    const invFm = investigate.slice(0, investigate.indexOf('\n---', 4));
+    expect(invFm).toContain('hooks:');
+    expect(invFm).toContain('gbrain:');
+  });
 });
 
 describe('plan-mode-info resolver (handshake-replacement)', () => {

test/helpers/capture-context-budget.ts

@@ -35,9 +35,19 @@ export const BUDGET_FIXTURE_PATH = path.join(REPO_ROOT, 'test', 'fixtures', 'con
 export const ALWAYS_ON_HEADROOM = 1.05;
 export const EAGER_HEADROOM = 1.10;
 
+/**
+ * Skill names come from path.relative in buildBill, which yields backslash
+ * separators on Windows. The fixture keys are POSIX. Normalize once here so
+ * the filter, the fixture keys, and checkBudget's name matching agree on
+ * every platform (the ratchet test runs in the curated Windows lane).
+ */
+export function toPosixName(name: string): string {
+  return name.split(path.sep).join('/');
+}
+
 /** Skills that exist only as context-bill test data — never budgeted. */
 export function isFixtureSkill(name: string): boolean {
-  return name.startsWith('test/');
+  return toPosixName(name).startsWith('test/fixtures/');
 }
 
 export interface ContextBudget {
@@ -46,20 +56,32 @@ export interface ContextBudget {
   eagerPerInvocation: Record<string, number>;
 }
 
-/** The bill the ratchet grades: repo tree minus test-fixture skill dirs. */
+/**
+ * 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).
+ */
 export function buildRatchetBill(root: string = REPO_ROOT): Bill {
   const bill = buildBill(root);
-  const skills = bill.skills.filter((s) => !isFixtureSkill(s.name));
+  const skills = bill.skills
+    .map((s) => ({ ...s, name: toPosixName(s.name) }))
+    .filter((s) => !isFixtureSkill(s.name));
   return {
     ...bill,
     skills,
     totals: {
-      ...bill.totals,
       skillCount: skills.length,
       alwaysOnBytes: skills.reduce((n, s) => n + s.frontmatterBytes, 0),
       alwaysOnTokens: skills.reduce((n, s) => n + s.frontmatterTokens, 0),
       eagerBytesBySkill: Object.fromEntries(skills.map((s) => [s.name, s.eagerBytes])),
       eagerTokensBySkill: Object.fromEntries(skills.map((s) => [s.name, Math.round(s.eagerTokens)])),
+      perInvocationBytesBySkill: Object.fromEntries(skills.map((s) => [s.name, s.perInvocationBytes])),
+      perInvocationTokensBySkill: Object.fromEntries(
+        skills.map((s) => [s.name, Math.round(s.perInvocationTokens)]),
+      ),
+      totalMdBytes: skills.reduce((n, s) => n + s.totalMdBytes, 0),
+      totalMdTokens: skills.reduce((n, s) => n + s.totalMdTokens, 0),
     },
   };
 }