mirror of
https://github.com/civitai/civitai.git
synced 2026-09-20 22:08:18 +08:00
aaf209dc21fdf2416355dfd0fd6b2018f802e8a5
24005 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Zachary Lowden
|
aaf209dc21 |
fix(cache): namespace cache keys per environment via CACHE_KEY_NAMESPACE (#3586)
Preview deployments run against a scratch database but shared the production cache instance. Cache keys carried no environment segment, so a preview page load could populate a key production then served - dev-shaped data answering production reads, and preview traffic evicting production entries. Keys are now prefixed with '<CACHE_KEY_NAMESPACE>:'. Production leaves it unset and is a STRUCTURAL no-op: applyCacheKeyPrefix returns the same object, prefixCacheKey is the identity, keys byte-identical. Non-production environments set it explicitly. Derived from an explicit namespace rather than the IS_PREVIEW boolean because IS_PREVIEW is overloaded - it also gates an auth path, and one standing non-production deployment sets it while running against the production database. Keying the cache namespace off it would have put production-data and scratch-data entries in the same keyspace. Single choke point: the key table itself is passed through applyCacheKeyPrefix, so every consumer inherits it. Wrapping client methods was rejected - the client exposes the full node-redis surface, so any method missed by a wrapper would go silently unprefixed, which is the original bug in an invisible form. Also covers two previously-unprefixed free-form minters, adds regression coverage for queryCacheRaw (a mutant that survived the earlier revision), and rejects array leaves in the key table at compile time. custody-sweep's payout mutex is deliberately left shared - it guards a real external payment, so cross-environment sharing is protective and namespacing it would permit a second payout. Verified live on this PR's own preview before merge: the container carries the namespace, prefixed keys appear on all three cache shards, and a production key and its preview twin coexist for the same entity id with production's copy untouched. Note: previews now start against a cold cache namespace instead of inheriting production's warm entries. |
||
|
briant
|
7edabd9322 | chore(creator-studio): release creator-studio-v0.0.33 creator-studio-v0.0.33 | ||
|
briant
|
9f54bf99c2 | 5.0.2236 v5.0.2236 | ||
|
briant
|
36edbded4c |
docs: triage the Creator Studio Review feedback for Jul 31 - Aug 4
Consolidates the Creator Studio Review group DM plus the three assigned ClickUp tasks from the Creator Studio article comments into one checklist, separating shipped fixes from open ones and noting where a report was never answered. |
||
|
briant
|
9a635a42f2 |
feat(monetization): require a rights affirmation before a version can be sold
Selling access to a model is a materially bigger exposure than hosting it, so
before a version can carry paid access or a licensing fee the creator now has to
affirm they hold the rights to monetize it, its training data, and its content.
The affirmation is recorded per model version on `ModelVersion.meta` as
{ userId, affirmedAt, version, statement }, following the Creator Shop pattern.
The wording is stored verbatim so a later dispute shows what was actually agreed;
bumping MONETIZATION_RIGHTS_AFFIRMATION_VERSION asks everyone again rather than
holding them to text they never saw.
Gated on every write path that can set a price: the tRPC upsert, the REST
early-access endpoint the Creator Studio calls, and the Creator Studio's own
licensing-fee writes (single, bulk, and CSV import) which bypass the main app.
Asked once per version, never to *clear* a fee or gate, and skipped for
moderators acting on someone else's model — staff monetizing their own models
affirm like any other creator.
Also reorganizes ModelVersionUpsertForm into two cards, moving paid access and
licensing fee out of the middle of the form into a Monetization card at the end.
CU 868kjuhj6
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
Zachary Lowden
|
59823f3aac |
feat(app-blocks): give the ClickHouse blockRenders table its first reader (#3613)
* feat(app-blocks): give the ClickHouse `blockRenders` table its first reader
`blockRenders` has been written since 2026-06-23 and read by nothing. It is the
only signal covering the viewers the author dashboard structurally cannot see:
the engagement counters come from `block_scope_invocations`, which is written
ONLY on authenticated, scope-gated API calls, so anonymous viewers and static /
no-scope blocks contribute nothing. This adds a "Views (range)" card backed by
that table — impressions plus unique viewers.
Measured against prod ClickHouse before building, and two findings changed the
design rather than confirming it:
- **Unique viewers must NOT dedup on `blockInstanceId`.** That id reads as
per-mount and is not: it is `page_apb_<ULID>`, one per PLACEMENT. 124 rows
carried 28 distinct `blockInstanceId` across 27 distinct `appBlockId` — ~1:1
with the app — so deduping on it would report "1 unique viewer" for an app
with hundreds of impressions.
- **Anonymous rows carry `userId = 0` for everyone**, so `uniqExact(userId)`
alone collapses every signed-out viewer into a single phantom. Hence the
`uniqExactIf(userId, isAnon = 0) + uniqExactIf(ip, isAnon = 1)` split.
Design notes:
- `views.unavailable` is a PER-SECTION flag, not a third value on the payload's
own `unavailable`. ClickHouse can be unconfigured (the client is literally
`undefined` without CLICKHOUSE_HOST/USERNAME) or down while every
Postgres-derived counter in the same response is genuinely measured; flagging
the whole payload would throw away good data, and omitting the flag would
reproduce the fabricated-zero defect #3557/#3581 fixed. The card renders an
em dash, never a 0.
- The read never throws — it degrades to `unavailable` so one flaky store
cannot take down a panel whose other counters are fine.
- Values ride in `query_params`, never interpolation: the client's `$query`
tagged template formats strings VERBATIM (`formatSqlType` returns the raw
value), so `${id}` would be an injection vector. Matches the parameterised
pattern in scanner-review.service.ts.
- Week buckets use `toStartOfWeek(time, 1)`. ClickHouse defaults to Sunday and
Postgres `date_trunc('week')` is Monday, so mode 1 keeps the views line in
phase with the runs line on the same chart.
- Ownership is unchanged and still resolved once, upstream; the reader applies
no check of its own and is handed already-resolved ids.
Also updates the "What engagement counts" note, which asserted "anonymous
viewers are not counted" — true before this card existed, false now.
Verification (12 mutations across two differently-constructed sweeps, all trees
restored byte-identically and checksum-verified):
- 38 unit + 12 component tests pass; typecheck 0 errors; prettier clean
(positive control: AppAnalyticsInline.tsx, which violates on main, was
correctly reported).
- Sweep 1 (9 mutations on the reader + wiring): 8 KILLED, each by its own
specific assertion. M9 SURVIVED and is an **equivalent mutant**, proven by
construction rather than asserted — when `appBlockId` is set,
`getOwnedAppBlockIds` returns `[appBlockId]` or `[]`, and `[]` hits the
notOwned early return before the reader is reached, so the mutated and
original expressions are identical at that call site.
- Sweep 2 (4 ownership mutations, wider blast radius incl. the router test):
4/4 KILLED.
- Sweep 3 (3 mutations on the card): P1 — replacing the em dash with the raw
count — initially SURVIVED. The test was named for the em dash but never
asserted one, so a literal "0" above "Not measured right now" passed. Fixed
by asserting the em dash itself; 3/3 KILLED after.
⚠️ The component tier ran on this host's non-canonical browser (nixpkgs Chrome
for Testing via PLAYWRIGHT_BROWSERS_PATH); `preview / component-tests` is the
authority.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017vvMdxcMKJP9ripQLEiPm9
* fix(app-blocks): bound the impressions read, pin the isolation predicate, drop the dead series
Addresses an adversarial audit of the first commit. Two findings were real
defects in what I shipped, and the audit was right that my own mutation sweep
had been the easy half.
🔴 THE ISOLATION PREDICATE WAS UNPINNED. The `WHERE` clause is the only thing
that applies tenant isolation to the impression read (ownership is resolved
upstream, but this is where the resolved ids constrain the scan). My tests
asserted FRAGMENTS of it with `toContain`, so two semantic mutants that delete
nothing both SURVIVED a fully green suite:
AND -> OR after the id filter -> filter becomes a no-op, returns EVERY author
IN -> NOT IN -> returns every OTHER author
Neither is a live bug — the shipped SQL was correct — but the next edit could
have introduced one with CI green. The clause is now a single `VIEWS_WHERE`
constant and the test pins the WHOLE normalised text plus a no-comment-markers
assertion (a commented-out clause keeps the text while killing the code). The
trade is explicit and intended: a cosmetic reformat now fails that test.
🔴 THE READ WAS UNBOUNDED ON A FAN-OUT PATH. `@clickhouse/client-common` 0.2.10
defaults `request_timeout` to FIVE MINUTES and `max_open_connections` to
Infinity, and this app overrides neither. The degrade-don't-throw contract
covered errors only — a merely SLOW ClickHouse would hold the whole 11-way
`Promise.all` open, and `AppAnalyticsInline` issues one `getMyAppAnalytics` per
approved-app ROW, so a single page load fans out to N of them. Now bounded at
10s both client-side (`abort_signal`) and server-side (`max_execution_time`) —
the latter matters because aborting the socket alone leaves the query running
on the cluster. A timeout degrades to `unavailable`, not to zero.
Also fixed from the same audit:
- **Dropped the time-series query.** It was a second round-trip per call for a
value NOTHING renders (the panel charts only installs/runs), on the fan-out
path above. Halves the new ClickHouse load. The `toStartOfWeek(time, 1)`
Monday-alignment rationale is preserved as a comment for whoever restores it.
- **`analytics.views` is now optional and read through a guard.** tRPC output is
not zod-validated on the client, so a rolling deploy can hand a new bundle a
payload from an old pod with no `views` key — an unguarded read threw in
render. Absent is a third state and renders like unavailable, never as zero.
(Same class as the pointer fix already made on the CLI side.)
- **`anonCount` is impressions, not viewers.** Rendered next to `uniqueViewers`
as "12 unique viewers · 4 signed-out" it parses as "4 of those 12". It can
exceed uniqueViewers in practice; the old fixture used 4 vs 12 and hid that.
Now labelled "signed-out loads", and the fixture uses 40 vs 12 so the units
cannot silently coincide.
- **The card is "App loads", not "Views".** `blockRenders` counts mount
ATTEMPTS — `/api/track/block-render` writes a row even when the launch FAILS
(it is a failed mount's only beacon) and the table has no status column, so
the reader cannot exclude them. "Views" overclaimed.
- **`SimpleGrid type="container"`.** Mantine's default breakpoints resolve
against the VIEWPORT, but this panel also renders inside AppAnalyticsInline's
`Modal size="xl"` (~780px) — so on any >=1200px screen the modal took the
5-column branch inside 780px (~135px/card, ~103px usable).
- `logToAxiom` now passes the `'clickhouse'` datastream like every other
ClickHouse failure site in tracker.ts.
- Removed the unused `EMPTY_VIEWS` export; documented the empty-ids branch as
deliberately defensive (unreachable from the only production caller).
- Corrected `AppAnalyticsInline`'s stale copy, which still promised the fix
"until render-event tracking ships" — it shipped; this PR reads it.
Settled by evidence rather than reasoning: the audit flagged `createdDate`'s
type as an unverified assumption whose failure (silently dropping the current
day) no test could catch. Prod `system.columns` reports `createdDate Date`,
`time DateTime` — the bound is correct. Recorded in a comment with the failure
mode, since a future migration to DateTime would reintroduce it.
NOT fixed here, flagged instead: `/api/track/block-render` is a PublicEndpoint
gated only by a forgeable Origin/Referer check with a client-supplied
appBlockId, so this number is attacker-inflatable. Pre-existing writer-side
weakness that this PR makes consequential; it needs its own change.
Verification — post-fix battery of 12 mutants (a fix round resets the gate),
tree restored byte-identically and checksum-verified, 12/12 KILLED. Includes
both mutants the audit found surviving, and deliberately favours mutants that
DELETE NOTHING: operand swap, branch inversion, comment-out, stale rebind.
56 unit + 13 component tests pass; typecheck 0 errors; prettier clean.
⚠️ Component tier ran on this host's non-canonical browser;
`preview / component-tests` is the authority.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017vvMdxcMKJP9ripQLEiPm9
* fix(app-blocks): the container-query "fix" was inert — it collapsed the grid to one column
Round-2 review of the round-1 audit fixes. The headline finding is a regression
I introduced while fixing something else, which is the failure mode this loop
exists to catch.
🔴 `SimpleGrid type="container"` WITH NAMED KEYS RENDERS ONE COLUMN AT EVERY
WIDTH. I switched to container queries so the panel would stop taking the
5-column branch inside AppAnalyticsInline's ~780px modal. But Mantine's
container branch interpolates the `cols` KEY VERBATIM —
`simple-grid (min-width: ${key})` (SimpleGridVariables.mjs:116) — while only the
media branch resolves `theme.breakpoints[key]` (:56). So `{sm:2, md:3, lg:5}`
emitted `(min-width: sm)`, not a valid <length>; the query never matched and
every width fell back to `base`. Net effect: /apps/revenue went from 4 columns
to 1 — strictly worse than the cramped modal I was fixing. Now explicit lengths
(`48em`/`62em`/`75em`, Mantine's own sm/md/lg, which this app does not
override), verified in the emitted CSS.
The two chart grids are converted too, so the rationale in the comment is
actually true of the whole panel rather than one grid of three.
🔴 …AND IT IS NOW PINNED, despite the review concluding no test could catch it.
The failure is invisible twice: the component tier loads no Mantine stylesheet
(so `grid-template-columns` computes to `none` and any layout assertion passes
either way), and the diff reads like an ordinary responsive prop. But Mantine
emits the `@container` query TEXT regardless of any stylesheet — so asserting
that every emitted breakpoint parses as a CSS length catches it exactly.
Reverting to named keys now fails that test (verified by mutation). The guard
carries its own positive control: if the panel stops emitting container queries
at all, it fails rather than passing over an empty set.
🟡 THE `AppAnalyticsInline` CAVEAT WAS WRONG IN BOTH DIRECTIONS. Round 1 left it
stale ("until render-event tracking ships" — it had shipped). Round 2 corrected
the staleness but asserted `runs` comes from `block_scope_invocations`; it comes
from `block_spend_attribution` (app-analytics.service.ts:246-252). The
undercount conclusion held, but the stated mechanism was false and would send a
maintainer to the wrong table. Now names both tables and why they differ, and
says so in the comment so the third revision does not repeat the second.
🟡 THE TIMEOUT'S MAGNITUDE WAS UNPINNED. `toBeGreaterThan(0)` asserted the
guard EXISTS, not that it is short. Measured: 10s -> 25s left the suite fully
green, so a drift to ~29s on a per-app-row fan-out path would have shipped
silently. The constant is exported and pinned by value.
Also from the same round:
- The hang test now uses fake timers. Same guard — the promise still resolves
only via the abort event, so a missing timer or unwired signal still fails —
but the two service files drop from 10.36s to 3.21s, and from 30s to ~0 when
the abort is broken. Added the matching positive control: advancing to just
BEFORE the deadline must still yield a measured result, so a timer that fired
immediately cannot masquerade as a correct one.
- The SQL-comment ban is scoped to the predicate rather than the whole query;
applied globally it also forbade a legitimate `/* app-views */` query tag.
- The attacker-inflatable caveat now lives in the module doc, not only in a
commit message, with an explicit "do not build payouts or ranking on this
until the writer is authenticated".
Verification: 57 unit + 14 component (panel) + 2 component (inline) pass;
typecheck 0 errors; prettier clean. Both new guards mutation-killed by their
exact intended test, tree restored byte-identically:
R1 revert to named breakpoint keys -> KILLED (container-length guard)
R2 timeout constant 10 -> 25 -> KILLED (was the review's SURVIVING control)
⚠️ Component tier ran on this host's non-canonical browser;
`preview / component-tests` is the authority.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017vvMdxcMKJP9ripQLEiPm9
* test(app-blocks): close three guards that were weaker than their own comments
Round-3 review found no production regression this time — the shipped behaviour
of the previous round is correct. What it found instead was GUARD EROSION: three
of the guards I added or edited were weaker than the comments next to them
claimed, and two were strict regressions against guards that had been working.
Each is proven by a mutant that DELETES NOTHING.
1. THE "does NOT abort within the timeout" CONTROL WAS VACUOUS. Its comment
claimed a too-aggressive timeout "would look identical to a correct one"
without it. Measured: shortening the timeout to 1ms — every production query
aborted — left the file GREEN. The fake client resolved instantly and ignored
`abort_signal`, and the service clears its timer in `finally` before any test
can advance the clock, so the abort could never win the race. The fake now
takes FAKE_QUERY_LATENCY_MS and honours the signal. The 1ms mutant now fails.
2. SCOPING THE COMMENT BAN TO THE PREDICATE LOST A MUTANT THE PREVIOUS ROUND
CAUGHT. I narrowed it to leave room for a ClickHouse query-tag comment —
hypothetical (no such tag exists) and paid for with a real regression: with
the narrow check, commenting out `uniqExactIf(ip, isAnon = 1) AS anonViewers`
and rebinding it to `0`, in the SELECT list ABOVE the WHERE, SURVIVED. That
silently drops every anonymous viewer from `uniqueViewers` in production, and
nothing else can see it — the fake client never executes SQL, so the
aggregation tests return the fixture whatever the SELECT says. Ban restored
across the whole query, with a note on how to widen it deliberately if a tag
is ever wanted.
3. THE CONTAINER-QUERY GUARD MISSED A PARTIAL REVERT, AND SAID IT DIDN'T. Its
comment promised it "must fail loudly" if the panel stops emitting container
queries. `toBeGreaterThan(1)` against 4 expected uniques had far too much
slack: reverting ONLY the metric grid to named keys — reintroducing exactly
the modal squeeze the guard exists to prevent — left the two chart grids
emitting `62em`, so the count stayed at 2 and it passed. Only an all-three
revert tripped it. Now pins the exact set, which also subsumes the
empty-set positive control.
Also corrected, all comment-vs-code accuracy:
- `vi.useFakeTimers()` could escape the file. The `finally` does NOT run when a
test TIMES OUT (the awaited promise never settles) — precisely the scenario a
timer test hits — leaving every later test on a frozen clock. Proven with a
probe, fixed with `afterEach(() => vi.useRealTimers())`.
- The fake-timer cost claim was inverted: dropping the explicit `}, 30000)` made
the broken-abort case inherit the project-wide 60s testTimeout, so that path
got 2× SLOWER, not faster. Comment now says what was measured.
- The `ContainerGrid2` precedent I cited was wrong. `Grid` resolves named keys
against the theme in BOTH modes (`GridVariables.mjs`), so it has no
verbatim-interpolation defect and its `breakpoints` prop swaps the SCALE. A
maintainer could have concluded `Grid` shares the trap, or that `SimpleGrid`
takes a `breakpoints` prop. Neither is true.
- "counts only activity that SPENT Buzz" is wrong for `runs`: a zero-cost run
(cache hit, free gen) still writes a `block_spend_attribution` row
(blocks.router.ts:4710, buzz-attribution.service.ts:539). It is authenticated
generation SUBMITS. The undercount conclusion was right, the mechanism wasn't
— the third wrong version of this sentence, so the correction now says which
part is load-bearing. The tooltip also stops calling the same number by a
different name than the panel it links to.
Verification — the three surviving mutants re-run, all KILLED, each by the guard
repaired for it; tree restored byte-identically and checksum-verified:
N1 timeout 10s -> 1ms -> KILLED
N2 comment out anonViewers above the WHERE -> KILLED
N3 revert ONLY the metric grid to named keys -> KILLED
39 unit + 14 component pass; typecheck 0 errors; prettier clean. The unit files
now run in 576ms.
⚠️ Component tier ran on this host's non-canonical browser;
`preview / component-tests` is the authority. Note the preview deploy for the
previous commit failed on a ghcr.io push rate-limit (HTTP 403 secondary limit,
build itself succeeded) — infrastructure, not code; this push re-triggers it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017vvMdxcMKJP9ripQLEiPm9
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
briant
|
a1e641ac3b |
fix(upload): stop large model files finishing upload without being saved
A part PUT that came back with an HTTP status fired neither 'error' nor 'abort', so its promise never settled: the worker parked forever, the multipart upload was never completed, and the row sat at ~100% with no error and no ModelFile record. A non-ok /api/upload/complete response returned out of the store with no status change and no callback, which read the same way. Both scale with part count, so 40GB+ uploads hit them routinely — one creator needed 3-5 attempts per file. - reject on non-200 part responses and classify the retry (Retry-After aware, 5 attempts), matching what useS3Upload already did - retry /api/upload/complete on transient failures and always leave a terminal 'error' behind, so the file can be retried instead of looking finished - add /api/upload/sign-part so a part whose 12h presign expired mid- transfer can be re-signed rather than dying; scoped to the caller's own key prefix and our upload buckets - size chunks against the file (<=1000 parts) and return chunkSize from the presign so clients can't drift from what the server signed - HeadObject before answering 409 from complete, so a retry whose first attempt succeeded reports 200 instead of stranding uploaded bytes Retry classification now lives in utils/upload-retry.ts shared by the store and the hook — the store never received the hook's fix, and that divergence is what this bug was. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
briant
|
6af670bec3 | 5.0.2235 | ||
|
Luis E. Rojas Cabrera
|
88a88e4cc1 |
Merge pull request #3603 from civitai/feat/move-recurring-bid-to-the-latest
feat(auction): move recurring bid to the latest model version |
||
|
Luis Rojas
|
8a0f703c60 |
feat(auction): let creators move a recurring bid to the latest version
Adds a `moveRecurringBidToLatest` mutation plus a `moveToLatest` hint on `getMyRecurringBids`, so a standing bid can be re-targeted in place at the newest eligible published version without touching today's already-charged bid. The card surfaces this as an arrow action, disabled with an explanatory tooltip when another recurring bid already covers the target. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> |
||
|
Briant Diehl
|
b8e8e38309 |
Merge pull request #3622 from civitai/ltxv-enhancements
@ |
||
|
briant
|
d248273c75 |
@
feat(generation): add Grok Imagine v1.5 Adds v1.5 as a selectable model version inside the existing Grok ecosystem, alongside v1.0. All three operations from the API spec are wired: textToVideo, imageToVideo, and referenceToVideo (1-7 reference images). v1.5 is video-only, so the image workflows and vid2vid:edit are excluded for it, and img2vid:ref2vid is excluded for v1.0. 1080p is offered on v1.5 text-to-video and image-to-video only; the reference-to-video endpoint caps at 720p. Duration keeps the v1.0 6-15s range - the client types declare no bounds for v1.5. Bumps @civitai/client to 0.2.0-beta.84 for the GrokV15* input types. That version also adds a required includeReasoning to XGuardModerationInput, set to false to preserve current behavior. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> @ |
||
|
Luis E. Rojas Cabrera
|
770dcbec20 |
Merge pull request #3621 from civitai/feat/add-takedown-path-with-refunds-and
feat(creator-shop): add takedown path with refunds and payout clawback |
||
|
Luis E. Rojas Cabrera
|
5c76813c61 |
Merge pull request #3600 from civitai/feat/allow-unpublishing-early-access-models-via
feat(models): allow unpublishing early access models via refund |
||
|
Luis Rojas
|
06bb48b98d |
feat(models): refund early access buyers when unpublishing a model
Owner unpublish no longer hard-fails on models with early access purchases: the service computes the refundable set from active PaidAccess gates and ledger amounts, and with explicit `refundEarlyAccess` consent refunds each buyer from the owner's yellow account and revokes their grant. The model page fetches the requirement first and confirms the buyer count and total Buzz before sending the mutation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> |
||
|
Luis E. Rojas Cabrera
|
251b2743a4 | Merge branch 'main' into feat/add-takedown-path-with-refunds-and | ||
|
Luis E. Rojas Cabrera
|
78b69c47a8 |
Merge pull request #3620 from civitai/fix/scope-review-queue-creator-filter-to
fix(creator-shop): scope review-queue creator filter to actual submit… |
||
|
Luis Rojas
|
203735109f |
feat(creator-shop): add takedown path with refunds and payout clawback
Adds a moderator-only `creatorShop.takedownItem` (plus a "Take down" action in the review queue) that pulls a cosmetic from sale, refunds every buyer, reverses the Buzz the seller and any reseller were paid, and strips the cosmetic from all accounts that own it. Purchases now record their payout split and each payout's transaction id on `UserCosmeticShopPurchases.meta` so the clawback reverses the exact transactions instead of guessing the split, with legacy rows falling back to the creator's guaranteed slice. Money moves are retried and logged to Axiom rather than aborting the run, so a partial failure leaves the ids needed to finish by hand. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> |
||
|
Luis Rojas
|
8ff496797f |
fix(creator-shop): scope review-queue creator filter to actual submitters
The moderator review queue's creator filter searched all users via `user.getAll`, so moderators could pick creators with nothing in the queue. It now pulls its options from a new `creatorShop.getReviewQueueCreators` endpoint that returns only users who have submitted a shop item, dropping the debounced search and id-lookup handling in favor of a single loaded list. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> |
||
|
Zachary Lowden
|
e0d5a483c0 |
test(apps): make the dark-flag analytics test fail loudly, and pin its loose locators (#3616)
`AppAnalyticsInline.browser.test.tsx`'s dark-flag test awaited
"Analytics unavailable" — text that only the branch under test renders. So the
one regression it exists to catch (the unavailable branch falling through to the
fabricated "0 runs / 0 users" stat) surfaced as a ~15s
"Matcher did not succeed in time" locator timeout, which reads like CI flake
rather than a value regression. Measured on a mutated component: 15.11s and a
timeout at the await, with the two absence assertions never reached.
Await the "Analytics" trigger instead — the one element rendered in EVERY
branch, matching what the sibling test already does — then assert the label's
presence explicitly. Same mutation now fails in 0.21s with
"expected [] to have a length of 1 but got +0".
Also documents why the `runs`/`users` locators deliberately stay NON-exact,
since the obvious "consistency" cleanup toward the sibling's `{ exact: true }`
is actively harmful here. These are ABSENCE assertions, so a looser matcher is a
stricter test. Verified against a regression that renders the fabricated stat as
one consolidated text node: the loose form fails, the `{ exact: true }` form
passes — a false green. The collision that makes `exact` load-bearing in the
sibling cannot occur in this branch either: it renders neither the stat nor its
hover tooltip, confirmed by parking the pointer on the label and re-querying
(0 matches for both words, hovered and unhovered).
No component change; behaviour under test is unchanged.
|
||
|
Zachary Lowden
|
15a14cd8f7 |
fix(freshdesk-agent): enforce the query_database timeout server-side and scope it to the tables it needs (#3584)
`query_database` is the one support-agent tool whose SQL the model writes, and
it was handed the app-wide read client behind a single startsWith('SELECT')
check. Three things about that were not what they looked like:
- The advertised 30s timeout was a `Promise.race` against a `setTimeout`. That
rejects the wrapper but does not cancel the query — the backend keeps running
and keeps holding its pooled connection. openrouter.ts:157-159 documents this
exact failure mode. The tool description promised a guarantee the server never
made.
- Nothing bounded which relations it could read. `dbRead` is the app-wide read
client, and packages/civitai-db/src/client.ts:24 aliases it to `dbWrite`
whenever DATABASE_REPLICA_URL === DATABASE_URL.
- openrouter.ts:322-338 Promise.all's every tool call in a turn, so several
query_database calls can run at once — and with an unbounded query each could
pin a connection indefinitely.
Route the statement through the primitive the repo already has:
db-helpers.ts:303 `queryWithTimeout` does BEGIN READ ONLY -> SET LOCAL
statement_timeout -> COMMIT and is documented PgBouncer-safe. The timeout is now
the database's to enforce, so it cancels the backend and frees the connection.
The Promise.race wrapper is removed rather than kept alongside — two mechanisms
where one is real is how the description drifted. Postgres 57014 is mapped to a
message the model can act on.
Add a 26-table allowlist (freshdesk-query-scope.ts), derived rather than
guessed: every relation the Prisma.sql queries in freshdesk-investigation-tools
already read, plus "User" for the email lookup freshdesk-prompts spells out
inline. The check is a small scanner plus a positional walk of the FROM list at
each paren depth — deliberately conservative, so it refuses any shape it cannot
positively confirm (CTEs, schema prefixes, table functions, FROM-inside-a-
function-call, comments, multi-statement, dollar quoting, prefixed literals)
rather than pattern-matching its way to a pass. Known false rejections and what
it does NOT bound (columns, rows, functions needing no FROM) are documented on
the function.
Update the tool description to state only what is actually enforced, including
the table list, so the model writes to the real contract. The
startsWith('SELECT') check is unchanged and still runs first.
Side effect: pg returns bigint as a string where Prisma returned BigInt, so
`SELECT count(*)` no longer fails on JSON.stringify.
Tests: 43 new (36 scope + 7 tool). The tool suite is 6 red / 1 pass against
pre-change freshdesk-tools.ts, each on its own assertion; the non-SELECT case is
an invariant guard, not regression coverage. The scope suite is covered by an
11-mutation sweep, 11 killed / 0 survivors, with an all-accept negative control.
|
||
|
Zachary Lowden
|
30be4b6811 |
feat(db-schema): schema<->database drift detector (declared vs enforced constraints) (#3591)
* feat(db-schema): add a schema<->database drift detector
Migrations here are applied by hand, per environment, so a constraint can be
declared in schema.full.prisma and simply absent from the database for a long
time without anything noticing. This makes that gap countable.
`pnpm --filter @civitai/db-schema drift` parses the schema, reads pg_catalog,
and reports four classes:
- foreign key declared but absent (matched on the ordered column tuple)
- foreign key present with a different ON DELETE / ON UPDATE than declared
- column nullability vs field optionality
- @unique / @@unique with no total unique index
Read-only: every statement is a SELECT against pg_catalog. Connection comes
from DATABASE_URL; nothing about any environment is baked in. Default exit
code is 0 — a gate that failed on the existing backlog would be red on every
run, so --strict is opt-in.
Three things it gets right on purpose, each of which produced a wrong answer
first:
- Prisma's implicit onDelete is SetNull for an optional relation and
Restrict for a required one, never Cascade.
- `references:` is read, never assumed to be `id`.
- column aggregates are selected as text[]. attname is `name`, and
node-postgres has no parser for name[], so array_agg(attname) arrives as
the string "{a,b}" — .length still answers and every tuple comparison
silently becomes a character count.
Models mapped to a view or an absent table, and @@ignore models, are skipped
and counted rather than reported as missing every constraint they declare.
Check constraints, defaults, column types, enum values and non-unique indexes
are NOT checked. The report says so, so a quiet section is not read as an
all-clear.
* fix(db-schema): see every owning-side relation, and stop reporting clean on nothing
Two defects from review, both outside the differ, which is where the unit
tests were not looking.
1. The relation regex accepted `@relation("X", fields: …)` but not
`@relation(name: "X", fields: …)`. Prisma takes both. This was not a
partial result: the six relations written the other way appeared in no
counter and produced no finding, so the tool reported clean on foreign
keys it had never looked at — the exact failure it exists to prevent.
Three of the six (Club.coverImageId, Club.headerImageId, Club.avatarId,
all -> Image, all declared SetNull) have no foreign key in production.
Corrected totals: 474 declared relations, not 468; 37 missing foreign
keys, not 34.
The test that missed it asserted `relations.length > 400` — a floor
cannot see an undercount. It now checks the parser against an
independently derived count of owning-side relation lines, which
self-updates as the schema grows.
2. `--strict` exited 0 on a catalog that checked nothing. An empty catalog —
a typo'd --db-schema, a wrong DATABASE_URL, a role that cannot read
pg_catalog — produced a fully clean report and a success exit. The CLI
now assesses its own coverage and exits 2 regardless of --strict, and
`cli.test.ts` runs the real entry point as a process to hold both halves
of that control: empty catalog must fail, covering catalog must pass.
Under --strict, referential actions that could not be compared now fail
too: "not measured" is not "clean".
Also from review:
- a new drift class, declared-column-with-no-column. 11 in production:
ModelFlag.sfwOnly and ten UserRank.thumbs{Up,Down}Count*Rank. A Prisma
read touching one errors outright, so it is reported separately rather
than folded into nullability.
- block attributes (@@map/@@ignore/@@unique) are matched with comments
stripped. A stray `// @@ignore` would otherwise skip a whole model.
- the CLI no longer echoes a connection string passed by mistake, and
reports pg's error CODE instead of its message, which embeds host and
user. This repo and its CI logs are public.
- primary keys (@id/@@id) and programmability (views, functions,
triggers) added to the not-checked list in both the report and the
README. Neither is audited.
- the production catalog snapshot is committed, so the documented numbers
are reproducible from the repo. It holds schema metadata only: table and
column names, nullability, and constraint column tuples.
* docs(db-schema): record why the checked-in pg_dump cannot pin these numbers
Review suggested pinning the acceptance counts against
containers/db/docker-init/02_all_dll.sql instead of shipping a snapshot,
with the caveat to verify its freshness first. Verified: it cannot.
It is a dev bootstrap DDL, not a production mirror — 195 tables against
production's 342, 178 production foreign keys absent, and 25 foreign keys
declared that production does not have. Against the 37 missing foreign keys
this PR documents it would contradict 19, fail to see 7 (table absent
entirely), and agree on 11.
All three Club image relations are in the contradicted 19, so pinning to it
would assert that the exact foreign keys this tool found missing are
present. It does agree on the *Rank nullability shape and on
ModelFlag.sfwOnly being absent, which is why a spot check there reads as a
match — the disagreement is confined to foreign keys, and only enumerating
them shows it.
Also recorded: this tool decides "backed by a view" from the catalog
(relkind IN ('r','p')), not from the /// @view annotation, so the detached
strip regex in scripts/prisma-migrate-with-views-workaround.mjs (26 of 32
annotations have a blank line before `model`) does not affect it. And the
second live view-drift instance, BountyEntryRank_Live, alongside
BountyRank_Live in the not-checked list.
* chore: retrigger preview pipeline (component-suite flake check)
No content change. Third data point on AppsSubmitEditView.browser.test.tsx 'retry RE-ARMS a
fresh ceiling', which failed on both prior runs of this branch. Same failure again => deterministic,
investigate before merge. Pass => suite flake, consistent with #3592 failing a different test in the
same App Blocks suite.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
Zachary Lowden
|
1b2ec15780 |
test(pgdb-parity): exempt a factory that spreads the real module — the gate's advice broke correct files (#3618)
The parity scan is a regex for `name:` over the factory span, so it read
`...(await importOriginal<typeof PgDbModule>())` as supplying nothing and
reported such a suite as "missing: pgDbReadLong, pgDbWrite". The suite was not
missing them — it had the REAL ones.
Worse, the assertion's own FIX advice ("add `pgDbReadLong: {}`") would have
replaced a real export with an empty stub, turning a correct file into a broken
one on the gate's instruction.
The hazard this gate exists for is kyselyDb.ts destructuring every export at
module-eval time; a spread omits no name, so the hazard is absent. Proof the
spread resolves: in the same CI run where this gate failed PR #3584, that PR's
own freshdesk suite collected and passed.
The spread form is also strictly safer than a literal — it cannot go stale the
day the module grows an export, which is the exact defect (#3579) this file was
written for. The gate was penalising the pattern most immune to its own bug.
Reproduced before fixing (probe flagged), verified after (accepted), and
negative-controlled (an incomplete literal is still flagged). #3584's real file
was copied in and passes. New test pins the exemption in both directions, and
states the textual-scan limit rather than implying none.
Claude-Session: https://claude.ai/code/session_01R7ZCxbgcsv3WfsSJrYdSCi
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
Zachary Lowden
|
54c2ff8e4b |
test(pgDb-parity): finish the retraction — the false figure was still printing into CI logs (#3617)
Follow-up to the retraction in #3604. That commit corrected the file HEADER but left the identical false claim in the assertion's failure MESSAGE, 130 lines lower — so every time the guard fired, CI printed a measurement the same file explicitly labels as false 130 lines above. Found by a CI probe that deliberately broke the invariant and read the real job output. The retraction was verified by reading the header; the message was never re-grepped. `grep -c` on the file returns 2, and only one was fixed — the exact "a count=1 replace on a pattern that occurs more than once" trap. Removed the performance justification from the message entirely. The message now states only what is true and useful at the point of failure: the factory is an inline sync literal, sharing one would require an async dynamic import in every mocked suite, and the single-sourcing lives in this test plus the canary. The header keeps the full retraction, since recording that the claim was made and why it was wrong has value; the CI message does not need to relitigate it. After this, the figure appears exactly once in the repo, inside the block whose next sentence is "That is false." Not verified locally: this worktree has no node_modules, and symlinking them from the primary clone produces a misleading `Cannot find package 'kysely'` (a known artifact, hit again while trying). The change is a template literal in an `expect` message parameter and cannot affect test logic; CI is the verification. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Zachary Lowden
|
472cfa5ce6 |
docs(claude): extend Git Worktrees with the three ways a worktree test run lies (#3567)
The section already covers the `event-engine-common` submodule and the typecheck/build wall of `Cannot find module` errors. Three measured gaps it did not cover — all of which produce FALSE results rather than loud failures: 1. Without the submodule, `src/server/routers/__tests__/blocks.router.workflow.test.ts` fails to COLLECT and contributes 0 tests. It does not report red, it reports nothing, and a run that collected nothing still reads as a pass to anyone checking an exit code. Validate a worktree run by confirming that file collected a nonzero count (308 on one base). 2. A fresh worktree has no `.envrc` (gitignored), so it silently gets system Node instead of the flake's pin. Measured: system 26.5.0 vs flake 22.22.2 produced 7 spurious `window.localStorage is undefined` failures under happy-dom plus 8 Prisma `linux-nixos` engine errors — all false reds attributed to the code. Also confirm cwd IS the worktree: one run with cwd set to another repo lost two suites to collection failures and 77 tests silently never ran (10849 -> 10772) while output looked normal. 3. Browser/component tests on NixOS: no `chromium` on PATH. Two routes are known to work and which one works has varied — `steam-run` + an LD_LIBRARY_PATH carrying nss/nspr, or PLAYWRIGHT_CHROMIUM_EXECUTABLE_PATH at a nix chromium with `--browser.api.host/--browser.api.port`. A stale `node_modules/.vite` cache (typical after a `kill -9`) hangs for minutes at near-zero CPU; clear it before blaming either route. Docs-only. `CLAUDE.md` already fails `prettier --check` on `origin/main` (verified against a positive control), and modified files are the report-only lint tier, so this neither fixes nor regresses that. Claude-Session: https://claude.ai/code/session_01858ymA3tEJQi83435u7npi Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Zachary Lowden
|
9354d369b4 |
fix(user-profile): bind the creator-card stats payload so apostrophes survive (#3577)
`updateUserProfile` built the jsonb payload for `creatorCardStatsPreferences` by splicing `JSON.stringify(...)` into a single-quoted SQL string literal. `JSON.stringify` escapes `"` and `\` but not `'`, so a preference value holding an apostrophe closed the literal early and the statement stopped meaning what it says — the write failed and the caller got a 500 instead of a saved profile. `creatorCardStatsPreferences` is `z.array(z.string()).max(3)` on `userProfileUpdateSchema`, and `updateUserProfileHandler` spreads the input straight through, so the three strings reach the statement unaltered. The profile edit modal only offers the six `creatorCardStats` constants, but nothing between the schema and the statement narrows the values. The statement is now a parameterised tagged template (`$executeRaw`), so the payload and the id bind instead of being pasted into the statement text — the same shape as the `setUserSetting` fix in #3570, where `${JSON.stringify(obj)}::jsonb` binds as a parameter that Postgres then casts. `jsonb_set` and the `{creatorCardStatsPreferences}` path are unchanged, so the write still touches that one key and nothing else. Raw SQL is kept rather than a typed `update`: Prisma can only assign a JSON column wholesale, so a typed update would mean a read-modify-write and would lose the atomic server-side single-key set. This is the only raw statement in the file — every other write on this path goes through the typed client. Tests: 6 new cases in `src/server/services/__tests__/update-user-profile.sql.service.test.ts`. They pin the emitted statement TEXT literally — the `jsonb_set` call, the `{creatorCardStatsPreferences}` path and the `$1::jsonb` cast — as well as the bound values, so a drift to a different operator or a different cast fails the suite rather than passing on "the value isn't in the text". 5 of the 6 fail on the pre-change code, each on its own assertion; the 6th covers the no-preferences branch, which this change does not touch, and is labelled an invariant guard rather than counted as regression coverage. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Zachary Lowden
|
e29b522a30 |
test(user-settings): pin the SQL shape setUserSetting emits, not just where values land (#3580)
The seven existing tests assert that user data reaches the database as a bound
parameter and never appears in the statement text. That is the property the file
was written for, and it holds — but it says nothing about whether the statement
around those parameters is well-formed.
Every key-removal assertion reads `values`, or asserts a negative on the text
(`not.toContain('}')`). Nothing pins the operator or the cast. Measured against
the current suite, three edits to `setUserSetting` leave all seven green while
making the statement fail outright at the database:
- `::text[]` -> `::jsonb` on the removal operand: `jsonb - text[]` is the only
form that takes a bound array of key names, so the cast is load-bearing.
- `settings - $1` -> `settings || $1` on the removal statement: an operator
with no definition at these operand types, and the opposite meaning from the
one that branch exists to express.
- the merge payload re-wrapped as `'$1'::jsonb`: still binds the value, so the
values-based assertions stay green, but Postgres casts the literal two
characters `$1` and rejects it. That is the original defect in a new costume.
Adds three tests that pin the emitted shape. Measured matrix, all in a clean
worktree off origin/main with the target suite counted per test:
clean origin/main 10 passed (10)
::jsonb cast 2 failed | 8 passed -- both removal-shape tests, each on
its own assertion's message
|| operator 1 failed | 9 passed -- the subtract test
quoted payload 1 failed | 9 passed -- the merge-placeholder test
A fourth edit (quoting the bound id as `'${userId}'`) was run purely to show the
merge test's second assertion is reachable and killing rather than decorative;
it fails there and nowhere else.
Test-only. `user.service.ts` is byte-identical to origin/main.
|
||
|
Zachary Lowden
|
35177835c5 |
fix(schema): align the seven *Rank models' nullability with the database (#3592)
* fix(schema): align the seven *Rank models' nullability with the database Declaration-only. These columns are already nullable in production, so there is no DDL to emit and no database was contacted. 245 scalar columns across ArticleRank, UserRank, TagRank, CollectionRank, BountyRank, BountyEntryRank and ClubRank were declared required while the database has them nullable. Each table was hand-built outside the Prisma migration path and carries exactly one not-null constraint -- on its entity-id column -- so the rule applied here is: every scalar becomes optional except the entity id, which stays required. Relation fields are untouched. The read path already assumed this. article.service.ts coalesces NULL rank columns to INT_MAX, with a comment describing the pagination bug that NULLs caused when the declaration was believed. Blast radius measured rather than assumed: pnpm run typecheck reports 0 errors. Every consumer reads these tables through $queryRaw with hand-written row types, and the one Prisma-relation consumer uses the relation only in orderBy, which is nullability-agnostic. Reconciling the declared count against the live catalog surfaced a separate drift class, reported in the PR body but not addressed here: 252 required scalars are declared and 242 exist, so 10 declared columns have no backing column at all. * style(schema): restore the space in TagRank's 'Int? @default(0)' The mechanical pass consumed one padding space on the 30 TagRank lines whose type column sat flush against the annotation, leaving Int?@default(0). It is the file's only non-uniform formatting -- the base file has 0 such occurrences against 1187 spaced ones -- and nothing would ever flag it: Prettier covers only *.ts/*.tsx, there is no prettier-plugin-prisma, and prisma format runs nowhere. Type-neutral: db:generate output is byte-identical. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Zachary Lowden
|
a4e886fc9d |
test(get-models-raw): hoist model.service import out of the per-test timeout budget (#3612)
* test(get-models-raw): hoist model.service import out of the per-test timeout budget
`get-models-raw.transient-503.test.ts` loaded the module under test with
`await import('~/server/services/model.service')` inside a test helper, so the
first test to run paid the cold TS transform + import of a ~4,800-line module
and its transitive service graph — and Vitest charges that to that test's
`testTimeout`. Measured, the file spent ~99.9% of its runtime in one test, and
that share is invariant to CPU (99.86% unconstrained on a 24-core host, 99.85%
under a 4-CPU quota). Against the 60s ceiling that left the file passing or
failing on ambient CI-runner speed, presenting as a single
`Test timed out in 60000ms` rather than a uniformly slow file.
Hoist the import to module scope. `vi.mock` is hoisted above static imports, so
the mocks still apply and no test semantics change; the transform now happens in
Vitest's collection phase, which no timeout bounds (Vitest exposes only
testTimeout / hookTimeout / teardownTimeout). This removes the cliff rather than
moving it, which raising the timeout or deferring to `beforeAll` would do.
Worst per-test, measured at three points (before -> after):
isolated, unconstrained 24-core host 4364ms -> 2ms
isolated, 4-CPU quota 3685ms -> 4ms
full 783-file suite, 4-CPU quota 2726ms -> 2ms
Suite unchanged: 783 files, 11588 passed + 1 skipped, before and after.
Mutation-verified that the 9 tests still discriminate (production code restored
afterwards; `git diff` on model.service.ts empty):
isTransientMeiliError(err) -> err instanceof MeiliCallTimeoutError
=> 6 failed / 3 passed, exactly the widening cases, each failing because the
raw MeiliSearchCommunicationError escaped unconverted
isTransientMeiliError(err) -> true
=> 2 failed / 7 passed, exactly the `does NOT convert` negatives, each
failing because a real app bug was masked as a retryable 503
Also adds a comment beside `testTimeout` in vitest.config.mts: the existing note
rationalised the 60s ceiling as the thing that absorbs cold `await import()`
cost, which normalises the pattern that caused this.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* docs(test): mark the CI-runner figure as a PROJECTION, not a measurement
Comment-only. The hoist comment presented two numbers under one "measured:"
label, but only the first was measured:
measured 2726ms of a 2730ms file, locally under a 4-CPU quota — and the
~99.9% first-test share is invariant to CPU (99.86% unconstrained
vs 99.85% under quota)
PROJECTED ~44.2s of the observed ~44.3s file total on the CI runner. Nobody
reproduced this in-pod; it is that invariant share applied to a
file total read out of a CI log.
Labelling a projection as a measurement is how a plausible number gets cited
later as a result. It is stated as a projection now, with the two things that
corroborate it: the failure signature was ONE test timing out (not a uniformly
slow file), and the same file — byte-identical — passed at 45.3s on a faster
runner and timed out at 60.2s on a slower one.
The projection being unverified does not weaken the fix. The mechanism is
directly observable locally: after the hoist this file reports
`9 tests | 7ms` with `transform 3.44s, import 5.78s` — the import cost is real
and now lands in Vitest's COLLECTION phase, which no timeout bounds, instead of
inside one test's 60s budget.
9/9 pass.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
Zachary Lowden
|
216bba5fbc |
fix(blocks): an errored XGuard label is a NON-ANSWER, not a clean one (#3609)
* fix(blocks): an errored XGuard label is a NON-ANSWER, not a clean one `XGuardLabelResult` carries `error?: null | string`. A label the scanner ATTEMPTED and FAILED on comes back as a PRESENT `results[]` entry with `triggered: false` and an `error` string. That entry satisfied the label-drift guard (whose only question is whether a `label` came back at all) and contributed nothing to `triggered` — so the policy read it as a CLEAN ANSWER and RELEASED the generated text. A scan where all fifteen labels errored was indistinguishable, to this policy, from a scan where all fifteen came back clean. That violates the invariant this file states in so many words: "'no answer' must never read as 'clean'". Drift is a label that was never EVALUATED; an error is a label whose evaluation FAILED. Both are non-answers, and every other branch here (scanner throw, hard deadline, no-verdict, over-cap, drift) already withholds on one. `erroredRequestedLabels` gives the failure its own signal rather than folding it into `missingLabels`: the two faults have different remedies (fix our label constant vs look at scanner health), so aggregating them would send an operator to the wrong place. It gets its own `stage: 'label-error'` and, like drift and the error branch, is deliberately NOT memoized — caching an operational fault pins a blip into a five-minute outage of the capability, and a memo hit logs nothing so the rate would go silent after one observation. `finishReason` is deliberately NOT acted on: unlike `error` it cannot be read by presence, and its healthy value set is orchestrator-side config we have never observed. Pinned by a test, with the probe gate written on the function. 17 tests added. 15 red at |
||
|
Zachary Lowden
|
e9f9809f0d |
fix(schema): correct 8 referential actions that misdescribe the database (#3589)
* fix(schema): correct 8 referential actions that misdescribe the database Declaration-only. No migration is generated and no database is touched: none of these relations has a foreign key in the database today, so there is no constraint to alter. Six *Rank relations, NoAction -> Cascade (UserRank.userId, TagRank.tagId, CollectionRank.collectionId, BountyRank.bountyId, BountyEntryRank.bountyEntryId, ClubRank.clubId). NoAction means the parent delete errors when a rank row exists; BountyRank covers 100% of bounties, so the FK as declared would make every bounty undeletable. Rank rows are derived and rebuilt wholesale by recreateRankTable / updateLeaderboardRank, and no delete path cleans one up before deleting its parent, so there is no step that could unblock such a delete. ArticleRank already declares Cascade. TagsOnImageNew.imageId, implicit Restrict -> explicit Cascade. Required relation with no explicit action defaults to Restrict, which would reject every Image delete -- silently, since deleteImageById swallows the failure into a log. The FK was created ON DELETE CASCADE in 20250303170613_tags_on_image_new and dropped in 20250314203912_drop_tags_on_image, one day after the after_image_delete trigger took over the cascade. Records the intent; does not add the FK. ClubPost.coverImageId, implicit SetNull -> explicit SetNull. The live constraint is already ON DELETE SET NULL and 8 of the 9 cover-image relations already declare it; this was the sole omission. The 235-column *Rank nullability alignment is deferred -- measured at 0 type errors, but the exact nullable column set cannot be pinned from the repo alone (a mechanical pass yields 245, not 235). * docs(schema): state that the TagsOnImageNew FK is intentionally absent The trigger, not a constraint, is the enforcement point, and it was verified enabled in production. Make that explicit at the declaration so onDelete: Cascade is not read as a to-do to add the FK back. * docs(schema): warn that db:migrate will try to add the TagsOnImageNew FK Name the trigger correctly (after_image_delete_trigger; after_image_delete is the function) and add the missing half of the warning: the model carries no /// @view annotation, so prisma migrate will emit an ADD CONSTRAINT for it. That generated DDL now reads as plausible -- it said RESTRICT before this branch, which was obviously wrong on sight -- so whoever reviews a generated migration has to know to delete the statement. |
||
|
Zachary Lowden
|
dc977dc0c0 |
fix(app-blocks): key step-moderation telemetry on AppBlock.id, not the publish slug (#3611)
`pollWorkflow` and `cancelWorkflow` passed `appBlockId: claims.blockId` into `attachModeratedStepTextOutputs`. A block token carries three distinct id claims and these are not aliases: `appBlockId` is `AppBlock.id` (the `apb_<ulid>` PK), `blockId` is `AppBlock.blockId` (the publish request's slug), and `blockInstanceId` is a per-render id. `publish-request.service.ts` writes `id: apb_<ulid>` and `blockId: request.slug` onto the same row, and the primary mint site signs `blockId: block.blockId` / `appBlockId: block.id`. The value reaches exactly one consumer: `logScan`'s Axiom event `block-step-text-output-moderation`, which `text-output-moderation.ts` documents as the per-app trigger-rate key. A slug logged under `appBlockId` joins to neither `AppBlock.id` nor `BlockScopeInvocation.app_block_id`. There is no DB write and no FK on this path, so the wrong value failed silently. These were the only 2 of the router's 15 claim-sourced `appBlockId:` assignments reading `claims.blockId`; the other 13 already read `claims.appBlockId`. Also updates three comments that asserted the now-closed hazard or a stale fact about `'textOutput'` adoption. Claude-Session: https://claude.ai/code/session_01R7ZCxbgcsv3WfsSJrYdSCi Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Zachary Lowden
|
0a67174d51 |
test: close the incomplete-pgDb-mock defect class (shared factory + static gates) (#3604)
* test: close the incomplete-pgDb-mock defect class (shared factory + static gates)
17 test files hand-wrote their own `vi.mock('~/server/db/pgDb', ...)` factory, each
listing only the exports that file happened to need. `kyselyDb.ts` destructures every
pgDb export at module-eval time, and Vitest's mocked-module proxy throws on a name the
factory omitted — during module LOAD, so the throw becomes the rejection value every
assertion inspects. The failures read as assertion failures against intact production
code, and a suite whose collection dies contributes zero tests instead of reporting a
failure. Adding one export to pgDb has reddened unrelated suites twice.
Three layers now close it, two of them blocking pre-merge:
1. `local-rules/no-wholesale-module-mock` gains `~/server/db/pgDb` as a target, so a
hand-written factory is rejected at authoring time ("ESLint (added files)" blocks).
2. `src/test-utils/pgDbMock.ts` is the single factory. Its stub object is typed
`Record<keyof typeof import('~/server/db/pgDb'), unknown>`, so an export added to
pgDb fails `tsc` naming the export, in one file. `Typecheck` blocks.
3. `pgDbMock.parity.test.ts` re-asserts both at runtime, covering the gap that the
ESLint step for MODIFIED files is report-only.
The rule's usual remedy — spread the real module via `importOriginal()` — is unsafe
for this particular module: importing pgDb runs `getClient()` and hands the suite a
live, query-capable `BoundPool` (verified: `.query('SELECT 1')` opens a real socket).
That would trade a silently-empty suite for a unit test that can reach a database. So
the rule gains a narrow `completeFactories` option letting a module nominate a shared
helper instead, accepted only when the factory demonstrably imported it from the
configured module. The completeness proof moves from the AST to the type system; it
stays static and blocking either way.
No behaviour change to any suite: 17 files / 133 tests before and after, per-file
counts identical.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* test: drop the completeFactories ESLint layer; keep the type + runtime guarantee
Acts on the adversarial audit of this PR. Two changes: fix the blocking Prettier
failure, and REMOVE the ESLint layer entirely rather than ship a safety gate that
asserts a property it does not have.
WHY THE LINT LAYER IS GONE
The `completeFactories` option was defeated with a single decoy import line. Its
binding check scanned the factory body for ANY declarator destructuring the helper
name out of the configured module, and never checked that the identifier actually
being CALLED resolved to that binding. The audit's fixtures, run against the real
rule and real config with validated controls (2 known-bad -> REPORT, 1 good ->
CLEAN):
decoy import + local shadow returned .................... ACCEPTED (wrong)
decoy import + same-named import from another module ..... ACCEPTED (wrong)
decoy import inside `if (false)` + shadow ................ ACCEPTED (wrong)
binding in a nested block, out of scope at the return .... ACCEPTED (wrong)
LEGITIMATE alias `{ createPgDbMock: makeMock }` .......... REPORTED (wrong)
So it accepted the wrong member of a rename pair and rejected the right one. The
rule's own invalid case carried the comment "Without these, the option would be a
rubber stamp" — asserting exactly the guarantee the implementation lacked. A gate
satisfiable without its safety property is worse than no gate, because it is
believed.
Fixing it properly needs real scope resolution (resolve the callee Identifier via
`getScope(node)` to its declarator). That is a genuine option, but it was 252 of
554 lines — 46% of this PR — to buy one thing the other layers don't: BLOCKING
coverage of a newly-ADDED file hand-rolling a pgDb factory. The runtime parity
test already covers that case, and the defect's current blast radius is zero (all
12 sites missing an export are unreachable from kyselyDb; the only 2 reachable
files already listed every export). Not worth the complexity, so it is dropped
whole: .eslintrc.js, eslint-local-rules.js and the rule's test file are restored
BYTE-IDENTICAL to main. `~/utils/trpc` keeps its existing protection untouched.
WHAT REMAINS — and it is the part that carries the guarantee
1. src/test-utils/pgDbMock.ts — `stubs` typed Record<keyof typeof import(pgDb),
unknown>. A TOTAL record: a new pgDb export fails the BLOCKING `Typecheck`
job, in one file, naming the export.
2. pgDbMock.parity.test.ts — runtime backstop with the same invariant.
3. 17 suites converted to the shared factory.
RE-VERIFIED AFTER DROPPING A LAYER (an audit fix resets the gate; the type gate is
now the PRIMARY blocking layer, so its teeth were re-proven, not assumed):
positive control — append `export let pgDbProbe` to pgDb.ts:
tsc 0 -> exactly 1 error
src/test-utils/pgDbMock.ts(78,9): TS2741
"Property 'pgDbProbe' is missing ... required in type 'PgDbStubs'"
parity test fails naming [pgDbProbe] with a one-line fix instruction
under test (probe reverted):
tsc 0 errors, parity 3/3 green
Reported as the PAIR — 1 on the positive control, 0 under test — never the zero
alone.
18 converted suites: 136/136 pass.
no-wholesale-module-mock.test.ts: 97 tests, matching main exactly (confirming the
+7 additions were reverted, not merely disabled).
Prettier: src/test-utils/__tests__/pgDbMock.parity.test.ts reformatted. The
"Prettier (added files)" step has no continue-on-error and was genuinely red.
Rebased onto current main so the checks gate the merged tree.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* test: revert the shared runtime factory — it regressed CI; keep the guarantee statically
The shared-factory approach caused a real CI regression and is withdrawn. The
durability guarantee is preserved by other means at zero runtime cost.
WHY THE SHARED RUNTIME FACTORY IS GONE
A `vi.mock` factory is hoisted above imports, so it CANNOT reference a statically
imported helper. Sharing one therefore requires making the factory async:
vi.mock('~/server/db/pgDb', async () => {
const { createPgDbMock } = await import('~/test-utils/pgDbMock');
return createPgDbMock();
});
That is not free. Measured in the CI pod by diffing per-file durations against an
unrelated PR (#3605) that ran the same suite minutes earlier, the async factory
slowed EVERY converted suite, proportionally to the work it does:
get-models-raw.transient-503 44,307ms -> 60,290ms TIMEOUT (+36%)
challenge.metrics.completing-stuck 14,807ms -> 24,284ms (+64%)
the other 14 (all small) 3-212 -> 6-304 +1..+280ms
The first blew the 60s per-test timeout and reddened 9 tests. It is INVISIBLE
locally — that file runs in ~3.5s here either way, a ~12x environment factor — so
this was only findable by reading the in-pod logs. `lint.yml`'s own comment
predicted exactly this ("a 2-core runner could trip the same way").
Two runs of this PR failed that gate; #3605 and #3606 passed it minutes either
side, so it was the change, not ambient load.
WHAT REPLACES IT
Each suite keeps an inline SYNC object literal (as before, and as on main), now
COMPLETE — the 12 factories missing `pgDbReadLong` (and in 3 cases `pgDbRead` or
`pgDbWrite`) now list all three. The single-sourcing moves off the runtime path:
1. src/test-utils/pgDbMock.ts — unchanged in purpose, reframed in role: it is now
a CANARY, not a factory suites call. `PgDbStubs = Record<keyof typeof
import(pgDb), unknown>` is a total record, so adding an export to pgDb fails
the BLOCKING `Typecheck` job right there, naming it.
2. pgDbMock.parity.test.ts — its scan is rewritten. It no longer asks "does this
file use the shared helper" (which would now flag all 17 by design); it parses
each `vi.mock('~/server/db/pgDb', ...)` factory's balanced-paren span and
asserts the key set is complete, naming the file AND the missing export.
This also closes the audit's finding #3: the old scan matched only the
single-quoted `~/` specifier, so a byte-identical double-quoted factory
scanned as zero offenders — a false green. The matcher now accepts any quote
style and relative specifiers.
VERIFICATION — reported as pairs, never a bare zero
type gate append `export let pgDbCanary` to pgDb.ts
-> tsc 0 -> exactly 1: pgDbMock.ts(84,9) TS2741
"Property 'pgDbCanary' is missing ... required in type 'PgDbStubs'"
-> under test (reverted): 0 errors
scan PC-1 drop pgDbReadLong from user-challenge-flag-gate (single-quoted)
-> "user-challenge-flag-gate.test.ts (missing: pgDbReadLong)"
scan PC-2 same, but DOUBLE-QUOTED specifier (the audit's gap)
-> detected identically; the old scan would have missed it
under test parity 3/3, 17 pgDb-mocking suites 133/133, tsc 0 errors
Stale claims corrected in both files' doc blocks: they described a three-layer
design whose first layer (the ESLint rule) was removed in the previous commit, and
pgDbMock.ts still told readers to migrate suites onto the helper.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* docs(test-utils): RETRACT the performance justification — it was pod variance, not the async factory
The previous commit justified keeping inline sync mock factories by claiming the
shared async factory cost +36% on a 44s suite and +64% on a 15s one in CI. That
claim is FALSE and is retracted in both files' doc blocks.
It was derived by comparing per-file durations across two DIFFERENT preview runs
on DIFFERENT pods, with no control for ambient pod speed. The correct control is a
within-run normalisation, and it says the opposite:
run global median vs base target file verdict
async 1.43x 1.36x NOT slower than the run
sync 1.43x 1.36x NOT slower than the run
Identical in both. Across ~193 files >=200ms, 149 were >20% slower than the
baseline run — including files this branch never touches
(audit-matching-equivalence +33.0s, prisma-inconsistent-orphan-relations +25.2s,
listForModel.behavior +18.6s). The whole POD is ~1.43x slower; the async factory
cost nothing measurable, and the target file was in fact slowed slightly LESS than
the run-wide trend.
What is actually true, and is now stated instead:
- `get-models-raw.transient-503.test.ts` consumes ~44s of a 60s per-test timeout
on a fast pod. A pod ~1.4x slower times it out. That is a pre-existing
marginal-file flake, unrelated to pgDb mocking, and it reproduces on a tree
where that file is BYTE-IDENTICAL to main.
- The inline literal is kept to avoid async indirection in 17 files — a
simplicity argument, which is honest — not a performance one.
The design is unchanged; only the justification was wrong. Keeping a false
performance claim in a comment is worse than having no comment: it would be cited
later as a measured result, and it was measured badly.
parity 3/3 green.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
Zachary Lowden
|
4a83e28163 |
docs(blocks): correct the FOURTH site claiming 'textOutput' withholds on any scanner failure (#3610)
#3607 and #3608 corrected this claim in three places. This is the fourth, and it survived both because it sits in `isModerationPostureImplemented`'s body — a different region of `index.ts` from the posture-union header #3607 fixed, and a different file from the two #3608 fixed. `'textOutput'` withholds on a policy hit and on MOST scanner failures — scan threw or hard deadline fired, no output, over the scanned-character cap, and a requested label absent from `results[]`. It does NOT withhold on a per-label `error`: such an entry carries a `label`, so it counts as evaluated and suppresses the drift guard, contributes nothing to `triggered`, and the verdict RELEASES. That gap is real on this tree and documented at the verdict function. Replaces the universal with the enumeration, names the gap, and adds an instruction to re-derive from `decideTextOutputVerdict` rather than trusting the summary in either direction — so it cannot rot into a false "still broken" claim once the gap is closed. Comment-only; verified zero non-comment changed lines. Claude-Session: https://claude.ai/code/session_01R7ZCxbgcsv3WfsSJrYdSCi Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Zachary Lowden
|
f959487c60 |
fix(app-blocks): make the AIR-scan rejection legible for chat-completion prose (no enforcement change) (#3608)
* fix(app-blocks): make the AIR-scan rejection legible for chat-completion prose
`containsAirReference` is a case-insensitive SUBSTRING scan for `urn:air:`
over every string, array element, object value and object key of a step's
built input. `chat-completion` is the first registered entry whose scanned
strings are PROSE: `messages[].content` is the only string in its built input
a caller can steer (`model` is `z.enum`-bounded, `maxTokens`/`temperature`
are numbers, every key is a fixed literal). So a user asking a chat block
"what does urn:air: mean?" gets a hard FORBIDDEN whose message asserted the
step "carries an AIR reference" — false for prose, and it read as a platform
bug rather than as something the app can fix in its own payload.
INVESTIGATED SCOPING THE SCAN AND DELIBERATELY DID NOT. Read at
civitai-orchestration origin/main e9ce862cb: string content deserialises to
`ChatCompletionContentPart { Text, ImageUrl = null }`;
`ChatCompletionInput.OnInitializedAsync` walks `Messages` but acts only on
`part.ImageUrl`; the single `urn:air:` test in the whole chat step is
`CalculateCostAsync`'s `input.Model.StartsWith(...)`; `GenerateJobsAsync`
copies `Messages` verbatim. Prose is inert there today — but that is a claim
about a separately deployed service that nothing in this repo can hold true,
so an exemption would be an unmonitored entitlement risk. The asymmetry
decides it: the false positive costs one bounced message and is fixable by
the app, an entitlement bypass is not recoverable by anyone.
What changed: the rejection now names the literal it matched, says the check
is a substring scan that does not parse, and tells the caller to strip or
escape it. `AIR_URN_PREFIX` is exported so the message cannot drift from the
scan. Plus the analysis written where the next person to touch this guard
will see it, and tests pinning the prose case as deliberate.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01R7ZCxbgcsv3WfsSJrYdSCi
* docs(app-blocks): replace the false "withheld on any scanner failure" claim with an enumeration
`chat-completion.step.ts` asserted in two places that the `'textOutput'`
posture withholds "on any scanner failure". That is a stronger safety
property than the implementation has, and a comment overstating a guard is
what a maintainer deletes the guard on the strength of.
Read off `./text-output-moderation` and `./moderation` rather than restated
from intent, the output phase withholds on exactly: `stage:'error'` (scan
threw / hard deadline), `stage:'no-verdict'` (submit failed, or the workflow
was still running when `wait` elapsed), `stage:'over-cap'` (>50,000 joined
chars, checked ahead of both the network call and the memo),
`stage:'label-drift'` (a requested label absent from `results[]`), and a
non-array / non-string extractor result caught in the output phase — plus the
ordinary `stage:'withheld'` policy hit. An EMPTY extraction is a release of
zero texts, not a withhold, and is called out as harmless.
Records the gap that sat inside the old phrase as OPEN: generated
`XGuardLabelResult` carries `error?: null | string`, this side's
`XGuardLabelResultLike` declares only `label`/`score`/`triggered`, and
`decideTextOutputVerdict` reads only those three — so a label the scanner
ATTEMPTED AND FAILED ON contributes no trigger, still lands in the
`evaluated` set (so the drift guard does not fire), and RELEASES. #3609 is in
flight against `./text-output-moderation`; this comment is written to be true
of the tree THIS commit produces and does not assume any merge order.
Comment-only. No behaviour change, and `text-output-moderation.ts` is not
touched.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01R7ZCxbgcsv3WfsSJrYdSCi
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
Zachary Lowden
|
49f7dac397 |
docs(blocks): correct six stale/contradicting comments in the step subsystem (#3607)
* docs(blocks): correct six stale/contradicting comments in the step subsystem Comment-only. The emitted JS is byte-identical to the base commit for all three files (verified by transpiling both sides with --removeComments and diffing; the detector was validated against a planted one-token code change). - request-time-policy: the chatCompletion row quoted a sentence out of index.ts in order to rebut it. #3605 deleted that sentence, so the quote pointed at text that no longer exists and sent a reader to the opposite of what it set up. Now references the corrected note instead of quoting it. - index.ts: the posture-union header claimed 'textOutput' was "still unanswered and still fails at load". False — isModerationPostureImplemented returns true for it, its output handler exists, and chat-completion is registered declaring it. - moderation.ts: appBlockId was documented as "the block instance id", which matches neither claim the token carries. Both live callers pass claims.blockId (a slug), not claims.appBlockId (the apb_<ulid> PK). The comment now describes reality; the fix belongs at the two router call sites. - request-time-policy: the three "GUARD" section headers read as live request-path gates. None has a request-path caller — guards 1 and 3 run only from two load-time asserts, guard 2 and the composite gate have no live caller at all. Stated plainly so a reviewer cannot conclude the registry's clauses are redundant with them. - Two registered-entry counts ("exactly one today (convertImage)", "seven no entry has yet claimed") went stale when chat-completion registered. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01R7ZCxbgcsv3WfsSJrYdSCi * docs(blocks): drop three overclaims from the previous commit's own new text Self-review of the comments added in |
||
|
Zachary Lowden
|
0ec1c4b9bc |
fix(test): anchor the Generations locator so the analytics component suite stops oscillating (#3606)
#3574 turned `preview / component-tests` red and I merged through it, having convinced myself CI did not run that tier. It does. Root cause is mine, and this fixes it. `AppAnalyticsPanel.browser.test.tsx` asserted `page.getByText('Generations')`. `getByText` is substring + case-insensitive, and the "Runs (range)" stat's own tooltip copy begins "Generations run through your app within the selected range...". Mantine mounts that tooltip only while the pointer rests on its target, and vitest browser mode shares ONE browser page across every `.browser.test.tsx` file — so the pointer position left behind by an earlier file can already sit there at mount. The locator then resolves to 2 elements and the assertion dies with a strict-mode violation after the matcher timeout. The collision was INTRODUCED by the label rename in that PR: the previous value, 'Generation submits', does not substring-match the tooltip sentence; 'Generations' does. Verified by computation rather than by eye, and the check history matches exactly — |
||
|
Zachary Lowden
|
fce7b918b5 |
docs(blocks): correct a false claim in the MEDIA-XOR-TEXT note — chatCompletion already has the dual shape (#3605)
`stepOutputShape`'s doc block ended "Nothing in the currently-licensed `$type` set has that shape", describing a media+text step as a future problem. That is false. `chatCompletion` has exactly that shape: `ChatCompletionInput.modalities` accepts `['image']`, and with it the orchestrator returns generated images on `choices[].message.images[].image_url.url` as base64 data URIs — bytes that never reach image ingestion, never become moderated `Image` rows, and are not seen by the text scan either. What actually keeps the XOR true is the `chatCompletion` entry's `.strict()` `paramSchema` omitting `modalities`, which `chat-completion.step.ts` already documents at length. The registry-level note contradicted it. This matters because the comment is the control here: someone registering a media+text step reads "nothing has that shape" and concludes the licensed types are single-natured, when the invariant they actually need to defend is per-entry schema discipline. Comment-only change; no behaviour, no API surface. Claude-Session: https://claude.ai/code/session_01R7ZCxbgcsv3WfsSJrYdSCi Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Luis E. Rojas Cabrera
|
dfc4e3ad12 |
Merge pull request #3602 from civitai/feat/require-and-record-creator-rights-affirmation
Feat/require and record creator rights affirmation |
||
|
Luis Rojas
|
06aee8f64d |
feat(auction): move recurring bid to the latest model version
Adds a `moveRecurringBidToLatest` mutation plus a hint on `getMyRecurringBids` so creators can re-target a standing recurring bid at the newest eligible published version of a model without deleting and re-creating it. The swap only rewrites the BidRecurring row — today's already-charged bid is untouched, so the new version takes effect with tomorrow's nightly run and nothing hits the refund path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> |
||
|
Luis E. Rojas Cabrera
|
e53fb25fb9 |
Merge pull request #3601 from civitai/feat/email-both-parties-when-a-gift
feat(membership-gift): email both parties when a gift is fulfilled |
||
|
Zachary Lowden
|
c924894a50 |
feat(apps): refuse go-live for an off-site listing whose primary CTA is non-actionable (#3594)
* feat(apps): refuse go-live for an off-site listing whose primary CTA is non-actionable * test(apps): pin the backfill + revision-strips-destination go-live cases * test(apps): discriminate the pre-tx approve fail-fast from the in-tx re-assert * test(apps): pin connect+https as ALLOWED now that #3585 has landed * test(apps): close four vacuous fixtures that made the actionable gate a no-op Audit finding F1. Two fixtures omitted `kind`, so the actionable-CTA gate hit its `listing.kind !== 'offsite'` early return and did nothing — four approve/revision tests passed with the guard DELETED. `kind` is NOT NULL in the DB, so both rows were also unrepresentable, and the edit fixture was internally inconsistent: applyApprovedRevision took the off-site copy branch while the off-site gate skipped. Measured rather than assumed. Instrumenting the gate's skip branch and running each fixture state: offsite-listing.edit.service.test.ts before SKIP=2 EVAL=0 -> after SKIP=0 EVAL=2 offsite-listing.onsite-revision.service.ts before SKIP=8 EVAL=1 -> after SKIP=7 EVAL=2 The 7 residual skips in the second file are onsite listings, which correctly skip an off-site gate. Two earlier attempts at this proof were the WRONG instrument and both reported a false all-clear: neutering the gate changed nothing (these fixtures PASS it, so removing a passing guard is unobservable), and throwing on gate entry fired identically in both states (the defect is that the gate is CALLED and skips, not that it is uncalled). Only instrumenting the skip branch discriminates. Gate file restored pristine; blocks suites 2424 passed / 107 files, rc=0. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Luis Rojas
|
54238dd65a |
feat(creator-shop): require and record creator rights affirmation
Submitting a cosmetic — or replacing its artwork — now requires the creator to confirm they hold the rights to sell it, and the service stores that affirmation (user, timestamp, version, and the exact wording) in the shop item's meta so a later takedown challenge can show who agreed to what. The moderator review queue surfaces the record alongside the automated checks. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> |
||
|
Luis Rojas
|
6cff222856 |
feat(membership-gift): email both parties when a gift is fulfilled
Adds `membershipGiftReceived` and `membershipGiftSent` email templates and sends them at the end of `fulfillMembershipGift`, respecting anonymous gifts and skipping users without an address. Sends are best-effort — a mail failure is logged rather than thrown, so the Stripe webhook doesn't retry an already-applied fulfillment. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> |
||
|
Luis Rojas
|
e4ab78ccbe |
feat(creator-shop): require and record a rights affirmation on submit
Submitting (or swapping the artwork on) a cosmetic now requires the creator to confirm they hold the rights to sell it, and the service stores the affirmation on the item's meta — user, timestamp, wording version, and the statement verbatim — so a later takedown challenge can show who agreed to what. A moderator replacing artwork neither affirms nor overwrites the creator's record, and the moderator review queue surfaces the affirmation alongside the automated checks. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> |
||
|
Luis Rojas
|
28052045c1 |
feat(models): allow unpublishing early access models via refund
Owners were hard-blocked from unpublishing any model with early access purchases; now `unpublishModelById` computes the still-active purchases, and with the owner's explicit `refundEarlyAccess` consent refunds each buyer from the owner's yellow account and revokes their grant before unpublishing. Adds a `getEarlyAccessRefundRequirement` query so the model page can show buyer count and total Buzz in a confirm modal, and keeps the hard error for owners who haven't consented. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> |
||
|
briant
|
c55eff1d0c | chore(creator-studio): release creator-studio-v0.0.32 creator-studio-v0.0.32 | ||
|
briant
|
930808a7e0 | 5.0.2234 v5.0.2234 | ||
|
Zachary Lowden
|
b5b292a2b4 |
test(apps): make the genuine-measured-zero analytics discriminator actually run (#3593)
* test(apps): make the genuine-measured-zero analytics discriminator actually run The `DISCRIMINATOR: a genuine measured zero still renders the 0 runs / 0 users stat` test added in #3557 has never completed. It asserted `expect.element(page.getByText('runs')).toBeInTheDocument()`, and `getByText` is substring + case-insensitive, so it also matches the stat's own tooltip copy — "Runs and unique users in the last 30 days…". Mantine mounts that tooltip only while the pointer rests on the stat (`mounted: !!tooltip.opened`, hover-only), and vitest browser mode shares ONE browser page across every `.browser.test.tsx` file, so in the full-suite run the pointer position left behind by an earlier file can already sit over the stat at mount. The locator then resolves to 2 elements and the assertion dies with a strict mode violation after the matcher timeout. Because the component suite is report-only in CI, that failure was never surfaced: the one test proving #3557's fix still shows a real measured zero (rather than suppressing it as "unavailable") has been reporting safety while asserting nothing. Fix is test-only — the component is not at fault: - anchor on `{ exact: true }`, which cannot match the tooltip sentence; - await the "Analytics" trigger (the one element rendered in every branch) instead of the stat, so a regression that stops rendering the stat fails on an assertion rather than a locator timeout; - assert the VALUE, not the word: the stat row's text must read `0 runs · 0 users`. Asserting only that "runs"/"users" appear would still pass if the component rendered the labels with no number at all — the same vacuous guard in a new costume. Proven by mutation (component temporarily mutated, then reverted): - render the counts as empty strings -> FAILS in 223ms with `AssertionError: expected 'runs·users' to match /^\s*0\s*runs\s*·\s*0\s*users\s*$/` - treat a genuine zero as unavailable (the exact regression #3557 fixed) -> FAILS in 243ms with `AssertionError: expected [] to have a length of 1 but got +0`. The sibling dark-flag test PASSES under this mutation, which is why the discriminator has to exist. Reverted, both tests pass — verified with the pointer both off and parked on the stat (tooltip open), the state that used to break it. * test(apps): fix the audit's two findings — a false committed comment and a brittle anchor Both from the adversarial audit of this PR. Test-only; no component change. 1. 🔴 The comment I committed asserted something FALSE. It said this discriminator "has never once completed since it was added in #3557". The audit falsified it: of the 5 most recent PRs containing #3557, THREE had `preview / component-tests` green, meaning the discriminator ran and passed with the old locator. The real mechanism is worse than a permanent red, so the comment now describes it: vitest's BaseSequencer sorts failed-first then longest-duration-first from a cache persisted on the preview workspace's reused volume. A run in which this file times out therefore promotes it to run FIRST next time — pointer still at (0,0), nothing hovered — and it passes. Since #3557 it has ALTERNATED between a ~20s strict-mode timeout and a pass that asserted nothing about the value. "Sometimes green, never meaningful" is exactly the shape that survives review, and a comment claiming "always red" would have sent the next reader looking for the wrong thing. 2. `runsLabels[0].parentElement` was structurally over-fitted AND misreported its own cause. The audit showed that wrapping the `runs` <Text> in a plain <span> (zero visual or behavioural change) red the test with `expected 'runs' to match /…0 runs · 0 users…/` — a message about the VALUE, so a maintainer would hunt a data regression rather than a markup change. Replaced with a bounded upward walk (`findStatRow`) to the nearest ancestor containing the whole stat, plus an explicit message naming markup-shape as the cause and stating it is NOT a value regression. Whitespace is normalized, and the separator is now tolerant (`·` vs `•` is cosmetic) while BOTH ZEROS remain mandatory — the value is the entire claim under test. An audit fix resets the verification gate, so the discriminator was re-proven from scratch against the refactored assertion — not carried over from the pre-fix runs: M-A runs.count + 1 (value) -> FAILS "expected '1runs·0users' to match /^0\s*runs\s*[^\d]*0\s*users$/" M-B <span> wrapper (cosmetic) -> now PASSES (this is finding 2 fixed; it FAILED misleadingly before) M-C genuine zero routed to the "—" -> FAILS "expected [] to have a length of 1 branch (the real #3557 bug) but got +0" The sibling dark-flag test stayed green under M-A and M-C, so the discriminator is still load-bearing and non-redundant rather than shadowed by its sibling. Rebased onto current main (was 9 behind) so the checks gate the merged tree. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
briant
|
18b17c7db1 |
feat(creator-studio): report client errors to Axiom
hooks.server.ts only sees thrown loads and actions, so an error during hydration went nowhere: the page server-rendered fine, the URL resolved, and the tab went blank with nothing recorded anywhere to act on. Diagnosing one meant trying to reproduce another user's browser, which is why a live each_key_duplicate report on /models has stayed unexplained -- every list the page renders is provably duplicate-free for the affected account. Add handleError in hooks.client.ts, beaconing to an auth-gated endpoint that logs to the same Axiom stream as the server hook. sendBeacon rather than fetch because a request started at hydration-failure time usually never leaves the tab. The captured query string is the point of this: whether the tab carried a filter or ?mode= is the likeliest difference between a report that reproduces and one that doesn't. Note the stack lands minified -- vite has no sourcemap setting, so it defaults off, and Svelte's production each_key_duplicate carries no key or component name. Route, URL and status are the reliable fields until build.sourcemap is turned on. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |