chore(ci): ratchet TypeScript errors, because npm test does not typecheck (#1980)

Vitest transpiles and discards types, so a type error passes all 18 000 tests
and only surfaces in npm run build several minutes later. That happened twice
on 2026-08-27: a widened union in the MCP server that a second declaration in
lib/events/types.ts still contradicted, and an interface that would not assign
into Record<string, unknown>[] because interfaces have no implicit index
signature. Both were caught by the build. Neither was caught by the tests,
which is the wrong order to learn it in.

This is not just a faster copy of the build job. tsc --noEmit also covers
__tests__ files, which the Next.js build never compiles, and that is where all
539 baseline errors live.

Baselined per FILE rather than per error code, unlike the lint ratchet: the
legacy errors sit in a handful of old test files and TS2322 is common enough
that a code-keyed budget would let a real regression hide behind a legacy fix
somewhere else.

Measured: 36s cold, which is what CI pays, and 4.4s warm locally.

Verified the gate fires by introducing a deliberate type error and watching it
fail with the exact location, then restoring.

Co-authored-by: Jakob Wennberg <311770904+jakobwennberg-oss@users.noreply.github.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Jakob Wennberg
2026-08-27 17:53:04 +02:00
committed by GitHub
parent e41cf50afe
commit 304baf1089
6 changed files with 243 additions and 0 deletions
+13
View File
@@ -58,6 +58,19 @@ jobs:
# baseline ratchets down as legacy errors get fixed.
run: npm run check:lint
- name: Typecheck ratchet (no new TypeScript errors)
# `npm test` does not typecheck: vitest transpiles and discards types,
# so a type error passes all 18 000 tests and only surfaces in the
# build. That happened twice on 2026-08-27. This also covers __tests__
# files, which the Next.js build never compiles, so it is not merely a
# faster copy of the build job.
#
# Baselined per FILE in scripts/checks/typecheck-baseline.json (539
# legacy errors across 83 files, all in test files) and ratchets down.
# Per-file rather than per-error-code on purpose: a code-keyed budget
# would let a real regression hide behind a legacy fix elsewhere.
run: npm run check:types
- name: Antipattern ratchet (no new MFA-bypassing routes / naive öre-rounding)
# Fails only if a PR ADDS a route that hand-rolls supabase.auth.getUser()
# instead of the MFA-enforcing guard, or a new Math.round(x*100)/100.
+1
View File
@@ -60,6 +60,7 @@ npm test # All Vitest tests
npx vitest run <dir> # Tests in one directory
npm run test:pg # pg-real tests against real Postgres
npm run check:guards # Ratchet guard (e.g. no hand-rolled route auth)
npm run check:types # Typecheck ratchet. `npm test` does NOT typecheck: run this before the build
npm run setup:extensions # Regenerate extension registry from extensions.config.json
npm run skills:generate # Regenerate agent_atom_registry seed after editing an atom SKILL.md
```
+1
View File
@@ -1296,3 +1296,4 @@ One line per decision: `[YYYY-MM-DD] <decision>: <why>`. Appended by agents and
[2026-08-27] gnubok_get_agent_briefing's outputSchema condensed 7 743 -> 4 565 chars by replacing four sub-schema interiors (ledger_context, dimensions, skatteverket_connection, recommended_tools) with a permissive `{type:'object'}` plus a fuller description, keeping the property declared so the top-level `additionalProperties: false` still holds. Deleting the properties was not an option for that reason, and adding `additionalProperties:false` to the condensed forms would have rejected the real payload. Safe because agent-briefing.test.ts pins the RUNTIME shape of all four blocks, so the contract stays guarded while the schema stops carrying 3 178 chars of documentation into every tools/list. Kept intact: `company` (its accounting_method prose drives the settlement posting), `atoms` and `memory` (they tell the agent to fetch bodies via gnubok_load_skill). Net with the new tool: guarded (accounted) projection 63 491 -> 62 942 tokens, and the payload ceiling TIGHTENS 63.6K -> 63.1K, the first downward move in that ledger.
[2026-08-27] Cockpit auto-landing gated to byrå owner/admin (isCockpitLandingRole; landing route + '/' bounce), superseding the 2026-08-05 all-members widening: plain members land like regular users and open the cockpit from the nav; the middleware zero-company steer stays ungated because a member with no client companies has nowhere else to land. Allowlist over role!=='member' so future roles default to the regular landing.
[2026-08-27] Klarmarkera (markPeriodClosedExternally) gets an undo, reopenExternallyClosedPeriod, allowed only while the closed state still comes from klarmarkera (closed_externally set, no closing entry): that close was a person's control decision without a bokslutsverifikat, so reversing it strands nothing, whereas a closePeriod close keeps its closing entry and stays irreversible here. The reopen clears the lock too, because the reason to reopen is to change the period's contents (Forsslund Systems 2026-08-27: five imported years klarmarkerade, then the prior-year SIE turned out wrong; replace refused the closed year, unlock refused the closed state, no way back). Audit_log row plus period.unlocked event; the MCP staged-op surface (lock/unlock) does not get a reopen op yet, follow-up.
[2026-08-27] Added `npm run check:types`, a typecheck ratchet (scripts/checks/no-new-type-errors.mjs + typecheck-baseline.json), wired into the core-build `checks` job next to check:lint. Reason: `npm test` does NOT typecheck. Vitest transpiles and discards types, so a type error passes all 18 000 tests and only surfaces in `npm run build` minutes later; that happened TWICE on 2026-08-27 (a widened errorKind union in the MCP server that lib/events/types.ts still contradicted, and an `interface` that would not assign into `Record<string, unknown>[]` because interfaces have no implicit index signature). It is not merely a faster copy of the build job: `tsc --noEmit` also covers `__tests__` files, which the Next.js build never compiles, and that is where all 539 baseline errors live. Baseline is keyed per FILE, deliberately unlike the per-RULE lint ratchet: the legacy errors are concentrated in a handful of old test files and TS2322 is common enough that a code-keyed budget would silently absorb a real regression somewhere else, whereas per-file trips the moment a previously-clean file gains an error. Verified the gate actually fires by introducing a deliberate `const x: number = 'str'` and watching it fail with the exact location, then restoring. Cost measured: 36 s cold (what CI pays, since tsconfig.tsbuildinfo is gitignored) and 4.4 s warm locally via the existing `incremental: true`. The script sets NODE_OPTIONS=--max-old-space-size=8192 because a bare tsc dies with "Ineffective mark-compacts near heap limit" on this graph after about two minutes, which reads like a hang rather than a misconfiguration; it also detects that OOM string and exits 2 with a "raise HEAP_MB" message rather than silently reporting zero errors. NOT changed: Definition of Done item 1 still says only lint + test. CI enforcement is the stronger mechanism and does not need the policy edit; adding it to DoD is a founder call.
+1
View File
@@ -23,6 +23,7 @@
"validate:packs": "npx tsx scripts/validate-packs.ts",
"validate:registry": "npx tsx scripts/validate-registry.ts",
"check:lint": "node scripts/checks/no-new-lint-errors.mjs",
"check:types": "node scripts/checks/no-new-type-errors.mjs",
"test": "vitest run --project unit",
"test:pg": "vitest run --project pg-real"
},
+139
View File
@@ -0,0 +1,139 @@
#!/usr/bin/env node
/**
* Ratchet guard for TypeScript errors (sibling of no-new-lint-errors.mjs).
*
* ## Why this exists
*
* `npm test` does not typecheck. Vitest transpiles and throws the types away,
* so a type error passes the entire 18 000-test suite and only surfaces in
* `npm run build`, several minutes later. That happened twice on 2026-08-27
* alone: a widened union in the MCP server that a second declaration in
* lib/events/types.ts still contradicted, and an `interface` that would not
* assign into `Record<string, unknown>[]` because interfaces have no implicit
* index signature. Both were caught by the build. Neither was caught by 18 000
* green tests, which is exactly the wrong order to learn it in.
*
* `tsc --noEmit` finds both in about two minutes, and unlike the build it also
* covers `__tests__` files, which the Next.js build never compiles.
*
* ## Why the baseline is keyed by FILE, not by error code
*
* The lint ratchet counts per rule, and accepts the tradeoff that fixing one
* legacy error of a rule lets a new one in. For types that tradeoff is worse:
* the pre-existing errors are concentrated in a handful of old test files, and
* TS2322 ("not assignable") is common enough that a per-code budget would
* silently absorb a real regression somewhere else entirely. Keyed by file, a
* new error in a previously-clean file trips immediately, which is the case
* that actually matters.
*
* Usage:
* node scripts/checks/no-new-type-errors.mjs # check
* node scripts/checks/no-new-type-errors.mjs --update # re-baseline
*
* Exit code 1 if any file's error count exceeds its baseline.
*/
import fs from 'node:fs'
import path from 'node:path'
import { spawnSync } from 'node:child_process'
import { fileURLToPath } from 'node:url'
const ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..', '..')
const BASELINE_PATH = path.join(ROOT, 'scripts', 'checks', 'typecheck-baseline.json')
/**
* This project's graph does not fit in Node's default heap: a bare
* `tsc --noEmit` dies with "Ineffective mark-compacts near heap limit" after
* about two minutes of work, which reads like a hang rather than a
* misconfiguration. The build sets the same flag for the same reason.
*/
const HEAP_MB = 8192
function runTsc() {
const tscBin = path.join(ROOT, 'node_modules', 'typescript', 'bin', 'tsc')
const result = spawnSync(process.execPath, [tscBin, '--noEmit', '--pretty', 'false'], {
cwd: ROOT,
encoding: 'utf8',
maxBuffer: 64 * 1024 * 1024,
env: { ...process.env, NODE_OPTIONS: `--max-old-space-size=${HEAP_MB}` },
})
const output = `${result.stdout ?? ''}${result.stderr ?? ''}`
if (/Ineffective mark-compacts|JavaScript heap out of memory/.test(output)) {
console.error(`no-new-type-errors: tsc ran out of memory at ${HEAP_MB} MB. Raise HEAP_MB.`)
process.exit(2)
}
// tsc exits non-zero when errors exist, which is the normal case here.
return output
}
/** `path/to/file.ts(12,34): error TS2322: ...` */
const ERROR_RE = /^(.+?)\((\d+),(\d+)\): error (TS\d+): (.*)$/
function collect(output) {
/** @type {Record<string, number>} */
const perFile = {}
/** @type {Record<string, string[]>} */
const locations = {}
for (const line of output.split('\n')) {
const match = ERROR_RE.exec(line.trim())
if (!match) continue
const [, file, lineNo, col, code, message] = match
const rel = path.relative(ROOT, path.resolve(ROOT, file)).split(path.sep).join('/')
perFile[rel] = (perFile[rel] ?? 0) + 1
;(locations[rel] ??= []).push(`${rel}:${lineNo}:${col} ${code} ${message}`)
}
return { perFile, locations }
}
const { perFile, locations } = collect(runTsc())
const total = Object.values(perFile).reduce((a, b) => a + b, 0)
if (process.argv.includes('--update')) {
const sorted = Object.fromEntries(Object.entries(perFile).sort(([a], [b]) => a.localeCompare(b)))
fs.writeFileSync(
BASELINE_PATH,
JSON.stringify({ totalErrors: total, perFile: sorted }, null, 2) + '\n',
)
console.log(
`no-new-type-errors: baseline updated: ${total} error(s) across ${Object.keys(perFile).length} file(s).`,
)
process.exit(0)
}
if (!fs.existsSync(BASELINE_PATH)) {
console.error(`no-new-type-errors: baseline missing at ${path.relative(ROOT, BASELINE_PATH)}.`)
console.error('Run: node scripts/checks/no-new-type-errors.mjs --update')
process.exit(2)
}
const baseline = JSON.parse(fs.readFileSync(BASELINE_PATH, 'utf8'))
const baselineFiles = baseline.perFile ?? {}
const regressions = []
for (const [file, count] of Object.entries(perFile)) {
const allowed = baselineFiles[file] ?? 0
if (count > allowed) regressions.push({ file, count, allowed })
}
if (regressions.length > 0) {
console.error('no-new-type-errors: FAILED: new TypeScript errors beyond the baseline:\n')
for (const { file, count, allowed } of regressions) {
console.error(` ${file}: ${count} (baseline ${allowed})`)
for (const loc of (locations[file] ?? []).slice(0, 10)) {
console.error(` ${loc}`)
}
}
console.error(`
Fix the new error(s): run \`NODE_OPTIONS=--max-old-space-size=${HEAP_MB} npx tsc --noEmit\` to see them all.
(If you fixed MORE legacy errors than you added and a file still trips,
re-baseline with: node scripts/checks/no-new-type-errors.mjs --update)
`)
process.exit(1)
}
const improved = total < (baseline.totalErrors ?? 0)
console.log(
`no-new-type-errors: OK: ${total} error(s), baseline ${baseline.totalErrors}.` +
(improved
? ' Count went DOWN: ratchet it: node scripts/checks/no-new-type-errors.mjs --update'
: ''),
)
+88
View File
@@ -0,0 +1,88 @@
{
"totalErrors": 539,
"perFile": {
"app/api/assets/__tests__/id.test.ts": 9,
"app/api/auth/email-hook/__tests__/route.test.ts": 1,
"app/api/auth/heartbeat/__tests__/route.test.ts": 1,
"app/api/bookkeeping/fiscal-periods/__tests__/route.test.ts": 23,
"app/api/bookkeeping/fiscal-periods/[id]/bokslutsdispositioner/__tests__/route.test.ts": 4,
"app/api/bookkeeping/journal-entries/__tests__/route.test.ts": 26,
"app/api/bookkeeping/journal-entries/[id]/chain/__tests__/route.test.ts": 2,
"app/api/bookkeeping/no-doc-required/batch/__tests__/route.test.ts": 6,
"app/api/bookkeeping/no-doc-required/bulk-missing/__tests__/route.test.ts": 9,
"app/api/bookkeeping/voucher-gaps/__tests__/route.test.ts": 9,
"app/api/customers/__tests__/viewer.test.ts": 2,
"app/api/documents/counts/__tests__/route.test.ts": 11,
"app/api/export/articles/__tests__/route.test.ts": 3,
"app/api/export/suppliers/__tests__/route.test.ts": 2,
"app/api/extensions/shopify/orders/cron/__tests__/route.test.ts": 2,
"app/api/extensions/skatteverket/agi/kvittenser/cron/__tests__/route.test.ts": 6,
"app/api/import/articles/__tests__/execute.test.ts": 23,
"app/api/import/bank-file/__tests__/route.test.ts": 1,
"app/api/import/bank-file/check-duplicates/__tests__/route.test.ts": 8,
"app/api/import/opening-balance/__tests__/correct.test.ts": 28,
"app/api/import/opening-balance/__tests__/execute.test.ts": 18,
"app/api/import/opening-balance/correct/__tests__/route.test.ts": 10,
"app/api/invoices/__tests__/route.test.ts": 24,
"app/api/invoices/[id]/peppol/__tests__/route.test.ts": 1,
"app/api/invoices/[id]/send-payment-confirmation/__tests__/route.test.ts": 8,
"app/api/invoices/bulk-book/__tests__/route.test.ts": 1,
"app/api/invoices/self-billed/__tests__/route.test.ts": 8,
"app/api/onboarding/state/__tests__/route.test.ts": 5,
"app/api/pending-operations/bulk-commit/__tests__/route.test.ts": 11,
"app/api/pending-operations/bulk-reject/__tests__/route.test.ts": 10,
"app/api/reports/audit-trail/__tests__/route.test.ts": 4,
"app/api/reports/bokslutsbilagor/__tests__/route.test.ts": 8,
"app/api/reports/full-archive/__tests__/route.test.ts": 11,
"app/api/rot-rut/__tests__/routes.test.ts": 13,
"app/api/rot-rut/beslut/__tests__/route.test.ts": 4,
"app/api/settings/api-keys/__tests__/route.test.ts": 6,
"app/api/settings/booking-templates/sync/cron/__tests__/route.test.ts": 1,
"app/api/settings/eu-trade-signal/__tests__/route.test.ts": 3,
"app/api/settings/ku-signal/__tests__/route.test.ts": 4,
"app/api/settings/rot-rut-signal/__tests__/route.test.ts": 4,
"app/api/supplier-invoices/__tests__/route.test.ts": 41,
"app/api/supplier-invoices/payment-batches/__tests__/route.test.ts": 12,
"app/api/tax-assessment-notices/__tests__/route.test.ts": 5,
"app/api/transactions/bulk-book/__tests__/route.test.ts": 16,
"app/api/v1/companies/[companyId]/customers/__tests__/route.test.ts": 1,
"app/api/webshop-orders/__tests__/bulk-book.test.ts": 1,
"app/api/webshop-orders/__tests__/list-and-settings.test.ts": 11,
"extensions/general/enable-banking/__tests__/session-expired.test.ts": 4,
"extensions/general/enable-banking/__tests__/supersede.test.ts": 1,
"extensions/general/invoice-inbox/__tests__/match-transaction.test.ts": 1,
"extensions/general/invoice-inbox/__tests__/retry-extraction.test.ts": 1,
"extensions/general/invoice-inbox/__tests__/sandbox-skip-extraction.test.ts": 1,
"extensions/general/invoice-inbox/__tests__/upload-page-count-gate.test.ts": 1,
"extensions/general/mcp-server/__tests__/account-tools.test.ts": 2,
"extensions/general/mcp-server/__tests__/connect-links.test.ts": 1,
"extensions/general/mcp-server/__tests__/payroll-staged-tools.test.ts": 21,
"extensions/general/mcp-server/__tests__/salary-tools.test.ts": 4,
"extensions/general/mcp-server/__tests__/set-voucher-note.test.ts": 1,
"extensions/general/mcp-server/__tests__/skills.test.ts": 2,
"extensions/general/skatteverket/__tests__/api-client.test.ts": 5,
"extensions/general/skatteverket/__tests__/skattekonto-mappers.test.ts": 1,
"lib/__tests__/logger.test.ts": 1,
"lib/api/v1/__tests__/with-api-v1.test.ts": 9,
"lib/auth/__tests__/require-write.test.ts": 15,
"lib/bokslut/__tests__/k3-framework-dispositions.test.ts": 2,
"lib/bokslut/__tests__/readiness-aggregator.test.ts": 6,
"lib/bookkeeping/__tests__/own-account-detector.test.ts": 1,
"lib/bookkeeping/__tests__/supplier-payment-lines.test.ts": 14,
"lib/branding/__tests__/public-brand.test.ts": 1,
"lib/email/__tests__/brand-mail-snapshots.test.ts": 1,
"lib/email/__tests__/brand-sender.test.ts": 1,
"lib/email/__tests__/reminder-templates.test.ts": 6,
"lib/import/__tests__/account-mapper.test.ts": 1,
"lib/import/__tests__/sie-import.test.ts": 1,
"lib/invoices/__tests__/pdf-template-amounts.test.ts": 1,
"lib/invoices/__tests__/peppol-bis-billing.test.ts": 1,
"lib/invoices/__tests__/reminder-processor.test.ts": 3,
"lib/invoices/__tests__/supplier-invoice-matching.test.ts": 1,
"lib/pending-operations/__tests__/commit-authorization-recoverable.test.ts": 1,
"lib/reports/__tests__/vat-declaration.test.ts": 5,
"lib/supabase/__tests__/middleware.test.ts": 2,
"tests/pg/categorize-calibration-samples.pg.test.ts": 2,
"tests/pg/match-batch-allocate.pg.test.ts": 1
}
}