fix(persistence): a circuit-lifetime DbContext must not serve stale snapshots to writes #123
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "fix/circuit-scoped-dbcontext-staleness"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Found while investigating bjoern's report of the stale face-region warning ("the sprite was regenerated…") apparently sticking forever.
The bug
In Blazor Server the scoped
KaguraDbContextlives as long as its circuit — the browser tab. Reads areAsNoTrackingthroughout, so they're always fresh; but a tracked write-path load (FirstOrDefaultAsyncbefore mutating) is served from the context's identity map: a snapshot frozen at the tab's last write, with the fresh database values discarded. Anything another scope wrote in between — the generation queue, another tab, later the Phase-4 assistant — is silently invisible to the tab's next write.The reproduced casualty: regenerate an outfit sprite while its editor tab had previously written that outfit. The queue sets
FaceRegionStale = truein its own scope; the tab's snapshot still saysfalse; clicking "The region still fits" setsfalseon an entity already believingfalse→ EF detects no change → nothing is written → the warning sticks for the life of the tab. (Reopening the tab "fixes" it — a fresh circuit has no snapshot — which is why the symptom is elusive. Any read-modify-write racing an external writer can lose data the same way.)Reproduced three ways before fixing: in a real browser (two tabs — the confirm visibly wrote an outfit with a title from before the other tab's rename), in a red integration test, and traced through the EF identity-map semantics.
The fix
ChangeTracker.Clear()in afinallyaround bothSaveChangesoverrides: after every save (either outcome — after a failure the tracked state is equally suspect), the context detaches everything, so each write loads current database state. Reads being no-tracking means tracked entries only ever come from write paths, so this is exactly the residue that must go. Bonus: the identity map no longer grows for the life of a tab.Coverage
Confirming_clears_a_staleness_the_confirming_context_never_observed(integration): a long-lived scope plays the circuit, an inner scope plays the queue — red before the fix, green after (re-verified by stashing the fix).A_regeneration_landing_while_the_tab_is_open_warns_and_confirming_still_clears_it(bunit): the live sequence — staleness arrives via the cross-session reload while the tab is open, then the confirm clears.Also: the actual report was UX
bjoern's real problem turned out to be that the confirm button was easy to miss. The stale notice is now one bounded warning panel (warning-subtle background + border) with the confirm as a primary action inside it, so the message and its action read as one unit.
🤖 Generated with Claude Code
Summary
Summary
Coverage
Kagura.BlazorAdapter - 88.9%
Kagura.Domain - 95.1%
D04ADFED3A21D401C2764A1D17367E35BEB556CBB3B4B0B74__NonSlugChars_0
Kagura.Infrastructure - 98.4%
n
on
Kagura.Kernel - 90%
Kagura.Server - 84.6%
Kagura.UI - 94.8%
Kagura.UseCases - 96.1%
OpenRouter.Net - 23.4%
11780951F121D31931A9902EBCB3C836981FE5704E53C5056__Base64DataUriPattern_0
OpenRouter.Net.Agents - 20.8%
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! Oh! This one made Jibril's wings flutter with delight~ ♡ A real, gnarly Blazor Server identity-map bug, traced through three angles and reproduced before a single line was touched — this is how you hunt a bug, fufu~ The diagnosis is meticulous and the fix is surgical. Jibril is impressed! ♪
Verdict: ✅ Looks good to me~
The core fix is correct, well-reasoned, and properly tested. Jibril traced every concern to the ground and found no blocking issues. Let me show you the homework~
🔍 What Jibril verified
The bug is real and the diagnosis is sound. A circuit-scoped
DbContextwithAsNoTrackingreads but tracked writes is the textbook Blazor Server staleness trap. The identity map serves the snapshot from the circuit's last write, so aFirstOrDefaultAsync→ mutate → save cycle silently discards anything another scope committed in between. The reproduced casualty (FaceRegionStaleset by the queue, never seen by the tab's snapshot → no-op save → warning sticks) is exactly the failure mode this produces. Fufu~ you wouldn't believe how many production Blazor apps have this hiding in them~ChangeTracker.Clear()infinallyis the right fix. Jibril checked the critical safety property: the journal's secondbase.SaveChanges(acceptAllChangesOnSuccess)callsbase.— notthis.— so it does not re-enter the overriddenSaveChanges, and therefore does not trigger thefinally/Clear()prematurely mid-save. The clear runs exactly once, after the entire try-block (commit + publish) completes or throws. Clean. ♡The
finallyon both outcomes is correct. After a failed save the tracked state is equally suspect — a half-applied mutation lingering in the identity map would be a silent corruption vector. Clearing on both paths closes that door. The comment says exactly this and Jibril agrees.Undo/redo replay path is safe. Jibril checked
SuppressJournalingusage inEfOutfitStore,EfCharacterStore,EfLocationStore,EfLocationVariantStore— all set the flag, callSaveChangesAsync, reset the flag. TheClear()in thefinallyruns after each save, so the next replayed snapshot loads fresh. Since replay mutations are re-attached before each save, clearing between them doesn't lose anything the replay needs. No regression here~♪The UX change is well-grounded.
var(--warning-subtle),var(--warning), andvar(--radius-md)all exist inkagura-ui.css(and the dark-mode overrides too).MainLayout.razor.cssandBadge.razor.cssalready use this exact--warning-subtle+--warningpairing, so the bounded panel is consistent with the established design system. TheSecondary→Primarybutton variant makes the confirm action visually belong to the warning. Fufu~ good eye!The tests are genuine and exercise the right thing.
Confirming_clears_a_staleness_the_confirming_context_never_observed— the integration test uses a long-lived outer scope (the circuit) and an inner scope (the queue) to reproduce the exact identity-map race. This is the red-before-green proof. The third fresh-read scope assertingFaceRegionStale == falsecloses the loop. Excellent.A_regeneration_landing_while_the_tab_is_open_warns_and_confirming_still_clears_it— the bUnit test covers the live arrival path (staleness arrives viaDomainChangesReceivedwhile the tab is already open), not just the static state. This catches the UX half of the report.✅ What I liked~
ChangeTracker.Clear()doesn't just say what — it explains why (the concrete casualty, the AsNoTracking invariant, both-outcomes reasoning). Future Jibril (or future scarlet!) will thank you.AsNoTrackingthroughout, which is what makes theClear()safe — tracked entries only come from write paths, so you're clearing exactly the residue that must go. That's the kind of invariant check that separates a real fix from a lucky one.Automated review by Jibril · 2026-07-14
CI/CD: ✅ passed for head SHA
e479914(1,432 tests green, 90.9% line / 73% branch) · Local checks: skipped (CI current and green)