Route Wolfskin setup and recovery through its own profile picker #63

Closed

justinzeus wants to merge 0 commits from fm/tv-skip-stock-setup into fm/tv-round2

justinzeus commented

2026-11-07 20:47:49 +01:00

Owner

Outcome

When Wolfskin screens is enabled, the stock Wholphin server and user selectors no longer render. Household setup, profile choice and failed-login recovery stay in Wolfskin. The rollback switch still exposes both original selectors.

Root cause

MainContent rendered SwitchServerContent / SwitchUserContent before the signed-in WolfskinApp hook. The stock user selector also calls SwitchUserViewModel.init → ServerRepository.switchServerOrUser, clearing the remembered server and user merely by visiting the screen. That can send the next start back to server selection.

The fork has a compiled Conductarr URL, but no compiled Jellyfin URL. Setup therefore recovers the supplied/current/recent server, then the most recently used Room server, before offering the approved one-time URL field on an unconfigured install. No new endpoint is hardcoded.

Change

Add one render-boundary interception point in MainContent, gated by the existing rollout preference. Navigation destination types remain compatible with upstream.
Read saved profiles locally without network validation or preference clearing. Use the existing Jellyfin discovery, Room persistence and authentication contracts for first-run connection and renewed credentials.
Enforce PIN and requireLogin; expired saved tokens open real credential entry. Failed selection/public-profile reads expose Wolfskin retry/sign-in actions. Retry reruns household startup server/profile recovery through an explicit controller callback and publishes Loading before its result.
Route expired-current-login failures from startup revalidation and foreground refresh to the picker. Ignore outages and failures belonging to a replaced identity.
Keep stock selector implementations untouched. Preserve the existing debug fixture seeding path. Document the flow in docker/README.md.

Every stock-selector entry path inspected

Entry / trigger	Wolfskin result
Cold start with automatic sign-in disabled, including activity restart after backgrounding	Saved server → own picker; missing recent preference → recover Room household first.
First run, missing server row, or no saved endpoint (`RestoredSession.None` → `ServerList`)	Recover a saved Room server if possible; otherwise own one-time URL state, discovery and retry.
Protected or missing saved user (`RestoredSession.ServerOnly` → `UserList`)	Own picker; PIN or server credentials required on selection.
Successful unprotected saved-session restore / hot reload (`AppContent`)	Existing Wolfskin launch/profile picker; expired startup or foreground validation now returns to own setup.
`MainActivityViewModel.appStart` exception fallback → `ServerList`	Own household recovery/configuration, never the stock server list.
Intent parsing/authentication/restoration error → `UserList(current.server)` or `ServerList`; valid intent target without a current user → `ServerList`	Same own picker/recovery boundary. Includes failed `IntentService` session restoration.
Old Home / `NavDrawer` user-avatar switch → `UserList(server)`	Own picker. No call to stock `SwitchUserViewModel.init`.
Legacy sign-out-equivalent state after `switchServerOrUser` cleared preferences	Room household recovery and own picker. There is no separate Jellyfin sign-out handler in these source sets; the stock switch-user preference clearing is the relevant legacy path. Entering Wolfskin setup never performs that clearing.
Stock add-server / switch-server actions → `UserList`; stock user's switch-server action → `ServerList`	These controls remain available in rollback only; the boundary also intercepts either destination if Wolfskin is enabled.
Missing/protected saved profile or failed login during the live Wolfskin profile switch	Hand off to own setup instead of silently stopping or showing the Conductarr-specific unavailable copy.
Saved server unreachable, public profiles unavailable, expired token, or unauthorized selection	Own retry/sign-in state; saved profiles remain local. An already signed-in library outage retains the existing Wolfskin Home recovery path, not a stock selector.
Settings	No independent stock server/user selector entry exists. Setup's own Settings gear offers another profile's credential entry. Advanced → Wolfskin screens deliberately restores original selectors when disabled.

Verification

12 tests passed; zero failures, errors or skipped tests. Only the three added WolfskinSetup* classes ran:

WolfskinSetupTest: 8 controller/auth regressions with real Room and remembered-server preferences; endpoint/user preservation, expired-token renewal, PIN/require-login gates, first-run discovery, public-profile failure, late-identity/rollback protection, and Unavailable → Loading → Picker/Unavailable retry transitions.
WolfskinSetupNavigationTest: 3 actual MainContent surface scenarios covering cold start, legacy sign-out state, drawer switch, intent failure, unconfigured first run, and the unreachable-household frame without either stock selector.
WolfskinSetupRollbackTest: 1 Hilt-backed test rendering both real stock selectors with Wolfskin disabled.

Executed through the serialized Gradle wrapper, using a task-local debug keystore:

