Skip to content

Easter Eggs Consolidation & Overhaul - #184

Merged
daniilperkin merged 72 commits into
devfrom
feature/easter-eggs-overhaul
Sep 26, 2026
Merged

daniilperkin merged 72 commits into
devfrom
feature/easter-eggs-overhaul

Conversation

@daniilperkin

@daniilperkin daniilperkin commented Aug 26, 2026 •

Copy link
Copy Markdown
Collaborator

📌 What this PR is

Two things in one branch, by design:

  1. The easter-eggs overhaul — all interactive micro-moments consolidated into one modular feature (src/features/easter-eggs/): a central registry for modal games, one accessible modal shell, a single app-wide effects layer driven by a bus, the dino waiting-game shared by chat / onboarding / buddy, discoverable triggers, prefers-reduced-motion honoured everywhere, and the legacy per-feature implementations deleted.
  2. A consolidation branch: while the overhaul was in review, every frontend PR open at the time (listed below) was merged into this branch, plus five rounds of dev backmerges. The branch's diff against dev is now 86 files, +5346 / −1075. It contains nothing that lives on another branch; the knowledge-base and tooling tweaks it used to carry moved to feat(knowledge-base): server-side pagination, URL-state filters, sort, date/language facets, bulk delete and AI status #264 (see Upgrade round), and the branch merges into feat(knowledge-base): server-side pagination, URL-state filters, sort, date/language facets, bulk delete and AI status #264 without conflicts.

🔀 What was absorbed along the way

