Fix system Back ownership for titles and Home #67

Merged

justinzeus merged 1 commit from fm/tv-back-title into fm/tv-round2

2026-11-07 22:36:20 +01:00

justinzeus commented

2026-11-07 22:32:23 +01:00

Owner

Outcome

Fix native Back ownership so a title returns to Home without removing Wolfskin’s task. From Home content, the next Back focuses navigation; Back from navigation exits. Preserve child menus, request sheets and unsaved-settings priority.

Base: fm/tv-round2, starting at b51dced5fed48d719d1fb456378cedb2d9ad9ac0. The walked 5fb37886 has identical Back sources. AI-assisted implementation and PR description for this internal Wolfskin fork.

Root causes

Two input ingress paths: readShellKey mapped native KEYCODE_BACK to local logical Back. Title/Home Compose previews therefore acted on key-down, before the activity’s system completion. Native tests reproduced premature title pop and premature Home-nav focus on both SDK 34 and 36. Native Back now belongs only to Android’s dispatcher; the existing Escape shortcut remains local.
Retained callback ownership: a title/person handler could remain registered after its entry popped, before Compose updated its enabled flag. A second system Back in the same frame still reached that old title instead of Home. The handler now checks the live navigation entry at invocation and forwards to the current destination if its own entry has retired. Root routing synchronizes the shell route to that current entry before applying the nav/exit ladder.

No debounce, timer, ignored Back, row-name special case or fallback through old setup UI. Keep the root Back handler: Navigation 3 does not handle the single-entry root itself. Child callbacks still get first refusal through the activity dispatcher.

Changes

Remove native Back from the shared Compose key adapter. All Home/Library/Discover/title/person/menu consumers therefore leave it to the activity; Escape continues to exercise local keyboard Back.
Share one destination-to-shell-route conversion between navigation composition, root Back and the title/person ownership check. The routing values are unchanged.
Replace the manual-Box title Back test seam with actual NavDisplay, loaded TitleDestination/person surface and Home. Cover normal and predictive dispatch, raw-key ownership and a second Back before recomposition.
Run title/root Back matrices on SDK 34 (Android 14) and SDK 36. Add the pinned Robolectric Android 16 runtime and its required test-worker-only java.base/jdk.internal.access export. No dependency-version upgrade or production JVM flag.
Migrate design-only logical Back replays to the already-supported Escape key; their state/position/modal assertions are unchanged. Android Back is separately exercised at the real activity dispatcher boundary, not replaced by Escape in the system regressions.
Update data/tv-spec/spec.md and docker/README.md. No layout, visual token, hero-transition, fetch-order or preset change.

Verification — touched only

Exercised coverage	Final result
Native ownership/retained-callback regressions before the fix, SDK 34 and 36	Six expected failing-before cases preserved
Selected app Back/modal/navigation cases	45 unique cases passed across the selected cohort and final two-case input-driver rerun
Selected design logical-Back/viewport/modal cases	21 passed, zero failures/errors/skips
Native UI smoke	Four 1920×1080 SDK 34/36 frames inspected: title pop returns to Home content; Home-row Back focuses nav without task exit

App coverage includes actual title/person normal and predictive callbacks, immediate second Back before exit recomposition, Home/Discover/Movies/Shows/Anime roots, cold/reopened picker, Coming-in dismissal, real Updates parent pops and unsaved Home Settings guards.

The selected app cohort first passed 43 cases; two title raw-key drivers did not synthesize WindowManager’s predictive completion from raw key-up under Robolectric/NavDisplay. Their navigation, focus and no-exit assertions were retained. The final driver injects raw key-down, verifies it did not navigate, then drives the actual activity dispatcher’s predictive start/progress/complete path; both SDK variants pass. There was no production change between that cohort and the final driver rerun. The Home-row hardware case exercises actual Activity key-down/key-up delivery.

Local evidence: /root/.treehouse/firstmate-7bab20/2/firstmate/data/tv-r2-home/back-title/ — before-sdk34/, before-sdk36/, after-app-first/, after-native-completion/, after-design/ and native/.

