mirror of
https://github.com/civitai/civitai.git
synced 2026-09-20 22:08:18 +08:00
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>
71 lines
3.5 KiB
TypeScript
71 lines
3.5 KiB
TypeScript
/**
|
|
* A DETERMINISTIC foothold inside React's commit→passive-effect window, for the
|
|
* browser tier.
|
|
*
|
|
* ── WHAT WINDOW, AND WHY IT NEEDS REPRODUCING ───────────────────────────────────
|
|
*
|
|
* A host component gates a postMessage handler on a piece of state. React writes
|
|
* commit-time output (a DOM attribute) during the COMMIT, but the `useEffect` that
|
|
* re-registers the handler with the fresh state is a PASSIVE effect, flushed later.
|
|
* A message arriving in between meets the PREVIOUS render's closure. Any guard for
|
|
* that bug has to post its message inside the window, on purpose, every run.
|
|
*
|
|
* ── TWO OBVIOUS APPROACHES, BOTH MEASURED, BOTH WRONG ───────────────────────────
|
|
*
|
|
* 1. `MutationObserver` on the attribute. Its callback is a MICROTASK: it runs when
|
|
* the JS stack unwinds, i.e. at the END of the whole scheduler task. React's work
|
|
* loop keeps running callbacks inside that SAME MessageChannel task while
|
|
* `shouldYieldToHost()` is false, so the passive flush frequently happens BEFORE
|
|
* the checkpoint. Measured: a MutationObserver-driven guard killed its mutant
|
|
* 2 / 2 / 3 times over three runs — each guard survived roughly one run in three.
|
|
* A guard that is a coin flip is not a regression test.
|
|
*
|
|
* 2. A SIBLING component with `useLayoutEffect`. Layout effects genuinely are ordered
|
|
* before the same commit's passive effects, so the ordering reasoning 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 layout tick, observing `data-block-ready="false"`, and the
|
|
* probe never fired at all.
|
|
*
|
|
* ── WHAT ACTUALLY WORKS: THE ATTRIBUTE WRITE ITSELF ─────────────────────────────
|
|
*
|
|
* React writes the attribute with `Element.prototype.setAttribute` during the
|
|
* commit's MUTATION phase. Wrapping that method means the callback runs
|
|
* SYNCHRONOUSLY, on React's own stack, at the exact instant the host publishes
|
|
* readiness — strictly before the layout phase and therefore strictly before ANY of
|
|
* this commit's passive effects. There is no scheduling involved, so there is
|
|
* nothing to lose a race to: it is ordered by the call stack, not by a queue.
|
|
*
|
|
* That is deliberately invasive, and it is scoped accordingly: the patch is
|
|
* installed for the duration of one drive and removed by the returned disposer,
|
|
* which callers run in a `finally`.
|
|
*/
|
|
|
|
/**
|
|
* Run `run()` exactly once, synchronously, the first time any element's
|
|
* `data-block-ready` attribute is set to `"true"` — i.e. inside React's commit,
|
|
* before this commit's passive effects flush.
|
|
*
|
|
* Returns a disposer that restores the original `setAttribute`. ALWAYS call it in a
|
|
* `finally`: leaving a patched prototype behind would leak into every later test in
|
|
* the file.
|
|
*/
|
|
export function onReadyAttributeWrite(run: () => void): () => void {
|
|
const original = Element.prototype.setAttribute;
|
|
let fired = false;
|
|
Element.prototype.setAttribute = function patched(
|
|
this: Element,
|
|
name: string,
|
|
value: string
|
|
): void {
|
|
original.call(this, name, value);
|
|
if (fired) return;
|
|
if (name !== 'data-block-ready' || value !== 'true') return;
|
|
fired = true;
|
|
run();
|
|
};
|
|
return () => {
|
|
Element.prototype.setAttribute = original;
|
|
};
|
|
}
|