4 Commits

Author	SHA1	Message	Date
Zachary Lowden	ec49115e55	feat(user-restriction): make the pending-review mute type a parameter (#4609) ... * feat(user-restriction): make the pending-review mute type a parameter Adds an optional `type` to `applyPendingReviewMute`, defaulting to 'generation' so both existing callers are byte-for-byte unchanged, and teaches the moderator queue to show a second type. This is the enabling seam for a bot-account detector that must file into the SAME review queue rather than a new board; no detector logic ships here. `UserRestriction.type` is free text with a [type, status] index, so a new type needs no migration. Dedupe is now scoped per type. Scoped to the user alone, the first queue to mute an account would permanently silence every other queue for it: a later finding of a different kind returns deduped against a row about something else and files nothing a moderator can see. Notifications are an opt-in per-type map, with null meaning "say nothing". createNotification validates its type against nothing, so an unregistered value is persisted and increments the unread badge while the bell drops it at render, leaving a phantom count with no click target; and reusing 'generation-muted' would tell a user their generation access was restricted for something unrelated to generation. A new type therefore stays silent until a processor is registered for it. Verdicts are still generation-shaped, so the moderator resolve and ban actions refuse a row of any other type rather than sending a misleading notice. Parameterising that path is deliberately left out of scope. * test(user-restriction): close two tests that passed against base code Both were found by running the new suites against pre-change source, and both were green there for a reason unrelated to what they claim to test. - The moderator ban refusal asserted only status 400. An invalid ban payload is also 400 with setBanned untouched, and the fixture used a reasonCode outside BAN_REASONS — so the schema rejected it before the type check could run. Uses a valid payload now, asserts the refusal message, and adds the generation-row positive control that proves the refusal is not simply rejecting every ban. - The SELECT-list assertion matched the whole statement, so it was satisfied by the `"ur"."type" = $1` in the WHERE that every version emits. Sliced to the select list. Also fixes a comment naming a function that does not exist. * fix(user-restriction): refuse an unrulable type inside the verdict path itself The type refusal added with the queue lived at ONE of the ruling surfaces. Five callers reach `resolveUserRestriction` — the tRPC router, `/api/mod/restriction/ resolve` (which is what both moderator-app ruling surfaces post through: the audit queue and the retool User Lookup panel), and `overturnPendingReviewMute` — and only the audit queue checked. Reaching the verdict path with a non-generation row would send a "your generation access has been restored" notice and an email, and call `resetProhibitedRequestCount`, wiping the account's real prompt-violation counter over an unrelated case. Moved down rather than replicated at a third route: a predicate open-coded at N sites is wrong at N-1 of them. `RULINGS_WIRED_FOR` and `unwiredRulingReason` now live beside the type vocabulary, and the refusal happens in the service, before any write and before the already-resolved check. Also validates `type` at runtime in `applyPendingReviewMute`. That seam exists to accept a caller-supplied type across an HTTP boundary and a JSON body, where the compiler's word is worth nothing; an out-of-vocabulary value used to mute the account, file a row no queue can select, and send no notification. Latent today — one writer, both callers pass no type, so no non-generation row can exist. Both are closed before a detector ships. Tests, red at the PR head and green here: - refuses Overturn and Uphold on a bot-account row, with no write, no notification, no subscription change and no counter reset - refuses before it argues about the row's status - rejects three out-of-vocabulary types with nothing muted and nothing filed - the seam test pins the moderator app's copy of the wired-for list AND the refusal wording to this one, in both directions The fake's `userRestriction.findUnique` now honours `select` for `type` alone, so a service that refuses non-generation rows but forgets to select the column reads `undefined` and fails the positive controls instead of passing vacuously. * fix(moderator): stop the lookup panel hiding an open case, and stop offering rulings that cannot land Three things, all from the audit on this PR. 1. User Lookup showed the newest restriction of ANY type — three correlated subqueries, `ORDER BY ur.id DESC LIMIT 1`, no type predicate. That was sound while a user could hold at most one open row. Per-type dedupe lets two coexist, so a Pending generation case sitting behind a later Upheld bot-account row rendered as no open restriction at all: the account stays muted, the ruling form is never drawn, and nobody can see the open case. Now `ORDER BY (ur.status = 'Pending') DESC, ur.id DESC` — a Pending case outranks a merely newer one; among several Pending rows, the newest wins; a resolved row is shown only when there is no open one. The panel still speaks for ONE row (a header chip and a single form; the audit queue is where a list belongs), and the ordering is now written once and called three times rather than copied per column, so the three cannot stop naming the same row. 2. `RULINGS_WIRED_FOR` / `unwiredRulingReason` move into `$lib/restriction-types` and are imported by the audit route instead of re-spelled there. The refusal that protects the account now lives in the main app's `resolveUserRestriction`; the route check is KEPT, not for defence in depth but for ordering — `ban` bans and THEN rules, so a refusal arriving inside the verdict call would leave the account banned against a restriction nobody can close. The seam test pins this copy to the main app's in both directions, list and wording. 3. The Bot account queue rendered live Uphold / Remove / Ban forms whose only possible outcome was a 400, and the retool panel offered Overturn / Uphold on a row the verdict path refuses. Both now disable those controls and say why. The server-side refusals are unchanged — they hold against a posted id, which nothing rendered can. Also: `restrictionById`'s comment claimed `unwiredRuling` governs which types an action can be handed; `flagSuspicious` calls it with `type: 'any'` and rules on nothing. The comment now says what the code does and why flagging is deliberately type-agnostic. And `capturingDb`'s `params` is `unknown[][]`, not a `readonly` contract cast away at its one use. Tests: new `user-lookup-restriction-row.test.ts` asserts the Pending-first ordering, the total tiebreak and the per-account scope on all three compiled subqueries; new `restriction-types.test.ts` covers the shared predicate the disabled controls read. The audit route gains an invariant guard that its refusal is the shared predicate's own output rather than a second copy. There is no component-test harness in this app, so the rendering half of (3) is not covered by a test. * fix(user-restriction): make the seam guard read the moderator vocabulary by executing it Round-2 audit F1. The guard that pins the two apps' restriction-type lists to each other read the moderator app's module as TEXT, and it passed green over a real divergence. Its regex captured to the FIRST closing bracket after the '=', so a comment naming an index truncated the capture, and it extracted single-quoted strings only, so a differently-quoted entry vanished. Measured here: with the moderator list written as export const RULINGS_WIRED_FOR: readonly RestrictionType[] = [ 'generation', // matches RESTRICTION_TYPES[0] 'bot-account', ]; the seam file reported 8 passed / 0 failed while the two lists genuinely disagreed. The length > 0 positive control could not see it either, because the first entry survives the truncation. Same green for a mixed-quote entry and for a list assembled with a spread. That is not cosmetic. If the moderator app believes a type is rulable, its audit queue bans the account and posts the ruling afterwards -- a ruling the main app refuses -- leaving a banned account with a Pending row nobody can close. Not fixed by a wider regex: a guard that pins source text by PATTERN is walkable by rewriting the text, and the rewrites that walk it are ordinary (a Prettier reflow, a comment, a quote style). The reader now IMPORTS AND EXECUTES the module and compares values, so formatting cannot be the difference between agreeing and disagreeing. This is available because the moderator vocabulary module has no imports of its own; a comment there says so and says to keep it that way. - moderator-restriction-vocabulary.harness.ts: the reader, plus runtime shape validation that throws (naming the export) rather than returning an empty list. - moderator-restriction-vocabulary.test.ts: 20 tests. Five fixtures, each a REAL divergence written in a different shape -- multi-line, a comment containing a closing bracket, double-quoted entries, a trailing comma, and values assembled at runtime -- plus seven refusal cases for the shape validation. - The refusal SENTENCE is now called rather than parsed out of a template literal, so a message built from constants is compared on what it produces. - apps/moderator restriction-types.test.ts: pin RULINGS_WIRED_FOR BY VALUE. The old unwired.length > 0 caught a widened list only by accident -- bot-account being the only unwired type. Measured: with a third filed type present, the length check passes over a wrongly-wired bot-account and the value pin is the only thing that fails. - .prettierignore: the fixtures' formatting IS the fixture; Prettier would normalise four distinct cases into one. Measured, old reader vs new, both run against the same mutated moderator module: comment containing ']' old 8/8 GREEN new RED (2 failures) mixed quote style old 8/8 GREEN new RED (2 failures) spread / computed list old 8/8 GREEN new RED (2 failures) all-double-quoted old RED (empty-list control) new RED multi-line, no comment old RED new RED single-line trailing , old RED new RED And with the reader reverted to the old text parser, 6 of the 20 new tests fail (the comment, double-quote and computed fixtures, on both their list and message cases); the multi-line and trailing-comma fixtures pass under both and are labelled as declared coverage rather than regression coverage. * fix(user-restriction): let the ruling refusal survive as a 400, and make two claims true Round-2 audit F3, F4 and F5. F4 (behaviour). resolveUserRestriction threw a plain Error, so handleEndpointError fell to its non-TRPCError branch and the refusal reached the wire as 500 "An unexpected error occurred" -- the moderator's panel rendered "Restriction ruling: An unexpected error occurred." and the reason was destroyed. Reachable today from the retool User Lookup panel, which has no local guard. Now throwBadRequestError, which keeps the status and the message. Scope decision on that one: the two neighbouring guards in the same function had the identical defect, and I fixed them in the same change rather than leaving one of three converted. "Restriction record not found" is now a 404 and "Restriction has already been resolved" a 400; both are facts about the request, neither is a server fault, and leaving two of three as opaque 500s would have recreated the same predicate spelled two ways one line apart. All three are covered. Covered by three tests that drive the REAL handleEndpointError over the REAL thrown value, not by asserting the message alone -- a message assertion stays green through exactly the 500 this fixes. Watched red at ff97751d20: "expected 500 to be 400", "expected 500 to be 404", "expected 500 to be 400". F5 (latent). The User Lookup panel's ORDER BY (ur.status = 'Pending') DESC is correct only while status is NOT NULL: Postgres defaults DESC to NULLS FIRST, so a NULL would outrank a genuinely Pending row and hide the open case -- the exact failure the preference exists to prevent, arriving through the column's nullability. DESC NULLS LAST makes it independent of that. The column is a NOT NULL enum today, so this is an unstated precondition made explicit, not a live defect. Two tests red at ff97751d20 on the compiled SQL text. F3 (comment truth). The runtime type guard's comment claimed the values reaching it "cross an HTTP boundary and a JSON body". Nothing does: neither production caller passes a type, and mute-user-pending-review.ts's zod schema has no type key, so no request body can supply one. The guard stays -- what it is actually for is the shape of the NEXT caller (this seam exists so a detector can file into the queue, and the obvious wiring is a route forwarding a JSON field) and the callers TypeScript cannot vouch for today (an `as` cast, a value read back off the free-text column, a JS caller). Corrected in the service and in the test file's docblock, which carried the same false sentence. * test(user-restriction): close the vocabulary guard's environment blind spot Round-4 delta audit, F-1/F-3/F-4. F-1. The execute-based reader resolves the moderator app's vocabulary IN THE MAIN APP'S TEST PROCESS, so an environment-conditional list is read under Vitest and never under the moderator app's production build. Reproduced: with export const RULINGS_WIRED_FOR: readonly RestrictionType[] = import.meta.env.DEV ? ['generation'] : ['generation', 'bot-account']; the seam + vocabulary suites report 28 passed / 0 failed while the shipped build carries both types — the ban-then-strand hazard, reached with every pinning guard green. The base commit's TEXT parser goes RED on that same module, so the reader that replaced it was not strictly stronger; it traded a formatting blind spot for a runtime-environment one. Keeps a TEXT assertion alongside the execute check rather than replacing it: the module's source may contain no import.meta and no process.env. The two mechanisms are complementary. With it, the mutant above fails the seam suite at its beforeAll and the vocabulary suite's real-module case, both naming the constant. Comments are stripped before the scan. Without that the guard is matched by its own documentation — the module has to be able to name the shapes it refuses, and a raw-text scan fires on the sentence forbidding the thing rather than on the thing. Covered by a control asserting a commented mention does not trip it. The moderator module's precondition comment said only 'keep this module import-free'. import.meta.env and process.env need no import statement, so that sentence never covered this; it now states both constraints and says they are separate. F-3. All five vocabulary fixtures wrote 'return RULINGS_WIRED_FOR.includes(type)' while the real module writes '(RULINGS_WIRED_FOR as readonly string[])'. The old parser's message regex requires 'return (RULINGS_WIRED_FOR', so it could not read the message out of ANY fixture and M8 was measuring the old reader against a shape the module does not have. Fixtures now carry the cast. Re-measured, M8 is 6 of 21 — the three fixture pairs the body names (comment-with-bracket, double-quoted, computed), on both their list and message cases. The audit's 8 was the drift; the multi-line-array and trailing-comma fixtures are read correctly by the old parser again, messages included. computed.ts's comment now distinguishes 'assembled from constants declared in this file' — the same value everywhere, which is what the execute reader can certify — from 'assembled from the environment', which is refused. It was the fixture that made the hazardous shape look sanctioned. F-4. The harness docblock now names the cross-app build coupling: resolving the moderator path makes Vite load apps/moderator/tsconfig.json, which extends the gitignored generated .svelte-kit/tsconfig.json, so without svelte-kit sync both suites fail with a TSConfckParseError naming a tsconfig rather than the seam. CI is unaffected; a fresh clone or worktree is not. * fix(moderator): read the refusal out of the envelope the endpoint actually sends Round-4 delta audit, F-2. handleEndpointError's 4xx pass-through emits { message } and no error key, while every other refusal from defineModeratorEndpoint emits { error, message, code }. The moderator app's readError read body.error and nothing else, so all three refusals added last round came back null and the operator saw 'Restriction ruling returned 400.' — the reason destroyed again, one layer further out than the opaque 500 that change removed. Fixed at the CONSUMER, not the emitter, and the choice is measured rather than assumed. handleEndpointError is the shared chokepoint for 36 REST route files; its 4xx and 503 pass-through bodies are pinned toStrictEqual({ message }) by endpoint-helpers-error-envelope.test.ts, the 503 case as an explicit documented carve-out; and restErrorBody needs a RestErrorCode that is not derivable at that point without a new status-to-code map, which would then have to be reconciled with the closed key ledger rest-error-envelope-ledger.test.ts enforces. Widening the reader costs one expression and makes every endpoint's 4xx legible to this app; widening the emitter changes the wire format of 36 routes and needs its own PR. The rule moves to apps/moderator/src/lib/server/rest-error-reason.ts, kept import-free so the main app's suite can load it by filesystem path — the same mechanism as the vocabulary harness. That is what lets the new test drive the REAL emitter into the REAL reader in one process, over all three refusals. Two suites each mocking the other side is precisely the arrangement that cannot see a disagreement about a field name, which is why the previous round's tests were green: they drove the real helper but asserted body.message, which the consumer never looked at. Red before the fix with 'expected null not to be null'; the reader's own null-capability is pinned as a positive control so the three not-null assertions cannot hold against a function that always answers. * docs(test): scope the environment guard's docblock to what it actually refuses Round 4 of the audit ladder found the guard's own documentation claimed more than the regex delivers. A guard description that reads as coverage while providing less is worse than none, because it stops the next reader looking. Two sentences, no behaviour change: - The scope note now says this refuses two spellings and NOT the class, and names the three measured escapes (an aliased global, a computed member access, and a regex literal containing a double slash on the same line as the read, which the comment stripper truncates). It also records what IS covered, so the note does not read as an indictment: the spellings a maintainer would plausibly reach for are caught, and the $app/environment and $env imports break loudly as a missing module. - assertEnvironmentIndependent now documents that string literals are deliberately NOT stripped, so the scanned module may not mention these tokens in a string either. That fails safe -- red with the guard's own message, never silently green -- but it is a real constraint, and a comment is the supported way to write one. Verified: the three affected suites are 99 passed / 3 files, unchanged.	2026-09-03 21:33:02 -05:00
Luis Rojas	470f0fd993	fix(db): stop prettier reformatting the generated Prisma client ... `db:check-generated` regenerates and diffs against the commit, so a commit that has been through prettier can never match: the generator emits `export type X = (typeof X)[keyof typeof X];` on one line and prettier wraps it. Every schema change then carried a ~700-line reformat on top of the real delta, and the gate stayed red no matter how faithfully the client was regenerated. Same reasoning as the drift-baseline and catalog entries already in this file — the generator owns the format. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-18 19:49:56 -04:00
Zachary Lowden	ebe9aa278b	ci(db-schema): gate a PR on NEW schema<->database drift, not on the backlog (#3643) ... * ci(db-schema): gate a PR on NEW schema<->database drift, not on the backlog The drift detector (#3591) reports the gap that exists. A gate has to answer a narrower question — did THIS change make it worse? `--strict` fails on any finding, and there are 61 on `main`; a gate red on every run teaches everyone to click through it and gets switched off within a week. Same reasoning as the report-only ESLint and Prettier steps in lint.yml. So: block only a PASS->FAIL regression the change introduces, warn about everything already broken, never block on a pre-existing finding. Modelled on the delta gate this org already runs on its infrastructure repo. `drift-baseline.json` records the 61 accepted findings, keyed by a fingerprint that deliberately EXCLUDES the `declared`/`actual` prose — #3589 corrected eight declared referential actions and touched no constraint, and a fingerprint that folded that in would have retired eight entries and raised eight identical-looking new ones. Nullability is the one exception: its `declared` is a single word and that word IS the finding, so a field flipped from optional to required against a NULLABLE column cannot inherit the old entry's pass. TWO SEVERITIES, decided structurally rather than by taste. Migrations here are applied by hand, so a declaration being ahead of the database is a normal intermediate state; a gate that could not tell that from real drift would block every PR that adds a column. The discriminator is whether the finding concerns database surface that already exists: enforced columns are in the catalog, the constraint is not -> BLOCKS pending the column is not in the catalog at all -> warns `missing-column` is always pending by construction, `nullability` and `uniqueness` always enforced by construction; `missing-foreign-key` is the only kind decided at runtime. On today's backlog that is 49 enforced, 12 pending. NOT MEASURED, and it says so rather than printing a clean zero: referential actions. The committed snapshot carries no ON DELETE/UPDATE data, so all 408 comparable foreign keys read "not comparable". A live run found 45 — every one an ON UPDATE Cascade-vs-NoAction on a hand-written App Blocks foreign key, zero ON DELETE mismatches, inert because `id` is never updated. They are absorbed by being structurally unmeasurable here, not by being waved through. READ-ONLY BY CONSTRUCTION. gate-cli.ts has no database code path — no `pg` import, no flag that opens a connection. This repo is public and so are these logs, so that is a requirement, not a convenience. Unknown arguments are redacted before being echoed, as in cli.ts. Positive controls, because "0 new drift" and "wired to nothing" print the same page: the verdict always reports matched-vs-baseline as a PAIR; an empty baseline or a run that reproduced none of its baseline exits 2, not 0; a catalog that covered nothing exits 2; and the workflow greps for the verdict line because `pnpm --filter` exits 0 when it matches no project. 25 tests, and every guard was mutation-tested — seven mutations, each killed by the test whose intent matches it. * fix(drift-gate): audit round 1 — NUL byte, snapshot decay, tier escalation Addresses the adversarial audit of #3643. A literal NUL byte in gate.ts made the file BINARY to git, to grep and to GitHub: all 293 lines of the gate's logic were absent from the PR diff, and `grep -c` returned rc=1, which reads as "0 matches". It is written as the escape now, matching compare.ts:15. This is the second independent occurrence of the exact defect (see #3647's plan.ts:53) — the class is a composite-key separator emitted as a raw byte rather than an escape. The gate compares against a FROZEN snapshot, so every column created after the capture can only ever be tiered pending/warn: blocking coverage shrinks monotonically while the check stays green, and nothing said so. The snapshot now carries `capturedAt` (stamped by --dump-catalog), the verdict prints its age on EVERY run rather than only past a threshold, and 90 days triggers an explicit STALE note. Advisory, never fatal — a gate that reddened because a date passed would be red for everyone at once, for a reason no PR author can fix. This makes the decay VISIBLE; it does not remove it. Only recapturing against a live database does that. Tier escalation was invisible. `tier` was written to the baseline and never read back, so a finding accepted as `pending` ("no such column yet") that became enforceable when its migration landed WITHOUT its constraint — the shape of 37 of the 61 baseline findings — was absorbed into `matched` and exited 0. `evaluateGate` now compares the live tier against the recorded one and blocks on `pending -> enforced`. A no-op `drift:baseline` produced a ~250-line reformat, because Prettier and JSON.stringify disagree about the file by 246 lines. That buried the one entry a reviewer needed to see, defeating the reason for committing a baseline at all. The generator now owns the format (.prettierignore): a no-op refresh is a zero-line diff, and accepting one finding is a twelve-line one. The fingerprint excluded the referenced table, so repointing a relation from Image to Post — same model, same field, same constrained column — inherited the old entry's pass silently. It is folded in now; the ON DELETE/UPDATE prose stays out, which is what #3589 required. The cross-module parse of compare.ts's `declared` string is pinned by a test over all 37 real missing-FK findings. The "this catalog carries no ON DELETE/UPDATE data" parenthetical was hardcoded and printed even against a catalog that did carry it. It is conditional now. Two mutation survivors closed. `.every -> .some` in the FK tier was structurally undetectable because every fixture used single-column keys — a composite case now distinguishes them. `assertCatalogSanity`'s CALL SITE was untested (the function was): the only exit-2 CLI case used an empty catalog, which trips assessCoverage instead, so deleting the call survived. A uniform-notNull catalog now exercises it. Sweep re-run against the changed source rather than citing the old table: 13 mutants, 13 killed, each by the test whose intent matches it, with a clean-tree control. Also: "blocks" softened throughout. `main` has branch protection but no required_status_checks, so this check is advisory and a PR can merge with it red. The README now states plainly what the gate can catch (a schema edit promising what the captured database lacked), what it cannot (anything the database itself does), and what degrades (any column younger than the capture). * fix(drift-gate): stamp the snapshot without reformatting 21,520 lines The capturedAt stamp was added by a JSON round-trip, which collapsed the prettier-formatted fixture to a single line: 1 insertion, 21,520 deletions. That is precisely the defect the audit raised against drift-baseline.json one finding earlier — a mechanical rewrite burying the one line a reviewer needs — so shipping it here while fixing it there would have been a straight regression. Inserting the key textually keeps the file's existing formatting: 1 insertion, 0 deletions, and prettier still agrees with the result. Generated JSON in this package now has two owners, deliberately and differently. The CATALOG fixture stays prettier-formatted: it is written once and read by a human when a finding is disputed. drift-baseline.json is prettierignored and owned by its generator: it is rewritten on every acceptance, so the generator has to be able to reproduce it byte-for-byte or the diff stops being reviewable. Also bounds a flake before it lands. Every case in the gate's CLI suite spawns the real entry point as a process, paying a cold tsx transpile plus a Node start before its first assertion. The slowest measures 3.5s on an idle machine against Vitest's 5s default — a 1.4x margin a 2-core runner will not honour, which is a test that goes red on ambient machine speed, a different case each run. 60s still bounds a genuine hang. Verified load-bearing rather than assumed: a deliberate 6s probe passes inside the describe and fails with "Test timed out in 5000ms" once the option is removed. Set on the describe rather than in the package's vitest.config.ts because that config is a shared file with a concurrent change already in flight for the sibling cli.test.ts, which has the same shape and the same problem. * fix(drift-gate): audit round 2 — test the capturedAt producer, close the refresh seam N1 (blocking). The staleness signal had a tested consumer and an untested producer. `snapshotAge` was exercised exhaustively; the line that WRITES the stamp had no test at all, so mutating it to `catalog.capturedAt` — which makes every fresh capture carry no date and silently disables the whole signal round 1 added — survived with PASS=44 FAIL=0. The only assertion touching the stamp checked that the committed FIXTURE has one, and that value is a hand-written midnight instant the command would never emit, so it never exercised the producer. A new suite drives `drift --dump-catalog` as a process and asserts the stamp is a real instant from this run, that an existing stamp is preserved rather than refreshed (a re-dump must not launder a stale snapshot young), and that the catalog is otherwise unchanged. The exact surviving mutant now dies. N2. `--dump-catalog` emitted single-line JSON, so a content-identical re-dump of the committed fixture was a 21,522-line diff — the same unreviewable-diff class this PR fixed for drift-baseline.json, and worse, because the gate's own STALE message instructs the operator to run precisely that command. Prettier cannot be the owner instead: it collapses short arrays in a way JSON.stringify will not reproduce (measured: 3,598 lines apart), so the artefact would only stay clean if every operator remembered a formatter afterwards, which nothing enforces — `prettier --check` on a MODIFIED file is report-only here. The generator now emits indented JSON and the fixture is prettierignored, matching how the baseline is handled. A re-dump is now byte-identical (sha256 verified). The fixture is reformatted once, +2,746/-852, content deep-equal before and after. N3. The escalation comment and README claimed the guard covers "the exact shape of 37 of the 61 baseline findings". Measured, escalation needs an entry that is both `pending` and `missing-foreign-key`, and the baseline has ZERO of those — all 12 pending entries are `missing-column`, hardcoded `pending`, which can never rise. The guard is purely forward-looking and now says so. N4. That guard also had a bypass: a catalog only gains a column via a recapture, and the documented recapture procedure refreshes the baseline in the same commit, turning the escalation into a tier flip inside a regenerated file rather than a failure. `drift:baseline` now reports every pending -> enforced transition it absorbs, so it lands in the recapture commit's log instead of nowhere. It does not fail; a refresh is deliberate. N5. A STALE note is advisory, so the check renders green and the one signal about shrinking coverage was a line inside a passing job's log. Promoted to a `::warning::` annotation on the PR. N6. The pending-tier text told developers a pending finding "will stop being reported once the snapshot is recaptured". True for missing-column; for missing-foreign-key a recapture makes it ENFORCED. Now spells out both, since this is the text read at the moment someone decides to accept a pending finding. Smaller: the `>=` staleness boundary is tested at the threshold, not one past it; a future-dated capture is treated as a problem rather than as maximally fresh (a negative age could never reach the threshold, disabling the signal indefinitely); the NUL tuple separator is pinned by a collision case that a printable separator gets wrong in the failing direction; the referencedTarget comment claimed a parse failure would "quietly merge" findings, when measured it gives 0 collisions and a loud 37-resolved/37-new block; README says eleven-line, not twelve; and remaining unqualified "blocks" wording is now "fails the check". Sweep re-run against changed source with the harness validated FIRST, which mattered: the initial runner counted reporter glyphs, and this reporter prints a tick per FILE rather than per test, so it read a real failure as FAIL=0. Rewritten to parse the summary line, with NO-SUMMARY treated as an unmeasured result rather than a kill — one mutant made the tool emit a single 226 KB line and blew ARG_MAX in the harness itself, which the first version scored as a kill. Controls: clean tree PASS=56 FAIL=0, deliberate break PASS=51 FAIL=5. 13 mutants, 13 killed. Also caught by that validation: STALE_AFTER_DAYS could be changed to 999999 with no test failing, because every staleness assertion was written in terms of the constant itself. The policy is now pinned in absolute terms. * fix(drift-gate): audit round 3 — finish the round-2 correction, cover --update-baseline Two blocking items, both instances of a fix landing in one place and being reported as landing in two. The round-2 correction to the escalation claim reached gate.ts and not the README, which then contradicted itself four lines apart: "the shape of 37 of the 61 baseline findings" at :372 against "the current baseline has zero of those" at :378. Re-measured: all 37 missing-foreign-key entries are already tier: enforced and cannot rise. The false sentence is gone; the PR body carried the same sentence and is corrected too. --update-baseline had no test of any kind — not the write, not the escalation report, not the previous-baseline read. Two mutants passed the full suite green: `absorbed = []`, which makes the entire N4 remedy inert, and reading `previous` AFTER the write instead of before, which is the likeliest real edit to that block. Both are now killed by a CLI case that seeds a pending tier, refreshes, and asserts the transition is named and the file rewritten. Six cases in total, all against a COPY in a temp dir so none can touch the committed artefact. This is the isolation seam reproduced inside a round-2 fix: the pure function was tested, its only caller was not. Folded in: A corrupt previous baseline printed nothing about escalations, so "0 absorbed" and "could not read the previous baseline" were byte-identical output — a reassuring zero indistinguishable from a probe wired to nothing, in a tool built against exactly that everywhere else. The refresh now prints either the pair (absorbed N, compared against M entries) or an explicit SKIPPED note. A malformed-but-parseable previous baseline (`{}`) crashed the refresh AFTER it had already written the file: exit 2, "Cannot read properties of undefined (reading 'map')", 677 lines on disk. The previous baseline is now validated at read time, before the write, and an unusable one degrades to the SKIPPED note. `set -euo pipefail` in the workflow aborted the step the moment the gate exited non-zero, skipping the no-verdict guard and the STALE annotation PRECISELY when the gate fires — and the comment beneath claimed the guard runs "including when it blocks", describing an unreachable path. The step now captures PIPESTATUS, runs both checks, and propagates the verdict last. Verified by extracting the step and running it against stubbed gates across five paths: pass, pass+STALE, FAIL+STALE, no-verdict-but-exit-0, and exit 2. The failing-with-STALE case emits 0 annotations under the old form and 1 under the new. --dump-catalog returned before assertCatalogSanity, so the recapture command the STALE message recommends could emit a catalog the next drift run would reject. The check now runs first: a uniform-notNull catalog exits 2 having written zero bytes, and a healthy one still dumps sha256-identically. cli.ts said the prettier/generator disagreement is 3,598 lines; re-measured, 3,589. The sibling figures (246, 21,522) re-derive exactly. The two-kind pending-tier message was unpinned prose next to a line that a CLI test pins; it now has its own assertion. Sweep re-run against changed source over the WHOLE package suite, with the harness asserting 17 files as well as both summary lines: 17 mutants, 17 killed (16 in the batch, G3's NUL-separator anchor run standalone because the escape sequence cannot be passed through the shell harness). Controls: clean tree PASS=377 FAIL=0, deliberate break PASS=371 FAIL=6. * fix(drift-gate): the errexit fix was inert in CI; restore the correct prettier figure Both findings this round have one root cause: a measurement taken in an environment that is not the one it targets. GitHub runs a `run:` step with no `shell:` key as `bash -e {0}`, so errexit is already on before the first line. `set -uo pipefail` does not clear it, and adding pipefail made it strictly worse: the pipeline now returns the gate's non-zero status, errexit fires on it, and the step dies at `tee` before `rc=${PIPESTATUS[0]}` is ever reached. The STALE annotation was still skipped precisely when the gate fires, and the comments describing that path could not run. `set +e -uo pipefail` explicitly. The reason this shipped is the finding itself: under plain `bash` both forms emit the annotation, and plain `bash` is what the previous round measured. Re-measured under `bash -e` with a stubbed gate exiting 1 and printing STALE: old form 0 annotations, new form 1. Positive control on a PASSING gate: all four combinations emit 1, so the harness can observe one under `-e`. All five paths re-verified under `bash -e` against the step extracted from the shipped file: pass 0/0, pass+STALE 0/1, FAIL+STALE 1/1, no-verdict 1/error, exit-2 2. cli.ts's prettier-disagreement figure goes back to 3,598. 3,589 is the count at prettier's DEFAULT printWidth 80 — the number you get by copying the file to /tmp, where `.prettierrc` cannot resolve. In the repo, where it resolves to printWidth 100, every method agrees on 3,598: 852 insertions + 2,746 deletions, plain diff, -u, -U0 and git --numstat, with a self-diff control of 0. The comment now records how the number must be measured, since getting it wrong is one `cp` away. Three guards that a differently-built sweep found surviving: assertCatalogSanity's move ahead of the --dump-catalog return had no regression coverage — deleting the call, and moving it back behind the return, both passed 382/0. Now pinned by a case asserting a uniform-notNull catalog exits 2 having written zero bytes. Same "only caller untested" shape the --update-baseline work existed to close. The NUL-separator guard was spelled to `_`: its fixture only collided under that one character, so mutating the separator to `|` or to empty survived. Rewritten as a property over five candidate separators — empty, underscore, pipe, dot, space — each building the pair that collides under ITS own separator, so any character legal in a Postgres identifier fails at least one. Second instance of the spelled-guard class in this file, and the reason the rewrite asserts the invariant rather than a character. The original `_` mutant still dies. A describe-scoped `let workBaseline` was assigned by one test and never read; it is a local const. Verified: 382 tests, 0 failures; typecheck 0 errors; gate 61 of 61 at exit 0; re-dump of the committed snapshot still sha256-identical. Four mutants that survived the independent sweep now die, each to the test whose name states that guard's intent. * test(drift-gate): derive the separator alphabet; the "property" was five spellings Round 4 rewrote the NUL-separator guard and claimed it made "any character legal in a Postgres identifier fail at least one" case. That was false, and the mechanism is exact: each case builds `A{S}B`/`C` against `A`/`B{S}C`, and those two keys are equal IFF S equals the implementation separator — so a case only ever catches its own. Reproduced: `-`, `#`, `::`, `--` and `~!~` all survived at 382/0. `-` and `#` are legal inside a QUOTED Postgres identifier, so this is the hazard class the guard exists for rather than a hypothetical. The case list is now derived from an alphabet instead of hand-picked: empty, space, all 32 ASCII punctuation characters, five multi-character candidates, and five non-ASCII ones. The non-ASCII entries were added because `§` survived the first version of this enumeration — an ASCII-only alphabet excludes exactly the characters a quoted identifier makes legal. Added alongside it, and constructed differently on purpose: a deterministic seeded fuzz that splits one character run at two different points, so the pairs are ambiguous BY CONSTRUCTION and no separator is named anywhere in it. It carries its own positive control on the loop counter, because a `continue` that swallowed every case would leave it green having asserted nothing. Ten separators that previously survived now die, each to its own named case: `-`, `#`, `::`, `--`, `~!~`, `_`, `|`, `@`, `^`, `§`. Single-character ones die twice — the enumerated case and the fuzz independently. The claim is now stated at its true scope in the test comment as well as the PR body. A finite enumeration cannot prove a property over an infinite alphabet: what is proven is every separator in that alphabet plus whatever the fuzz catches, and what is NOT proven is an arbitrary unenumerated multi-character run — `ZZQ` still survives it, and the comment says so rather than implying otherwise. Overstating the scope is what produced this round. Also recorded, as a known gap rather than a fix: the workflow's STALE and no-verdict logic has no automated test in this repo, and the live check only ever exercises the passing path, so it structurally cannot observe the FAIL+STALE branch. That is precisely why the round-3 errexit fix shipped inert. It is verified by hand each round against the step extracted from the shipped file, under `bash -e`. 422 tests, 0 failures.	2026-08-05 22:22:46 -05:00
briant	d6f3defc81	chore(creator-studio): wire up eslint + prettier for this app ... Neither tool covered the app. Prettier's root scripts glob "**/*.{ts,tsx}" and prettier-plugin-svelte was absent, so a .svelte file failed with "No parser could be inferred". ESLint's root script is scoped to src/ (the Next app), and the root config cannot parse .svelte at all - @typescript-eslint/parser reads the markup as TypeScript and throws. Scoped to this app rather than fixed at the root, since an app-wide eslint/ prettier overhaul is in flight separately. Prettier is a second major here (3.9 alongside root 2.8) out of necessity: no prettier-plugin-svelte supports svelte 5 on prettier 2 (@2.10 caps at svelte 4, svelte 5 support starts at @3.2 which peers prettier ^3). The two majors are NOT interchangeable - prettier 3 collapses leading-pipe union types that prettier 2 breaks across lines - so root .prettierignore hands this directory over wholesale rather than letting both format the same files and revert each other forever. Run `pnpm -F @civitai/creator-studio-app format` from the app. ESLint needed no version split: eslint-plugin-svelte@2.46 peers eslint ^8 and still ships eslintrc configs, matching the pinned 8.57.1. Four rules are adjusted where they are categorically wrong rather than merely inconvenient, each with the reason inline: - prefer-const off in .svelte - svelte 5 REQUIRES let for $props() destructuring, so it fired on all 118 props in the app - no-undef off - TS resolves types itself; it reported generics and DOM lib types (T, FormDataEntryValue) as undefined globals - prefer-const destructuring:'all' - the default reports unreassigned bindings in a let{...} pattern with a reassigned sibling, unfixable without splitting the statement (see getEdgeUrl) - svelte/valid-compile off - re-reports compiler diagnostics that svelte-check and the build already own, including a custom-element warning that cannot apply to an app never compiled as a custom element Also uncomment WEBHOOK_TOKEN in .env.example. It is required for permanent paid access (the main-app endpoint gates the `permanent` flag on it) but shipped commented out, so deployments provisioned from the template omitted it - which is how creator-studio came to send `?token=` and every creator got the misleading 403 "Permanent access can only be set from the Creator Studio." Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-29 22:38:49 -06:00