fix(chat): let Back close the mobile sessions drawer

[chenmingwei23]

1. What is the problem?

On a phone, the sessions drawer covers the screen, and Back is the gesture a user reaches for to dismiss a panel that covers the screen. The drawer was pure component state (drawerPhase), present in no history entry, so a back swipe with it open left /chat entirely -- and the drawer was still open when they navigated back.

Nothing about this changed in the drawer work that landed since the issue was filed: #7133 added the swipe gesture to every page and touched no history at all, and there is still no popstate handling anywhere in the drawer's path on main. Confirmed two ways -- the two failing cases in the new test file on the pre-fix tree (opening the drawer reports REPLACE, and Back leaves the route), and the before/after recording below.

2. Why this issue matters to the user

Back is the only dismissal affordance the mobile layout does not have to teach. The drawer has three others (leftward drag, backdrop tap, header toggle), so this is a papercut rather than a trap -- but the papercut is paid by the user who reaches for the gesture the platform trained them on, and it costs them their place in the app rather than just doing nothing.

3. How our fix solves it

The drawer now owns exactly one history entry, and it exists exactly while the drawer is open. That single invariant is what makes each step follow:

• Open pushes one entry, a bare duplicate of the entry below it. Same URL, because the drawer is view state and not a location; a URL that moved would have to be unwound on the pop, and unwinding a ?sid= is exactly what the sid effect misreads.
• Back pops it: while the drawer is open and the page holds an entry, that entry is on top, so any POP is a pop off it. The drawer closes and the route does not change. The pop is the consumption, so this path deliberately does not spend a second entry.
• Every other close spends it (navigate(-1)): backdrop tap, header toggle, gesture settle, session switch, and the crossing out of the mobile viewport. Leaving it behind would be the twin-entry defect SidePanelLayout's own back control documents -- two entries with the same URL, so the next back-swipe visibly does nothing.

The chain from that to the two effects the issue said would need rework:

• Consuming the entry on a session switch lands on an entry still carrying the outgoing ?sid=, and switchSlot.pending has already moved activeSlot. The ?sid= -> activeSlot effect treats any POP as the user retracing sessions, so it would switch them straight back to the session they just left. drawerPopRef marks that pop as bookkeeping. It is checked before the arming block, not after, because the popReadyRef and duplicate-key guards there return early and would leave the flag set to swallow a later genuine POP.
• That pop lands on an entry whose pathname is identical, which the activeSlot -> ?sid= effect could not see -- so it did not re-run and left ?sid= naming the outgoing session, which a reload would then restore. It now keys on location.key as well: the key changes on any history move, which is the thing that actually happened. POPs are still funnelled through the existing popInFlightRef bail, so this adds a re-check rather than a new writer.

Ownership is a ref, and the entry carries no history.state marker. That is a deliberate difference from SUBNAV_PUSH_STATE: a SubNav drill-in changes the URL, so a cold deep link can land on the drilled-in entry and the marker is the only way to tell "we pushed this" from "the user arrived here". Nothing can deep-link a drawer open, so there is no such question, and a marker would be write-only state. A ref is also the only correct form -- a marked entry can outlive the mount that pushed it (a reload restores history.state, and Forward can walk back into one), so reading a marker would have the page consume an entry it never pushed.

4. What tests we did

Recording

Same scenario both times, on a real pod running this branch at 390x844: open the sessions drawer by its own control, then press platform Back twice.

Before (main)	After (this PR)
One Back and you are on Settings -> Overview. The drawer is still open when you return.	One Back closes the drawer, same URL. A second Back leaves chat, as it always did.

The driver logged the URL and the scrim's presence at each step, which is the part a GIF cannot assert:

BEFORE  drawer-open   url=/chat/new-session?sid=chat-1  backdrop=1
BEFORE  after-back-1  url=/settings/overview            backdrop=0   <- left the route
AFTER   drawer-open   url=/chat/new-session?sid=chat-1  backdrop=1
AFTER   after-back-1  url=/chat/new-session?sid=chat-1  backdrop=0   <- drawer closed, route kept
AFTER   after-back-2  url=/settings/overview

drawer-open keeping the URL byte-identical is the other half of the contract: the pushed entry is a duplicate, so nothing about the session moves.

