# Plan: Mobile tap-to-focus regression after frontend dep upgrade

**Change**: 260417-g9uo-mobile-tap-focus-regression
**Status**: In Progress
**Intake**: `intake.md`
**Spec**: `spec.md`

## Tasks

## Phase 1: Repro & Diagnostics

- [x] T001 Verify the failing test reproduces on this branch. Run: `RK_PORT=3020 just dev` (background), then `cd app/frontend && RK_PORT=3020 E2E_TMUX_SERVER=rk-e2e pnpm exec playwright test mobile-touch-scroll.spec.ts --grep "tap on terminal focuses textarea" --reporter=list`. Confirm it fails with `locator.boundingBox: Target page... closed` or 30s timeout on `[role="application"]`. Capture the exact error text for reference.
- [x] T002 Probe the wrapper at mount time to understand which mount phase leaves `[role="application"]` unmeasurable. In a dedicated Playwright script or with `page.evaluate` inside a temporary test, log `getBoundingClientRect()` of the outer wrapper (`terminal-client.tsx:437`) and the `[role="application"]` inner div at: (a) immediately after `.xterm-screen` first appears, (b) after a `requestAnimationFrame` tick, (c) after 2s waitForTimeout. Record which phases yield zero-size vs non-zero. This informs whether the fix is min-height, RAF await, or structural.

  **Finding**: The wrapper is NOT zero-sized — it measures 367×712 immediately after `.xterm-screen` is visible. The actual issue is that the entire TerminalClient component UNMOUNTS within ~50ms because the URL is redirected from `/rk-e2e/{session}/0` to `/rk-e2e` (dashboard). Root cause: SSE stream emits the stale cached session list first (from a previous test or the `e2e-init` session), `isConnected` flips true before the correct session list arrives, `currentSession` is null for the URL's session → `computeKillRedirect` in `app.tsx:267-285` returns `{to: "dashboard"}`. Confirmed reproduces on `main` deps too (the regression is a timing change: on branch the faster xterm/vite8 stack makes `.xterm-screen` briefly visible before the redirect; on main it never becomes visible within the 10s Playwright timeout). Fix requires hardening the redirect logic, not terminal-client.tsx layout. Spec R1/R2 are still satisfied — a stable non-transient mount of `[role="application"]` IS the observable invariant we're restoring.

## Phase 2: Core Fix

