fix(scripts): coverage-ratchet sentinel matching immune to prose mentions + write sanity check

.github/workflows/ci.yml

@@ -83,10 +83,12 @@ jobs:
 
       # Backfill scripts have their own vitest context (scripts/vitest.config.ts)
       # and are not part of any workspace package, so the turbo test pipeline
-      # never sees them. Scoped to backfill-* because the older tenant-* and
-      # changelog-* suites in that config are broken independently of CI.
+      # never sees them. Explicitly listed by file (not a glob) because the
+      # older tenant-* and changelog-* suites in that config are broken
+      # independently of CI — a new one-off scripts/__tests__ file must be
+      # added to this list by hand to actually run here.
       - name: Run backfill script tests
-        run: cd scripts && bunx vitest run __tests__/backfill-home-drives.test.ts __tests__/backfill-task-verb-tools.test.ts __tests__/backfill-trash-verb-tools.test.ts __tests__/backfill-legacy-ciphertext-reencrypt.test.ts __tests__/promote-admin.test.ts __tests__/setup-onprem-admin.test.ts
+        run: cd scripts && bunx vitest run __tests__/backfill-home-drives.test.ts __tests__/backfill-task-verb-tools.test.ts __tests__/backfill-trash-verb-tools.test.ts __tests__/backfill-legacy-ciphertext-reencrypt.test.ts __tests__/promote-admin.test.ts __tests__/setup-onprem-admin.test.ts __tests__/coverage-ratchet-sentinel.test.ts
 
       - name: Coverage report
         if: always()

scripts/__tests__/coverage-ratchet-sentinel.test.ts

@@ -0,0 +1,194 @@
+import { describe, it, expect } from 'vitest';
+import {
+  matchThresholdBlock,
+  buildThresholdBlock,
+  parseThresholds,
+  assertValidSyntax,
+} from '../lib/coverage-ratchet-sentinel.mjs';
+
+// Mirrors the real corrupting comment from apps/web/vitest.config.ts (#2075):
+// a prose line that mentions the marker text literally, directly above the
+// real sentinel-delimited threshold block.
+const CONFIG_WITH_PROSE_MENTION = `export default defineConfig({
+  test: {
+    coverage: {
+      thresholds: {
+        // The /* ratchet:start */.../* ratchet:end */ markers below are load-bearing:
+        // some-script.mjs matches this exact comment-delimited region
+        /* ratchet:start */
+        lines: 44,
+        branches: 85,
+        functions: 56,
+        statements: 44,
+        /* ratchet:end */
+        'src/lib/foo/*.ts': { lines: 100, branches: 100, functions: 100, statements: 100 },
+      },
+    },
+  },
+})
+`;
+
+const CONFIG_WITHOUT_SENTINEL = `export default defineConfig({
+  test: {
+    coverage: {
+      thresholds: {
+        lines: 50,
+        branches: 89,
+        functions: 66,
+        statements: 50,
+      },
+    },
+  },
+})
+`;
+
+describe('matchThresholdBlock', () => {
+  it('ignores marker text embedded in prose and finds the real sentinel block', () => {
+    const found = matchThresholdBlock(CONFIG_WITH_PROSE_MENTION);
+
+    expect(found).not.toBeNull();
+    expect(found!.isSentinel).toBe(true);
+    expect(found!.match[0]).toContain('lines: 44,');
+    expect(found!.match[0]).toContain('statements: 44,');
+    // The fake span from the prose line must not be what matched.
+    expect(found!.match[0]).not.toContain('markers below are load-bearing');
+  });
+
+  it('extracts the real numeric thresholds, not zeros from a fake match', () => {
+    const found = matchThresholdBlock(CONFIG_WITH_PROSE_MENTION)!;
+
+    expect(parseThresholds(found.match[0])).toEqual({ lines: 44, branches: 85, functions: 56, statements: 44 });
+  });
+
+  it('falls back to the plain thresholds block when there is no sentinel', () => {
+    const found = matchThresholdBlock(CONFIG_WITHOUT_SENTINEL);
+
+    expect(found).not.toBeNull();
+    expect(found!.isSentinel).toBe(false);
+    expect(found!.match[0]).toContain('lines: 50,');
+  });
+
+  it('returns null when no thresholds block exists at all', () => {
+    expect(matchThresholdBlock('export default {}')).toBeNull();
+  });
+
+  it('matches a well-formed sentinel block with CRLF line endings (Windows/core.autocrlf checkouts)', () => {
+    const crlfConfig = CONFIG_WITH_PROSE_MENTION.replace(/\n/g, '\r\n');
+    const found = matchThresholdBlock(crlfConfig);
+
+    expect(found).not.toBeNull();
+    expect(found!.isSentinel).toBe(true);
+    expect(found!.match[0]).toContain('lines: 44,');
+  });
+
+  it('throws instead of guessing when two well-formed sentinel blocks exist', () => {
+    const twoSentinels = `thresholds: {
+        /* ratchet:start */
+        lines: 44,
+        branches: 85,
+        functions: 56,
+        statements: 44,
+        /* ratchet:end */
+      },
+      other: {
+        /* ratchet:start */
+        lines: 1,
+        branches: 2,
+        functions: 3,
+        statements: 4,
+        /* ratchet:end */
+      },`;
+
+    expect(() => matchThresholdBlock(twoSentinels)).toThrow(/2 ratchet:start\/ratchet:end sentinel blocks/);
+  });
+
+  it('throws instead of silently falling back to the corrupting plain match when the sentinel is malformed', () => {
+    // Trailing text on the ratchet:end line (e.g. from an autoformatter) means
+    // the marker no longer occupies its own line — this must NOT silently
+    // fall back to PLAIN_REGEX, which would truncate at the first `}` in the
+    // per-glob keys below and corrupt the config exactly like the original bug.
+    const malformed = `thresholds: {
+        /* ratchet:start */
+        lines: 44,
+        branches: 85,
+        functions: 56,
+        statements: 44,
+        /* ratchet:end */ // do not edit
+        'src/lib/foo/*.ts': { lines: 100, branches: 100, functions: 100, statements: 100 },
+      },`;
+
+    expect(() => matchThresholdBlock(malformed)).toThrow(/not a well-formed sentinel block/);
+  });
+});
+
+describe('parseThresholds', () => {
+  it('extracts all four numeric thresholds from a block of text', () => {
+    expect(parseThresholds('lines: 44,\nbranches: 85,\nfunctions: 56,\nstatements: 44,')).toEqual({
+      lines: 44,
+      branches: 85,
+      functions: 56,
+      statements: 44,
+    });
+  });
+
+  it('defaults missing keys to 0', () => {
+    expect(parseThresholds('lines: 44,')).toEqual({
+      lines: 44,
+      branches: 0,
+      functions: 0,
+      statements: 0,
+    });
+  });
+});
+
+describe('buildThresholdBlock', () => {
+  it('preserves the sentinel start marker\'s original indentation', () => {
+    const found = matchThresholdBlock(CONFIG_WITH_PROSE_MENTION)!;
+    const block = buildThresholdBlock({
+      isSentinel: true,
+      indent: found.indent,
+      thresholds: { lines: 60, branches: 90, functions: 70, statements: 60 },
+    });
+
+    expect(block.startsWith(`${found.indent}/* ratchet:start */`)).toBe(true);
+    expect(block).toContain('lines: 60,');
+    expect(block.endsWith('/* ratchet:end */')).toBe(true);
+  });
+
+  it('round-trips through a real rewrite without corrupting the surrounding config', () => {
+    const found = matchThresholdBlock(CONFIG_WITH_PROSE_MENTION)!;
+    const block = buildThresholdBlock({
+      isSentinel: true,
+      indent: found.indent,
+      thresholds: { lines: 60, branches: 90, functions: 70, statements: 60 },
+    });
+    const rewritten = CONFIG_WITH_PROSE_MENTION.replace(found.regex, block);
+
+    // The prose comment line and the trailing per-glob key must survive untouched.
+    expect(rewritten).toContain('markers below are load-bearing');
+    expect(rewritten).toContain("'src/lib/foo/*.ts': { lines: 100, branches: 100, functions: 100, statements: 100 },");
+    expect(rewritten).toContain('lines: 60,');
+    expect(rewritten).not.toContain('lines: 44,');
+  });
+});
+
+describe('assertValidSyntax', () => {
+  it('does not throw on valid source', () => {
+    expect(() => assertValidSyntax('export default { foo: 1 };\n', 'test-pkg')).not.toThrow();
+  });
+
+  it('throws on a corrupted rewrite (unterminated block comment)', () => {
+    const corrupted = 'export default {\n  /* ratchet:end */ markers below are load-bearing:\n';
+    expect(() => assertValidSyntax(corrupted, 'test-pkg')).toThrow(/test-pkg/);
+  });
+
+  it('accepts TS-only syntax (satisfies, type assertions) that a JS-only check would wrongly reject', () => {
+    const tsOnlySyntax = `
+      interface Thresholds { lines: number }
+      const t = { lines: 44 } satisfies Thresholds;
+      const u = t as Thresholds;
+      export default t;
+    `;
+    expect(() => assertValidSyntax(tsOnlySyntax, 'test-pkg')).not.toThrow();
+  });
+});