recorded from 98bd1c4 - kirodotdev/KiroCrew fix/mobile-back-closes-drawer-5795 - mode: pod kirocrew-fix-5795 :8005, viewport 390x844, headless chromium - real server, no fixtures; the BEFORE clip is main's ChatPage.tsx built into the same pod (verified by the absence of chatDrawerPush in the served bundle). The head has since moved to b6db610, whose only change is deleting the inert history.state marker -- zero production readers, as the First Principles review independently grepped -- so the recorded behaviour is unchanged. Re-record from either SHA to check.

Automated

New src/test/ChatPage.drawerBackClose.test.tsx (4 cases). Two are RED on the pre-fix tree; the other two were green there only because there was no entry to mismanage, and both caught a real defect in this change (the stale ?sid= above).

Every guard is mutation-verified -- each reverted in turn, each reddens a case:

mutation	result
drawerPopRef guard deleted	1 failed
location.key dep removed	1 failed
entry never consumed	2 failed
entry never pushed	2 failed
POP no longer closes the drawer	1 failed

The harness arms sseConnected() deliberately: the ?sid= effect returns early while offline, and the first version of this file passed with the drawerPopRef guard deleted because it never reached the revert.

Neighbouring suites, re-run after the marker subtraction: ChatPage.sid (36), ChatPage.drawerFrameBudget (2), sessionUrlHistory (3), ChatSidebar.historyDeepLink (3), ChatPage.embedded (3) -- 47 passed, plus ChatPage.drawerBackClose (4). Earlier: ChatPageCoverage + ChatPageDrafts (75). tsc -b clean. eslint 13 warnings on ChatPage.tsx both before and after and 0 on the new test, so the --max-warnings 659 ratchet is untouched.

5. Any other suggestions on the work

• The issue's test sketch reads "pick a session, press back, assert the route did not change". This implements the stricter reading -- the pick already consumed the entry, so the drawer is closed and Back then means what it means everywhere else on mobile: leave the chat route. Absorbing a Back with no panel on screen is the invisible-no-op defect the repo already calls out. In a bounded-history harness the two readings are indistinguishable, which is why the sketch passes either way.
• One edge is unfixable by design and shared with SUBNAV_PUSH_STATE: reloading while the drawer is open leaves the pushed entry behind, since an entry you are standing on cannot be removed except by leaving it. Cost is one dead Back press after that specific reload.
• The issue notes the same shape applies to the other mobile layers that push nothing (side panel, file viewer, diff view). Deliberately not done here -- this PR is scoped to the drawer, and those want the same push on open / spend on close pair lifted into a shared hook rather than four copies.

Pattern harvest

Rule candidate: review checklist (not mechanically greppable)

Pattern: a mobile layer that covers the screen must own a history entry, and that entry's lifetime must equal the layer's. Both halves are load-bearing and both were wrong here in different ways: no entry at all is this bug, and an entry that outlives its layer is the twin-entry dead-Back defect SidePanelLayout's back control already documents. The issue names three more layers that push nothing (side panel, file viewer, diff view), so the next one should lift the pair into a shared hook rather than add a third hand-rolled copy.

Second, narrower candidate, worth a lint rule if it recurs: an effect that writes the URL from state must key on location.key, not only location.pathname. A history move between two entries with the same pathname is invisible to a pathname dep, and the effect then leaves the URL describing state that is no longer current. That is a silent class of bug -- nothing throws, and it only surfaces on reload.

Fixes #5795

[github-actions]

UX Review (Fable 5) — ✅ PASS

UX-level review of 2d592ffb4b010336bbb3d931ea8b861bfeb40174 — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

All evidence gathered. The change is behavioral only — no new user-facing strings, no layout changes — and it aligns the mobile drawer with the platform convention that Back dismisses a layered surface. The tests pin the flows that would have degraded the experience (invisible no-op Back, resurrected outgoing session, Back during the opening slide). The one coherence signal: the mirrored right-hand activity overlay in the same file keeps the old behavior, so Back dismisses one overlay but exits the chat over the other.

UX-Verdict: PASS

Back now dismisses the mobile sessions drawer instead of ejecting the user from /chat — matching platform convention, with the no-op-Back and resurrected-session traps explicitly closed.

Suggestions