Source What it brought
dev (5 merge rounds, last = d531e21e) industry detection, starter work, TanStack Query migration, page navigation, chat conversation rail, KB filter round, Hire Setup rework (#256), project switcher (#257), word-glue hotfix (#255), PATH_STEP board cards + checklist (#260), buddy quote-from-selection, AI skill suggestions (#261)
#250 format hotfix
#249 chat error toasts + message queue
#229 in-chat reply (quote-to-reply)
#225 chat source filter + popover redesign
#248 buddy board actions
#228 blueprint onboarding (Supersedes #217, which was skipped as fully contained)
#251 KB artifact repository display + facet (incl. review-addressing commits)
#252 buddy team mode: stored proposals, confirm-by-id, per-user persistence
#259 closed-source state on the current-task card

Conflict policy throughout: incoming (dev/PR) changes win, except where the branch itself carries newer consolidated work (the egg architecture, the sidebar's Blueprints entry beside dev's Hire Setup, the reviewed buddy-session fixes). After every round the tree is swept for the known trap — dev-side branches re-importing the pre-overhaul dino state in ChatPage — and the sweep is at zero dead markers on this head.


🥚 The easter-egg overhaul (core changes)

Architecture

  • Central registry (registry.ts): modal eggs (game-2048, space-invaders) addressed by id everywhere; each game component is React.lazy-loaded, so a game's chunk is fetched only on first open. The dino waiting-game is deliberately not a registry entry — it plays inline in the chat thread, the onboarding step and the buddy, never in the shell. Verified: it stays its own lazy chunk.
  • Unified modal shell (EggModalShell.tsx): one accessible shell (role="dialog", aria-modal, scroll lock) replaces three hand-rolled modal implementations.
  • Decoupled effects layer (eggEffectBus.ts, EggEffectsLayer.tsx, ConfettiBurst.tsx): phrase detection (eggPhrases.ts) fires app-wide effects drawn once, app-wide — barrel roll, matrix rain, confetti. EggEffectsLayer sits outside the auth gate deliberately: a fired effect always has its renderer.
  • Legacy cleanup: dino/, game2048/, space-invaders/ and the stale global shortcut hooks are deleted; README updated.

Behaviour

  • Dino waiting-game (useDinoWaitingGame.ts): Space opens it while an AI turn runs, shared by chat, onboarding generation and buddy; a shared app-wide slot guarantees one game at a time; it can outlive its turn (keepActiveUntilExit), which is exactly why both the trigger and the game's own key handlers ignore keystrokes landing in text fields — the composer regains focus the moment a turn ends, and Space/w/s/arrows/Escape keep working mid-sentence.
  • Completion claims are honest: chat shows "Reply ready" only once the stream has actually ended, onboarding only when status === "done", ingestion only when the source really reached connected — not merely "busy flipped off" (a failed sync/turn no longer promises a reply, path or sync that does not exist).
  • Discoverability: eggHint glow pulse on header icons, triple-click 2048 on the dashboard, cogwheel triple-click dino unlock in Settings, sidebar-logo physics roll, 404 rocket teaser, egg phrases in chat and buddy.
  • 2048: one Escape mechanism per press — the iframe captures it and posts a same-origin EGG_EXIT message; the frame also takes focus on load so arrows work without a click first.
  • Reduced motion: barrel roll (whole-app rotation has no honest still frame), glow pulse, hover expansion, logo drop — all skipped under prefers-reduced-motion.

🧾 Review responses this branch has been through

Pass Findings fixed
Eggs review (d1a7ffc9, 8a0335e0) 14 — the two behavioural ones: the buddy's Space trigger never fired (composer focus parity with the chat, added), and the barrel roll ignored prefers-reduced-motion. Plus the app-wide one-game-at-a-time slot, a WCAG 2.5.3 accessible-name fix on the 404 teaser, the 2048 double-Escape, and a batch of docs/hygiene items
Owner-work review (3f701f5e) A dedicated review of the 16 owner-authored commits (buddy team mode, KB repo facet, waiting-game evolution, current-task card): the dead "Try again" retry on a refused hire action, the waiting-game eating keystrokes in text fields, the three lying completion badges, a stale team-target binding across a user switch, the dock's fresh-visit control clearing a thread mid-stream, uncached repository-metadata parsing (re-parse + console.warn on every render), org-parser strictness, facet count allocations, and warn hygiene in tests
Upgrade round (ba9f7d0d…17cdf9b6) Full three-part review of the whole branch. Focus stays inside the 2048 dialog and returns to the opener; failed game chunks show an error state instead of blanking the app; matrix rain honours reduced motion; Space no longer opens the game from focused controls or with modifiers; chat/buddy composers stop pulling focus into the text box mid-game; a minimised buddy dock can no longer open a hidden game; the game closes on a hire/team switch; honest badges after Stop/failure and during an in-flight source update; failed onboarding generation shows its retry screen; one Escape closes only the game inside a drawer; the running score is no longer announced; unlock popover and confetti announce reliably; spring tokens, ui/Button and app tokens throughout; revived StarterWorkIcon removed; 404 teaser copy fixed. The knowledge-base and tooling tweaks moved to #264 (9cd5081e)
Review fact-check (061276ed) Game slot freed after commit instead of during render; Space Invaders ignores Ctrl/Cmd/Alt shortcuts and text-field typing, Escape claimed in the capture phase like DinoGame
David's review 5317855360 (b3dd474e) GenerationScreen gets the run status (isRunning) instead of guessing from phases, so "Path ready" can no longer show while the run is still persisting; key guards moved to easter-eggs/lib/keyTargets.ts; import-specifier churn reverted; registry lazy() inline for both games

🧪 Verification — head b3dd474e

Check Result
npm run build (tsc -b && vite build) ✅ 0 errors
npm run lint ✅ 0 errors, 0 warnings
npm run format:check ✅
npm run test ✅ 368 files / 3,083 tests passed
npm run a11y (vitest-axe) ✅ included in the npm run test run above
Merge probe ✅ git merge-tree against dev and against #264 (feature/KB-updates-final): no conflicts
CI (push trigger) build + gitleaks — see the checks tab on this head

Manual verification (carried from the original pass; surfaces unchanged since)

  • Triple-clicking the Dashboard header icon opens 2048; Escape closes it from both the iframe and the shell.
  • Clicking the sidebar logo 5× triggers the physics roll-off animation and resets.
  • Typing party / party time / let's party in Chat or Buddy fires confetti; do a barrel roll and matrix fire their effects via the app-level layer.
  • Triple-clicking the Settings cogwheel toggles the dino unlock; the waiting game opens on Space while the AI thinks, on chat, onboarding and buddy.
  • All animations respect prefers-reduced-motion: reduce — glow pulse, hover expansion, logo drop and barrel roll included.
  • The 404 rocket teaser opens Space Invaders.

The manual pass predates the backmerge rounds and the upgrade round. Behaviour changed since (focus contract, disabled-control and modifier rules for Space, badges, drawer Escape, the error boundary) is covered by the unit suite, not a click-through. A fresh manual pass is welcome.


📋 Definition of Done

Why this change
===============
The easter-egg games were reachable only via Ctrl+Shift+1/2/3 keyboard
chords, which we are retiring: hidden chords are undiscoverable by design,
and the product direction is that eggs should be found by users who pay
attention to details (clicking real UI elements, trying phrases in chat).
This commit is the structural groundwork for that direction; the actual
new discovery triggers land on top of it.

What was wrong structurally
===========================
- Three byte-identical chord hooks (useDinoShortcut, useGame2048Shortcut,
  useSpaceInvadersShortcut) differing only in which digit they matched.
- Two near-identical modal wrappers (DinoGameModal, SpaceInvadersModal)
  plus a third variant for the 2048 iframe.
- The dino "waiting game" logic (unlock flag + Space trigger + auto-close)
  was copy-pasted across ChatPage and OnBoardingPage (~40 lines each),
  including duplicated localStorage/dinoUnlockChanged/storage sync code.
- The chat phrase easter eggs ("do a barrel roll", matrix variants) were
  matched with inline string comparisons inside ChatPage.
- All game code was statically imported through DashboardPage, so ~1,600
  lines of canvas/iframe game shipped in the main bundle even for users
  who never open an egg.

What replaces it
================
features/easter-eggs/ now owns the shared machinery:
- registry.ts: every modal egg as { id, label, kind, component }, with all
  components lazy-loaded so each game becomes its own chunk fetched on
  first open. Adding a future egg = one registry entry + one component.
- components/EggModalShell.tsx: one Framer Motion modal for every egg.
  Canvas games render bare (they own their chrome and Escape handling via
  onExit); iframe games get a header bar + close button + window-level
  Escape (the iframe swallows keys, so the shell must listen). One
  Suspense wraps both branches because an unsuspended lazy child would
  tear down the whole tree.
- components/Game2048Frame.tsx: thin iframe wrapper so the self-contained
  vanilla-JS 2048 page fits the shared { onExit } game shape.
- hooks/useDinoWaitingGame.ts: useDinoUnlocked() (live localStorage flag
  synced via dinoUnlockChanged + storage events) and useSpaceOpensDino()
  (Space opens while armed+unlocked, typing-guarded, closes when the flag
  flips off — via React's adjust-state-during-render pattern, not an
  effect, matching ChatPage's existing style and the lint rule).
- hooks/useRepeatClicks.ts: generalized N-consecutive-clicks detector;
  first consumer is the dashboard header icon (triple-click = dino), the
  same gesture language the Settings cogwheel already uses.
- lib/eggPhrases.ts: phrase matcher extracted verbatim from ChatPage so
  behavior is unchanged there and other surfaces (buddy chat later) can
  reuse it.

Consumer changes
================
- DashboardPage: all three chords removed; triple-clicking the page-header
  icon now opens the dino runner (ungated, deliberately - finding it IS
  the unlock). 2048/invaders lose their only triggers for now; new ones
  follow in upcoming commits.
- NotFoundPage: no longer force-opens Space Invaders on load; uses the
  shared shell instead of its own modal instance. A visible teaser lands
  separately.
- ChatPage / OnBoardingPage: waiting-game logic replaced by the shared
  hooks; OnBoardingPage additionally closes the game when a regeneration
  starts (same as before) and keeps its DinoGame mount.
- useDinoEasterEgg (Settings cogwheel): rewritten onto useRepeatClicks +
  shared unlock helpers. Its dinoUnlockChanged event is now dispatched on
  a microtask because the old synchronous dispatch ran during a React
  render phase (setState-in-render hazard flagged by lint). Consumers
  listen for the event asynchronously anyway.

Verification
============
npm run build: PASS (pre-existing chunk-size warning only)
npx eslint src tests: PASS (0 problems)
npm run unit: 205 files / 1814 tests PASS
npm run a11y: 52 files / 60 tests PASS
format:check: my files clean; 5 unrelated files were already drifted on
feature/minor-upgrades (verified against committed blobs), untouched here.
Why this change
===============
Space Invaders used to force itself open the moment the 404 page loaded,
covering the actual error content before the user could read it. That is
the opposite of what an easter egg should be: an egg is a reward for
paying attention, not something thrown at you. It also contradicted its
own copy - the page invited the user to "save the galaxy" but never let
them choose to.

What changed
============
- The modal now starts closed. Arriving on 404 shows exactly the error
  content: headline, copy, Return to Dashboard.
- Below the copy sits one small teaser row, deliberately styled to blend
  in (subtle text color, small size): "While you wait for your manager's
  approval... [rocket emoji]". Clicking anywhere in the row opens Space
  Invaders via the shared EggModalShell.
- The whole row is the button so the hit target stays generous; the
  emoji is aria-hidden and the button carries an explicit accessible
  name ("Open Space Invaders") instead of reading out the ellipsis.
- Hover/focus states follow the app's standard surface-hover + focus ring
  pattern.

Tests
=====
- NotFoundPage.test.tsx: asserts the game is NOT mounted on arrival,
  that the teaser reads correctly, and that clicking it mounts the shell
  with eggId "space-invaders" (mocked shell keeps the test hermetic -
  no lazy game chunks load).
- NotFoundPage.a11y.test.tsx: mock updated from the deleted
  SpaceInvadersModal to EggModalShell for the same reason.
Why this change
===============
The easter-egg overhaul moved discovery from keyboard chords to real UI
details. But a trigger nobody notices is just a dead button, so the icons
that hide a game need the smallest possible tell - enough for an attentive
user to think "huh?" and click, not enough to stop being an easter egg.
The Settings cogwheel (dino) is the first consumer; the dashboard header
icon gets the same treatment with its own game in the next commit.

What changed
============
PageHeader gains an `eggHint` prop (default false, purely opt-in). When
set on an icon that already has `onIconClick`:

- The icon grows ~15% on hover (the button already had
  transition-transform + active:scale-95, so this rides the existing
  motion system rather than adding a new one).
- On mount it plays exactly ONE 0.5s brand-colored glow pulse via a
  drop-shadow keyframe. One-shot by construction (`1 both`, never
  re-triggered), so it reads as a flicker of "this thing is special",
  not as a notification or badge.

Implementation notes
====================
The effects are two small CSS rules keyed off classes +
data-[egg-hint="true"] in styles/index.css, next to the barrel-roll
keyframes - not Tailwind arbitrary variants - because both the hover
descendant selector and the prefers-reduced-motion override read better
as plain CSS and the file is already the home for one-off app-wide
animations.

Accessibility: the hint is decorative only. It is deliberately NOT
reflected in aria labels or roles - screen readers get the same plain
"Settings icon" button as before (asserted by a dedicated test). Users
with prefers-reduced-motion get neither the hover scale nor the pulse;
both overrides sit inside one media query next to the base rules.

subtitle became optional in PageHeaderProps (was required): ChatPage
already renders PageHeader without one and passed runtime undefined;
typing it optional makes the contract honest. No behavior change -
empty subtitle renders nothing, exactly as before.

Tests
=====
New tests/unit/components/layout/PageHeader.test.tsx covers: plain icon
stays non-clickable, onIconClick turns it into a named button, the
data-egg-hint attribute appears exactly when eggHint is set, and the
accessible name is unchanged by the hint. Existing PageHeader a11y scan,
SettingsPage tests and the full unit/a11y suites pass unchanged.
Why this change
===============
When the chords died, the dashboard's triple-click-the-header-icon
trigger was parked on the dino runner as a placeholder. The plan assigns
2048 to that spot: the dino already has two discovery homes (the Settings
cogwheel unlock plus Space during AI thinking / onboarding generation),
while 2048 had none left after losing Ctrl+Shift+2. The chart-column icon
over a grid of widgets also rhymes nicely with a game about merging tiles
in a grid.

What changed
============
Same gesture, same target grammar ("header icons hide games"), different
game: three quick clicks on the dashboard page-header icon now open 2048
in the shared EggModalShell instead of the dino modal. Still ungated -
finding it is the fun, no localStorage flag involved. The empty-board /
edit-mode concern does not apply: the header icon exists on the dashboard
in every state, unlike the edit-mode-only Add-widget button considered
earlier.

Dino therefore has no chord-free dashboard entry anymore - intentional;
its homes are the cogwheel and the waiting screens.

Verification
============
npm run build PASS, npx eslint src/tests PASS (0 problems),
npm run unit 206 files / 1819 tests PASS, npm run a11y 52 files / 60
tests PASS. Dashboard/PmDashboard/PageHeader suites re-run individually.
The AI chat already offers the dino runner (press Space while the AI
thinks, when unlocked) and two phrase easter eggs ("do a barrel roll",
"the matrix"). They stayed stuck on the AI chat: typing a phrase to the
*buddy* did nothing, and while the buddy churned there was never another
game to click — so the two chats drifted apart, the exact inconsistency
the BuddyProvider exists to prevent (dock vs. /buddy page sharing one
conversation).

What changed
------------
1. App-level effect layer. The barrel roll and Matrix rain used to be
   local useState inside ChatPage. They now play through a shared
   `eggEffectBus` (playEggEffect / useActiveEggEffect / clearEggEffect)
   and render from one EggEffectsLayer mounted once in App.tsx — so any
   surface, not just the AI chat, can fire them. ChatPage was migrated
   onto the bus (~35 lines of local state/effect deleted; phrase matching
   routes through matchEggPhrase + playEggEffect instead of calling
   setIsBarrelRolling/setIsMatrixActive). ChatPage keeps its MatrixRain
   import deleted; the layer owns it now.

2. Buddy waiting-game. useBuddyConversation now calls useSpaceOpensDino
   with (isThinking || isStreaming, dinoUnlocked) — Space opens the
   runner while the buddy works, and it auto-closes when the answer
   arrives (mirrors ChatPage's render-adjust close). The props flow up
   through its return value and down through BuddyThread → Buddydock/
   BuddyConversation → BuddyWidget → BuddyDock, so both the dock and
   the /buddy page show the running game inline under the typing
   indicator.

3. Buddy phrase interception. useBuddyConversation.handleSubmit checks
   matchEggPhrase(draft) BEFORE sending: a match clears the composer,
   fires playEggEffect, and swallows the message — no request, no reply.
   Same silent contract as the AI chat, so "do a barrel roll" / "the
   matrix" behave identically in both chats.

4. Tests. New eggEffectBus.test.tsx (4 cases: idle→barrel, barrel
   timeout+cleanup, matrix mount/unmount, clear on unmount) and
   buddyEasterEggs.test.tsx (phrase swallowed silently vs. normal send
   reaching the wire, dino Space trigger while thinking). useDinoUnlock
   coverage moved into the bus test's storage-event case.

Design notes
------------
- EggEffectsLayer is mounted outside `showSidebar` (i.e. always, even on
  /login) because a fired effect must always have its renderer; the
  barrel-roll body class is harmless on login, and matrix rain on a
  login screen is the kind of secret that rewards attention.
- The waiting-game uses React's documented "adjust state when a value
  changes" pattern (guarded setState during render) rather than
  set-state-in-effect, matching ChatPage's existing approach — keeps the
  close-on-answer synchronous with the render that knows the turn ended.
- dinoGameActive/closeDinoGame are optional on BuddyDock (defaults false /
  no-op) so the existing dock test mocks, which don't pass them, keep
  compiling and green.

Tests: unit 208/1825, a11y 52/60, eslint 0 problems, tsc clean.
Why this change
===============
The chord retirement left two kinds of dead weight behind. The moments
feature still carried its own five development-only chords
(Ctrl+Shift+4..8) — a tuning aid, marked "remove before merging" in its
own header, whose reason to exist (replaying progress-gated moments)
has nothing to do with shipping. And the docs/comments still described
a world that no longer exists.

What is removed
===============
- `useMomentDevShortcuts.ts` (whole file) and its call in
  `MomentsProvider`. The hook was DEV-only (`import.meta.env.DEV`
  compiled it out of production), so this changes nothing for users;
  it just deletes the tuning scaffolding before the branch lands.
- The `EggEffects = Record<EggEffectId, ReactNode>` type from
  eggPhrases.ts — a leftover from the pre-bus design where each page
  rendered its own effects. The bus replaced it; nothing imported it.
  eggPhrases.ts now re-exports `EggEffectId` from the bus instead of
  declaring a second copy of the union.
- A stale comment block in SpaceInvaders.tsx pointing new readers at
  the deleted SpaceInvadersModal and the retired Ctrl+Shift+3 chord.

Verification
============
npm run build (tsc -b + vite), eslint src tests, prettier --check on
touched dirs, unit suites for easter-eggs/buddy/moments/pages (373
tests) all pass.
Why this change
===============
The phrase easter eggs had two entries (barrel roll, matrix) and the
bus had exactly one renderer per id — adding a third effect is the
proof that the EggEffectsLayer design scales without new plumbing.
"Party" was picked over more game-adjacent ideas because it needs no
new surface, no persistence, and no trigger UI: someone types "party"
into the AI or buddy composer and the app celebrates. It is also the
first alias set that includes an emoji ("🎉"), which is deliberately
the most guessable trigger in the whole system — no language barrier,
and typing one character into a chat box is a very low-risk experiment
for whoever suspects there is something hidden.

Trigger
-------
matchEggPhrase gains ["party", "party time", "let's party", "🎉"] →
"party". Matching stays exact after trim/lowercase, so "partying" and
"a party for two" remain normal messages. The union type on
EggEffectId makes the compiler force the new branch into
EggEffectsLayer — no silent fall-through possible.

ConfettiBurst
-------------
Same architecture contract as MatrixRain: all mutable particle state
lives in local variables owned by one effect, a single delta-time-scaled
rAF loop renders it, DPR-aware canvas, cleanup cancels everything.

- Two cannons (bottom-left/bottom-right) fire up-and-inward, 75
  particles each, staggered across the first 150ms so the burst reads
  as two pops rather than one wall.
- Particles are rects and circles with per-particle spin, gravity
  (900 px/s²), air drag, and a sinusoidal horizontal flutter whose
  phase advances faster while falling — that flutter is what makes it
  read as paper instead of ballistics.
- Colors come from the computed CSS custom properties of the theme
  (brand plus the extended accent hues), read once at spawn: confetti
  automatically matches light/dark mode and any future palette change.
  A hardcoded five-color fallback covers SSR/test environments where
  getComputedStyle resolves nothing.
- When every particle has died, the canvas fades out (~250ms) and the
  effect clears itself off the bus; the layer keys ConfettiBurst by
  seq, so re-triggering mid-burst replays cleanly instead of being
  swallowed by equal-state bailing.

Reduced motion
--------------
Particle animation IS the effect, so there is no honest way to keep
it: under prefers-reduced-motion the component renders a static
"🎉 Party!" status chip that fades via opacity only and clears the
bus after 1.5s. Nobody who types "party" ends up staring at nothing.

Tests
-----
- eggPhrases.test.ts: all four aliases match, plus trim/case
  invariance; "partying" and substring cases stay null.
- ConfettiBurst.test.tsx (via EggEffectsLayer): canvas mounts
  pointer-events-none + aria-hidden; the full lifecycle
  (spawn → simulate → fade → bus clear) completes under fake timers
  using a stubbed 2D context and faked rAF/performance (jsdom has no
  canvas, so without the stub the loop would exit before doing
  anything); the reduced-motion chip renders instead of the canvas
  and also clears its bus effect when its timer runs out.

Verification
------------
build ✅ · eslint src tests ✅ 0 problems · prettier (touched files)
✅ · unit 209 files / 1836 tests ✅ · a11y 52 files / 60 tests ✅
Why this change
===============
The easter-egg overhaul replaced keyboard chords with real UI details a
user can stumble on. The strongest remaining surface for that grammar is
the app logo itself: it already reacts to hover (badge lift, rocket nudge,
flames light up), so it reads as interactive — but clicking it did
nothing. That gap between "looks alive" and "is clickable" is exactly
where an egg belongs, and repeated clicks on a decorative element is the
same discovery gesture as the cogwheel and dashboard-icon triggers.

Research note: the logo only *looks* clickable — it is decorative markup
at all three usage sites (desktop sidebar header, mobile top bar,
Keycloak login template). Per the agreed approach it stays decorative:
plain onClick plus cursor-pointer, no button, no tab stop. Rationale:
the egg is pure whimsy with zero function, and adding a focusable
element to the login page's keyboard path (of all places) would make
navigation worse for every screen-keyboard user to serve a secret.

The animation
-------------
A four-phase gravity sequence on the outer badge div, driven by framer-
motion's onAnimationComplete rather than hand-tuned delays, so each beat
starts exactly when the previous one lands:

1. falling  (~0.45s, easeIn): y +140px with a slight forward tumble —
   gravity accelerates, it does not slide.
2. impact   (~90ms): sideways squash (scaleX 1.14 / scaleY 0.8), the
   classic cartoon beat.
3. bouncing (~0.62s): two diminishing bounces via keyframed arrays
   (34% then 16% rebound heights) with re-squashes that shrink per
   landing.
4. hopping  (spring, stiffness 260 / damping 11): back to the shelf
   with a small overshoot while rotation settles to 0.

The fixed 140px translateY keeps the effect layout-independent — parents
are overflow-visible, so the badge visually escapes its header row
without portals or measured rects.

Deliberately NOT routed through the egg effect bus: barrel roll and
matrix are whole-window effects owned by EggEffectsLayer; this one is
scoped to a single element that three different layouts place
differently. Element-scoped animation, element-owned state.

Interplay details
-----------------
- The rocket's permanent idle float lives on the inner <svg>, the drop
  on the outer div — separate DOM nodes, transforms compose.
- whileHover="lift" is suppressed while any phase is playing so hover
  never fights the drop for the same transform; it re-arms at idle.
- Clicks during a run are absorbed by the phase gate (only idle arms a
  new drop); useRepeatClicks resets after firing either way.
- prefers-reduced-motion skips the sequence entirely — pure motion has
  no honest static equivalent — though the click counter still consumes.
- One targeted behavior kept from review: DROP_PHASES is passed as the
  animate target object directly (not by variant name), because the
  phases are not part of the badge's rest/lift variants family.

Tests
-----
SidebarLogo.test.tsx asserts the trigger contract under a motion-stripping
proxy: the logo remains decorative markup (no button role) with
cursor-pointer; four clicks leave it untransformed; five rapid clicks do
not throw even under reduced motion. Pixel choreography is not asserted
(jsdom has no animation loop — same boundary as the game tests).

Verification
------------
build ✅ · eslint src tests ✅ 0 problems · prettier (touched files)
✅ · unit 210 files / 1839 tests ✅ · a11y 52 files / 60 tests ✅
…ose for 2048

- Enable eggHint on DashboardPage PageHeader so the ChartColumn icon triggers the opening glow pulse and scales on hover
- Enable Escape key close handling for 2048 game modal via postMessage from 2048.html and keydown listeners in Game2048Frame
- Add unit test coverage in DashboardPage.test.tsx and Game2048Frame.test.tsx
- Apply Prettier formatting across codebase
@daniilperkin
daniilperkin marked this pull request as draft August 29, 2026 12:44
@daniilperkin

Copy link
Copy Markdown
Collaborator Author

should be merged after confluence, so undraft after it also

@DavidLeuter

Copy link
Copy Markdown
Collaborator

Draft, so no approval — but I went through the branch's own commits (3b56ebf…a90a45f plus 5eda917), i.e. the easter-egg work without the merged-in #183/#185/#182 content.

The consolidation is genuinely good: one EggModalShell in place of the near-identical DinoGameModal / SpaceInvadersModal / Game2048Modal trio, a registry that keeps every game out of the main bundle behind lazy(), and an effect bus via useSyncExternalStore with a seq so re-firing the same effect actually replays it. Reduced motion is handled in both the CSS (@media (prefers-reduced-motion: reduce) kills the hover scale and the glow) and in JS (ConfettiBurst renders a static chip instead of particles). No objections to the direction at all. A few things:

The diff against dev isn't reviewable in isolation

101 files, because the branch merges in feature/minor-upgrades (#183) and feature/269-confluence-connector (#185, which itself carries #182). The easter-egg work is ~49 files.

Retarget the PR base to feature/269-confluence-connector and GitHub will show only this branch's own work; flip it back to dev once the stack lands. (Same note on #185.)

EggModalShell: hand-rolled close button

<button type="button" onClick={onClose} aria-label={`Close ${egg.label}`}
  className="flex h-8 w-8 items-center justify-center rounded-lg text-app-text-muted transition-colors hover:bg-app-surface-hover hover:text-app-text focus-visible:ring-2 …">

That's ui/Button variant="ghost" size="sm" iconOnly spelled out by hand — including a re-derived ghost hover and focus ring. #183 in the same stack is migrating exactly this pattern in the escalation inbox, and Button's own TSDoc says every <button> carrying an action should be it. Worth converting here so the new shared shell doesn't reintroduce the drift.

EggModalShell: dead id on the title

The header renders <h2 id={${eggId}-title}>, but the dialog is labelled with aria-label={egg.label}, so nothing references that id. Either switch to aria-labelledby={${eggId}-title} for the iframe branch (canvas games have no header, so they'd keep aria-label), or drop the id.

index.css: hardcoded colour in the glow keyframe

0%, 100% { filter: drop-shadow(0 0 0 rgba(37, 99, 235, 0)); }
50%      { filter: drop-shadow(0 0 6px var(--brand-border-strong)); }

AGENTS §7 is explicit about never hardcoding colours, and the 50% stop already does it right. At α=0 nothing renders, so transparent gets the identical result without the literal.

Smaller

  • Suspense fallback h-64 w-[680px] — arbitrary pixel width (AGENTS §7 asks for the shared scale), and it won't match the iframe game's actual box, so first open will jump. Minor, but a token-scale size or min-h/min-w on the dialog would be steadier.
  • useRepeatClicks has no reset window: five clicks spread over ten minutes still trip the egg, since countRef only resets on success. Matches the old cogwheel behaviour so probably intentional — just flagging, since the doc says "consecutive".
  • 5eda917 mixes "Apply Prettier formatting across codebase" into a feature commit (CreateProjectWizard, AddSourceModal, JiraCredentialAddForm). Harmless, but it puts unrelated files in the diff — a separate formatting commit is easier to skip past in review.

Ping me when it's out of draft and I'll do a proper pass on the games themselves.

Reviewed with Claude Code.

@daniilperkin
daniilperkin changed the base branch from dev to feature/269-confluence-connector August 31, 2026 13:37
@daniilperkin

Copy link
Copy Markdown
Collaborator Author

Claude is right, i will point this PR into feature/269-confluence-connector

Base automatically changed from feature/269-confluence-connector to dev September 8, 2026 10:53
Brings 141 commits of dev — industry detection, starter-work, the TanStack
Query migration, smooth page navigation, the chat conversation rail — together
with the easter-egg overhaul (one shared egg module behind a bus, the waiting
games, the 404 rocket teaser).

Eight files conflicted. Resolved as follows.

App.tsx — kept both sides. dev renamed the shell's auth gate from `showSidebar`
to `signedIn`, so the rocket pet reads that now. `EggEffectsLayer` deliberately
sits outside that gate: a fired effect must always have its renderer, whoever
is looking. The comment above the pet points at `AppearanceSection`, since the
branch's `MomentsSection` no longer exists.

DashboardPage.tsx — took the egg shell (`EggModalShell` + `useRepeatClicks`)
over dev's three per-game modals and their Ctrl+Shift chords: this branch
deleted `src/features/{game2048,dino,space-invaders}` and moved the games into
`features/easter-eggs`, so six imports had nowhere to point. dev's
`WidgetPickerModal` replaces `AddWidgetModal` (the JSX already said so).

ChatPage.tsx — kept dev's conversation rail, `surfaceFromPathname` and
`useNewConversationShortcut`; dropped dev's `MatrixRain` import, because matrix
rain is now fired through the egg bus and drawn once, app-wide, by
`EggEffectsLayer`. dev's inline barrel-roll / matrix / dino wiring in this file
is superseded by `playEggEffect` plus `useSpaceOpensDino` / `useDinoUnlocked`.

OnBoardingPage.tsx — dev wrapped the generator in `useCallback` and
invalidated the onboarding status on both the success and the recovery path;
this branch had replaced the inline dino state with `useSpaceOpensDino`. Both
kept: `closeGame()` closes the waiting-game, `invalidateMyOnboardingStatus()`
still runs in both places (that was the stale-status bug dev fixed), and
`closeGame` joins the dependency array — it is a stable `useCallback`, and
leaving it out left an `exhaustive-deps` warning in a repo that runs at zero
warnings. The branch's `eslint-disable` for that array is gone with it.

Buddy chain (BuddyThread / BuddyConversation / BuddyDock / BuddyWidget) — both
features thread props through the same four components, so each now forwards
both concerns: the dino waiting-game pair and the visit divider's
clear-previous action. The dock forwards the clear action without naming a
keyboard chord, and that is deliberate: over a page like /chat that chord
starts a chat, not a visit.

Verified before committing: `tsc -b` silent, `eslint .` 0 errors / 0 warnings,
2328 unit tests, 63 a11y tests, production build green. The five files still
failing `prettier --check` are dev-side drift this branch never touched and are
left alone on purpose.
@daniilperkin
daniilperkin marked this pull request as ready for review September 15, 2026 19:52
…ne game at a time

Review findings on the consolidation branch, fixed in one commit. Each was
reproduced against the branch head (46ff21a) before being changed, and the
reasoning is repeated in the code comments where a future reader will look.

Buddy: Space can open the waiting-game at last
- The chat blurs its composer on submit precisely so the Space trigger can
  fire (see ChatPage's focus effect), but the buddy composer never gave the
  caret up: after Enter the textarea still held focus, and the trigger's
  "don't eat the space bar while somebody is typing" guard refused every
  press. The waiting-game this branch brought to the buddy was therefore only
  reachable after clicking away from the box.
- BuddyComposer now blurs on submit and takes the caret back when the turn
  ends — the same two-way dance the chat does — and only when *this* box gave
  the caret up in the first place. The dock auto-focuses its composer and is
  mounted on every page, so an unconditional refocus would steal the caret
  out of whatever the hire was actually typing in.
- "Busy" is isThinking || isStreaming, matching the chat's notion of a live
  turn. BuddyConversation takes a new isStreaming prop (the page and the dock
  already had the flag) instead of folding streaming into isThinking, which
  would change what the thread draws.

Barrel roll stops spinning for prefers-reduced-motion
- .barrel-roll-active had no reduced-motion guard, so a full-screen 360°
  rotation of the whole app played for users who explicitly asked for less
  motion — while everything else this branch touched respects it (the
  confetti falls back to a static chip, the logo's drop is skipped outright,
  the header hint's glow is disabled).
- The layer skips the class and clears the effect immediately: the spin has
  no honest still frame, the same call the logo's drop makes ("the counter
  still consumes, nothing plays"). Typing the phrase still works; there is
  simply nothing to see.
- MatrixRain is deliberately left alone: it is a dismissible overlay the
  player asked for, it is pre-existing behaviour and this branch did not
  change it. Left as a residual rather than silently rewritten.

One waiting-game at a time, app-wide
- Three surfaces arm the trigger against their own turn and all listen on
  the window, so a single Space press with two of them busy opened two dino
  games at once (chat streaming with the dock open mid-answer). The first
  host to claim the shared slot plays; the rest stay shut.
- The slot is released on close, on the unlock toggle and on unmount, so a
  page that goes away mid-game cannot leave every other surface unarmed.
- A module-level symbol rather than context: the hosts are a page, a floating
  dock and a wizard step, with no component above all three to own it.

404 teaser: the accessible name now contains the visible words
- aria-label="Open Space Invaders" replaced the visible sentence, so the
  button's name was not in its text (WCAG 2.5.3). The hint moved into an
  sr-only suffix inside the name instead, and the test asserts the contract
  rather than the mismatch.

2048 frame: one mechanism per Escape, same-origin only
- The frame attached the same Escape handler to both the frame's window and
  its document, so one press called onExit twice — and the frame's own page
  already reports Escape through EGG_EXIT, which made the injected listener
  redundant as well as doubled. What is left is the focus on load, so arrow
  keys work without a click first, plus that message path.
- The message listener now insists on event.origin === window.location.origin.
  The game page is ours, so a message from anywhere else can only be some
  other frame closing a modal that is not its business.

Hygiene and docs (no behaviour change)
- registry: iframeSrc was dead config (the wrapper hardcodes the one path it
  has) and the `dino` entry had no call site left — the waiting-game is
  played inline in chat, onboarding and buddy, never in the shell. EggId is
  now the two eggs that actually open as modals.
- EggModalShell: dropped the unused id on the iframe header (the dialog is
  named by aria-label) and made the close button's testid egg-relative
  instead of naming 2048 inside a shared shell.
- PageHeader's icon button is type="button"; it defaulted to submit.
- ConfettiBurst states why three components now share its name.
- Stale references are gone from SpaceInvaders (the Ctrl+Shift+3 chord and
  the deleted SpaceInvadersModal), MatrixRain (ChatPage's old inline arrow)
  and the README, whose feature list and structure tree still named dino/,
  game2048/ and space-invaders/ after this branch deleted them.
- The Keycloak login copy of the logo now says what it is: the same mark,
  deliberately without the gravity egg, still synced by hand.

Tests: Game2048Frame covers both message directions (our own origin closes,
a foreign one is ignored); EggModalShell covers the two eggs that exist.
npm run format:check fails on five files that predate this branch:
ProjectIndustryPanel, useStarterWorkReview, projectService,
ProjectIndustryWidget.test and starterWorkService.test. Formatting only —
npx prettier --write, nothing else — and deliberately kept out of the
easter-egg review commit so the behaviour change and the churn do not hide
each other in blame.

CI never ran format:check (ci.yaml runs lint, build, unit, a11y and the
docker build), which is how the drift shipped in the first place. These five
were the whole of it on this branch, so the check is green from here.
The shared slot behind the Space-to-play dino game was released on host
unmount and when the unlock flag flipped off, but never when the wait
itself ended. The game's DOM belongs to the host's waiting state, so it
disappears on its own the moment the answer arrives — leaving
`gameActive` true kept the slot claimed by a game nobody could see, and
the next surface to arm found the trigger dead.

The onboarding generation step is where that bites hardest: it arms on
`loadingState === "generating"`, has no close of its own, and stays
mounted after the path arrives, so one finished generation ate Space for
the rest of the visit — chat, buddy and onboarding alike. Chat and buddy
had each grown a copy-pasted "close when busy flips" block to work around
it; both are gone now that the hook owns the close, which is what the
buddy hook's own comment already claimed it did.

The close is a render-phase adjustment next to the existing unlock flip,
so there is no extra effect, no cascading render, and no host that has to
watch `armed` itself.

A second armed host can open its game once the first one's wait has ended
— a test that fails without the release.
The bus has carried a `seq` since it was written, and ConfettiBurst was
keyed by it — but neither of the other two ids ever saw it. `MatrixRain`
was rendered without a key, and the barrel-roll effect listed only
`effect?.id` in its dependencies, so firing the same phrase again while
its effect was running handed the second trigger the first one's
deadline: the rain ended on the first instance's 6s timer, a second
barrel roll was cut short by the first 2s timer. The layer's own
documentation promises the opposite ("re-triggering replays the effect"),
and the two re-fireable ids were precisely the ones that did not.

MatrixRain is keyed by `seq` now and the barrel-roll effect depends on
it, so a re-fire restarts the clock. The body class itself stays
idempotent: cleanup drops it, the effect puts it straight back.

ConfettiBurst's fade timer is ref-held and cleared on unmount, for the
same class of reason. It fires into the bus rather than into the
component, so the cleanup that looked complete — rAF, both spawn timers,
the resize listener — still let a burst that was remounted (or replaced
by another effect) clear the *new* effect a few hundred ms in.

Three tests, one per re-fire path, each fake-timer driven and each
failing without its fix.
Reviewed the 16 owner-authored commits since the last push (buddy team
mode, the GitHub repository facet, the dino waiting game evolution and
the closed-task card) with three parallel read-only reviews, verified
every finding against the sources, and fixed what was real. BabuPlk's
three commits (arriving via the #248 re-merge) were excluded as other
people's work.

Buddy team mode (major + minors):

- useBuddyConversation: a refused hire action's "Try again" button was
  dead. The card renders it only for a resolved refusal
  (!action.ok && "action" in action), but confirmAction's guard let
  only idle/error through - every click hit the guard and returned.
  The component-level test stubbed onConfirm, so it passed while the
  hook refused the retry. Now a resolved refusal that still carries
  its action passes the guard; a permanent refusal (no action payload)
  and everything confirming/confirmed/declined stays spent. The
  per-card flight lock already cleared in finally, so a retry runs.
- useBuddyConversation: teamTargetRef survived a signed-in-user
  change. The userId effect set team mode directly, bypassing
  setTeamMode - the only place that cleared the target - so the next
  user's adopt effect compared their selection against the previous
  user's binding and overwrote the restored preference with a bogus
  exit. The ref now lives above the effect and is cleared on every
  subject change.
- useBuddyConversation: the dock's "Clear the earlier conversation"
  control is handed to BuddyThread unconditionally, and
  startFreshVisit's guard checked greeting/decisions but not a live
  reply - clicking it mid-stream silently dropped the tokens,
  citations and proposed actions still arriving. The guard now also
  refuses while isThinking/isStreaming (one-line fix at the hook; the
  durable stream-abort redesign stays a separate piece of work).
- BuddyWidget: stale comment claimed the mode switcher still carries
  the restore audit; it moved into the session - the comment now
  points at the session.

Dino waiting game (major + minors):

- DinoGame: the window keydown/keyup handlers had no typing-target
  guard, and the game now outlives its turn - exactly when the
  composer regains focus. Space/w/s/arrows were prevented and eaten
  mid-sentence, and a stray Escape closed the game while typing. Both
  handlers now bail when the event lands in a text field, reusing the
  same isTypingTarget guard the trigger applies (now exported,
  widened to EventTarget for event.target).
- The completion badges claimed success from "not busy anymore":
  - ChatPage promised "Reply ready" while tokens were still streaming
    (isThinking flips false at the first token) and on a failed or
    stopped turn; now it waits for the stream to actually end. A
    provider-level clean-end flag would fix the error/stop labels
    too - left as a follow-up, failures surface as toasts so there is
    no state to read from the page today.
  - OnBoardingPage promised "Path ready" for a failed generation
    (status error also leaves running); now only status done claims
    it.
  - SourceDetailsPanel promised "Sync complete" when a sync failed
    into attention or was disabled; now only state connected claims
    it.
- ThinkingIndicator: the suppression comment claimed the
  ReasoningPanel displays the thinking state - it does not render
  thinkingState, so a post-reasoning tool label has no surface. The
  comment now documents the accepted gap instead of a fiction.

Knowledge base (minors):

- githubMetadata: getArtifactRepository re-parsed and re-warned the
  raw metadata JSON on every card render, drawer render and facet
  memo evaluation - the WeakMap cache only covered
  matchesRepository. The parse now runs once per artifact object for
  every reader (compute core + cache wrapper), which also caches the
  org-login lookup.
- orgMetadata: parseOrgMetadata accepted a blank-string login
  (unlike the repo parser's blank rejection) and returned it
  untrimmed, silently missing the case-folded owner-half match
  against a trimmed repositoryFullName. It now rejects blank logins
  and trims like the repo parser does.
- useKnowledgeBase: the facet count allocated a Set inside the
  per-candidate filter - O(repos x candidates) throwaway allocations
  per evaluation; now one per offered repository.
- githubMetadata.test: the matchesRepository tests leaked real
  console.warns for the malformed-metadata case, masking unexpected
  warnings; silenced like the parse tests, asserting the warn fired.

Verified: npm run build, npm run lint, npm run format:check green;
unit suite 2911/2911 across 353 files. Not pushed, per instruction.
…PATH_STEP board, buddy quotes, AI skill suggestions)
roll, dialog focus contract, and a truthful confetti palette

Fact-checked pass over the six strict-review findings, keeping the ones
that survived verification and refuting the rest:

- ConfettiBurst: the fifth palette candidate read a variable that does not
  exist (--color-app-yellow-text); the real token is
  --color-app-highlight-yellow, so the burst now paints all five app hues
  instead of silently running on four. The claim that the whole reader was
  dead is refuted: the --color-app-* tokens are defined via @theme inline
  in styles/index.css, so getComputedStyle resolves them. The inert
  animate-fade-in / shadow-card classes (defined nowhere) are dropped.
- Buddy waiting game: the one surface where the game died at the first
  token. The hook now arms useSpaceOpensDino with keepActiveUntilExit like
  the chat and the drawer; the thread keeps the typing bubble mounted while
  the game is open even after thinking ends; isStreaming flows from both
  callers (dock and full page) into the thread, and replyReady is only
  true once thinking AND streaming are done — so the dots row stops lying
  once the reply has arrived and the game's completion badge takes over.
- EggEffectsLayer: re-triggering the barrel roll mid-run removed and
  re-added the class inside one commit, which the engine answers with a
  shrug — the animation kept its already-finished state. A forced style
  recalc between cleanup and re-add makes the second roll actually roll
  from the top.
- EggModalShell: focus contract without fighting the games — focus lands
  on the dialog on open (tabIndex={-1}, so it joins no Tab order) and
  returns to the opener on close; Tab wraps inside the dialog only. Tab
  is deliberately the only trapped key: the canvas games own
  arrows/w/s/space, and keydowns inside the 2048 iframe never reach this
  window, so neither surface can collide with the trap.
- OnBoardingPage: the queueMicrotask around the last-generation snapshot
  stays — fact-checking showed it looks redundant but exists to satisfy
  react-hooks/set-state-in-effect, which errors on the direct setState the
  reviewer suggested. A comment now records that so the next reader does
  not "simplify" it away.

Refuted by verification, left unchanged: the useRepeatClicks window
finding — the inter-click-gap semantics are the documented design ("the
guard only has to rule out accumulation across a session"), and the
proposed total-window reading would tighten a deliberate gesture.

Verified: npm run build, npm run lint, npm run format:check green; unit
suite 3017/3017 across 360 files.
daniilperkin added a commit that referenced this pull request Sep 24, 2026
…ogins

Three small hardening changes to the GitHub metadata readers, plus a
format-check exclude for local worktrees. They were first written on
feature/easter-eggs-overhaul (#184) against the KB code already on dev,
where they had no business living: two of the touched files overlap
this branch and made #184 and #264 conflict whichever landed second.
They now live here, next to the code they harden, and #184 drops them.

- githubMetadata: getArtifactRepository now reads through the same
  per-artifact WeakMap cache as the org-login lookup. The card list, the
  drawer and the facet pass all call it on every render, and each call
  re-ran JSON.parse (and re-logged the warning for a malformed payload)
  for metadata that never changes for a given artifact object. The
  uncached core is one private computeRepositoryInfo both public
  readers share, so the two can no longer disagree.
- orgMetadata: a blank login is rejected and a padded one is trimmed.
  The owner-half repository match compares against a trimmed
  `owner/repo` string, so "  org " silently never matched its own
  repositories. This is the same normalisation the repo parser already
  applies to repositoryFullName. A new test pins both cases.
- githubMetadata test: the malformed-blob case now silences and asserts
  the expected parser warning, so real warnings stay visible in the
  test output.
- .prettierignore: skip .worktrees. eslint and vitest on this branch
  already exclude it, but format:check still walked every per-task
  worktree copy and flagged files formatted against other branches.

Not ported: #184's facet-count Set hoisting in useKnowledgeBase. This
branch moved facet counting server-side, so that loop no longer exists.
The vite/eslint worktree excludes are already here in this branch's
own form.

Gates: build, lint (0 warnings), format:check clean; unit 362 files /
3131 tests.


This branch carried seven files that have nothing to do with easter
eggs: hardening in the knowledge-base metadata readers, a Set hoist in
useKnowledgeBase, and .worktrees excludes for vite, eslint and
prettier. They were review fixes to code already on dev, parked here
while this branch was the only open one. Now #264 is open and rewrites
the same KB code: merge-tree showed useKnowledgeBase.ts (about 240
lines) and vite.config.ts conflicting whichever PR landed second.

All seven now match origin/dev again:
- githubMetadata/orgMetadata hardening + its test: moved to #264
  (9cd5081) with a new trim test the original lacked.
- useKnowledgeBase Set hoist: dropped. #264 moved facet counting
  server-side, so the loop it optimised no longer exists.
- vite/eslint .worktrees excludes: #264 carries its own versions. This
  also drops the duplicate `vitest/config` import the vite change left.
- .prettierignore: moved to #264 with the KB commit.

No worktree nests inside this branch's own checkout, so its gates are
unaffected.
…ailed chunks

The modal shell promised a focus contract it could not keep, and the
lazy game chunks could take the whole app down.

- Focus trap: keydowns inside the 2048 iframe never reach the parent,
  so Tab from its last link walked out of the aria-modal dialog onto
  the page behind it (WCAG 2.4.3). A document focusin guard now pulls
  focus back to the dialog's first or last control, depending on the
  direction it left.
- Focus restore: on a cached reopen the iframe focused itself (a fresh
  about:blank already reads readyState "complete") before the shell's
  effect ran, so the shell recorded the iframe as the opener and
  restored focus to a removed node. The shell now captures the opener
  in useLayoutEffect, drops the guard before restoring, and restores
  only an opener that is still connected. Game2048Frame focuses on the
  iframe load event only.
- EggErrorBoundary: a game chunk that fails (stale tab after a deploy,
  offline) used to unmount the root, since the app has no boundary.
  The shell now shows an EmptyState "Couldn't load <game>" with a
  working close. The 2048 header renders outside Suspense so it can
  always be closed.
- Reduced motion: MatrixRain ignored the preference the effects layer
  documents. It now shows a static chip like confetti, via the new
  shared ReducedMotionEffectChip.
- Space trigger: isInteractiveTarget exempts buttons, selects, links
  and ARIA widgets, so Space presses the focused control instead of
  opening the dino. Modifier keys, auto-repeat and already-handled
  presses are also ignored. isTypingTarget keeps its text-field-only
  meaning for the game's own key filter. Unlock reads are guarded
  against a throwing localStorage.
- useRepeatClicks: the window now spans the whole gesture, not each
  gap, which is what the docs said (3 clicks in 3 s, not 3 s per gap).
- Confetti: its status now mounts empty and fills a tick later so
  screen readers announce it. The pale highlight yellow is dropped;
  the hex fallback is kept and documented as jsdom-only.
- Hardening: ui/Button close control. 2048 messages must come from
  our iframe; the page posts to location.origin, not "*". Phrases
  accept curly apostrophes (iOS "let’s party").

Tests: focus contract (open, restore, removed opener, Tab wrap, focus
pulled back), failing chunks, two armed hosts on one Space, focused
controls keep Space, load-only frame focus, foreign messages, matrix
reduced motion, curly apostrophes, gesture window.
The unlock popover's live region was mounted together with its text
inside AnimatePresence, so screen readers often skipped the "unlocked"
announcement entirely. The emoji's aria-label also repeated the
"shh…" copy, so it was read twice.

- The status region is now always mounted; only the bubble animates.
  The emoji is aria-hidden.
- useDinoEasterEgg returns an explicit kind ("unlocked" | "locked").
  The popover used to pick lock vs unlock by matching substrings of
  the toast text, so any wording change would flip it silently.
- The popover uses the floating shadow-lg and app tokens instead of
  shadow-black/10 with dark: overrides.
…evived icon

- SidebarNavIcons: StarterWorkIcon came back in merge fix-up 44f3cc5
  after dev deleted it (d922f8e, Starter Work became Hire Setup).
  Nothing imports it. Removing it and its Target import leaves the file
  byte-identical to dev, so this PR no longer touches it.
- SidebarLogo springs: the hover springs use hoverSpringToken instead
  of inline 400/16 and 420/14. The five-click drop really needs a
  bouncier landing, so it gets a named logoHopSpringToken in tokens.ts
  instead of magic numbers. The comment claiming the egg lives on the
  login page too now says two in-app surfaces; the login copy
  deliberately has none.
- SidebarLogo tests: the old motion mock turned every tag into a div
  and dropped `animate`, so "idle below five clicks" could never fail.
  The logo root now exposes data-drop-phase. The tests check idle
  after four clicks, falling after five, each stage in turn, no restart
  mid-drop, and staying idle under reduced motion (a stubbed
  useReducedMotion, since framer caches matchMedia).
- NotFoundPage: the Space Invaders teaser said "While you wait for your
  manager's approval…", which makes no sense on a 404. It now reads
  "While you're lost in space…" with a screen-reader-only "play Space
  Invaders" suffix, so the accessible name still starts with the
  visible text (WCAG 2.5.3). It is a ui/Button ghost instead of a
  hand-styled button.
The game now stays open past the end of the wait, which exposed bugs in
every surface that hosts it. Each one is fixed and pinned by a test.

- Chat and buddy refocus: when the reply landed, the composer took
  focus back mid-run (during play focus sits on body). DinoGame then
  ignored every key aimed at the text field: the dino died, Esc was
  dead, and Space typed into the box. The refocus now waits until the
  game closes, then returns focus if nothing else holds it.
- Buddy visibility: Space was armed on isThinking alone, but the
  session lives app-wide and the dock mounts only while open. So a
  minimised dock plus Space opened an invisible game that held the
  one-game slot for good. Surfaces now register themselves (the open
  dock, BuddyPage); Space arms only while one is visible, and the game
  closes when none is. The hire/team conversation switch also closes it.
- Outcome labels: "Reply ready" used to show after Stop and after
  failed streams too. The chat records each turn's outcome
  (finished / stopped / failed, exposed per chat by useChat.turnOutcome);
  buddy derives it from the last message's error. The badge now reads
  "Stopped" or "Reply failed", in words not just colour. The shared
  label lives in dinoOutcome.ts.
- Onboarding: a failed generation fell back to the last snapshot,
  whose working phases kept isGenerating true. Phases froze, the timer
  kept running, and retry never appeared. Failure now wins and closes
  the game; on success the snapshot shows every phase completed.
- Data sources: "Sync complete" ignored an update still in flight;
  replyReady now also needs !isSyncing.
- Drawer Escape: SidePanel's document listener closed the drawer
  before DinoGame's window listener saw the key. DinoGame now takes
  Escape in the capture phase, and SidePanel skips a key already
  handled, so the first Esc closes the game and the second the drawer.
- Screen readers: the whole game sat in role=status, so the score was
  announced about 12 times a second. The game is out of the live
  region now; one status line gives the result and final score, the
  running score is aria-hidden, and the typing bubbles keep a short
  "Thinking…".
- Consistency: the game's raw emerald/white colours are now app
  tokens, its buttons ui/Button, the buddy bubble uses
  centralSpringToken, and dino-game* test ids are added. BuddyDock's
  prop docs are back on their own props. ChatPage closes the game on a
  chat switch from an effect instead of during render.
The drawer test for "an Escape already handled inside is ignored" was
appended to SidePanel.test.tsx. #264 also adds imports and tests at the
end of that file, so whichever PR merged second hit a conflict on its
import lines and on its final block.

The test moves unchanged into SidePanel.escape.test.tsx, and
SidePanel.test.tsx matches dev again. merge-tree against
feature/KB-updates-final is now conflict-free.
The trigger's "this control owns Space" guard also covered disabled
controls. Space on a disabled button does nothing, and a busy ui/Button
(`loading`) is disabled while it can still hold focus. So clicking
"Update repo" in the source drawer and then pressing Space never opened
the game, during the one wait that is exactly what the game is for.

isInteractiveTarget now skips `:disabled` and `[aria-disabled="true"]`
controls. Text fields stay protected even when disabled: the typing
guard is unchanged.

This was also why the SourceDetailsPanel "update in flight" test was
flaky. In jsdom, blur() does nothing on a disabled element, so focus
stayed on the busy button. The test passed only when the drawer's
initial-focus frame happened to land after the click. The test now waits
for that frame, clicks, asserts focus is still on the busy button, and
presses Space once, without the retry loop. Passed 8/8 runs alone and
3/3 with its folder.
… out of Space Invaders

Addresses the verified findings from the PR #184 review.

1. useSpaceOpensDino released the shared game slot during render.
   `releaseGameSlot(host)` mutated the module-level `gameHost` inside the
   render-phase "adjust state when a prop changes" blocks (unlock flipped
   off, wait ended). Render must stay pure: a render React throws away
   (concurrent retry, StrictMode double render) would free a slot the
   committed game still holds, and a second armed surface could then claim
   it and open a second game - exactly what the slot exists to prevent.
   The render-phase blocks now only call `setGameActive(false)`; a new
   effect releases the slot whenever `gameActive` is false after commit.
   `close()` still releases eagerly because it runs in an event handler,
   where mutating module state is fine. The suggested alternative (move
   both the release and `setGameActive(false)` into one effect) was not
   used: a setState inside an effect trips `react-hooks/set-state-in-effect`
   and costs an extra render with the stale game still on screen.

2. Space Invaders handled keys with no modifier or typing guard.
   Ctrl/Cmd+A (select all) and Alt+D matched `KeyA`/`KeyD`, got
   `preventDefault` and moved the ship. The keydown handler now ignores
   Ctrl/Meta/Alt presses and keys typed into a text field, the same guards
   DinoGame already has. keyup stays unguarded on purpose so a held key is
   always released. Escape moved to a capture-phase window listener with
   `preventDefault` + `stopPropagation`, again mirroring DinoGame, so one
   press closes only the game and never a surrounding surface too.

New tests/unit/features/easter-eggs/SpaceInvaders.test.tsx covers movement
keys taken, modified presses and text-field typing left alone, and Escape
claimed before document listeners.

Findings checked and deliberately not changed:
- Raw <button>s in SpaceInvaders: game surfaces are a listed exception in
  FRONTEND_CODING_STANDARDS.md section 4.
- `text-white` on `bg-app-brand`: the same pairing `ui/Button` primary uses;
  `text-app-text-inverse` would turn dark in dark mode.
- EggLoadingFallback `w-[680px]`: the dialog is `max-w-full overflow-hidden`,
  so it clips instead of widening the viewport; `w-full` inside that
  shrink-to-fit dialog would collapse the reserved box to zero width.

Gates: prettier format:check clean, npm run build OK, npm run lint 0
problems, npm run test 368 files / 3082 tests passed.
@DavidLeuter DavidLeuter self-assigned this Sep 25, 2026

@DavidLeuter DavidLeuter left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Went through the full source diff against dev (87 files) — really solid work. The registry + single shell, the effect bus with one renderer, the shared game slot and the capture-phase Escape are all clean, and the test coverage for the tricky bits (focus contract, slot release, honest badges, drawer Escape) is impressive. CI is green on 061276ed.

One behavioural finding (inline on GenerationScreen.tsx) and a few non-blocking nits:

Nits (non-blocking)

  • isTypingTarget / isInteractiveTarget live in hooks/useDinoWaitingGame.ts, but SpaceInvaders and DinoGame import them from there too. They're generic key guards — easter-eggs/lib/ would be a more natural home than a dino hook.
  • Import specifiers are touched in both directions: ThinkingIndicator / BuddyMessage add .tsx/.ts extensions, while NotFoundPage removes them. Pure churn — maybe pick one and leave untouched imports alone.
  • registry.ts: Space Invaders' lazy() is hoisted into a const while 2048's is inline — small inconsistency.
  • The Verification section still says head 17cdf9b6; the head is now 061276ed (CI is green there, so just the description).

Happy to approve once the GenerationScreen point is addressed or you explain why it can't happen with the current stage stream.

Comment thread src/features/onboarding/components/journey/GenerationScreen.tsx Outdated
…es the game, key guards to lib

Responds to review 5317855360 on PR #184. Every point was checked against
the code.

1. GenerationScreen derived "still generating" from the phases (real).
   `isGenerating` was `!isCompleted && (no phases || some phase
   waiting/working)`. OnboardingJourneyProvider builds `phases`
   incrementally from `onStage` events (unknown names are pushed) and a
   phase's state comes from its detail line, so every phase can read done
   while `generation.status` is still "running": after the last phase
   reports "Completed..." but before `onDone` (path still persisting), and
   in any gap before the next phase's first stage event. In that window an
   open game showed "Path ready", the trigger disarmed, the Space hint
   vanished and the clock stopped - although the run could still fail.
   GenerationScreen now takes a required `isRunning` prop, OnBoardingPage
   passes `generation.status === "running"` (which it already computed),
   and `isGenerating = isRunning && !isCompleted`. The phases are purely
   presentational. Made the prop required rather than optional so no
   caller can silently fall back to the old phase-based guess. New test:
   all phases done but still running -> hint shown, game opens, no
   "Path ready"; existing tests now pass `isRunning` explicitly.

2. isTypingTarget / isInteractiveTarget moved to easter-eggs/lib/keyTargets.ts.
   They are generic key guards used by DinoGame, SpaceInvaders and the
   Space-to-play hook, not dino-hook internals. No re-export from the old
   module: all three importers (plus the hook test) now import from lib.

3. Import-specifier churn reverted. ThinkingIndicator's untouched imports
   are back to dev's extensionless form and NotFoundPage's are back to
   dev's `.tsx` form; the one import each file genuinely adds follows that
   file's style. BuddyMessage was not churn - its extension-bearing lines
   are all new imports - so it is left alone.

4. registry.ts: Space Invaders' lazy() is inline like 2048's instead of a
   hoisted const.

Not changed here: the PR description's Verification head SHA is updated
on the PR itself after this push.

Gates: format:check clean, npm run build OK, npm run lint 0 problems,
npm run test 368 files / 3083 tests passed.
@daniilperkin

Copy link
Copy Markdown
Collaborator Author

Thanks @DavidLeuter. Addressed in b3dd474e:

  • GenerationScreen (behavioural): real, fixed. Confirmed it in OnboardingJourneyProvider: phases are pushed from onStage, and their state comes from the detail line, so every phase can read done while status is still "running". GenerationScreen now takes a required isRunning prop (the page passes generation.status === "running") and uses isGenerating = isRunning && !isCompleted. The phases are presentational only. It's required rather than optional, so no caller falls back to the phase guess. New test: all phases done but still running → hint shown, game opens, no "Path ready".
  • Key guards: moved to easter-eggs/lib/keyTargets.ts. DinoGame, SpaceInvaders, the hook and its test import from there, with no re-export.
  • Import churn: ThinkingIndicator's untouched imports are back to dev's extensionless form, NotFoundPage's back to dev's .tsx. The single new import in each follows its file. BuddyMessage wasn't churn: all of its extension-bearing lines are new imports, so it's unchanged.
  • registry.ts: Space Invaders' lazy() is inline like 2048's.
  • Description: Verification now points at b3dd474e (368 files / 3083 tests; build, lint, format:check clean; merge-tree against dev and feat(knowledge-base): server-side pagination, URL-state filters, sort, date/language facets, bulk delete and AI status #264 clean).

@DavidLeuter DavidLeuter left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @daniilperkin — checked b3dd474e against my review, all points are resolved:

  • GenerationScreen: isGenerating = isRunning && !isCompleted now comes from the real run status, and the phases are purely presentational. I also walked the status transitions on OnBoardingPage: error unmounts the screen, and idle is only ever reached via clearGeneration() after done/error (it's a no-op while running), where lastGeneration.completed keeps "Path ready" honest. So there's no path left where an open game claims completion early. Making the prop required is a nice touch, and the new "all phases done but still running" test covers exactly the case I raised.
  • Key guards in easter-eggs/lib/keyTargets.ts with no re-export, import churn reverted to each file's dev style, registry lazy() inline, description updated. Fair point on BuddyMessage.

CI is green on the head. Approving 🚀

Reviewed with Claude Code.

@daniilperkin
daniilperkin merged commit 356c1b1 into dev Sep 26, 2026
4 checks passed
@daniilperkin
daniilperkin deleted the feature/easter-eggs-overhaul branch September 28, 2026 07:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants