feat(invariants): shared format contracts + upgrade-path CI (#1364)
* feat(invariants): centralise shared format contracts, reconcile the org-number paths
The same format rules were written out independently across the codebase, and
where they disagreed the disagreement was invisible until a filing failed.
Worst case, now fixed: four Skatteverket- and Bolagsverket-bound export paths
each had their own idea of a valid organisationsnummer.
lib/skatteverket/format.ts strip '-' only threw on any input with a space
lib/salary/ku/ku10-generator.ts replace('-', '') first hyphen only, spaces survived
lib/salary/agi/xml-generator.ts strip non-digits stray letters passed the length check
lib/bokslut/ixbrl/validate /^\d{6}-?\d{4}$/ rejected the 12-digit form, no Luhn
A company stored with a space or in 12-digit form could file AGI all year and
then fail at the arsredovisning deadline with a message that did not say why.
lib/invariants/ now owns account number, ISO date, four-digit fiscal year and
org number, each with the rationale recorded next to the rule. normalizeOrgNumber
moves here from lib/company-lookup/ and isSaneDateString from lib/utils.ts; both
old paths re-export, so no caller changes. lib/api/schemas.ts builds its
primitives on the module, so ~100 schemas inherit any correction.
The arsredovisning check-digit verdict is a warn, not an error: a wrong Luhn
digit is almost certainly a typo worth surfacing, but whether every org number
Bolagsverket accepts satisfies Luhn is a Swedish domain question we have not
verified against a primary source, and an error there blocks Skicka in. We do
not block a statutory filing on an unverified assumption.
KU10 still passes a 12-digit stored org number through unfolded. That is
pre-existing, and whether the KU10 schema wants 10 or 12 digits is not covered
by the swedish-payroll skill, so it is pinned by a test rather than changed
silently.
Guard 8 (hand-rolled-invariant) tracks the remaining 114 inline copies as a
ratchet that may only go down, same mechanism as the roundOre guard.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* test(ci): add an upgrade-path job that applies new migrations against real data
The pg-real job applies all 548 migrations to an EMPTY database. Empty means
zero rows, so a migration that adds a NOT NULL, adds a CHECK, creates a unique
index or backfills passes trivially in CI and can still fail on production,
where the rows exist. CI proved that a fresh install works; nothing proved that
an existing install upgrades.
The new pg-upgrade job: apply the schema as it stands at the merge base, seed a
small real company (three posted verifikat, balanced lines, one ore-level
amount), then apply ONLY the migrations this PR adds, then assert the data
survived (entries still posted, lines intact, ledger still balances, ore
unchanged, voucher numbers sequential). A PR with no migration no-ops.
Verified locally against supabase/postgres:15.8.1.060 rather than assumed, with
three deliberately bad migrations:
rescale money on posted lines empty: would pass seeded: ERROR (immutability trigger)
CHECK violating the ore row empty: exit 0 seeded: exit 3
NOT NULL on a populated column empty: exit 0 seeded: exit 3
Base migrations are read out of the merge-base git tree, not the working tree,
so a PR that edits an already-shipped migration still gets the original applied
and the edit surfaces as a failure here.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* docs: record the invariants and upgrade-CI decisions
Two entries covering what this PR changes and, more importantly, the calls that
are not obvious from the diff: why the arsredovisning check-digit verdict is a
warning rather than an error, why KU10's 12-digit passthrough is pinned instead
of fixed, and why the ROT/RUT brf org-number schemas stay on their own rule.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* docs(test): mark the upgrade fixture as CI-only, never a production template
The fixture writes posted journal_entries and their lines directly, bypassing
the engine and the atomic commit RPC. That is the only way to hand a migration
pre-existing posted rows to break, and it is safe against a throwaway CI
database, but it reads like a sanctioned pattern to anyone who finds it later.
Says so explicitly, with the reason it is confined here (no voucher sequence to
keep gapless, no retention obligation on a database destroyed with the job) and
a pointer back to Hard Rule 2 for anything touching a real database.
Raised by the Swedish compliance review bot on #1364.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---------
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
@@ -9,6 +9,9 @@
|
||||
"naiveOreRound": {
|
||||
"count": 641
|
||||
},
|
||||
"handRolledInvariants": {
|
||||
"count": 114
|
||||
},
|
||||
"ledgerScanningReports": {
|
||||
"count": 4,
|
||||
"files": [
|
||||
|
||||
@@ -51,6 +51,13 @@
|
||||
* extensionRegistry.get('<id>') so a disabled extension never exposes a
|
||||
* live surface (allowlisted file-set, may only shrink). Implementation
|
||||
* and rationale in extension-route-guards.mjs.
|
||||
* 8. hand-rolled-invariant: a shared format rule (BAS account number, ISO
|
||||
* date, four-digit fiscal year) spelled out inline instead of imported
|
||||
* from lib/invariants/. The BAS account rule was written out at 20 sites
|
||||
* and the ISO date rule at 68, with error messages that differed per site;
|
||||
* the four Skatteverket-bound org-number paths disagreed outright about
|
||||
* what "valid" meant, which is the kind of drift a customer only discovers
|
||||
* when a filing fails at the deadline. Tracked as a count.
|
||||
*
|
||||
* Usage:
|
||||
* node scripts/checks/no-new-antipatterns.mjs # check (CI)
|
||||
@@ -83,6 +90,25 @@ const RAW_AUTH_RE = /\.auth\.getUser\(/
|
||||
const GUARD_RE = /requireAuth\(|withRouteContext[<(]/
|
||||
const NAIVE_ROUND_RE = /Math\.round\([^\n]*\*\s*100\s*\)\s*\/\s*100/
|
||||
|
||||
// 8. hand-rolled-invariant. Shared format contracts live in lib/invariants/
|
||||
// (account number, ISO date, four-digit fiscal year, org number). Before that
|
||||
// module the BAS account rule was written out at 20 sites and the ISO date rule
|
||||
// at 68, with error messages that differed per site, and the four
|
||||
// Skatteverket-bound org-number paths did not agree on what "valid" meant.
|
||||
//
|
||||
// Only the two unambiguous regex families are counted. An org-number
|
||||
// digit-strip is too varied in shape to match reliably by regex; the
|
||||
// cross-path test in lib/invariants/__tests__/org-number-cross-path.test.ts is
|
||||
// the guard on that one instead.
|
||||
const HAND_ROLLED_INVARIANT_RES = [
|
||||
// /^\d{4}$/ or /^[0-9]{4}$/ → accountNumberSchema or fiscalYearSchema
|
||||
/\/\^(?:\\d|\[0-9\])\{4\}\$\//,
|
||||
// /^\d{4}-\d{2}-\d{2}$/ → isoDateSchema or ISO_DATE_RE
|
||||
/\/\^(?:\\d|\[0-9\])\{4\}-(?:\\d|\[0-9\])\{2\}-(?:\\d|\[0-9\])\{2\}\$\//,
|
||||
]
|
||||
// The sanctioned home of these rules: must not count against itself.
|
||||
const INVARIANT_EXEMPT_PREFIX = 'lib/invariants/'
|
||||
|
||||
function walk(dir, exts, out = []) {
|
||||
let entries
|
||||
try {
|
||||
@@ -231,6 +257,31 @@ function countNaiveRound() {
|
||||
return count
|
||||
}
|
||||
|
||||
/**
|
||||
* Occurrences of a shared format rule written out by hand instead of imported
|
||||
* from lib/invariants/. Counted, not file-setted: the campaign lowers the
|
||||
* number file by file and the count may only go down.
|
||||
*/
|
||||
function countHandRolledInvariants() {
|
||||
const files = [
|
||||
...walk(path.join(ROOT, 'lib'), ['.ts', '.tsx']),
|
||||
...walk(path.join(ROOT, 'app'), ['.ts', '.tsx']),
|
||||
...walk(path.join(ROOT, 'components'), ['.ts', '.tsx']),
|
||||
...walk(path.join(ROOT, 'extensions'), ['.ts', '.tsx']),
|
||||
]
|
||||
let count = 0
|
||||
for (const f of files) {
|
||||
const relPath = rel(f)
|
||||
if (relPath.startsWith(INVARIANT_EXEMPT_PREFIX)) continue
|
||||
// Tests legitimately spell out the pattern they are asserting about.
|
||||
if (relPath.includes('__tests__/') || relPath.endsWith('.test.ts')) continue
|
||||
for (const line of fs.readFileSync(f, 'utf8').split('\n')) {
|
||||
if (HAND_ROLLED_INVARIANT_RES.some((re) => re.test(line))) count++
|
||||
}
|
||||
}
|
||||
return count
|
||||
}
|
||||
|
||||
// Dependencies pinned to an EXACT version on purpose, because a bump broke prod
|
||||
// and must not silently return via `npm update`, a dependabot bump, or a manual
|
||||
// install. Any drift (in package.json OR the lockfile) fails CI. See DECISIONS.md.
|
||||
@@ -563,6 +614,7 @@ function findRawUserErrors() {
|
||||
const current = {
|
||||
rawRouteAuth: findRawRouteAuth(),
|
||||
naiveOreRound: countNaiveRound(),
|
||||
handRolledInvariants: countHandRolledInvariants(),
|
||||
ledgerScanningReports: findLedgerScanningReports(),
|
||||
directJelInsert: findDirectJelInserts(),
|
||||
pinnedDepViolations: findPinnedDepViolations(),
|
||||
@@ -579,6 +631,7 @@ if (isUpdate) {
|
||||
'Ratchet baseline for scripts/checks/no-new-antipatterns.mjs. These counts may only decrease. Re-run with --update after a migration lowers them. Goal: both reach 0 (A1 route-auth campaign, D1 rounding codemod).',
|
||||
rawRouteAuth: { count: current.rawRouteAuth.length, files: current.rawRouteAuth },
|
||||
naiveOreRound: { count: current.naiveOreRound },
|
||||
handRolledInvariants: { count: current.handRolledInvariants },
|
||||
ledgerScanningReports: {
|
||||
count: current.ledgerScanningReports.length,
|
||||
files: current.ledgerScanningReports,
|
||||
@@ -675,6 +728,20 @@ if (current.sekLabelledAmounts.length) {
|
||||
)
|
||||
}
|
||||
|
||||
// 1e2. hand-rolled-invariant: counted, may only go down.
|
||||
if (current.handRolledInvariants > (baseline.handRolledInvariants?.count ?? Infinity)) {
|
||||
failed = true
|
||||
console.error(
|
||||
`\n✗ hand-rolled-invariant: ${current.handRolledInvariants} inline copies of a shared format rule ` +
|
||||
`(baseline ${baseline.handRolledInvariants?.count}):`,
|
||||
)
|
||||
console.error(
|
||||
' → import the rule instead: accountNumberSchema / isoDateSchema / saneIsoDateSchema /\n' +
|
||||
' fiscalYearSchema from @/lib/invariants/zod, or the ACCOUNT_NUMBER_RE / ISO_DATE_RE\n' +
|
||||
' constants from @/lib/invariants. See lib/invariants/README.md.',
|
||||
)
|
||||
}
|
||||
|
||||
// 1f. cross-extension-import: a physical extension route may only import its
|
||||
// own extension. No baseline: the count is 0 today, any hit is a hard failure.
|
||||
if (current.extensionRoutes.crossImports.length) {
|
||||
@@ -772,5 +839,5 @@ if (failed) {
|
||||
process.exit(1)
|
||||
}
|
||||
console.log(
|
||||
`\n✓ Antipattern guard passed (raw-route-auth: ${current.rawRouteAuth.length}, naive-ore-round: ${current.naiveOreRound}, ledger-scanning-report: ${current.ledgerScanningReports.length}, direct-jel-insert: 0, pinned-dep: 0, raw-user-error: 0, sek-labelled-amount: 0, cross-extension-import: 0, ungated-extension-route: ${current.extensionRoutes.ungated.length}/${UNGATED_EXTENSION_ROUTES.size} allowlisted).`,
|
||||
`\n✓ Antipattern guard passed (raw-route-auth: ${current.rawRouteAuth.length}, naive-ore-round: ${current.naiveOreRound}, hand-rolled-invariant: ${current.handRolledInvariants}, ledger-scanning-report: ${current.ledgerScanningReports.length}, direct-jel-insert: 0, pinned-dep: 0, raw-user-error: 0, sek-labelled-amount: 0, cross-extension-import: 0, ungated-extension-route: ${current.extensionRoutes.ungated.length}/${UNGATED_EXTENSION_ROUTES.size} allowlisted).`,
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user