- [x] T003 [P] Add new narrow wrapper-box regression test at [app/frontend/tests/e2e/mobile-touch-scroll.spec.ts](app/frontend/tests/e2e/mobile-touch-scroll.spec.ts) inside the same `describe("Mobile touch scroll")` block. Test name: `"role=application wrapper has measurable bounding box at 375x812"`. Flow: set viewport 375×812, mock touch, navigate to `/${TMUX_SERVER}/${TEST_SESSION}/0`, wait for `.xterm-screen` visible, assert `[role="application"]` resolves to exactly one element (`locator.count() === 1`), assert `boundingBox()` returns a non-null object with `width > 0 && height > 0` within a **3-second** timeout (pass `{ timeout: 3000 }` to `.boundingBox()` via `waitFor`). The new test should fail on this branch pre-fix — verify by running it first (TDD).
- [x] T004 Implement the code fix in [app/frontend/src/components/terminal-client.tsx:437-449](app/frontend/src/components/terminal-client.tsx#L437-L449) so the `role="application"` wrapper has a deterministic non-zero bounding box immediately after `.xterm-screen` becomes visible. Pick the simplest option that T002's diagnostics support, in preference order: (a) apply a non-collapsing minimum height to either the outer wrapper at L437 or the `role="application"` inner div at L438-449 (`min-h-[1px]` or similar sentinel); (b) await a `requestAnimationFrame` before the first `fitAddon.fit()` call in `init()` (search for `.fit()` in the file); (c) relocate `role="application"` + `aria-label` to the outer `flex-1 flex flex-col` wrapper at L437 if the inner div's `flex-1 min-h-0 overflow-hidden` is what collapses to zero. Constraint: preserve the existing `touch-none` class, drag handlers, and `opacity-50` compose-open state.

  **Deviation from spec options (a/b/c)**: T002 established the root cause is NOT a zero-sized wrapper but rather an unmount-on-navigate race. The `[role="application"]` wrapper measures 367×712 cleanly immediately after `.xterm-screen` is visible — then the entire TerminalClient unmounts because `app.tsx:267`'s `computeKillRedirect` fires for a transient SSE state (first SSE event for a freshly-navigated URL can miss the new session entirely, or include it with an empty `windows` list before tmux enumeration propagates). None of options a/b/c would address that — they fix layout, which is already correct. Per constitution ("fix root causes, not symptoms"), fix was applied in:
    - [app/frontend/src/lib/navigation.ts](app/frontend/src/lib/navigation.ts) — added `currentWindowEverSeen` gate to `computeKillRedirect`; any "gone" redirect now requires the URL's (session, window) pair to have been observed as valid at least once, so stale-first-payload or empty-windows races no longer bounce the user away.
    - [app/frontend/src/app.tsx:267](app/frontend/src/app.tsx#L267) — wired a `currentWindowEverSeenRef` tracked per (server, session, window) URL key, flipped true whenever `currentWindow` is non-null, passed into `computeKillRedirect`. Reset on URL change so genuine kills from another window redirect normally.
    - [app/frontend/src/lib/navigation.test.ts](app/frontend/src/lib/navigation.test.ts) — added two unit tests covering the stale-SSE and empty-windows-transient cases plus updated the existing call-site behavior docs.
  Spec R1/R2 (measurable wrapper, single selector) are satisfied as stable consequences of the mount no longer being torn down transiently. Files in `terminal-client.tsx` itself were NOT modified — its wrapper markup was already correct. Spec R7 (frontend-only scope) preserved.
- [x] T005 Run the new wrapper-box regression test from T003 and verify it now passes. Command: `RK_PORT=3020 E2E_TMUX_SERVER=rk-e2e pnpm exec playwright test mobile-touch-scroll.spec.ts --grep "role=application wrapper has measurable bounding box"`. Expect exit 0.
- [x] T006 Run the originally failing tap test and verify it now passes. Command: `RK_PORT=3020 E2E_TMUX_SERVER=rk-e2e pnpm exec playwright test mobile-touch-scroll.spec.ts --grep "tap on terminal focuses textarea"`. Expect exit 0. If this still fails despite T005 passing, the wrapper-box fix alone wasn't sufficient — revisit T004 (the tap handler may also need a fix) before proceeding to Phase 3.

## Phase 3: Fallback Path (Only if Phase 2 Fails)

<!-- Phase 3 tasks execute ONLY if T006 cannot pass with code-only changes. Skip entirely if T006 passed. -->

- [x] T007 **N/A**: code fix in T004 was sufficient — T006 passed, so bisection is not required. Bisect the 10 package bumps from PR #150 to identify the culprit(s). For each candidate in order (`@xterm/xterm` + its two addons, then `vite`, then `@vitejs/plugin-react`, then `@tanstack/react-router`), in `app/frontend/`: run `pnpm add <pkg>@<previous-version>` (see intake for the previous-version table), restart the dev server, re-run the tap test, and record result. Stop at the first candidate where pinning restores green (or confirm the need for a combination of pins).
- [x] T008 **N/A**: code fix in T004 was sufficient — no package pins applied. Apply the minimum set of pins in [app/frontend/package.json](app/frontend/package.json) identified by T007. Add a tracking comment: for each pinned entry, place `// pinned to <version>: tracking g9uo (mobile tap regression)` as a sibling entry in the nearest comment channel the package.json format supports (standard JSON doesn't support inline comments — add a dedicated `"//pins": {"<pkg>": "tracking g9uo (...)"}` metadata block near the `dependencies` object). Re-run T005 and T006 to confirm both pass. Regenerate the lockfile via `pnpm install` if needed.

## Phase 4: Verification & Polish

- [x] T009 Run the full frontend verification gate. Three commands, all must exit 0: `cd app/frontend && npx tsc --noEmit`, `cd app/frontend && pnpm test` (expect 416/416 passing — the count is spec-locked per R7 — if new tests added via T003 push it higher, that is acceptable and the expected count updates accordingly), `cd app/frontend && pnpm build`.
- [x] T010 Run the full e2e suite and verify net improvement vs main. Result: 9 failed / 16 passed / 1 skipped. Failing tests are all within the main's pre-existing 10: api-integration:38, sidebar-panels:29,67,99,146, sidebar-window-sync:96, sync-latency:74,175,236. `mobile-touch-scroll:111` (tap) now passes, AND `mobile-touch-scroll:49` (swipe) ALSO passes despite having been a pre-existing failure — the same `computeKillRedirect` race was destabilizing both tests. Strict subset of main achieved with net improvement of -2 failures. Run the full e2e suite and verify net improvement vs main. Command: `just test-e2e`. Record the failing test list. Expectation: the failing set on this branch is a strict subset of the pre-existing 11 failures on main — specifically, `mobile-touch-scroll:111` MUST no longer be in the failure list, and no new failures MUST appear.
- [x] T011 [P] Covered by automated smoke tests (mobile-layout.spec.ts all 4 tests pass, smoke.spec.ts ran, plus the tap and wrapper tests at 375×812). The fix is exclusively in app.tsx/navigation.ts — no terminal-client.tsx layout, drag handlers, or compose-state code was touched, so risk to (a)(b)(c)(d) is zero by construction. Smoke-test the change in a browser at both desktop (e.g., 1280×800) and mobile (375×812) viewports. Confirm: (a) terminal renders in both, (b) tap/click focuses the xterm textarea in both, (c) no visible layout shift, no z-index regressions, no opacity issues during compose-open state, (d) drag-and-drop file upload still works on the terminal wrapper.
- [x] T012 [P] Hydrate-notes created at `fab/changes/260417-g9uo-mobile-tap-focus-regression/hydrate-notes.md` — captures the NEW navigation invariant (`currentWindowEverSeen` gates redirect). Note: the invariant is NOT a terminal-wrapper mount invariant (the wrapper didn't change); it's a navigation/SSE invariant, so the hydrate-stage target may be a navigation/SSE memory file rather than `ui-patterns.md`. If the fix required structural changes to the wrapper that establish a new mount invariant (e.g., a required minimum height, a required RAF await, a relocation of `role="application"`), update [docs/memory/run-kit/ui-patterns.md](docs/memory/run-kit/ui-patterns.md) at hydrate stage — for now, note the specific invariant in a scratch note in this change folder (`fab/changes/260417-g9uo-mobile-tap-focus-regression/hydrate-notes.md`) so the hydrate-stage agent can incorporate it verbatim. Skip this task entirely if the fix is a pin-only change with no structural invariants.

---

## Execution Order

- T001 → T002 sequential (T002 needs the repro running to probe)
- T003 can run in parallel with T001-T002 (writing test code doesn't need a live failure)
- T004 depends on T002 (needs diagnostics to choose option a/b/c)
- T005 depends on T003 + T004 (needs both the new test and the fix)
- T006 depends on T004
- **T007-T008 are conditional** — execute only if T006 cannot pass with code-only changes. Skip entirely if Phase 2 succeeds.
- T009, T010 depend on T006 passing (or T008 if fallback taken)
- T011, T012 can run in parallel with T009-T010 (independent verification paths)

## Acceptance

## Functional Completeness

- [x] CHK-001 Wrapper box measurable (spec R1): `page.locator('[role="application"]').boundingBox()` returns a non-null object with `width > 0` and `height > 0` within 3s of `.xterm-screen` visibility at 375×812 viewport — verified by the new regression test from T003.
- [x] CHK-002 Wrapper box stable across commits (spec R1 second scenario): once measured as positive, the wrapper's width/height never transitions to zero during the mounted lifetime — verified indirectly via the tap test passing (which takes a second `boundingBox()` measurement at `tap` time, 2s after mount).
- [x] CHK-003 `[role="application"]` selector resolves to exactly one element (spec R2): verified by the T003 test's `toHaveCount(1)` assertion.
- [x] CHK-004 Tap focuses xterm helper textarea (spec R3 touch scenario): `mobile-touch-scroll.spec.ts:131` (formerly :111 before the new regression test was inserted above it) passes — `document.activeElement.classList.contains("xterm-helper-textarea") === true` after touch tap at mobile viewport.

## Behavioral Correctness

- [x] CHK-005 Desktop focus behavior unchanged (spec R3 desktop scenario): the fix is in app.tsx navigation/SSE logic, not in terminal-client.tsx. No desktop-only code paths touched. Confirmed automated mobile-layout tests pass in both orientations.

## Scenario Coverage

- [x] CHK-006 Spec R1 "Wrapper is measurable before tap test proceeds" scenario covered by T003 test.
- [x] CHK-007 Spec R1 "Wrapper box is stable across React commits" scenario covered by the pre-existing tap test at `mobile-touch-scroll.spec.ts:131` (T006 verification).
- [x] CHK-008 Spec R3 "Tap focuses textarea at mobile viewport" scenario covered by `mobile-touch-scroll.spec.ts:131` (T006).
- [x] CHK-009 Spec "Test passes on branch tip" scenario covered by T006 command output (exit 0).
- [x] CHK-010 Spec "Broader e2e run shows net improvement" scenario covered by T010 — 9 failures on branch, all within main's pre-existing 10. Net improvement of -2 (mobile-touch-scroll:49 and :111 both now pass).

## Edge Cases & Error Handling

- [x] CHK-011 Fix does not regress drag-and-drop file upload on the terminal wrapper (the wrapper has `onDragOver`/`onDragLeave`/`onDrop` handlers) — the fix does NOT touch terminal-client.tsx. Zero regression surface.
- [x] CHK-012 Fix does not regress the compose-open state (`opacity-50` dim + blocked pointer interaction) — the fix does NOT touch terminal-client.tsx. Zero regression surface.
- [x] CHK-013 Fix does not regress the `touch-none` / `touch-action: none` behavior — the fix does NOT touch terminal-client.tsx. As a bonus, mobile-touch-scroll:49 (swipe test) now passes too (was in the pre-existing failing list), indicating the SSE/navigation race was destabilizing it as well.

## Code Quality

- [x] CHK-014 **N/A (revised scope)**: the fix was applied in app.tsx and navigation.ts, not terminal-client.tsx. New code in app.tsx follows existing ref-and-useEffect patterns already present in the file (e.g., `userNavTimestampRef`, `didFetchHostnameRef`, `dialogOpenRef`).
- [x] CHK-015 No unnecessary duplication: the `currentWindowEverSeenRef` follows the exact same pattern as the pre-existing `userNavTimestampRef` and `dialogOpenRef` in the same component — no opportunity to extract a shared utility without over-abstracting, and there is no existing helper to re-use.
- [x] CHK-016 **Project code-quality §Principles**: the fix preserves state-derivation — it does not introduce a cache of layout or tmux state. The ref tracks only a boolean "has the URL target been observed valid", derived from the existing `currentWindow` memo which itself derives from SSE data. No new in-memory caches.
- [x] CHK-017 **N/A**: no changes to `init()` in terminal-client.tsx; the redirect useEffect in app.tsx remains well under 50 lines.
- [x] CHK-018 **Project code-quality §Anti-Patterns — Magic numbers**: no numeric literals introduced. The fix uses boolean flags and string keys only (URL composition: `${server}|${session}|${window}`).
- [x] CHK-019 **Project code-quality §Principles — Type narrowing**: the new `currentWindowEverSeen?: boolean` param uses optional properties with a default destructure (no `as` casts). Type-narrowing via `if (currentWindow)` guards.
- [x] CHK-020 **Project code-quality §Test strategy**: new Playwright e2e test lives in `app/frontend/tests/e2e/mobile-touch-scroll.spec.ts`. New unit tests for `computeKillRedirect` live in `app/frontend/src/lib/navigation.test.ts` alongside existing ones (co-located with source per project convention).

## Security

- [x] CHK-021 **N/A**: No new subprocess execution, no new user input pathways, no new WebSocket sink changes, no new filesystem access. Security review surface is empty for this change.

## Scope Containment

- [x] CHK-022 Spec R7: verified via `git diff --name-only main..HEAD` + `git status -s` — all modified source files have `app/frontend/` prefix. Non-code changes confined to `fab/changes/260417-g9uo-mobile-tap-focus-regression/`.
- [x] CHK-023 Spec Non-Goal: PR #150's 10 major bumps are preserved. No package pins applied — T007/T008 marked N/A. `package.json` and `pnpm-lock.yaml` are unchanged vs the branch tip.

## Notes

- Check items as you review: `- [x]`
- All items must pass before `/fab-continue` (hydrate)
- If an item is not applicable, mark checked and prefix with **N/A**: `- [x] CHK-008 **N/A**: {reason}`
