fix(vat): momsdeklaration defaults respect the configured cadence and persist manual changes (#1998)

* fix(vat): momsdeklaration defaults respect the configured cadence and persist manual changes

The period picker re-seeded from scratch on every visit: an arsmoms user
whose moms_period was never set landed on a silently guessed quarterly
declaration (companies without a company_settings row bypassed every
gate), and a manually chosen cadence evaporated on the next visit.

- Gate the view when no company_settings row exists, matching the
  existing "registered but no period" gate: a declaration for the wrong
  period type is a compliance hazard, not a convenience.
- Persist the manually chosen cadence per company (localStorage,
  FyPicker pattern) and restore it while moms_period is unchanged; the
  concrete period still re-seeds to the most recently ended one, and a
  changed setting discards the stored cadence.
- Extract the seeding decision into lib/vat/period-selection.ts with
  unit tests.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012pQn9kC742B9R7Ggi8wdn9

* fix(vat): drop cadence persistence; the moms_period re-seed is the control

Skeptic review refuted the persistence half of the previous commit twice:
the render-phase localStorage restore diverged from SSR (hydration error
on every visit once a cadence was stored), and restoring a manually
chosen cadence that deviates from moms_period kept the filing pipeline
open on the wrong period type across visits, with no downstream path
validating period type against the setting.

The redovisningsperiod has exactly one lawful value per company, so the
mount-time re-seed from company_settings.moms_period is the self-healing
control, not a bug. The settings-row gate and the extracted, tested
seeding resolver stay.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012pQn9kC742B9R7Ggi8wdn9

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Mattsson
2026-08-27 19:09:47 +02:00
committed by GitHub
co-authored by Claude Fable 5
parent 4f6ecad549
commit fca57dc470
4 changed files with 147 additions and 17 deletions
+1
View File
@@ -1301,4 +1301,5 @@ One line per decision: `[YYYY-MM-DD] <decision>: <why>`. Appended by agents and
[2026-08-27] Brand domains gate signup server-side (brands.signup_mode + brand_signup_allowlist), not in the register page: the browser used to call supabase.auth.signUp directly, so any client-side host check would be cosmetic. Email signup moved to POST /api/auth/signup on ALL hosts (byte-identical GoTrue call for open hosts); BankID gates in /bankid/complete; Google gates via the dashboard layout's brand-domain bounce (the account exists after OAuth, but gets no branded experience). Company invites bypass the allowlist because the invite is the authorization. Allowlisted signups' companies attach to the brand's byrå team via create_company_for_brand_signup (allowlist entry = the byrå's standing WL-15 authorization, recorded by an owner/admin); without the attach, WL-01 would home the company on the canonical domain, invisible on the very domain the user signed up on. Rejected a Supabase before-user-created hook: it does not reliably see the originating host and adds dashboard config coupling.
[2026-08-27] New `tool-pg` vitest project: MCP tools driven through a REAL supabase-js client against a REAL PostgREST (tests/tool-pg/, scripts/tool-pg/reset.sh, `npm run tools:pg:reset` + `npm run test:tools`, plus a tool-pg CI job). NOT a duplicate of pg-real: that project holds a `pg` Pool and writes SQL, which structurally cannot see the half of a tool that PostgREST resolves at request time (the `.select()` column strings, the resource embeds, the `or=(...)` grammar, `.contains()` operand types). Before this, all 100 files in extensions/general/mcp-server/__tests__ faked supabase and query-journal.test.ts deferred its query chain to "the live MCP smoke test", which does not exist in CI: the PostgREST grammar of 157 tools was gated by nothing. Three findings worth keeping. (1) supabase-js hard-codes a `/rest/v1` prefix that a bare PostgREST does not serve, so the first version of the harness 404'd all 55 sweep queries, the tools reported the empty response as "Database error: undefined", and the suite passed GREEN while exercising nothing; fixed with a URL-rewriting `global.fetch` in createToolPgClient, and a permanent self-test now injects a bad column and asserts the harness detects 42703, so a green sweep means something. (2) Errors are captured at the TRANSPORT, not from the thrown Error: the tools wrap failures in their own prose and lose the payload, so a real 42703 arrives as an unclassifiable string. (3) The reset recreates the CONTAINER rather than dropping schemas: `storage` is owned by supabase_storage_admin so `DROP SCHEMA storage` fails as postgres, and dropping only `public` leaves the storage RLS policies migration 20240101000024 creates unconditionally, aborting the next replay partway and leaving a half-migrated database that looks like a migration bug. CI runs PostgREST via `docker run --network host` rather than a service container, because service containers on a non-containerized job are reachable from the runner but not from each other by name. Current coverage is honest and partial: 74 read tools, 87 real requests, 0 malformed queries, 4 failures all 22P02 from the empty argument set. Per-tool argument fixtures are what deepen it, and the harness is the thing that makes writing them worthwhile.
[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.
[2026-08-27] Dropped VAT cadence localStorage persistence from PR #1998 (kept the settings-row gate): skeptic pass refuted it twice (SSR hydration mismatch from render-phase localStorage read; persisting a cadence that deviates from moms_period keeps the filing pipeline open on the wrong period type with no downstream period-type validation). The mount-time re-seed from moms_period is the self-healing control; FyPicker already persists the rakenskapsar pick.
[2026-08-27] Invite-only brand signup ships accepting a low-severity allowlist enumeration residual: POST /api/auth/signup returns 403 for a non-allowlisted email vs 200/400 for an allowlisted one, and the 403 short-circuits before GoTrue, so it is captcha-free and unthrottled: someone with candidate emails can test which are on a brand's allowlist. Not closed because (a) the app deliberately never holds the Turnstile secret (it lives in Supabase/GoTrue; a repo test forbids TURNSTILE_SECRET_KEY in app env), and (b) the clear "you're not invited, go to Accounted" redirect UX inherently reveals the verdict. It leaks membership of guessed emails, not the list, and no ledger/credential data. Follow-up option if it matters later: add signup-endpoint rate limiting. The related fail-OPEN (a brands-table error was read as unbranded, opening invite-only signup during a DB blip) WAS fixed: the gate now returns lookupFailed and both signup routes answer 503.