• The mirrored right-hand activity overlay (useDrawerSwipe(..., side: 'right') in ChatPage.tsx) is the same layered-surface job, but Back over it still leaves /chat — after this PR users learn "Back dismisses overlays" from the drawer and the sibling breaks that model. Worth a named follow-up extending pushDrawerEntry/consumeDrawerEntry to it.
• Both demo GIFs open on a near-black frame, so the PR page shows two black rectangles until playback; re-capture starting on the rendered chat frame.

[UX-REVIEWED] 2d592ff

[github-actions]

Design Review (Fable 5) — ✅ PASS

Design-level review of 2d592ffb4b010336bbb3d931ea8b861bfeb40174 — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

All checks done. The design here is sound: a real, platform-conventional UX problem (mobile Back should dismiss a screen-covering layer), solved with the standard one-entry-per-layer pattern, with the two known interaction hazards (the ?sid= retrace misread and the twin-entry dead-Back) explicitly handled and mutation-tested, edge cases disclosed, and the shared-hook generalization correctly deferred until a second consumer exists. The temp-screenshots/ media follows an established repo convention (600+ files already there).

Design-Verdict: PASS

One history entry whose lifetime equals the drawer's is the platform-correct shape; both known hazards of that shape are handled and pinned by mutation-verified tests.

[DESIGN-REVIEWED] 2d592ff

[github-actions]

First Principles Review (Fable 5) — ✅ PASS

Premise-level review of 2d592ffb4b010336bbb3d931ea8b861bfeb40174 — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

All checks are done: the recordings are a documented repo convention, the near-duplicate mechanism (SUBNAV_PUSH_STATE) is meaningfully different for grounded platform reasons, and the deferred sibling layers are declared with counts I verified. Final review:

First-Principles-Verdict: PASS

Every item is the fix or its mutation-tested bookkeeping; nothing rides along, and the near-duplicate mechanism is meaningfully, verifiably different.

What this change ships