Limits / integration risk

No full suite, APK build, device access, installation or merge. JVM/native callback proof does not replace the integrated emulator re-walk after the candidate is rebuilt. This is not dismissed as emulator-only: the failing ownership cases and repaired root/title paths exercise Android 14 / SDK 34 as well as SDK 36.

## Outcome Fix native Back ownership so a title returns to Home without removing Wolfskin’s task. From Home content, the next Back focuses navigation; Back from navigation exits. Preserve child menus, request sheets and unsaved-settings priority. Base: `fm/tv-round2`, starting at `b51dced5fed48d719d1fb456378cedb2d9ad9ac0`. The walked `5fb37886` has identical Back sources. AI-assisted implementation and PR description for this internal Wolfskin fork. ## Root causes 1. **Two input ingress paths:** `readShellKey` mapped native `KEYCODE_BACK` to local logical Back. Title/Home Compose previews therefore acted on **key-down**, before the activity’s system completion. Native tests reproduced premature title pop and premature Home-nav focus on both SDK 34 and 36. Native Back now belongs only to Android’s dispatcher; the existing Escape shortcut remains local. 2. **Retained callback ownership:** a title/person handler could remain registered after its entry popped, before Compose updated its enabled flag. A second system Back in the same frame still reached that old title instead of Home. The handler now checks the **live navigation entry** at invocation and forwards to the current destination if its own entry has retired. Root routing synchronizes the shell route to that current entry before applying the nav/exit ladder. No debounce, timer, ignored Back, row-name special case or fallback through old setup UI. Keep the root Back handler: Navigation 3 does not handle the single-entry root itself. Child callbacks still get first refusal through the activity dispatcher. ## Changes - Remove native Back from the shared Compose key adapter. All Home/Library/Discover/title/person/menu consumers therefore leave it to the activity; Escape continues to exercise local keyboard Back. - Share one destination-to-shell-route conversion between navigation composition, root Back and the title/person ownership check. The routing values are unchanged. - Replace the manual-Box title Back test seam with actual `NavDisplay`, loaded `TitleDestination`/person surface and Home. Cover normal and predictive dispatch, raw-key ownership and a second Back **before recomposition**. - Run title/root Back matrices on **SDK 34 (Android 14)** and **SDK 36**. Add the pinned Robolectric Android 16 runtime and its required test-worker-only `java.base/jdk.internal.access` export. No dependency-version upgrade or production JVM flag. - Migrate design-only logical Back replays to the already-supported Escape key; their state/position/modal assertions are unchanged. Android Back is separately exercised at the real activity dispatcher boundary, not replaced by Escape in the system regressions. - Update `data/tv-spec/spec.md` and `docker/README.md`. No layout, visual token, hero-transition, fetch-order or preset change. ## Verification — touched only | Exercised coverage | Final result | |---|---| | Native ownership/retained-callback regressions before the fix, SDK 34 and 36 | Six expected failing-before cases preserved | | Selected app Back/modal/navigation cases | **45 unique cases passed** across the selected cohort and final two-case input-driver rerun | | Selected design logical-Back/viewport/modal cases | **21 passed**, zero failures/errors/skips | | Native UI smoke | Four **1920×1080** SDK 34/36 frames inspected: title pop returns to Home content; Home-row Back focuses nav without task exit | App coverage includes actual title/person normal and predictive callbacks, immediate second Back before exit recomposition, Home/Discover/Movies/Shows/Anime roots, cold/reopened picker, Coming-in dismissal, real Updates parent pops and unsaved Home Settings guards. The selected app cohort first passed 43 cases; two title raw-key drivers did not synthesize WindowManager’s predictive completion from raw key-up under Robolectric/NavDisplay. Their **navigation, focus and no-exit assertions were retained**. The final driver injects raw key-down, verifies it did not navigate, then drives the actual activity dispatcher’s predictive start/progress/complete path; both SDK variants pass. There was no production change between that cohort and the final driver rerun. The Home-row hardware case exercises actual Activity key-down/key-up delivery. Local evidence: `/root/.treehouse/firstmate-7bab20/2/firstmate/data/tv-r2-home/back-title/` — `before-sdk34/`, `before-sdk36/`, `after-app-first/`, `after-native-completion/`, `after-design/` and `native/`. ## Limits / integration risk No full suite, APK build, device access, installation or merge. JVM/native callback proof does not replace the integrated emulator re-walk after the candidate is rebuilt. This is not dismissed as emulator-only: the failing ownership cases and repaired root/title paths exercise Android 14 / SDK 34 as well as SDK 36.

justinzeus added 1 commit 2026-11-07 22:32:23 +01:00

Route system Back through the current navigation owner

review/code-claude Code APPROVE: single Back owner, tests fail before and pass after

Details

review/design-claude Design PASS: no visual change

Details

a268cdd7b8

justinzeus commented

2026-11-07 22:36:09 +01:00

Author

Owner

Review of the Back fix at head a268cdd7b8d887157bcf6749a54c0f9ab0ccf4f1 (base fm/tv-round2).

Code verdict: APPROVE (review/code-claude = success).
Design verdict: PASS (review/design-claude = success). Nothing visual changes.

One owner per Back.

readShellKey no longer maps Key.Back, only Escape. I found no other Key.Back or KEYCODE_BACK in the Wolfskin sources. Hardware Back therefore reaches Wolfskin only through the Activity dispatcher.
The dispatcher runs one callback per Back: the title/person handler, then NavDisplay's handler, then the root handler.
The title handler still gives sheets, menus, pickers and request overlays first refusal (TitleState.back).
Home, Library, Discover overlays, the Updates notes, the requests sheet and the picker are reached through shell.key(Back), so their priority order is preserved.
Exit happens only when Back is pressed on a focused nav, or from goBack on a one-entry stack.
A retained title/person callback now checks the live top of the back stack when it runs. If its entry has already popped it hands Back to the current destination and syncs the shell route first. A second Back before recomposition therefore focuses the Home nav instead of popping again or exiting, and the third Back exits. A retained handler over a surviving title page pops that page. The same route function is used for composition, root Back and the ownership check.
One Back from a Home row focuses the nav only, and a second Back from the nav exits. The picker behaves as before.

API 34 and 36.

On the box (Android 14, manifest predictive flag not set) the raw Back key goes unconsumed through Compose to onBackPressed and the dispatcher. [INFERENCE: not run on a device]
On Android 16 (targetSdk 37) the new tests dispatch predictive start, progress and completion and assert one pop.

Tests.

TitleEntryBackTest and WolfskinRootExitTest run on sdk 34 and 36 with a real NavDisplay, the real activity dispatcher and raw KEYCODE_BACK. They cover the second Back before recomposition and the Home-row down/up path.
The stored before-fix XML shows the raw-key, second-Back and Home-row tests failing on both SDKs with the intended messages. Design-suite logical-Back replays moved to Escape with unchanged assertions.
Caveat the PR states: two raw-key title drivers inject key-down, assert no navigation, then drive the dispatcher's predictive completion. Robolectric does not synthesize that completion from a raw key-up.

Unrelated screens and merge. No layout, token or logic changes elsewhere. The Robolectric Android 16 runtime and its --add-exports are test-worker only. A merge-tree check against current fm/tv-round2 (985ebc53) is clean.

Non-blocking notes:

handleWolfskinBack and handleBack use backStack.last(), which throws on an empty stack, where the old code used lastOrNull(). The stack is only cleared and refilled synchronously on the main thread, so this cannot interleave. A lastOrNull() ?: return would be a free safety.
I could not reproduce the exact on-device mechanism that turned one Back into an exit. The PR proves the premature key-down pop and the retained-handler second Back natively and removes the double ingress by design. The integrated emulator re-walk is still needed: title then one Back lands on Home, Home row then one Back focuses the nav, nav then Back exits, and a fast double Back from a title.
A predictive gesture on a screen where content consumes Back (for example the Updates notes) with NavDisplay's handler on top could replay NavDisplay's back animation without a pop. Cosmetic. [INFERENCE]
Back now acts at system completion (key-up) instead of key-down, which is the point of the fix.