bash docker/gradle.sh debug :wolfskin-design:cacheRobolectricSdk   :app:testDefaultDebugUnitTest   --tests 'org.justinvisser.wolfskin.tv.WolfskinSetup*'   -x :wolfskin-design:testDebugUnitTest

The brittle full-MainContent native retry-click driver was replaced with controller-level transition coverage, as directed; it is not skipped or time-padded. Exactly one native 1920×1080 same-data unreachable-household frame remains, reviewed against the existing token-backed palette, typography and layout. Proof and final XML are retained at /root/.treehouse/firstmate-7bab20/2/firstmate/data/tv-skip-stock-setup/proof/.

No full test suite, APK/Opt build, install, ADB, TV launch or TV screenshot was performed. Full merged-candidate/Opt validation remains the integration gate. Home rows, hero motion and existing Home Back handling are untouched. Base is fm/tv-round2; no merge requested or performed.

Implementation and verification were AI-assisted.

## Outcome When **Wolfskin screens** is enabled, the stock Wholphin server and user selectors no longer render. Household setup, profile choice and failed-login recovery stay in Wolfskin. The rollback switch still exposes both original selectors. ## Root cause `MainContent` rendered `SwitchServerContent` / `SwitchUserContent` before the signed-in `WolfskinApp` hook. The stock user selector also calls `SwitchUserViewModel.init` → `ServerRepository.switchServerOrUser`, clearing the remembered server **and** user merely by visiting the screen. That can send the next start back to server selection. The fork has a compiled Conductarr URL, but no compiled Jellyfin URL. Setup therefore recovers the supplied/current/recent server, then the most recently used Room server, before offering the approved one-time URL field on an unconfigured install. No new endpoint is hardcoded. ## Change - Add one render-boundary interception point in `MainContent`, gated by the existing rollout preference. Navigation destination types remain compatible with upstream. - Read saved profiles locally without network validation or preference clearing. Use the existing Jellyfin discovery, Room persistence and authentication contracts for first-run connection and renewed credentials. - Enforce PIN and `requireLogin`; expired saved tokens open real credential entry. Failed selection/public-profile reads expose Wolfskin retry/sign-in actions. Retry reruns household startup server/profile recovery through an explicit controller callback and publishes `Loading` before its result. - Route expired-current-login failures from startup revalidation and foreground refresh to the picker. Ignore outages and failures belonging to a replaced identity. - Keep stock selector implementations untouched. Preserve the existing debug fixture seeding path. Document the flow in `docker/README.md`. ## Every stock-selector entry path inspected | Entry / trigger | Wolfskin result | | --- | --- | | Cold start with automatic sign-in disabled, including activity restart after backgrounding | Saved server → own picker; missing recent preference → recover Room household first. | | First run, missing server row, or no saved endpoint (`RestoredSession.None` → `ServerList`) | Recover a saved Room server if possible; otherwise own one-time URL state, discovery and retry. | | Protected or missing saved user (`RestoredSession.ServerOnly` → `UserList`) | Own picker; PIN or server credentials required on selection. | | Successful unprotected saved-session restore / hot reload (`AppContent`) | Existing Wolfskin launch/profile picker; expired startup or foreground validation now returns to own setup. | | `MainActivityViewModel.appStart` exception fallback → `ServerList` | Own household recovery/configuration, never the stock server list. | | Intent parsing/authentication/restoration error → `UserList(current.server)` or `ServerList`; valid intent target without a current user → `ServerList` | Same own picker/recovery boundary. Includes failed `IntentService` session restoration. | | Old Home / `NavDrawer` user-avatar switch → `UserList(server)` | Own picker. No call to stock `SwitchUserViewModel.init`. | | Legacy sign-out-equivalent state after `switchServerOrUser` cleared preferences | Room household recovery and own picker. There is no separate Jellyfin sign-out handler in these source sets; the stock switch-user preference clearing is the relevant legacy path. Entering Wolfskin setup never performs that clearing. | | Stock add-server / switch-server actions → `UserList`; stock user's switch-server action → `ServerList` | These controls remain available in rollback only; the boundary also intercepts either destination if Wolfskin is enabled. | | Missing/protected saved profile or failed login during the live Wolfskin profile switch | Hand off to own setup instead of silently stopping or showing the Conductarr-specific unavailable copy. | | Saved server unreachable, public profiles unavailable, expired token, or unauthorized selection | Own retry/sign-in state; saved profiles remain local. An already signed-in library outage retains the existing Wolfskin Home recovery path, not a stock selector. | | Settings | No independent stock server/user selector entry exists. Setup's own Settings gear offers another profile's credential entry. Advanced → Wolfskin screens deliberately restores original selectors when disabled. | ## Verification **12 tests passed; zero failures, errors or skipped tests.** Only the three added `WolfskinSetup*` classes ran: - `WolfskinSetupTest`: 8 controller/auth regressions with real Room and remembered-server preferences; endpoint/user preservation, expired-token renewal, PIN/require-login gates, first-run discovery, public-profile failure, late-identity/rollback protection, and `Unavailable → Loading → Picker/Unavailable` retry transitions. - `WolfskinSetupNavigationTest`: 3 actual `MainContent` surface scenarios covering cold start, legacy sign-out state, drawer switch, intent failure, unconfigured first run, and the unreachable-household frame without either stock selector. - `WolfskinSetupRollbackTest`: 1 Hilt-backed test rendering both real stock selectors with Wolfskin disabled. Executed through the serialized Gradle wrapper, using a task-local debug keystore: ```sh bash docker/gradle.sh debug :wolfskin-design:cacheRobolectricSdk :app:testDefaultDebugUnitTest --tests 'org.justinvisser.wolfskin.tv.WolfskinSetup*' -x :wolfskin-design:testDebugUnitTest ``` The brittle full-`MainContent` native retry-click driver was replaced with controller-level transition coverage, as directed; it is not skipped or time-padded. Exactly one native 1920×1080 same-data unreachable-household frame remains, reviewed against the existing token-backed palette, typography and layout. Proof and final XML are retained at `/root/.treehouse/firstmate-7bab20/2/firstmate/data/tv-skip-stock-setup/proof/`. No full test suite, APK/Opt build, install, ADB, TV launch or TV screenshot was performed. Full merged-candidate/Opt validation remains the integration gate. Home rows, hero motion and existing Home Back handling are untouched. Base is `fm/tv-round2`; no merge requested or performed. Implementation and verification were AI-assisted.

justinzeus added 1 commit 2026-11-07 20:47:49 +01:00

Route Wolfskin setup through the household profile picker

review/code-claude Opus review PASS: no blockers, 10 non-blocking notes

review/safety-opus Opus review PASS: no blockers, 10 non-blocking notes

a5b5e92017

justinzeus commented

2026-11-07 20:57:17 +01:00

Author

Owner

Review (Opus, behaviour + safety): PASS, no blocking findings

Reviewed head: a5b5e9201741693b4d4f922ae9e6d945b9ff58ce. Statuses review/code-claude and review/safety-opus are set to success on this SHA.

Safety checks, all passed

No stock selector with Wolfskin screens on. The stock selectors render in exactly one place (MainContent), and the new interceptor runs before it inside every NavEntry. Every route into server or user selection (app-start exception, intent errors, saved-session restore, drawer, stock add/switch) lands in Wolfskin setup. The rollout switch is read from SharedPreferences before it is used, so a cold start never acts on the default. With the switch off, both stock selectors render unchanged.
PIN and require-login still enforced. A profile with a PIN or requireLogin (or no saved token) cannot reach changeUser without the PIN or real credentials. Expired token → credential entry. A failed login leaves the current user, the remembered server/user and the shared Jellyfin client untouched. No password or token is stored or logged. The password field is masked.
Late failures ignored. An expired-login failure from another session is dropped (current === expected). Outages, 5xx and 403 are ignored. A 401 only opens the picker and clears nothing, so there is no sign-out or picker↔Home loop.
Saved household never cleared or overwritten. Setup never calls switchServerOrUser or any other clearing path. The one-time URL field appears only when Room holds no server at all, and it saves a server only after discovery verifies it, same checks as upstream.
No dead ends. Every failure (unreachable, public profiles unavailable, 403, Room read error, startup/intent error) offers Try again and/or Sign in. Retry shows Loading and reruns the startup recovery.

Non-blocking notes

