mirror of
https://github.com/civitai/civitai.git
synced 2026-09-20 22:08:18 +08:00
Zachary Lowden
5c76917c24
* fix(app-blocks): PageBlockHost's status gates read a stale closure at commit
The five status-gated PageBlockHost message handlers (REQUEST_CONSENT,
OPEN_BUZZ_PURCHASE, REQUEST_SIGN_IN, OPEN_IMAGE_UPLOAD, NAVIGATE) closed over
`status` from the render that registered them. React writes `data-block-ready`
during the COMMIT but flushes passive effects — the ones that re-register the
listener with a fresh `status` — in a LATER scheduler task. So there is a window
where the host publicly reports ready while the LIVE handler still holds
`status === 'loading'` and drops the message.
Two independent changes:
1. Root cause. `statusRef.current = status` moves to the RENDER BODY (it was
updated in an effect, which has exactly the deferral being fixed), and the
handlers read it through one shared `readGateStatus()` instead of four
open-coded copies of the `'error' -> 'no_token'` shim plus a fifth hand-rolled
comparison in NAVIGATE. `status` drops out of those five effects' dependency
arrays, so they now subscribe once per mount rather than re-registering on
every status transition.
2. Never hang. The two gates that carry a `requestId` now NACK when they genuinely
refuse, mirroring the reviewMode branches that already did ("so it fails fast
(no hang)"). REQUEST_CONSENT / REQUEST_SIGN_IN / NAVIGATE get no NACK: they are
fire-and-forget with no requestId and no reply message. A missing/non-string
requestId is still dropped silently — there is nothing to reply to.
Measured, not inferred: the handler is stale at the instant `data-block-ready`
flips (MutationObserver checkpoint), and 9 existing browser tests across three
files pass today only because a 50ms `vi.waitFor` poll usually lands after the
flush — swapping their `driveToReady` for a commit-window version turns all 9 red
at the base commit and all 9 green with this change.
Inferred, NOT observed: that a deployed block hits this. No SDK code path
auto-posts a gated request on ready — all four are `useCallback`s the app invokes
(@civitai/blocks-react 0.39.0) — so reaching the window needs app code that calls
one in the same turn it observes ready. No such block was observed. This is a
latent defect being closed, not a reported outage.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Aj1HosDFkP6LgxMAtJdDmJ
* test(app-blocks): count only BUZZ_PURCHASE_RESULT in the unrepliable-drop guard
The new "OPEN_BUZZ_PURCHASE with NO requestId is dropped silently" guard asserted
a zero by comparing `replies.received.length` — a count of EVERY host→block
message. The host pushes BLOCK_INIT / TOKEN_REFRESH / ROUTE_CHANGED from its own
effects on a schedule the test does not control, so that total drifts for reasons
unrelated to the assertion.
Found by re-running the commit-window exposure probe against the committed fix:
with the drive handing control back inside React's commit window, the late
effect flush landed an unrelated push between the two reads and the guard failed
`expected 1 to be +0` against a host that was behaving correctly. That is a CI
flake this PR would otherwise have shipped.
Now filters to BUZZ_PURCHASE_RESULT, and the positive control is strengthened to
drive the control request all the way to a REPLY — so the zero is read only after
a BUZZ_PURCHASE_RESULT has been proven observable, rather than after merely
proving a modal opened.
Re-verified after the change (an audit fix resets the gate): the
`m3_buzz_nack_too_wide` mutant — widening the NACK to also answer the unrepliable
no-requestId case — still turns exactly this guard red.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Aj1HosDFkP6LgxMAtJdDmJ
* test(app-blocks): make the commit-window guards deterministic, retire the workaround
Audit follow-up. Four claims from the previous commits did not hold; one was
load-bearing.
F1 — THE GUARDS WERE STOCHASTIC, AND THE "3 RED" RESULT WAS NOT REPRODUCIBLE.
Reverting `statusRef.current = status` into an effect killed 2 / 2 / 3 guards
across three runs: each commit-window guard survived the mutant roughly one run in
three, so a future revert would slip all three with ~4% probability and any
single-file run was ~33% blind. The mechanism is that a MutationObserver callback
is a MICROTASK — it runs when the stack unwinds, i.e. at the end of the whole
scheduler task — while React's work loop keeps running callbacks inside that same
MessageChannel task while `shouldYieldToHost()` is false. The passive flush
therefore often happened BEFORE the checkpoint. The old comment's reassuring "it
can never false-FAIL" was in fact an admission that the guard silently degrades to
a no-op.
Fixed by hooking the attribute write itself (`test/commitWindow.tsx`,
`onReadyAttributeWrite`): React calls `Element.prototype.setAttribute` during the
commit's mutation phase, so the callback runs SYNCHRONOUSLY on React's own stack,
ordered by the call stack rather than by a queue. Now 4/4 guards killed on 5/5
runs (20/20 per-guard).
A sibling component with `useLayoutEffect` was tried first and rejected: the
ordering argument is sound but the premise is not — the host's internal
`setStatus` re-renders the HOST, not its sibling, so the sibling's layout effect
never runs again after mount. Measured: exactly one tick, observing
`data-block-ready="false"`. Both dead ends are written up in the helper so the
next person does not re-derive them.
F3 — RETIRED THE NOW-INERT TEST-SIDE WORKAROUND. `openUploadModal`'s fresh-id
retry loop existed to work around this production bug; with the host fixed it can
never take a second attempt. It is deleted rather than left in place, because an
inert retry whose comment calls both halves load-bearing is a false claim, and the
next person to hit a similar flake would copy it. The comments in
PageBlockHost.browser.test.tsx and PageBlockHostReviewMode.browser.test.tsx that
still described the gate as lagging `data-block-ready` are corrected too.
F4 — ADDED THE MISSING POSITIVE CONTROL. "REQUEST_CONSENT with nothing missing is
a no-op" asserts only zeros (no dialog, no toast), and a host that DROPPED the
message produces the same zeros — so pre-fix it could pass without ever exercising
its own subject. It now first posts an un-grantable scope and waits for the toast,
proving the handler is live under exactly those props before the zero is read.
Also dropped an overstated claim from three comments: the transport's 5s requestId
dedup is real in `usePostMessage` but UNREACHABLE from the SDK, whose
`nextRequestId()` is `random6 + a monotonic counter`, so every call mints a fresh
id. The 10-minute / 30-second hang argument stands on its own.
Verified: 342/342 AppBlocks browser tests; typecheck 0 errors; prettier clean;
eslint 7 errors at base and 7 at HEAD on the touched files (zero delta).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Aj1HosDFkP6LgxMAtJdDmJ
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>