Full report (review 6): /root/.treehouse/firstmate-7bab20/2/firstmate/data/round2-review/report.md. Not run: Gradle/make, nothing on the box.

Review of the Back fix at head `a268cdd7b8d887157bcf6749a54c0f9ab0ccf4f1` (base `fm/tv-round2`). **Code verdict: APPROVE (`review/code-claude` = success).** **Design verdict: PASS (`review/design-claude` = success).** Nothing visual changes. **One owner per Back.** - `readShellKey` no longer maps `Key.Back`, only Escape. I found no other `Key.Back` or `KEYCODE_BACK` in the Wolfskin sources. Hardware Back therefore reaches Wolfskin only through the Activity dispatcher. - The dispatcher runs one callback per Back: the title/person handler, then NavDisplay's handler, then the root handler. - The title handler still gives sheets, menus, pickers and request overlays first refusal (`TitleState.back`). - Home, Library, Discover overlays, the Updates notes, the requests sheet and the picker are reached through `shell.key(Back)`, so their priority order is preserved. - Exit happens only when Back is pressed on a focused nav, or from `goBack` on a one-entry stack. - A retained title/person callback now checks the live top of the back stack when it runs. If its entry has already popped it hands Back to the current destination and syncs the shell route first. A second Back before recomposition therefore focuses the Home nav instead of popping again or exiting, and the third Back exits. A retained handler over a surviving title page pops that page. The same route function is used for composition, root Back and the ownership check. - One Back from a Home row focuses the nav only, and a second Back from the nav exits. The picker behaves as before. **API 34 and 36.** - On the box (Android 14, manifest predictive flag not set) the raw Back key goes unconsumed through Compose to `onBackPressed` and the dispatcher. [INFERENCE: not run on a device] - On Android 16 (targetSdk 37) the new tests dispatch predictive start, progress and completion and assert one pop. **Tests.** - `TitleEntryBackTest` and `WolfskinRootExitTest` run on sdk 34 and 36 with a real NavDisplay, the real activity dispatcher and raw `KEYCODE_BACK`. They cover the second Back before recomposition and the Home-row down/up path. - The stored before-fix XML shows the raw-key, second-Back and Home-row tests failing on both SDKs with the intended messages. Design-suite logical-Back replays moved to Escape with unchanged assertions. - Caveat the PR states: two raw-key title drivers inject key-down, assert no navigation, then drive the dispatcher's predictive completion. Robolectric does not synthesize that completion from a raw key-up. **Unrelated screens and merge.** No layout, token or logic changes elsewhere. The Robolectric Android 16 runtime and its `--add-exports` are test-worker only. A merge-tree check against current `fm/tv-round2` (`985ebc53`) is clean. **Non-blocking notes:** 1. `handleWolfskinBack` and `handleBack` use `backStack.last()`, which throws on an empty stack, where the old code used `lastOrNull()`. The stack is only cleared and refilled synchronously on the main thread, so this cannot interleave. A `lastOrNull() ?: return` would be a free safety. 2. I could not reproduce the exact on-device mechanism that turned one Back into an exit. The PR proves the premature key-down pop and the retained-handler second Back natively and removes the double ingress by design. The integrated emulator re-walk is still needed: title then one Back lands on Home, Home row then one Back focuses the nav, nav then Back exits, and a fast double Back from a title. 3. A predictive gesture on a screen where content consumes Back (for example the Updates notes) with NavDisplay's handler on top could replay NavDisplay's back animation without a pop. Cosmetic. [INFERENCE] 4. Back now acts at system completion (key-up) instead of key-down, which is the point of the fix. Full report (review 6): `/root/.treehouse/firstmate-7bab20/2/firstmate/data/round2-review/report.md`. Not run: Gradle/make, nothing on the box.