From 6e5096453c2b073558eeb65add8fc7fad5258f4c Mon Sep 17 00:00:00 2001 From: 2witstudios <2witstudios@gmail.com> Date: Wed, 15 Jul 2026 00:37:44 -0500 Subject: [PATCH 1/5] fix(scripts): coverage-ratchet sentinel matching immune to prose mentions + write sanity check coverage-ratchet.mjs's sentinel regex was fooled by apps/web/vitest.config.ts's own explanatory comment, which contains the literal substrings `/* ratchet:start */` and `/* ratchet:end */` in a single-line prose comment. The non-greedy regex matched that fake 40-char span instead of the real threshold block: --dry-run reported fabricated zeros for current thresholds, and a real (non-dry-run) invocation would have rewritten the wrong span, corrupting vitest.config.ts into unparseable syntax and breaking every apps/web test. - sentinelRegex now requires the ratchet:start/ratchet:end markers to occupy their own line, so prose mentioning the marker text no longer matches. - Extracted the matching/rewrite logic into scripts/lib/coverage-ratchet-sentinel.mjs so it's directly unit-testable. - Added a node --check write-sanity-check: before any write (dry-run or real), the rewritten config is validated as parseable syntax; a mismatch aborts with a non-zero exit instead of writing corrupted output. - New tests in scripts/__tests__/coverage-ratchet-sentinel.test.ts, wired into scripts/vitest.config.ts's include glob and the CI backfill-script test step. Co-Authored-By: Claude Fable 5 --- .github/workflows/ci.yml | 2 +- .../coverage-ratchet-sentinel.test.ts | 133 ++++++++++++++++++ scripts/coverage-ratchet.mjs | 47 ++++--- scripts/lib/coverage-ratchet-sentinel.mjs | 61 ++++++++ scripts/vitest.config.ts | 2 +- 5 files changed, 223 insertions(+), 22 deletions(-) create mode 100644 scripts/__tests__/coverage-ratchet-sentinel.test.ts create mode 100644 scripts/lib/coverage-ratchet-sentinel.mjs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f037ca1924..eb74e7d3f1 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -86,7 +86,7 @@ jobs: # never sees them. Scoped to backfill-* because the older tenant-* and # changelog-* suites in that config are broken independently of CI. - 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() diff --git a/scripts/__tests__/coverage-ratchet-sentinel.test.ts b/scripts/__tests__/coverage-ratchet-sentinel.test.ts new file mode 100644 index 0000000000..7c6aed98fd --- /dev/null +++ b/scripts/__tests__/coverage-ratchet-sentinel.test.ts @@ -0,0 +1,133 @@ +import { describe, it, expect } from 'vitest'; +import { mkdtempSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { + matchThresholdBlock, + buildThresholdBlock, + 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)!; + const lines = parseInt(found.match[0].match(/lines:\s*(\d+)/)?.[1] ?? '0'); + const branches = parseInt(found.match[0].match(/branches:\s*(\d+)/)?.[1] ?? '0'); + + expect(lines).toBe(44); + expect(branches).toBe(85); + }); + + 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(); + }); +}); + +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.match[1], + thresholds: { lines: 60, branches: 90, functions: 70, statements: 60 }, + }); + + expect(block.startsWith(`${found.match[1]}/* 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.match[1], + 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', () => { + let dir: string; + + it('does not throw on valid source', () => { + dir = mkdtempSync(join(tmpdir(), 'ratchet-sentinel-test-')); + try { + expect(() => assertValidSyntax('export default { foo: 1 };\n', 'test-pkg', dir)).not.toThrow(); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + + it('throws on a corrupted rewrite (unterminated block comment)', () => { + dir = mkdtempSync(join(tmpdir(), 'ratchet-sentinel-test-')); + try { + const corrupted = 'export default {\n /* ratchet:end */ markers below are load-bearing:\n'; + expect(() => assertValidSyntax(corrupted, 'test-pkg', dir)).toThrow(/test-pkg/); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); +}); diff --git a/scripts/coverage-ratchet.mjs b/scripts/coverage-ratchet.mjs index f5686671ee..4f0a9565f2 100644 --- a/scripts/coverage-ratchet.mjs +++ b/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, 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,23 +48,15 @@ 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) { + const found = matchThresholdBlock(config); + + if (!found) { console.log(`[skip] ${pkg.name}: no thresholds block found in vitest config`); continue; } + const { match, regex: thresholdRegex, isSentinel } = found; + // 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'); @@ -88,14 +82,23 @@ 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: isSentinel ? match[1] : '', + thresholds: finalThresholds, + }); + + const newConfig = config.replace(thresholdRegex, newBlock); + + try { + assertValidSyntax(newConfig, pkg.name, root); + } catch (err) { + console.error(`[fail] ${err.message}`); + hadError = true; + continue; + } - config = config.replace(thresholdRegex, newBlock); + config = newConfig; if (dryRun) { console.log(`[dry] ${pkg.name}: would update thresholds:`); @@ -112,3 +115,7 @@ for (const pkg of packages) { } console.log(`\n${dryRun ? 'Would update' : 'Updated'} ${updated} package(s).`); + +if (hadError) { + process.exit(1); +} diff --git a/scripts/lib/coverage-ratchet-sentinel.mjs b/scripts/lib/coverage-ratchet-sentinel.mjs new file mode 100644 index 0000000000..71d73cf7a3 --- /dev/null +++ b/scripts/lib/coverage-ratchet-sentinel.mjs @@ -0,0 +1,61 @@ +/** + * Pure, unit-testable pieces of scripts/coverage-ratchet.mjs's threshold-block + * matching, rewriting, and pre-write syntax check. + */ + +import { writeFileSync, unlinkSync } from 'node:fs'; +import { execFileSync } from 'node:child_process'; +import { resolve } from 'node:path'; + +// 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. +export const SENTINEL_REGEX = /^([ \t]*)\/\* ratchet:start \*\/[ \t]*\r?\n[\s\S]*?^[ \t]*\/\* ratchet:end \*\/[ \t]*\r?$/m; + +export 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. +export function matchThresholdBlock(config) { + const sentinelMatch = config.match(SENTINEL_REGEX); + if (sentinelMatch) { + return { match: sentinelMatch, regex: SENTINEL_REGEX, isSentinel: true }; + } + const plainMatch = config.match(PLAIN_REGEX); + if (plainMatch) { + return { match: plainMatch, regex: PLAIN_REGEX, isSentinel: false }; + } + 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 }`; +} + +// Writes `source` to a throwaway .mjs file and runs `node --check` on it — +// 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. +export function assertValidSyntax(source, label, tmpDir) { + const checkPath = resolve(tmpDir, `.coverage-ratchet-check-${process.pid}.mjs`); + writeFileSync(checkPath, source); + try { + execFileSync(process.execPath, ['--check', checkPath], { stdio: 'pipe' }); + } catch (err) { + throw new Error(`${label}: rewritten config fails a syntax check — refusing to write.\n${err.stderr?.toString() ?? err.message}`); + } finally { + unlinkSync(checkPath); + } +} diff --git a/scripts/vitest.config.ts b/scripts/vitest.config.ts index da3b6cd1f9..e7e48168fa 100644 --- a/scripts/vitest.config.ts +++ b/scripts/vitest.config.ts @@ -5,7 +5,7 @@ export default defineConfig({ globals: false, testTimeout: 30_000, hookTimeout: 60_000, - include: ['__tests__/tenant-*.test.ts', '__tests__/cutover-*.test.ts', '__tests__/changelog-*.test.ts', '__tests__/backfill-*.test.ts', '__tests__/promote-admin.test.ts', '__tests__/setup-onprem-admin.test.ts'], + include: ['__tests__/tenant-*.test.ts', '__tests__/cutover-*.test.ts', '__tests__/changelog-*.test.ts', '__tests__/backfill-*.test.ts', '__tests__/promote-admin.test.ts', '__tests__/setup-onprem-admin.test.ts', '__tests__/coverage-ratchet-*.test.ts'], pool: 'forks', poolOptions: { forks: { singleFork: true }, From 57a245faeb3fac9fc543dcc4d1ab1db50dd5adee Mon Sep 17 00:00:00 2001 From: 2witstudios <2witstudios@gmail.com> Date: Wed, 15 Jul 2026 00:51:21 -0500 Subject: [PATCH 2/5] harden: sentinel uniqueness + malformed-marker guard, temp-dir fix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Self-triggered 8-angle review (no external reviewer available yet) on the coverage-ratchet fix surfaced two real residual hazards and some cleanup, all fixed here: - assertValidSyntax was writing its throwaway syntax-check file into the repo root (tmpDir=root passed from the caller) instead of the OS temp dir. If the process were killed between the write and the finally's cleanup, a stray file would be left in the working tree. Now uses mkdtempSync(os.tmpdir()) internally; the tmpDir param is gone. (Independently flagged by 5 of 8 finder agents.) - matchThresholdBlock now refuses to guess in two ambiguous cases instead of silently doing the wrong thing: - More than one well-formed sentinel block in the same config file throws (which one is real?). - Marker text present but NOT forming a well-formed block (e.g. an autoformatter appends a trailing comment to the ratchet:end line) throws instead of silently falling back to the plain thresholds match — that fallback is only safe for configs with no sentinel at all, and applying it to a malformed sentinel reproduces the original truncation bug on apps/web's per-glob keys. - Extracted parseThresholds() as its own tested pure function instead of four inline regex lines duplicated at the call site. - Simplified indent handling: matchThresholdBlock now returns `indent` directly (empty string for the non-sentinel case) instead of callers re-deriving it from `isSentinel ? match[1] : ''`. - Dropped the speculative `\r?` CRLF tolerance in the regex — this repo has no CRLF line endings (no .gitattributes eol=crlf, CI runs ubuntu-latest), so it was untested dead-code generality. - Small CI comment fix: the "Run backfill script tests" step's comment said it was "scoped to backfill-*" when it already listed several non-backfill one-off files by name; reworded to describe the actual explicit-list mechanism. 4 new tests added (12 total, up from 8) covering the uniqueness guard, the malformed-marker guard, and parseThresholds. Co-Authored-By: Claude Fable 5 --- .github/workflows/ci.yml | 6 +- .../coverage-ratchet-sentinel.test.ts | 87 ++++++++++++++----- scripts/coverage-ratchet.mjs | 32 ++++--- scripts/lib/coverage-ratchet-sentinel.mjs | 59 ++++++++++--- 4 files changed, 134 insertions(+), 50 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index eb74e7d3f1..b46a4179ee 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -83,8 +83,10 @@ 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 __tests__/coverage-ratchet-sentinel.test.ts diff --git a/scripts/__tests__/coverage-ratchet-sentinel.test.ts b/scripts/__tests__/coverage-ratchet-sentinel.test.ts index 7c6aed98fd..2e1845b0a1 100644 --- a/scripts/__tests__/coverage-ratchet-sentinel.test.ts +++ b/scripts/__tests__/coverage-ratchet-sentinel.test.ts @@ -1,10 +1,8 @@ import { describe, it, expect } from 'vitest'; -import { mkdtempSync, rmSync } from 'node:fs'; -import { tmpdir } from 'node:os'; -import { join } from 'node:path'; import { matchThresholdBlock, buildThresholdBlock, + parseThresholds, assertValidSyntax, } from '../lib/coverage-ratchet-sentinel.mjs'; @@ -76,6 +74,65 @@ describe('matchThresholdBlock', () => { it('returns null when no thresholds block exists at all', () => { expect(matchThresholdBlock('export default {}')).toBeNull(); }); + + 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', () => { @@ -83,11 +140,11 @@ describe('buildThresholdBlock', () => { const found = matchThresholdBlock(CONFIG_WITH_PROSE_MENTION)!; const block = buildThresholdBlock({ isSentinel: true, - indent: found.match[1], + indent: found.indent, thresholds: { lines: 60, branches: 90, functions: 70, statements: 60 }, }); - expect(block.startsWith(`${found.match[1]}/* ratchet:start */`)).toBe(true); + expect(block.startsWith(`${found.indent}/* ratchet:start */`)).toBe(true); expect(block).toContain('lines: 60,'); expect(block.endsWith('/* ratchet:end */')).toBe(true); }); @@ -96,7 +153,7 @@ describe('buildThresholdBlock', () => { const found = matchThresholdBlock(CONFIG_WITH_PROSE_MENTION)!; const block = buildThresholdBlock({ isSentinel: true, - indent: found.match[1], + indent: found.indent, thresholds: { lines: 60, branches: 90, functions: 70, statements: 60 }, }); const rewritten = CONFIG_WITH_PROSE_MENTION.replace(found.regex, block); @@ -110,24 +167,12 @@ describe('buildThresholdBlock', () => { }); describe('assertValidSyntax', () => { - let dir: string; - it('does not throw on valid source', () => { - dir = mkdtempSync(join(tmpdir(), 'ratchet-sentinel-test-')); - try { - expect(() => assertValidSyntax('export default { foo: 1 };\n', 'test-pkg', dir)).not.toThrow(); - } finally { - rmSync(dir, { recursive: true, force: true }); - } + expect(() => assertValidSyntax('export default { foo: 1 };\n', 'test-pkg')).not.toThrow(); }); it('throws on a corrupted rewrite (unterminated block comment)', () => { - dir = mkdtempSync(join(tmpdir(), 'ratchet-sentinel-test-')); - try { - const corrupted = 'export default {\n /* ratchet:end */ markers below are load-bearing:\n'; - expect(() => assertValidSyntax(corrupted, 'test-pkg', dir)).toThrow(/test-pkg/); - } finally { - rmSync(dir, { recursive: true, force: true }); - } + const corrupted = 'export default {\n /* ratchet:end */ markers below are load-bearing:\n'; + expect(() => assertValidSyntax(corrupted, 'test-pkg')).toThrow(/test-pkg/); }); }); diff --git a/scripts/coverage-ratchet.mjs b/scripts/coverage-ratchet.mjs index 4f0a9565f2..2d5c25424c 100644 --- a/scripts/coverage-ratchet.mjs +++ b/scripts/coverage-ratchet.mjs @@ -10,7 +10,7 @@ import { readFileSync, writeFileSync, existsSync } from 'node:fs'; import { resolve, dirname } from 'node:path'; import { fileURLToPath } from 'node:url'; -import { matchThresholdBlock, buildThresholdBlock, assertValidSyntax } from './lib/coverage-ratchet-sentinel.mjs'; +import { matchThresholdBlock, buildThresholdBlock, parseThresholds, assertValidSyntax } from './lib/coverage-ratchet-sentinel.mjs'; const __dirname = dirname(fileURLToPath(import.meta.url)); const root = resolve(__dirname, '..'); @@ -48,20 +48,28 @@ for (const pkg of packages) { statements: Math.floor(t.statements.pct), }; - const found = matchThresholdBlock(config); + 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; } - const { match, regex: thresholdRegex, isSentinel } = found; + const { match, regex: thresholdRegex, isSentinel, indent } = found; - // 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 { + lines: currentLines, + branches: currentBranches, + functions: currentFunctions, + statements: currentStatements, + } = parseThresholds(match[0]); // Only ratchet UP, never down const finalThresholds = { @@ -82,16 +90,12 @@ for (const pkg of packages) { continue; } - const newBlock = buildThresholdBlock({ - isSentinel, - indent: isSentinel ? match[1] : '', - thresholds: finalThresholds, - }); + const newBlock = buildThresholdBlock({ isSentinel, indent, thresholds: finalThresholds }); const newConfig = config.replace(thresholdRegex, newBlock); try { - assertValidSyntax(newConfig, pkg.name, root); + assertValidSyntax(newConfig, pkg.name); } catch (err) { console.error(`[fail] ${err.message}`); hadError = true; diff --git a/scripts/lib/coverage-ratchet-sentinel.mjs b/scripts/lib/coverage-ratchet-sentinel.mjs index 71d73cf7a3..8d3ca4e5af 100644 --- a/scripts/lib/coverage-ratchet-sentinel.mjs +++ b/scripts/lib/coverage-ratchet-sentinel.mjs @@ -3,16 +3,18 @@ * matching, rewriting, and pre-write syntax check. */ -import { writeFileSync, unlinkSync } from 'node:fs'; +import { writeFileSync, mkdtempSync, rmSync } from 'node:fs'; import { execFileSync } from 'node:child_process'; -import { resolve } from 'node:path'; +import { join } from 'node:path'; +import { tmpdir } from 'node:os'; // 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. -export const SENTINEL_REGEX = /^([ \t]*)\/\* ratchet:start \*\/[ \t]*\r?\n[\s\S]*?^[ \t]*\/\* ratchet:end \*\/[ \t]*\r?$/m; +export const SENTINEL_REGEX = /^([ \t]*)\/\* ratchet:start \*\/[ \t]*\n[\s\S]*?^[ \t]*\/\* ratchet:end \*\/[ \t]*$/m; +const SENTINEL_REGEX_GLOBAL = new RegExp(SENTINEL_REGEX.source, `${SENTINEL_REGEX.flags}g`); export const PLAIN_REGEX = /thresholds:\s*\{[^}]+\}/s; @@ -22,14 +24,34 @@ export const PLAIN_REGEX = /thresholds:\s*\{[^}]+\}/s; // 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 sentinelSpans = config.match(SENTINEL_REGEX_GLOBAL); + if (sentinelSpans && sentinelSpans.length > 1) { + throw new Error(`found ${sentinelSpans.length} ratchet:start/ratchet:end sentinel blocks — expected at most one; refusing to guess which is real`); + } + const sentinelMatch = config.match(SENTINEL_REGEX); if (sentinelMatch) { - return { match: sentinelMatch, regex: SENTINEL_REGEX, isSentinel: true }; + 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 }; + return { match: plainMatch, regex: PLAIN_REGEX, isSentinel: false, indent: '' }; } return null; } @@ -44,18 +66,29 @@ export function buildThresholdBlock({ isSentinel, indent, thresholds }) { : `thresholds: {\n lines: ${lines},\n branches: ${branches},\n functions: ${functions},\n statements: ${statements},\n }`; } -// Writes `source` to a throwaway .mjs file and runs `node --check` on it — -// 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. -export function assertValidSyntax(source, label, tmpDir) { - const checkPath = resolve(tmpDir, `.coverage-ratchet-check-${process.pid}.mjs`); - writeFileSync(checkPath, source); +// 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'), + }; +} + +// Writes `source` to a throwaway .mjs file (in the OS temp dir, never the +// repo) and runs `node --check` on it — 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. +export function assertValidSyntax(source, label) { + const dir = mkdtempSync(join(tmpdir(), 'coverage-ratchet-check-')); + const checkPath = join(dir, 'check.mjs'); try { + writeFileSync(checkPath, source); execFileSync(process.execPath, ['--check', checkPath], { stdio: 'pipe' }); } catch (err) { throw new Error(`${label}: rewritten config fails a syntax check — refusing to write.\n${err.stderr?.toString() ?? err.message}`); } finally { - unlinkSync(checkPath); + rmSync(dir, { recursive: true, force: true }); } } From 20b870c7e35780aeef270031a2cedb2995124cc8 Mon Sep 17 00:00:00 2001 From: 2witstudios <2witstudios@gmail.com> Date: Wed, 15 Jul 2026 00:55:00 -0500 Subject: [PATCH 3/5] fix(scripts): use TS-aware syntax check, not node --check on a renamed .mjs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review feedback from chatgpt-codex-connector (P2): assertValidSyntax wrote the rewritten config to a .mjs file and ran `node --check` on it, which validates plain JavaScript syntax — but all five configs are .ts files. If a future PR added TS-only syntax to any of them (satisfies, a type assertion, a generic), coverage-ratchet.mjs would refuse to ratchet that package forever with a misleading "fails a syntax check" error, even though the config is valid TypeScript that Vitest loads fine. Replaced the node --check subprocess (temp dir + write + spawn + cleanup) with an in-process ts.transpileModule() syntax-only check (no type-checking, no file I/O, no subprocess) using the TypeScript compiler already a devDependency of this workspace. This is strictly better than the previous fix: correctly TS-aware, faster (no subprocess spawn), and has no temp-file lifecycle to get wrong. New test proves the exact scenario the reviewer described: a config using `satisfies`/type assertions is accepted (13 tests total, up from 12). Co-Authored-By: Claude Fable 5 --- .../coverage-ratchet-sentinel.test.ts | 10 ++++++ scripts/lib/coverage-ratchet-sentinel.mjs | 31 +++++++++---------- 2 files changed, 24 insertions(+), 17 deletions(-) diff --git a/scripts/__tests__/coverage-ratchet-sentinel.test.ts b/scripts/__tests__/coverage-ratchet-sentinel.test.ts index 2e1845b0a1..b08b4bb509 100644 --- a/scripts/__tests__/coverage-ratchet-sentinel.test.ts +++ b/scripts/__tests__/coverage-ratchet-sentinel.test.ts @@ -175,4 +175,14 @@ describe('assertValidSyntax', () => { 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(); + }); }); diff --git a/scripts/lib/coverage-ratchet-sentinel.mjs b/scripts/lib/coverage-ratchet-sentinel.mjs index 8d3ca4e5af..d29ea217f6 100644 --- a/scripts/lib/coverage-ratchet-sentinel.mjs +++ b/scripts/lib/coverage-ratchet-sentinel.mjs @@ -3,10 +3,7 @@ * matching, rewriting, and pre-write syntax check. */ -import { writeFileSync, mkdtempSync, rmSync } from 'node:fs'; -import { execFileSync } from 'node:child_process'; -import { join } from 'node:path'; -import { tmpdir } from 'node:os'; +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 @@ -76,19 +73,19 @@ export function parseThresholds(matchText) { }; } -// Writes `source` to a throwaway .mjs file (in the OS temp dir, never the -// repo) and runs `node --check` on it — 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. +// 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 dir = mkdtempSync(join(tmpdir(), 'coverage-ratchet-check-')); - const checkPath = join(dir, 'check.mjs'); - try { - writeFileSync(checkPath, source); - execFileSync(process.execPath, ['--check', checkPath], { stdio: 'pipe' }); - } catch (err) { - throw new Error(`${label}: rewritten config fails a syntax check — refusing to write.\n${err.stderr?.toString() ?? err.message}`); - } finally { - rmSync(dir, { recursive: true, force: true }); + 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}`); } } From 2426c4e05531b6846d18236d494845dbd1dc901c Mon Sep 17 00:00:00 2001 From: 2witstudios <2witstudios@gmail.com> Date: Wed, 15 Jul 2026 01:24:14 -0500 Subject: [PATCH 4/5] fix(scripts): tolerate CRLF sentinel marker lines MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review feedback from chatgpt-codex-connector (P3): apps/web/vitest.config.ts is not forced to LF by .gitattributes, so a Windows/core.autocrlf checkout can have \r\n line endings. The line-anchored sentinel regex required a bare \n after the start marker and only [ \t]* before end-of-line on the end marker, so a CRLF checkout's well-formed sentinel block would fail to match — and the malformed-marker guard added earlier would then treat it as corrupt and refuse to update thresholds, a false failure on an otherwise valid config. Restored \r? tolerance around both line anchors (this had been present in an earlier iteration and was dropped as "unexercised generality" by an internal simplification pass before external review caught the actual portability gap). New test proves a CRLF-normalized copy of the real sentinel fixture still matches (14 tests total, up from 13). Co-Authored-By: Claude Fable 5 --- scripts/__tests__/coverage-ratchet-sentinel.test.ts | 9 +++++++++ scripts/lib/coverage-ratchet-sentinel.mjs | 6 ++++-- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/scripts/__tests__/coverage-ratchet-sentinel.test.ts b/scripts/__tests__/coverage-ratchet-sentinel.test.ts index b08b4bb509..a4e057e641 100644 --- a/scripts/__tests__/coverage-ratchet-sentinel.test.ts +++ b/scripts/__tests__/coverage-ratchet-sentinel.test.ts @@ -75,6 +75,15 @@ describe('matchThresholdBlock', () => { 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 */ diff --git a/scripts/lib/coverage-ratchet-sentinel.mjs b/scripts/lib/coverage-ratchet-sentinel.mjs index d29ea217f6..60b4e5f4f9 100644 --- a/scripts/lib/coverage-ratchet-sentinel.mjs +++ b/scripts/lib/coverage-ratchet-sentinel.mjs @@ -9,8 +9,10 @@ import ts from 'typescript'; // 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. -export const SENTINEL_REGEX = /^([ \t]*)\/\* ratchet:start \*\/[ \t]*\n[\s\S]*?^[ \t]*\/\* ratchet:end \*\/[ \t]*$/m; +// 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. +export 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`); export const PLAIN_REGEX = /thresholds:\s*\{[^}]+\}/s; From 5d9c069cb8bb8b8f0c237e20254d3fc842220e34 Mon Sep 17 00:00:00 2001 From: 2witstudios <2witstudios@gmail.com> Date: Wed, 15 Jul 2026 01:54:00 -0500 Subject: [PATCH 5/5] simplify: consolidate sentinel regex scan, un-export unused constants MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ran /simplify (4 parallel cleanup agents: reuse, simplification, efficiency, altitude) against the full diff. Reuse, efficiency, and altitude all came back clean (altitude explicitly confirmed the regex+guards+ts.transpileModule approach is proportionate to the problem for this manually-run, git-diff-reviewed internal tool — an AST-based rewrite would be over-engineering here). Simplification found four real items; applied the two cheap, in-scope ones: - matchThresholdBlock scanned the config twice (once via a global regex just for the ambiguity count, once via a plain regex to recover the indent capture group). Replaced with a single matchAll() pass that yields both the count and the capture group. - SENTINEL_REGEX/PLAIN_REGEX were exported with no consumer outside this file (confirmed via grep) — un-exported them. - Simplified a test that re-implemented parseThresholds's regex extraction inline to just call parseThresholds directly. Skipped: dropping the unused `indent: ''` field from the non-sentinel branch — real but marginal (an unread destructured field, not actual complexity), not worth the interface churn. Co-Authored-By: Claude Fable 5 --- scripts/__tests__/coverage-ratchet-sentinel.test.ts | 5 +---- scripts/lib/coverage-ratchet-sentinel.mjs | 12 ++++++------ 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/scripts/__tests__/coverage-ratchet-sentinel.test.ts b/scripts/__tests__/coverage-ratchet-sentinel.test.ts index a4e057e641..d80a789524 100644 --- a/scripts/__tests__/coverage-ratchet-sentinel.test.ts +++ b/scripts/__tests__/coverage-ratchet-sentinel.test.ts @@ -56,11 +56,8 @@ describe('matchThresholdBlock', () => { it('extracts the real numeric thresholds, not zeros from a fake match', () => { const found = matchThresholdBlock(CONFIG_WITH_PROSE_MENTION)!; - const lines = parseInt(found.match[0].match(/lines:\s*(\d+)/)?.[1] ?? '0'); - const branches = parseInt(found.match[0].match(/branches:\s*(\d+)/)?.[1] ?? '0'); - expect(lines).toBe(44); - expect(branches).toBe(85); + 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', () => { diff --git a/scripts/lib/coverage-ratchet-sentinel.mjs b/scripts/lib/coverage-ratchet-sentinel.mjs index 60b4e5f4f9..96d6038a08 100644 --- a/scripts/lib/coverage-ratchet-sentinel.mjs +++ b/scripts/lib/coverage-ratchet-sentinel.mjs @@ -12,10 +12,10 @@ import ts from 'typescript'; // 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. -export const SENTINEL_REGEX = /^([ \t]*)\/\* ratchet:start \*\/[ \t]*\r?\n[\s\S]*?^[ \t]*\/\* ratchet:end \*\/[ \t]*\r?$/m; +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`); -export const PLAIN_REGEX = /thresholds:\s*\{[^}]+\}/s; +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 @@ -34,12 +34,12 @@ export const PLAIN_REGEX = /thresholds:\s*\{[^}]+\}/s; // it to a malformed sentinel reproduces the original truncation bug on // apps/web's per-glob keys. export function matchThresholdBlock(config) { - const sentinelSpans = config.match(SENTINEL_REGEX_GLOBAL); - if (sentinelSpans && sentinelSpans.length > 1) { - throw new Error(`found ${sentinelSpans.length} ratchet:start/ratchet:end sentinel blocks — expected at most one; refusing to guess which is real`); + 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 = config.match(SENTINEL_REGEX); + const sentinelMatch = sentinelMatches[0]; if (sentinelMatch) { return { match: sentinelMatch, regex: SENTINEL_REGEX, isSentinel: true, indent: sentinelMatch[1] }; }