scripts/coverage-ratchet.mjs

@@ -10,10 +10,12 @@
 import { readFileSync, writeFileSync, existsSync } from 'node:fs';
 import { resolve, dirname } from 'node:path';
 import { fileURLToPath } from 'node:url';
+import { matchThresholdBlock, buildThresholdBlock, parseThresholds, assertValidSyntax } from './lib/coverage-ratchet-sentinel.mjs';
 
 const __dirname = dirname(fileURLToPath(import.meta.url));
 const root = resolve(__dirname, '..');
 const dryRun = process.argv.includes('--dry-run');
+let hadError = false;
 
 const packages = [
   { name: 'apps/web', config: 'apps/web/vitest.config.ts', summary: 'apps/web/coverage/coverage-summary.json' },
@@ -46,28 +48,28 @@ for (const pkg of packages) {
     statements: Math.floor(t.statements.pct),
   };
 
-  // Match the sentinel-marked ratchet region first — packages with per-glob
-  // threshold keys after the four ratcheted scalars (e.g. apps/web's 100%
-  // gates on new pure modules) would otherwise have `[^}]+` stop at the
-  // glob sub-objects' first `}`, truncating the match and corrupting the
-  // rewrite. Fall back to the plain block match for packages with no
-  // sentinel, so processor/realtime/db/lib keep working unchanged.
-  const sentinelRegex = /\/\* ratchet:start \*\/[\s\S]*?\/\* ratchet:end \*\//;
-  const plainRegex = /thresholds:\s*\{[^}]+\}/s;
-  const sentinelMatch = config.match(sentinelRegex);
-  const match = sentinelMatch ?? config.match(plainRegex);
-  const thresholdRegex = sentinelMatch ? sentinelRegex : plainRegex;
-
-  if (!match) {
+  let found;
+  try {
+    found = matchThresholdBlock(config);
+  } catch (err) {
+    console.error(`[fail] ${pkg.name}: ${err.message}`);
+    hadError = true;
+    continue;
+  }
+
+  if (!found) {
     console.log(`[skip] ${pkg.name}: no thresholds block found in vitest config`);
     continue;
   }
 
-  // Extract current thresholds
-  const currentLines = parseInt(match[0].match(/lines:\s*(\d+)/)?.[1] ?? '0');
-  const currentBranches = parseInt(match[0].match(/branches:\s*(\d+)/)?.[1] ?? '0');
-  const currentFunctions = parseInt(match[0].match(/functions:\s*(\d+)/)?.[1] ?? '0');
-  const currentStatements = parseInt(match[0].match(/statements:\s*(\d+)/)?.[1] ?? '0');
+  const { match, regex: thresholdRegex, isSentinel, indent } = found;
+
+  const {
+    lines: currentLines,
+    branches: currentBranches,
+    functions: currentFunctions,
+    statements: currentStatements,
+  } = parseThresholds(match[0]);
 
   // Only ratchet UP, never down
   const finalThresholds = {
@@ -88,14 +90,19 @@ for (const pkg of packages) {
     continue;
   }
 
-  // The sentinel rewrite preserves everything after `ratchet:end` (the
-  // per-glob 100% keys) — only the four ratcheted scalars inside the markers
-  // are replaced.
-  const newBlock = sentinelMatch
-    ? `/* ratchet:start */\n        lines: ${finalThresholds.lines},\n        branches: ${finalThresholds.branches},\n        functions: ${finalThresholds.functions},\n        statements: ${finalThresholds.statements},\n        /* ratchet:end */`
-    : `thresholds: {\n        lines: ${finalThresholds.lines},\n        branches: ${finalThresholds.branches},\n        functions: ${finalThresholds.functions},\n        statements: ${finalThresholds.statements},\n      }`;
+  const newBlock = buildThresholdBlock({ isSentinel, indent, thresholds: finalThresholds });
+
+  const newConfig = config.replace(thresholdRegex, newBlock);
 
-  config = config.replace(thresholdRegex, newBlock);
+  try {
+    assertValidSyntax(newConfig, pkg.name);
+  } catch (err) {
+    console.error(`[fail] ${err.message}`);
+    hadError = true;
+    continue;
+  }
+
+  config = newConfig;
 
   if (dryRun) {
     console.log(`[dry]  ${pkg.name}: would update thresholds:`);
@@ -112,3 +119,7 @@ for (const pkg of packages) {
 }
 
 console.log(`\n${dryRun ? 'Would update' : 'Updated'} ${updated} package(s).`);
+
+if (hadError) {
+  process.exit(1);
+}

scripts/lib/coverage-ratchet-sentinel.mjs

@@ -0,0 +1,93 @@
+/**
+ * Pure, unit-testable pieces of scripts/coverage-ratchet.mjs's threshold-block
+ * matching, rewriting, and pre-write syntax check.
+ */
+
+import ts from 'typescript';
+
+// Markers must occupy their own line (only leading/trailing whitespace besides
+// the marker) — real usage always looks like that, whereas a prose comment
+// mentioning the markers (e.g. "the /* ratchet:start */.../* ratchet:end */
+// markers below are load-bearing") has other text sharing the line and is
+// correctly rejected. `\r?` tolerates CRLF line endings: .gitattributes does
+// not force LF for .ts files, so a Windows/core.autocrlf checkout can have
+// \r\n here even though the repo's own files are LF.
+const SENTINEL_REGEX = /^([ \t]*)\/\* ratchet:start \*\/[ \t]*\r?\n[\s\S]*?^[ \t]*\/\* ratchet:end \*\/[ \t]*\r?$/m;
+const SENTINEL_REGEX_GLOBAL = new RegExp(SENTINEL_REGEX.source, `${SENTINEL_REGEX.flags}g`);
+
+const PLAIN_REGEX = /thresholds:\s*\{[^}]+\}/s;
+
+// Match the sentinel-marked ratchet region first — packages with per-glob
+// threshold keys after the four ratcheted scalars (e.g. apps/web's 100% gates
+// on new pure modules) would otherwise have `[^}]+` stop at the glob
+// sub-objects' first `}`, truncating the match and corrupting the rewrite.
+// Fall back to the plain block match for packages with no sentinel, so
+// processor/realtime/db/lib keep working unchanged.
+//
+// Two "refuse to guess" guards beyond the line-anchoring above:
+//  - More than one well-formed sentinel block in the same file is ambiguous
+//    (which one is real?) — error instead of silently taking the first match.
+//  - Marker text present but NOT forming a well-formed block (e.g. an
+//    autoformatter appends a trailing comment to the `ratchet:end` line) must
+//    NOT silently fall back to the plain match — that fallback is only safe
+//    for configs that never intended to use a sentinel at all, and applying
+//    it to a malformed sentinel reproduces the original truncation bug on
+//    apps/web's per-glob keys.
+export function matchThresholdBlock(config) {
+  const sentinelMatches = [...config.matchAll(SENTINEL_REGEX_GLOBAL)];
+  if (sentinelMatches.length > 1) {
+    throw new Error(`found ${sentinelMatches.length} ratchet:start/ratchet:end sentinel blocks — expected at most one; refusing to guess which is real`);
+  }
+
+  const sentinelMatch = sentinelMatches[0];
+  if (sentinelMatch) {
+    return { match: sentinelMatch, regex: SENTINEL_REGEX, isSentinel: true, indent: sentinelMatch[1] };
+  }
+
+  if (config.includes('ratchet:start') || config.includes('ratchet:end')) {
+    throw new Error('found "ratchet:start"/"ratchet:end" marker text that is not a well-formed sentinel block (each marker must occupy its own line) — refusing to fall back to the plain thresholds match, which would corrupt the per-glob keys after it');
+  }
+
+  const plainMatch = config.match(PLAIN_REGEX);
+  if (plainMatch) {
+    return { match: plainMatch, regex: PLAIN_REGEX, isSentinel: false, indent: '' };
+  }
+  return null;
+}
+
+// The sentinel rewrite preserves everything after `ratchet:end` (the per-glob
+// 100% keys) and the start marker's original indentation — only the four
+// ratcheted scalars inside the markers are replaced.
+export function buildThresholdBlock({ isSentinel, indent, thresholds }) {
+  const { lines, branches, functions, statements } = thresholds;
+  return isSentinel
+    ? `${indent}/* ratchet:start */\n        lines: ${lines},\n        branches: ${branches},\n        functions: ${functions},\n        statements: ${statements},\n        /* ratchet:end */`
+    : `thresholds: {\n        lines: ${lines},\n        branches: ${branches},\n        functions: ${functions},\n        statements: ${statements},\n      }`;
+}
+
+// Extracts the current numeric thresholds from a matched block's text.
+export function parseThresholds(matchText) {
+  return {
+    lines: parseInt(matchText.match(/lines:\s*(\d+)/)?.[1] ?? '0'),
+    branches: parseInt(matchText.match(/branches:\s*(\d+)/)?.[1] ?? '0'),
+    functions: parseInt(matchText.match(/functions:\s*(\d+)/)?.[1] ?? '0'),
+    statements: parseInt(matchText.match(/statements:\s*(\d+)/)?.[1] ?? '0'),
+  };
+}
+
+// Runs a TypeScript syntax-only check (no type-checking, no file I/O, no
+// subprocess) on the rewritten config string — catches any rewrite that
+// produces unparseable output (e.g. a sentinel match landing mid-comment/
+// mid-token) before it ever reaches disk, rather than trusting the regex
+// match alone. Uses the TypeScript parser rather than `node --check` because
+// these are .ts files: a config using TS-only syntax (`satisfies`, a type
+// assertion, a generic) is valid input that a JS-only syntax check would
+// wrongly reject.
+export function assertValidSyntax(source, label) {
+  const { diagnostics } = ts.transpileModule(source, { reportDiagnostics: true });
+  const errors = (diagnostics ?? []).filter((d) => d.category === ts.DiagnosticCategory.Error);
+  if (errors.length > 0) {
+    const messages = errors.map((d) => ts.flattenDiagnosticMessageText(d.messageText, '\n')).join('\n');
+    throw new Error(`${label}: rewritten config fails a syntax check — refusing to write.\n${messages}`);
+  }
+}