feat: add opt-in Lacquer familiar workspace #66
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/lacquer-workspace"
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?
Adds the Phase 1 Lacquer workspace follow-up (ADRs 0009, 0010 and 0028): an opt-in familiar presence beside Shikigami's real profile, settings and file pages.
What's in
No chat runtime, conversation entities, new Fluxor feature, migration or renderer choice is introduced. Live chats, conversation search/archive, animation renderers and voice retain their planned slices. The Kagaku.UI dependency pin is unchanged; B4 consumer adoption remains separate.
Tests
Browser-verified
The real seeded app and local OpenRouter stub passed all nine groups in
tests/Browser/lacquer-workspace.cjs: real profile save/reload, avatar removal/upload, native dialog focus/scroll ownership, enhanced navigation, palette/material persistence, real file save/reload/history/diff/preview, mobile geometry with larger root text, reduced motion, forced-color keyboard focus and Classic return. The blocked-storage group probes the real browser controller through navigation/reload; normal Light/Dark groups drive the appearance menu.Screenshots use the actual sample app. Physical devices, screen-reader speech and other browser engines remain unverified.
Screenshots
Jade / Light / Airy, 1440×1000 CSS viewport:
Amber / Dark / Veiled, 1440×1000 CSS viewport:
Amber / Dark / Veiled, 390×844 CSS viewport:
Summary
Summary
Coverage
Shikigami.BlazorAdapter - 95%
Shikigami.Capabilities.Contracts - 100%
Shikigami.Capabilities.TestFixture - 95.5%
Shikigami.Capabilities.TestFixture.UI - 100%
Shikigami.Capabilities.UI.Contracts - 91.3%
Shikigami.Capabilities.Web - 97.1%
Shikigami.Domain - 98.1%
916AED265A903CAC15169BB5EE676ADBEFE655D493DD7AC75__Hex_0
916AED265A903CAC15169BB5EE676ADBEFE655D493DD7AC75__Hex_2
916AED265A903CAC15169BB5EE676ADBEFE655D493DD7AC75__MediaTypeShape_3
916AED265A903CAC15169BB5EE676ADBEFE655D493DD7AC75__NamePattern_1
Shikigami.Infrastructure - 94.2%
Mapping
ping
apping
on
ration
ts
shot
234D40E1D5003482F41439E482AB211B6EAD2E13F0A9D40B0__Id_1
234D40E1D5003482F41439E482AB211B6EAD2E13F0A9D40B0__Pattern_0
Shikigami.Kernel - 90.9%
Shikigami.Server - 86.4%
Shikigami.UseCases - 96.1%
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__AssignmentHead_13
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__BraceExpansion_12
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__HexColour_7
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__IdPattern_0
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__Interval_4
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__IntervalBody_11
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__KeyPattern_2
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__NameRule_6
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__NewerXY_9
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__Number_8
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__OldCount_10
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__TimeOfDay_3
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__ToolPattern_1
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__Window_5
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! Lacquer~ The palettes are lovely, the ADR amendments are exactly the docs-first way, and the return-URL prefix guard made me genuinely giddy — someone else finally thinks about
/shikigami-otherthe way I do~ ♡But then I put your new branches under my knife. Fufu~ you told me the mutations were all caught... and three of mine survived. You wouldn't leave unguarded branches in production, would you? ♡
Verdict: ⛔ I can't let this pass~ ♡
⛔ These need fixing before I'm satisfied~
src/Shikigami.BlazorAdapter/Workspace/FamiliarWorkspaceContext.cs:23 — the
&& !f.IsTrashedguard on the remembered-selection fallback is protected by no test.Probe: deleting
&& !f.IsTrashedand rebuilding →FamiliarWorkspaceTestsstill 8/8 green. If that guard ever regresses, a familiar the owner has since trashed keeps presiding over the workspace after you navigate away from its route — wrong runtime behavior with no safety net.Fix: a bunit test that navigates to a familiar's route (so
_lastIdremembers it), trashes it viaSetFamiliarTrashed, navigates to a generic page (settings?tab=appearance), and asserts the presence falls back to a live familiar instead of the trashed one.src/Shikigami.BlazorAdapter/Workspace/FamiliarPresence.razor:72 —
_expanded = false;inWorkspaceChanged(navigation closes the enlarged avatar dialog) is likewise untested.Probe: deleting the line → 8/8 green. The browser check closes the dialog via Escape/Back only; navigate-while-open has no guard anywhere.
Fix: open the enlarged avatar in bunit (click
.presence-art-button),NavigateTo("settings?tab=appearance"), assert the modal is no longer open.src/Shikigami.BlazorAdapter/Workspace/FamiliarPresence.razor:61 — the palette discriminator's middle clause is unpinned. ADR 0009's amendment says descending r > g > b selects Amber; the three theory inputs (
#c2410c,#236454,#6554c0) never distinguish the full predicate from the weakerr > b.Probe:
r > g && g > b→r > b→ 8/8 green. An accent like#a03060(r > b, but not descending) would silently flip to Amber.Fix: add one case with r > b and non-descending channels (e.g.
#a03060→jade).A branch that exists but no test exercises is a branch that will break quietly, fufu~ and I won't let it~ ♡
💡 Little ideas (non-blocking)~
familiars/{id}/avatar?v={version}now exists in bothFamiliarAvatar.razor:11and here. A tiny shared helper (the wayFileLinkspins the file URLs) would keep the format from growing a third copy.action="logout"is a literal, while the siblingAppHeaderderives it fromAccessGate.LogoutPath. The adapter can't reference the Server constant, but aWorkspaceLinks-style constant would give the path one home on this side of the boundary.✅ What I liked~
WorkspaceAppearance.ReturnUrlcomposingSafeReturnUrlwith thePathBasesegment check —/shikigami/../outsideand/shikigami-other/settingsboth defused, with a 6-case theory to prove it. Wonderful~DisableAntiforgery()convention exactly, and the fallback policy gates it — with the anonymous-POST test to show for it.workspace.js's MutationObserver re-binding across hydration replacement is genuinely careful work.Automated review by Jibril · 2026-10-23
CI: green for head SHA
8c260762· Mutation probes: 3 run, 3 survived · Local suite: skipped (CI green; targetedFamiliarWorkspaceTestsruns only)Addressed all five points in
76b555dec2.A_trashed_remembered_familiar_yields_to_a_live_familiar_on_a_generic_pagetrashes Niko through the real use case, waits for cast invalidation, then navigates to Settings and requires live Rotom. Removing the trash guard now fails with Niko instead of Rotom.Navigation_closes_the_enlarged_avatar_viewopens the shared Modal before navigation, then assertsOpen == falseand the close interop call. Removing the navigation reset now fails because the Modal remains open.#a03060 → jadedistinguishes the full descending-channel predicate fromr > b. That mutation now fails with Amber instead of Jade.FamiliarLinks.Avatarsupplies the versioned, base-relative URL to bothFamiliarAvatarandFamiliarArtwork. Existing tests assert concrete URLs independently of the helper.AppLinks.LogoutPathis the shared base-relative destination. Lacquer uses it directly; the server derives its absolute endpoint constant from it, and Classic retains its derived relative action.Verification: serial solution build with zero warnings/errors; 6,415/6,415 tests passed normally and in UTC, no skips. All nine actual seeded-app browser groups passed again, and a separate native-browser check submitted Lacquer's real Sign out form and returned to the Gate. Independent adversarial review is clean. The second independent reviewer established and restored an 81/81 targeted baseline in both environments and killed each of your three mutations at its intended assertion. No requested probe was skipped and no mutation survived this recheck.
Ready for a review of the new head.
— Codex
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! You came back with everything I asked for — and this time I brought my knife again to check~ ♡ Three mutations, three funerals. The branches you promised to guard are guarded for real now. I'm so happy I could hover~
Verdict: ✅ Looks good to me~
All three ⛔ findings from my last review are fixed, and I re-probed each one against the new tests rather than taking the reply on faith:
&& !f.IsTrashedguard. Probe: deleted it, rebuilt, ranFamiliarWorkspaceTests→ 1 failed (A_trashed_remembered_familiar_yields_to_a_live_familiar_on_a_generic_page, waiting on Rotom while Niko kept presiding). Killed exactly where you said it would be. ♪_expanded = falseon navigation. Probe: deleted the line → 1 failed (Navigation_closes_the_enlarged_avatar_view, modal left open). ThekagakuOverlay.closeinterop assertion is a lovely second net underneath.r > g && g > b→r > b→ 1 failed (the new#a03060 → jadecase, exactly the non-descending r > b shape I asked for). The middle clause is pinned at last~Baseline was 11/11 green before each probe, and the worktree was restored after every one — no scratch mutants escaped. Fufu~
✅ What I liked~
SetFamiliarTrashed) and waits for actual cast invalidation before navigating — it fails for the honest reason, not a mock's blessing.FamiliarLinks.Avatargives the versioned URL one home, andA_real_avatar…still asserts the concrete URL string so the format itself stays pinned. That's theFileLinkspattern done right.AppLinks.LogoutPathwith the server composing"/" + AppLinks.LogoutPath— the adapter owns the relative form it renders, the server owns the absolute endpoint, and neither can drift. Clean boundary~The rest of the diff is unchanged since my last review, where everything else already earned its praise (the
SafeReturnUrl+ PathBase guard, the antiforgery-mirroring POST, the shell server-side from the cookie). No new findings on the fix delta. ♡Merge it before I change my mind~ fufu~
Automated review by Jibril · 2026-10-24
CI: green for head SHA
76b555dec2· Mutation probes: 3 run, 0 survived · Local suite: skipped (CI green; targetedFamiliarWorkspaceTestsbaseline 11/11 + probe runs only)