Room fallback can pick the wrong server. JellyfinServer.lastUsed is never written, so the fallback takes the first Room row, not the most recently used server. That is fine with one server. With several saved servers it can open on the wrong one, and setup has no way to switch server. Suggest choosing by the newest user lastUsed. The README wording is inaccurate.
Back on the setup picker. After any profile choice, pickerLaunch becomes false. A later Back then shows the shell nav and the "not built in this slice" placeholder inside setup. It is recoverable and exposes no data. After the Home PR (#59) merges, Back on the launch picker sends ShellAction.Exit, which setup ignores, so Back never exits from setup.
Re-sign-in with a prefilled profile (inherited from upstream): (a) a successful server sign-in removes the profile's PIN; (b) editing the username to another person overwrites the prefilled person's saved Room row. Suggest passing existingUser only when the authenticated user id matches.
Pre-existing: a failed switch leaves the shared Jellyfin client on the new person's token while the current user stays the old one. Not introduced by this PR.
Rollback behaviour changed. With the switch off, a protected or failed selection in the Wolfskin picker now opens the stock user selector, which clears the remembered server/user (stock behaviour). The base did nothing in that case. This is not documented.
Retry drops the hint. retry() reloads with load(null), ignoring the UserList(server) destination.
Copy: the first-run button reads "Try again" before any attempt. The username field lacks autocorrect-off and no-capitalisation, and there is no Go key action.
Unreachable-household frame: it uses only approved tokens, but it does not follow the approved picker-recovery layout (panel, alert glyph, warn heading). The 5dp gap between body copy and buttons looks cramped.
Test gaps: login 401 not changing the current user; the Wolfskin picker's protected/failed hand-off; multi-server Room fallback. The legacy sign-out step reuses a model already at Picker, so it does not prove Room recovery (a controller test does). Only the 3 new test classes ran; the full suite is left to the integration gate.
Merges: clean pairwise against #58, #59, #61 and #62 (git merge-tree exit 0). The only conflict is between #58 and #59 (DiscoverScreen.kt), which does not involve this PR. Semantic overlap with #59 is in note 2.

Full report: firstmate/data/review-63-auth/report.md.

## Review (Opus, behaviour + safety): **PASS, no blocking findings** Reviewed head: `a5b5e9201741693b4d4f922ae9e6d945b9ff58ce`. Statuses `review/code-claude` and `review/safety-opus` are set to success on this SHA. **Safety checks, all passed** - **No stock selector with Wolfskin screens on.** The stock selectors render in exactly one place (`MainContent`), and the new interceptor runs before it inside every `NavEntry`. Every route into server or user selection (app-start exception, intent errors, saved-session restore, drawer, stock add/switch) lands in Wolfskin setup. The rollout switch is read from SharedPreferences before it is used, so a cold start never acts on the default. With the switch off, both stock selectors render unchanged. - **PIN and require-login still enforced.** A profile with a PIN or `requireLogin` (or no saved token) cannot reach `changeUser` without the PIN or real credentials. Expired token → credential entry. A failed login leaves the current user, the remembered server/user and the shared Jellyfin client untouched. No password or token is stored or logged. The password field is masked. - **Late failures ignored.** An expired-login failure from another session is dropped (`current === expected`). Outages, 5xx and 403 are ignored. A 401 only opens the picker and clears nothing, so there is no sign-out or picker↔Home loop. - **Saved household never cleared or overwritten.** Setup never calls `switchServerOrUser` or any other clearing path. The one-time URL field appears only when Room holds no server at all, and it saves a server only after discovery verifies it, same checks as upstream. - **No dead ends.** Every failure (unreachable, public profiles unavailable, 403, Room read error, startup/intent error) offers **Try again** and/or **Sign in**. Retry shows Loading and reruns the startup recovery. **Non-blocking notes** 1. **Room fallback can pick the wrong server.** `JellyfinServer.lastUsed` is never written, so the fallback takes the first Room row, not the most recently used server. That is fine with one server. With several saved servers it can open on the wrong one, and setup has no way to switch server. Suggest choosing by the newest user `lastUsed`. The README wording is inaccurate. 2. **Back on the setup picker.** After any profile choice, `pickerLaunch` becomes false. A later Back then shows the shell nav and the "not built in this slice" placeholder inside setup. It is recoverable and exposes no data. After the Home PR (#59) merges, Back on the launch picker sends `ShellAction.Exit`, which setup ignores, so Back never exits from setup. 3. **Re-sign-in with a prefilled profile** (inherited from upstream): (a) a successful server sign-in removes the profile's PIN; (b) editing the username to another person overwrites the prefilled person's saved Room row. Suggest passing `existingUser` only when the authenticated user id matches. 4. **Pre-existing:** a failed switch leaves the shared Jellyfin client on the new person's token while the current user stays the old one. Not introduced by this PR. 5. **Rollback behaviour changed.** With the switch off, a protected or failed selection in the Wolfskin picker now opens the stock user selector, which clears the remembered server/user (stock behaviour). The base did nothing in that case. This is not documented. 6. **Retry drops the hint.** `retry()` reloads with `load(null)`, ignoring the `UserList(server)` destination. 7. **Copy:** the first-run button reads "Try again" before any attempt. The username field lacks autocorrect-off and no-capitalisation, and there is no Go key action. 8. **Unreachable-household frame:** it uses only approved tokens, but it does not follow the approved picker-recovery layout (panel, alert glyph, warn heading). The 5dp gap between body copy and buttons looks cramped. 9. **Test gaps:** login 401 not changing the current user; the Wolfskin picker's protected/failed hand-off; multi-server Room fallback. The legacy sign-out step reuses a model already at Picker, so it does not prove Room recovery (a controller test does). Only the 3 new test classes ran; the full suite is left to the integration gate. 10. **Merges:** clean pairwise against #58, #59, #61 and #62 (`git merge-tree` exit 0). The only conflict is between #58 and #59 (`DiscoverScreen.kt`), which does not involve this PR. Semantic overlap with #59 is in note 2. Full report: `firstmate/data/review-63-auth/report.md`.