Intent: on a phone, the platform Back gesture should dismiss the open sessions drawer instead of leaving /chat — a FIX (#5795).

1. Back with the drawer open closes it and keeps the route — justified (reported defect)
2. Opening the drawer adds one same-URL history entry — declared, the fix's mechanism
3. Backdrop tap, toggle, session pick, drag-close, and leaving mobile width each spend that entry — declared, prevents the documented twin-entry defect
4. Session switch no longer lets Back/reload restore the outgoing session — declared, caught by this change's own tests
5. Drag-open mints the entry at release, so Back mid-slide also dismisses — declared
6. Four recording binaries under temp-screenshots/ — justified: documented convention (temp-screenshots/README.md, PR template, cleanup workflow)
7. New 5-case test file (description says 4) — declared
8. Residue: one dead Back press after reloading with the drawer open — declared, shared with SUBNAV_PUSH_STATE

Duplication check I ran: navigate(-1) has 3 production sites (SidePanelLayout.tsx:289, SettingsSubNav.tsx:180, this one); the existing SUBNAV_PUSH_STATE mechanism (2 consumers) is for URL-changing drill-ins where a deep link can land on the pushed entry — inapplicable here, and the ref-not-marker reasoning rests on a real platform rule (reload restores history.state). Sibling layers with the same shape (side panel, file viewer, diff view): 3, counted, declared as deliberately deferred — accepted-and-deferred, not a demand.

No blockers, no subtractions: consumer counts are 2–3 per new helper, and every guard's deletion reddens a named test.

[FIRST-PRINCIPLES-REVIEWED] 2d592ff

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

Reviewed 2d592ffb4b010336bbb3d931ea8b861bfeb40174 — this comment is updated in place on each push.

Review details

I've analyzed the single candidate and the full diff.

Candidate 1 (twin-entry survives reload): The candidate's own confidence is "low," and it explicitly requires assuming browser history-stack persistence across a hard reload — behavior not present in any code I can open (MemoryRouter has no persistent stack, as the candidate concedes). The scenario is a deliberately documented tradeoff: the code comment at the drawerEntryRef declaration weighs the ref-vs-history.state-marker choice and picks the ref precisely because a marker "can outlive the mount that pushed it." The harm — a single invisible no-op Back press in the narrow reload-with-drawer-open window — is neither a crash, data loss, nor a security boundary. It cannot be re-derived at 80+ from code alone, and its own fix line reads "No code change likely warranted." Dropped.

I traced the new flag mechanics (drawerPopRef, drawerEntryRef, the POP-close effect, consumeDrawerEntry, the location.key dep, and the split of runDrawerClose/closeSidebar) against the four scenarios the test file pins. Ordering is consistent: drawerEntryRef is cleared before navigate(-1) so the POP-close effect skips its own consumption; drawerPopRef gates the sid effect exactly on the bookkeeping pop and is cleared before the arming block so it cannot outlive its pop. No grounded defect surfaced.

No findings.

[OPUS-REVIEWED] 2d592ff

Verdict parsed from the review's SHA-scoped output markers for commit 2d592ffb4b010336bbb3d931ea8b861bfeb40174.

False positive or not applicable? A repository writer can comment:
/ai-review override fable 2d592ffb4b010336bbb3d931ea8b861bfeb40174: <one-sentence reason>

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of 2d592ffb4b010336bbb3d931ea8b861bfeb40174 and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 2d592ff

False positive or not applicable? A repository writer can comment:
/ai-review override gpt 2d592ffb4b010336bbb3d931ea8b861bfeb40174: <one-sentence reason>

[chenmingwei23]

First Principles blocker taken, and the advisory subtraction with it. b6db610 deletes both:

• isDrawerHistoryEntry -- gone. It was dead on arrival (0 callers) and I should have caught that before pushing.
• DRAWER_PUSH_STATE and the state: payload -- gone. The drawer now pushes a bare duplicate entry. website/src/utils/sessionUrlHistory.ts is byte-identical to main again, so the diff is ChatPage.tsx + one test file.

The "inherited, not derived" reading in the Watch section is correct and is the part worth recording. SUBNAV_PUSH_STATE earns its marker because a SubNav drill-in changes the URL, so a cold deep link can land on the drilled-in entry and the marker is the only way to separate "we pushed this" from "the user arrived here". Nothing can deep-link a drawer open, so that question does not exist here and the marker could only ever be write-only. Worth adding: a marker would have been wrong, not merely idle -- history.state survives a reload and Forward can walk back into a marked entry, so a page reading the marker as ownership would consume an entry it never pushed. drawerEntryRef is the only form that cannot. That reasoning now lives in the comment above the ref instead of the previous "ownership is this ref rather than the marker", which was the tell you picked up on.

Verification after the subtraction: 4/4 in ChatPage.drawerBackClose, and the entry never pushed mutation still reddens 2 cases -- which also pins the thing the bare push depends on, that react-router does not dedupe a navigate to the current URL. Neighbours re-run green (ChatPage.sid 36, drawerFrameBudget 2, sessionUrlHistory 3, ChatSidebar.historyDeepLink 3, ChatPage.embedded 3). tsc -b clean, eslint unchanged at 13.

The recording was made at 98bd1c4, one commit before this subtraction. Not re-recorded: the only delta is the marker you grepped as having zero production readers, so it cannot move observable behaviour. The provenance line in the PR body states both SHAs rather than implying the clips came from the head.

[chenmingwei23]

On the CONCERNS Watch items, for anyone reading the thread later:

Description/diff drift -- already fixed, the review read the previous body. The marker subtraction and the description rewrite were two separate operations: I pushed b6db610 first, then rewrote the body. The review ran on the push, so it checked out post-subtraction code against the pre-edit description and correctly spotted the mismatch. Against the body as it now stands:

$ gh pr view 7295 --json body -q .body | grep -c 'DRAWER_PUSH_STATE'
0

The single chatDrawerPush occurrence left in the body is in the recording's provenance line, where it names the marker's absence as how the BEFORE bundle was verified. The description now says the entry carries no history.state marker and why. Nothing to change in the code; the next review pass should clear this by itself.

Counted siblings -- accepted as deferred, and I agree with the deadline framing. Worth pinning the two you found that I had not: the file and diff panels in usePanelState.ts, and the right-hand activity overlay at ChatPage.tsx:7010. That last one is in the file this PR already touches, which is the strongest argument for the shared hook rather than a fourth copy -- the two overlays would sit side by side in one component, each hand-rolling the same push/spend pair, and they would then have to agree about who owns the top entry when both are open. That interaction is real design work and does not belong in a fix PR, but it is the reason the fourth copy should not be written.

[buluoray]

Reviewed at b6db61010. 1 blocking / 1 yellow / 2 blue — comment only. The blocker is the merge conflict, not a code defect: the change itself is clean and its tests are mutation-verified.

The code is right

Making the drawer own exactly one history entry for its lifetime — push a bare duplicate on open, pop it on Back, spend it via navigate(-1) on every other close path — is the correct shape, and I verified the mechanism against origin/main rather than the description:

• The URL-sync write effect's early return (if (current === activeSlot && location.pathname === expectedPath) return, main:4058) is why the duplicate PUSH does not fire a redundant navigate, so navigationType correctly stays PUSH. That is the load-bearing detail the whole approach rests on.
• popInFlightRef (main:4044) shields genuine session Back/Forward, and the new POP effect is gated on drawerPhaseRef === 'open' so it cannot fire during session navigation.
• Every closeSidebar caller (session switch, backdrop, header toggle, selectSource, preview-expand) inherits consumption because closeSidebar itself was redefined to call consumeDrawerEntry — so the close paths are covered by construction rather than by enumeration.
• No Escape handler for this drawer exists on main, so Escape is not a missed close path. I checked because it is the usual gap in this pattern.

All 8 load-bearing body claims are true against source; none is false. AUTOSDE blocking rules are clean — the production diff adds no user-facing strings (comments only), no icons, no buttons, no dangerouslySetInnerHTML, no layout changes, no locale-less formatting. Mutations reproduce your table: neutralizing the POP-closes-drawer effect fails "Back closes the drawer"; making pushDrawerEntry a no-op fails both "opening PUSHES" and "Back closes".

BLOCKING — conflicting, and the CI on it is stale

gh api reports mergeable=false, mergeable_state=dirty. A read-only git merge-tree origin/main b6db610 produces exactly one conflict: website/src/pages/ChatPage.tsx, from #7207 ("stop chat transcript blanking content above the viewport", cec5f7146) landing after the fork.

Two consequences worth stating explicitly:

The 78 green check-runs do not mean what they look like. A CONFLICTING PR cannot regenerate refs/pull/N/merge, so no pull_request-event workflow has run against the tip merged with current main. Those greens are from a pre-conflict SHA.

The rebase is not mechanical. Main also gained #7320 ("hand a mobile panel's swipe to its sibling at release"), which restructured the drawer gesture config into onCommit + onSettle (main:7031-7032), whereas this PR edits an onSettle-only base. So the rebase has to wire pushDrawerEntry/consumeDrawerEntry into both hooks correctly and reconcile #7207 — after which the tests and the mutation table need re-running on the rebased tip, because what merges will be materially different from what I reviewed.

Yellow

ChatPage.tsx:6788 — a genuine PUSH navigation away while the drawer is open unmounts ChatPage with drawerEntryRef=true, orphaning the pushed entry and leaving a phantom duplicate /chat entry, i.e. one dead Back after returning. This is the same mechanism as the reload-while-open edge you acknowledge as unfixable-by-design, but it is not among the enumerated close paths. Reachability is low on mobile — the drawer covers the screen and normal actions route through closeSidebar or the POP effect — so it is non-blocking, but worth confirming no programmatic push-away path exists while the sessions drawer is open.

Blue

Four binary evidence files are committed (temp-screenshots/mobile-back-drawer-5795/*.mp4 plus 2 .gif). The GIFs already serve the description, so the two .mp4s are usually worth stripping before merge.

Side panel, file viewer and diff view have the same "pushes nothing" defect, deliberately out of scope — and I agree with your note that the third instance is the point to lift the push/spend pair into a shared hook rather than hand-roll it again.

---

Rebase onto main (reconciling #7207 and #7320), re-run the tests and the mutation table on the rebased tip, and I will re-review. Nothing in the code itself needs changing.

[chenmingwei23]

Rebased onto current main after the 13-PR merge batch put this branch in conflict; the conflict is resolved and the PR is MERGEABLE again.

The remaining reds on this head are main-owned and reproduce on PRs with disjoint diffs, so they are not actionable here:

• Backend Tests shard 3 (3.10, 3 / 3.12, 3 / Windows 3) -- filed as Backend Tests shard 3 reds every PR: log-site census has slack for dashboard/handlers/files.py #7504. test_security_posture.py::test_the_census_holds_no_slack fails because _BASELINE_LOG_SITE_CENSUS pins dashboard/handlers/files.py: 3 while the code now has 1. Reproduced on fix(chat): let Back close the mobile sessions drawer #7295, whose diff is one .tsx, one new .test.tsx and four media files -- zero Python -- which is what establishes it as main-owned.
• Frontend Lint & Type Check (where it runs) -- filed as Frontend Lint reds every PR: main sits at 660 eslint warnings against a 659 ratchet #7499. main carries 660 eslint warnings against the --max-warnings 659 ratchet in ci.yml:1298. Measured locally in a worktree at merge-base 271428883: pristine base = 660, base + branch diff = 660, so the branch contributes zero.
• Coverage Gate and PR Readiness are downstream aggregates of the above.

Per house rule the main-owned fixes are not being folded in here. Once main heals I will rebase to cut a fresh merge ref and re-run, rather than re-triggering against a stale one.

[chenmingwei23]

• FIXED span=083e7a6856df: the drawer's history entry was minted on arrival, not on release

Verified against the hook and fixed, not rebutted. The premise holds:
onCommit fires at gesture release (useDrawerSwipe.ts:1244, immediately
before settle), while onSettle runs only as the settle animation's
completion callback (useDrawerSwipe.ts:944). The hook's own doc says
onSettle "deliberately waits for the animation" and that onCommit "fires
~300ms earlier" and is the right signal "for anything gating on intent". An
entry standing for "a drawer is open, so Back dismisses it" is gating on
intent, so the entire 120-450ms opening slide ran with the panel covering the
screen and no entry to pop: #5795 again, reachable by drag instead of by tap.

Both endpoints moved to the commit points, not only the mint. Spending on
arrival had the mirror hazard: a Back during the closing slide popped the
still-unspent entry while the POP effect ignored it (the phase is closing,
not open), and the settle then spent a second, real entry. Fixing only the
mint would have left that half standing.

Pinned by a new test that drives the real touch gesture and presses Back with
the settle still queued. Mutation-verified: restoring the mint to onSettle
kills that test alone and leaves the other four green. That is also why the
original four missed this, since all four open via the toggle, whose push is
synchronous. The test cannot use the click-driven Back stand-in, because a
drag's release arms the hook's click swallower by design.

Head 2d592ff: tsc clean, eslint 598/599 (an A/B against main shows zero new
warnings), i18n:check exit 0, 153 tests green across 12 explicitly named files.

[chenmingwei23]

Answering the two advisory bullets in the UX Review for 2d592ffb4b010336bbb3d931ea8b861bfeb40174. Both are recorded here as prose rather than as ai-review-disposition records: neither is tagged [FINDING], so neither carries a span= id, and a disposition record claiming a span that does not exist would itself be a rule violation.

1. "the mirrored right-hand activity overlay ... Back over it still leaves /chat" - CORRECT, and deliberately out of scope.

Verified rather than assumed: every call site of the two entry helpers is a LEFT-drawer path (ChatPage.tsx 7035 openSidebar, 7064 closeSidebar, 7091 the isMobile crossing, 7241/7247 the left gesture's onCommit). The right overlay's own onCommit/onSettle never touch them, so Back over the activity panel does still leave /chat after this PR.

Not folded in, for three reasons. #5795 names the sessions drawer, so this is a second defect rather than an unfinished half of this one. The right overlay's open state is not symmetric with the drawer's: it is gated on activitySlot, search.isOpen and the desktop actbar column owning the panel, and its store mount predicate can refuse to keep it open, so an entry standing for "the overlay is open" has failure modes the drawer's does not and needs its own tests. And this PR is currently green across 69 checks with five PASS verdicts; extending the mechanism would re-roll every lane on a materially larger diff. Flagged to the maintainer as a follow-up.

2. "Both demo GIFs open on a near-black frame" - accepted, not fixed in this revision.

A fair presentation criticism: the PR page shows two dark rectangles until playback. It is also purely cosmetic and lives entirely in temp-screenshots/, touching no shipped code. Re-capturing means a new head, which re-rolls all five review lanes on a PR that has just converged. Held for the maintainer to call, since re-capture is cheap to do but not free to land.

Neither item blocks: the verdict is UX-Verdict: PASS, and the body's own legend states PASS/CONCERNS are advisory.

[buluoray]

Verdict: 0 blocking, 2 non-blocking. Reviewed at head 2d592ffb4b010336bbb3d931ea8b861bfeb40174.

What I verified

• Read the full diff: only website/src/pages/ChatPage.tsx and the new website/src/test/ChatPage.drawerBackClose.test.tsx carry code; the other four files are temp-screenshots/ media (documented repo convention).
• Entry lifetime is balanced (no accumulation). pushDrawerEntry early-returns when drawerEntryRef.current is already set, and consumeDrawerEntry early-returns when it is not. One entry maximum per open; open mints, every non-Back close spends. Verified all close paths reach consumeDrawerEntry: backdrop tap and header toggle via closeSidebar -> runDrawerClose + consumeDrawerEntry; drag-close via onCommit(open=false); session switch via the activeSlot effect -> closeSidebar; and the mobile->desktop crossing effect (if (!isMobile) { setDrawerPhase('closed'); consumeDrawerEntry() }).
• Back path does not double-consume. The POP-close effect clears drawerEntryRef and calls runDrawerClose() only (no navigate(-1)) — the pop is the consumption. When closeSidebar triggers its own navigate(-1), the resulting POP hits the effect but both guards (drawerEntryRef.current already false, drawerPhaseRef.current === 'closing' not 'open') make it a no-op. No off-by-one.
• Stale ?sid= guard. On a session-switch close, drawerPopRef is set before navigate(-1); the ?sid=->activeSlot effect checks and clears it before the arming block (so the early-returning popReadyRef/duplicate-key guards cannot strand it), preventing the outgoing session from being resurrected. Test 3 pins exactly this.
• Desktop unaffected. pushDrawerEntry early-returns on !isMobile, so drawerEntryRef never becomes true off-mobile; consumeDrawerEntry and the POP-close effect are therefore no-ops on desktop.
• Panel mutual exclusivity is intact and untouched. The right-side gesture (ChatPage.tsx:7202 on main) is gated ... && drawerPhase !== 'open', so it cannot fire while the left drawer is open; the PR modifies only the left useDrawerSwipe onCommit/onSettle, leaving the right overlay binding unchanged.
• No manual popstate listener is added — the fix rides react-router's useNavigationType() (ChatPage.tsx:967) + a location.key-keyed effect, so there is no listener to leak against a dead component. On the initial render navigationType is 'POP' but drawerEntryRef is false, so the effect early-returns.
• i18n: the diff adds no JSX and no user-facing strings (only refs, callbacks, effects, and comments), so there is nothing new to route through the catalog.
• AUTOSDE.yaml blocking rules: none implicated — no layout/gutter change (page-layout-pattern), no icons (use-lucide-icons/icon-buttons-need-labels/no-emoji-as-icons), no new interactive elements (accessible-interactive-elements), no security surface (frontend-security). Automated Rule Check is green.
• All 69 checks green at this SHA; all five review lanes (GPT 5.6, Opus 4.8, Design, UX, First Principles) read PASS/no-blocking with bodies citing this exact SHA.

Non-blocking findings

1. PR body / test-count drift (PR description, section 4): the body says the new file has "(4 cases)" but ChatPage.drawerBackClose.test.tsx contains 5 it() blocks (the drag-open-at-release case is the 5th). Consequence: a reviewer counting cases against the description sees a mismatch. Suggestion: update the body to "(5 cases)". Pure prose edit, no SHA change. (First Principles already noted this as declared.)
2. Documented residue — one dead Back after reload/navigate-away with the drawer open (ChatPage.tsx drawerEntryRef declaration + POP-close effect): if the component unmounts (hard reload, or leaving /chat) while the entry is still held, the entry is left on the stack, costing one invisible no-op Back press. Consequence: a single silent Back in a narrow window. This is disclosed in the PR body and in the drawerEntryRef comment, and is the same tradeoff SUBNAV_PUSH_STATE carries. Suggestion: none required; acceptable as declared.

What I could not verify

• I did not execute the vitest suite or the mutation reverts (read-only shared checkout, no build). I relied on reading the test file plus the green "Frontend Tests" checks and the author's documented mutation table. The test structure (real touch gesture for the drag case, sseConnected() armed so the ?sid= effect is actually exercised, manual settle draining) is consistent with the claimed guards.
• The mirrored right-hand activity overlay still leaves /chat on Back (it owns no history entry). This is a pre-existing sibling of #5795, not introduced here, and is correctly deferred to a shared-hook follow-up per the UX Review thread — out of scope for this PR.