fix(appMode): also preserve a fresh cross-tab hint through the AuthProvider hydrate - #31335
Merged
Merged
Conversation
…ovider hydrate Follow-up to #31316. That PR taught `hydrateAndResolveAppMode` to skip its boot-time `writeAppMode` when a session tuple already exists — fine for a returning tab or an in-tab toggle. But the *newly-opened* tab path (cmd+click from an AI tab, or any browser-issued "new tab in the same context") starts with an empty session tuple and a *fresh* `omAppModeHint` in localStorage. My previous fix passed straight through that guard and wrote `DEFAULT_APP_MODE` to the session tuple, which then satisfied `useResolvedAppMode`'s "valid session" check — the resolver returned early and never consulted the hint. Result: the sibling tab's active mode failed to carry, breaking exactly the cross-tab-inheritance behaviour the hint exists for. Extend the guard to skip the write when either signal is present: 1. A session tuple this tab already owns. 2. A fresh cross-tab hint (`isAppModeHintFresh(readAppModeHint())`). `useResolvedAppMode` already handles the hint-adoption path correctly once the tuple isn't in the way — it writes the hint's mode if it's registered, waits for the registry when it isn't, and falls through cleanly if the hint isn't fresh. Fresh boot with neither signal is unchanged. Fixes the following Collate Playwright regressions that #31316 alone did not resolve: - AppModeResolver.spec.ts:137 (new tab in the same browser inherits AI mode via the cross-tab hint) - AppMode.spec.ts:155 (uninstall → refresh → reinstall → refresh lands user in default mode) - AppModeSidebarState.spec.ts:139 (sub-nav resets when the user re-enters AI mode) A stale (TTL-expired) hint continues to fall through to the default — `isAppModeHintFresh` returns false and the guard doesn't fire — so `AppModeResolver.spec.ts:227` ("stale hint does not carry AI") remains green.
chirag-madlani
requested review from
ShaileshParmar11 and
karanh37
as code owners
August 11, 2026 10:19
Contributor
❌ PR checklist incompleteThis PR cannot be merged until the following are addressed on its linked issue:
The fields live on the linked issue in the Shipping project (open the issue → right sidebar → Projects). After you set them, re-run this check (or push a commit) — issue/project changes do not re-trigger it automatically. Maintainers can bypass this check by adding the |
github-actions
Bot
added
safe to test
chirag-madlani
enabled auto-merge
August 11, 2026 10:22
harsh-vador
approved these changes
Aug 11, 2026
Code Review ✅ ApprovedExtends the OptionsDisplay: compact → Showing less information. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Powered by Gitar — free for open source |
karanh37
approved these changes
Aug 11, 2026
Contributor
|
| Count | Rule |
|---|---|
| 9 | react-hooks/exhaustive-deps |
| 3 | sonarjs/no-nested-functions |
| 1 | sonarjs/cyclomatic-complexity |
All findings
| Location | Rule | Message | |
|---|---|---|---|
| 🟡 | src/components/Auth/AuthProviders/AuthProvider.tsx:262:9 |
react-hooks/exhaustive-deps |
The 'onLoginHandler' function makes the dependencies of useMemo Hook (at line 900) change on every render. Move it inside the useMemo callback. Alternatively, w |
| 🟡 | src/components/Auth/AuthProviders/AuthProvider.tsx:339:6 |
react-hooks/exhaustive-deps |
React Hook useCallback has missing dependencies: 'navigate', 'setApplicationLoading', 'setCurrentUser', and 'setIsAuthenticated'. Either include them or remove |
| 🟡 | src/components/Auth/AuthProviders/AuthProvider.tsx:397:9 |
react-hooks/exhaustive-deps |
The 'resetUserDetails' function makes the dependencies of useMemo Hook (at line 900) change on every render. To fix this, wrap the definition of 'resetUserDetai |
| 🟡 | src/components/Auth/AuthProviders/AuthProvider.tsx:479:6 |
react-hooks/exhaustive-deps |
React Hook useEffect has a missing dependency: 'startTokenExpiryTimer'. Either include it or remove the dependency array. |
| 🟡 | src/components/Auth/AuthProviders/AuthProvider.tsx:522:6 |
react-hooks/exhaustive-deps |
React Hook useEffect has a missing dependency: 'startTokenExpiryTimer'. Either include it or remove the dependency array. |
| 🟡 | src/components/Auth/AuthProviders/AuthProvider.tsx:532:9 |
react-hooks/exhaustive-deps |
The 'handleFailedLogin' function makes the dependencies of useMemo Hook (at line 900) change on every render. Move it inside the useMemo callback. Alternatively |
| 🟡 | src/components/Auth/AuthProviders/AuthProvider.tsx:590:5 |
react-hooks/exhaustive-deps |
React Hook useCallback has missing dependencies: 'authConfig?.provider', 'handledVerifiedUser', 'navigate', 'resetUserDetails', and 'startTokenExpiryTimer'. Eit |
| 🟡 | src/components/Auth/AuthProviders/AuthProvider.tsx:635:9 |
react-hooks/exhaustive-deps |
The 'initializeAxiosInterceptors' function makes the dependencies of useMemo Hook (at line 900) change on every render. To fix this, wrap the definition of 'ini |
| 🟡 | src/components/Auth/AuthProviders/AuthProvider.tsx:703:67 |
sonarjs/no-nested-functions |
Refactor this code to not nest functions more than 4 levels deep. |
| 🟡 | src/components/Auth/AuthProviders/AuthProvider.tsx:722:37 |
sonarjs/no-nested-functions |
Refactor this code to not nest functions more than 4 levels deep. |
| 🟡 | src/components/Auth/AuthProviders/AuthProvider.tsx:731:27 |
sonarjs/no-nested-functions |
Refactor this code to not nest functions more than 4 levels deep. |
| 🟡 | src/components/Auth/AuthProviders/AuthProvider.tsx:800:30 |
sonarjs/cyclomatic-complexity |
{"message":"Function has a complexity of 17 which is greater than 10 authorized.","cost":7,"secondaryLocations":[{"line":800,"column":29,"endLine":800,"endColum |
| 🟡 | src/components/Auth/AuthProviders/AuthProvider.tsx:889:6 |
react-hooks/exhaustive-deps |
React Hook useEffect has missing dependencies: 'cleanup', 'fetchAuthConfig', 'initializeAxiosInterceptors', and 'startTokenExpiryTimer'. Either include them or |
Fix locally (fast - only checks files changed in this branch):
make ui-checkstyle-changed
Contributor
Jest test CoverageUI tests summary
|
|
Contributor
✅ Playwright Results — workflow succeededValidated commit ✅ 321 passed · ❌ 0 failed · 🟡 0 flaky · ⏭️ 0 skipped · 🧰 0 lifecycle flaky PerformanceBlocking targets: ✅ met · Optimization targets: 🟡 in progress Shard-job maxima below are not the full workflow wall time; the linked run includes build, fixture, planning, and reporting. 🕒 Full workflow signal wall (to summary) 44m 1s ⏱️ Max setup 1m 40s · max shard execution 10m 32s · max shard-job elapsed before upload 14m 15s · reporting 3s 🌐 211.65 requests/attempt · 2.22 app boots/UI scenario · 8.54% common-shard skew Optimization targets still in progress:
How to debug locally# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip # view trace |
Merged
5 tasks
k-anshul
pushed a commit
to k-anshul/OpenMetadata
that referenced
this pull request
Aug 12, 2026
…ovider hydrate (open-metadata#31335) Follow-up to open-metadata#31316. That PR taught `hydrateAndResolveAppMode` to skip its boot-time `writeAppMode` when a session tuple already exists — fine for a returning tab or an in-tab toggle. But the *newly-opened* tab path (cmd+click from an AI tab, or any browser-issued "new tab in the same context") starts with an empty session tuple and a *fresh* `omAppModeHint` in localStorage. My previous fix passed straight through that guard and wrote `DEFAULT_APP_MODE` to the session tuple, which then satisfied `useResolvedAppMode`'s "valid session" check — the resolver returned early and never consulted the hint. Result: the sibling tab's active mode failed to carry, breaking exactly the cross-tab-inheritance behaviour the hint exists for. Extend the guard to skip the write when either signal is present: 1. A session tuple this tab already owns. 2. A fresh cross-tab hint (`isAppModeHintFresh(readAppModeHint())`). `useResolvedAppMode` already handles the hint-adoption path correctly once the tuple isn't in the way — it writes the hint's mode if it's registered, waits for the registry when it isn't, and falls through cleanly if the hint isn't fresh. Fresh boot with neither signal is unchanged. Fixes the following Collate Playwright regressions that open-metadata#31316 alone did not resolve: - AppModeResolver.spec.ts:137 (new tab in the same browser inherits AI mode via the cross-tab hint) - AppMode.spec.ts:155 (uninstall → refresh → reinstall → refresh lands user in default mode) - AppModeSidebarState.spec.ts:139 (sub-nav resets when the user re-enters AI mode) A stale (TTL-expired) hint continues to fall through to the default — `isAppModeHintFresh` returns false and the guard doesn't fire — so `AppModeResolver.spec.ts:227` ("stale hint does not carry AI") remains green.
timothybrush
pushed a commit
to timothybrush/OpenMetadata
that referenced
this pull request
Aug 13, 2026
…le wins unconditionally (open-metadata#31369) * fix(appMode): unify precedence between boot and resolver; session tuple wins unconditionally The AppMode feature had two contradictory precedence chains — the async runtime resolver used one, the boot-time pre-session write used another — and the runtime resolver silently clobbered a valid session tuple whenever the persona changed. This PR fixes both. ## Unified precedence `resolveEffectiveAppMode(userPref, personaMode, appDefault)` in `useAppMode.ts` is now the single source of truth for the "no session tuple, no fresh hint" case: userPref -> personaMode -> appDefault -> DEFAULT_APP_MODE Both entry points use it: * `AuthProvider.hydrateAndResolveAppMode` (boot-time pre-session write — passes `null` for persona, since persona isn't known synchronously). * `useResolvedAppMode`'s async `candidate` (passes the resolved persona once its docStore doc has settled). Before this PR the resolver placed persona BEFORE userPref in its `candidate` chain (`useResolvedAppMode.ts:236-240`, deleted here), which meant an admin persona edit silently overrode a user's server- side "remember" choice for the resolver run — while the boot-time write kept respecting userPref. The two writes disagreed within one session. ## Session tuple wins unconditionally `useResolvedAppMode.ts:204` previously kept the session only when `session.personaAppMode === currentPersonaAppMode`. If persona had changed since the tuple was written, the resolver fell through and overwrote the user's manual in-tab switch. Rung 1 of the target precedence is "manual in-tab switch wins" — an admin persona edit must not silently revert a user's right-now click. Drop the persona- match check: `if (validSession) return;`. ## New precedence (final) Top wins: 1. Session tuple (manual in-tab switch) — sessionStorage, per-tab. 2. Fresh cross-tab hint (`omAppModeHint`, TTL-gated) — localStorage. 3. User pref (`user_preferences.appMode`, server-persisted "remember"). 4. Persona (docStore `personaPreferences[].appMode`). 5. Tenant default (`appConfiguration.defaultAppMode`). 6. `DEFAULT_APP_MODE` constant. Boot-time write (`hydrateAndResolveAppMode`) bails on rungs 1 & 2 (my earlier open-metadata#31316 + open-metadata#31335 guards, unchanged here). The remaining rungs (3 → 6) are what `resolveEffectiveAppMode` encodes. ## Tests * Rewrote `useResolvedAppMode.test.ts:248` ("persona overrides session when personaAppMode differs from tuple's snapshot") into a regression guard for the opposite: session survives persona change (rung 1). * Added three new tests covering rung transitions: - user pref beats persona - persona beats tenant default - tenant default beats DEFAULT * `beforeEach` now resets `setAppDefaultMode(null)` so tenant- default tests don't leak into siblings. All 69 existing + new tests pass. ## Not in scope `resolveInitialAppMode` (`useAppMode.ts:346`) still considers only session/hint/userPref — no persona, no tenant default. It's used by the post-login redirect ONLY to pick a landing route, and the resolver corrects a moment later. Fixing that helper is a separate cleanup. * fix(appMode): defer boot write when user has a persona but no server-side pref Under the unified precedence (userPref -> persona -> appDefault -> DEFAULT), `hydrateAndResolveAppMode` was writing to the session tuple at boot with `personaMode = null` (persona isn't known synchronously). If the user had no userPref but DID have a defaultPersona, the boot write computed the wrong candidate — falling through to `appDefault` or `DEFAULT_APP_MODE` instead of persona's opinion. Once the tuple was written, `useResolvedAppMode`'s `if (validSession) return;` guard kept the wrong mode; the persona- doc fetch that ran a moment later never got to influence the write. Symptom: `AppModePrecedence.spec.ts:242` (rung 4 persona beats rung 5 tenant default) — fresh user, persona=AI, tenant default=Classic, no user pref. Boot wrote 'default' (falling through to appDefault since persona wasn't known and userPref was null); resolver's session guard locked it. Expected 'ai', got 'default'. Fix: third guard in `hydrateAndResolveAppMode` — skip the boot write when `userPref === null && user.defaultPersona`. Defer to `useResolvedAppMode`, which fetches the persona doc and can compute the correct candidate against the full chain. When userPref IS set it wins over persona under the unified precedence, so writing at boot remains safe in that branch. All 69 existing Jest tests still pass. * fix(appMode): provisional boot-session; resolver overrides with persona-based candidate Rung 4 (persona beats tenant default) failed because AuthProvider's boot resolver writes a sessionStorage tuple *before* the persona doc is fetched, and `useResolvedAppMode`'s `if (validSession) return` locked in that pre-persona value forever. Deferring the boot write broke fast-boot rendering (the resolver's async chain — persona-doc fetch + registrySettled + install-gate — leaves the store in DEFAULT long enough to time out UI assertions). Fix: tag each tuple write with a `source` field: - 'manual' — UI toggle (default; sticky) - 'resolver' — useResolvedAppMode's authoritative write (sticky) - 'boot' — AuthProvider's pre-persona provisional write (overrideable once the resolver has persona context) The resolver only bails on non-'boot' sessions; a 'boot' tuple is cleared and the full precedence chain runs. `readSession` now preserves the field on the parse path so the guard is not a silent no-op. Boot writes do not update the cross-tab hint — a provisional value should not leak to sibling tabs as if the user chose it. Also revert the earlier Guard 3 in AuthProvider that tried to defer the boot write when the user has a persona but no server-side pref — that broke 6+ downstream tests by leaving the store in DEFAULT long enough for install-gated UI to time out. Adds a Jest regression test that seeds a `source: 'boot'` tuple, mocks the persona doc to return AI, and asserts the resolver overrides the provisional tuple with the persona-based candidate. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(appMode): preserve store during resolver's boot-provisional override The provisional-session fix cleared the boot tuple via `clearAppModeSessionOnly()` before the resolver's `writeAppMode(candidate)`. That helper calls `useAppModeStore.reset()` which sets currentMode to DEFAULT_APP_MODE. Zustand notifies subscribers synchronously, so between `.reset()` and the resolver's `setMode(candidate)`, one render fires with currentMode = DEFAULT. That's enough to route a non-default URL (/ai-automations, /observability/*, admin app-mode settings, any AI-only path) through the Classic route tree's `path='*'` catch-all and redirect to /404 — or, for pages the Classic tree lacks entirely, hang on the "loading" state past the test timeout. Fix: add `removeAppModeSession()` which drops ONLY the sessionStorage tuple and leaves the store untouched. The resolver's boot-override branch uses it in place of `clearAppModeSessionOnly()`. `writeAppMode(candidate)` then transitions the store from boot's value directly to the resolver's value with no intermediate reset — subscribers see a single mode change, not two. `clearAppModeSessionOnly()` stays as-is for the stale-mode-cleanup branch where resetting the store to DEFAULT is the intended behaviour (session was invalid, no candidate to write). Fixes AppMode Playwright tests on the Collate PR that navigated to AI routes or the admin app-mode settings and timed out on the transient DEFAULT render. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
chirag-madlani
added a commit
that referenced
this pull request
Aug 14, 2026
…ovider hydrate (#31335) Follow-up to #31316. That PR taught `hydrateAndResolveAppMode` to skip its boot-time `writeAppMode` when a session tuple already exists — fine for a returning tab or an in-tab toggle. But the *newly-opened* tab path (cmd+click from an AI tab, or any browser-issued "new tab in the same context") starts with an empty session tuple and a *fresh* `omAppModeHint` in localStorage. My previous fix passed straight through that guard and wrote `DEFAULT_APP_MODE` to the session tuple, which then satisfied `useResolvedAppMode`'s "valid session" check — the resolver returned early and never consulted the hint. Result: the sibling tab's active mode failed to carry, breaking exactly the cross-tab-inheritance behaviour the hint exists for. Extend the guard to skip the write when either signal is present: 1. A session tuple this tab already owns. 2. A fresh cross-tab hint (`isAppModeHintFresh(readAppModeHint())`). `useResolvedAppMode` already handles the hint-adoption path correctly once the tuple isn't in the way — it writes the hint's mode if it's registered, waits for the registry when it isn't, and falls through cleanly if the hint isn't fresh. Fresh boot with neither signal is unchanged. Fixes the following Collate Playwright regressions that #31316 alone did not resolve: - AppModeResolver.spec.ts:137 (new tab in the same browser inherits AI mode via the cross-tab hint) - AppMode.spec.ts:155 (uninstall → refresh → reinstall → refresh lands user in default mode) - AppModeSidebarState.spec.ts:139 (sub-nav resets when the user re-enters AI mode) A stale (TTL-expired) hint continues to fall through to the default — `isAppModeHintFresh` returns false and the guard doesn't fire — so `AppModeResolver.spec.ts:227` ("stale hint does not carry AI") remains green.
chirag-madlani
added a commit
that referenced
this pull request
Aug 14, 2026
…le wins unconditionally (#31369) * fix(appMode): unify precedence between boot and resolver; session tuple wins unconditionally The AppMode feature had two contradictory precedence chains — the async runtime resolver used one, the boot-time pre-session write used another — and the runtime resolver silently clobbered a valid session tuple whenever the persona changed. This PR fixes both. ## Unified precedence `resolveEffectiveAppMode(userPref, personaMode, appDefault)` in `useAppMode.ts` is now the single source of truth for the "no session tuple, no fresh hint" case: userPref -> personaMode -> appDefault -> DEFAULT_APP_MODE Both entry points use it: * `AuthProvider.hydrateAndResolveAppMode` (boot-time pre-session write — passes `null` for persona, since persona isn't known synchronously). * `useResolvedAppMode`'s async `candidate` (passes the resolved persona once its docStore doc has settled). Before this PR the resolver placed persona BEFORE userPref in its `candidate` chain (`useResolvedAppMode.ts:236-240`, deleted here), which meant an admin persona edit silently overrode a user's server- side "remember" choice for the resolver run — while the boot-time write kept respecting userPref. The two writes disagreed within one session. ## Session tuple wins unconditionally `useResolvedAppMode.ts:204` previously kept the session only when `session.personaAppMode === currentPersonaAppMode`. If persona had changed since the tuple was written, the resolver fell through and overwrote the user's manual in-tab switch. Rung 1 of the target precedence is "manual in-tab switch wins" — an admin persona edit must not silently revert a user's right-now click. Drop the persona- match check: `if (validSession) return;`. ## New precedence (final) Top wins: 1. Session tuple (manual in-tab switch) — sessionStorage, per-tab. 2. Fresh cross-tab hint (`omAppModeHint`, TTL-gated) — localStorage. 3. User pref (`user_preferences.appMode`, server-persisted "remember"). 4. Persona (docStore `personaPreferences[].appMode`). 5. Tenant default (`appConfiguration.defaultAppMode`). 6. `DEFAULT_APP_MODE` constant. Boot-time write (`hydrateAndResolveAppMode`) bails on rungs 1 & 2 (my earlier #31316 + #31335 guards, unchanged here). The remaining rungs (3 → 6) are what `resolveEffectiveAppMode` encodes. ## Tests * Rewrote `useResolvedAppMode.test.ts:248` ("persona overrides session when personaAppMode differs from tuple's snapshot") into a regression guard for the opposite: session survives persona change (rung 1). * Added three new tests covering rung transitions: - user pref beats persona - persona beats tenant default - tenant default beats DEFAULT * `beforeEach` now resets `setAppDefaultMode(null)` so tenant- default tests don't leak into siblings. All 69 existing + new tests pass. ## Not in scope `resolveInitialAppMode` (`useAppMode.ts:346`) still considers only session/hint/userPref — no persona, no tenant default. It's used by the post-login redirect ONLY to pick a landing route, and the resolver corrects a moment later. Fixing that helper is a separate cleanup. * fix(appMode): defer boot write when user has a persona but no server-side pref Under the unified precedence (userPref -> persona -> appDefault -> DEFAULT), `hydrateAndResolveAppMode` was writing to the session tuple at boot with `personaMode = null` (persona isn't known synchronously). If the user had no userPref but DID have a defaultPersona, the boot write computed the wrong candidate — falling through to `appDefault` or `DEFAULT_APP_MODE` instead of persona's opinion. Once the tuple was written, `useResolvedAppMode`'s `if (validSession) return;` guard kept the wrong mode; the persona- doc fetch that ran a moment later never got to influence the write. Symptom: `AppModePrecedence.spec.ts:242` (rung 4 persona beats rung 5 tenant default) — fresh user, persona=AI, tenant default=Classic, no user pref. Boot wrote 'default' (falling through to appDefault since persona wasn't known and userPref was null); resolver's session guard locked it. Expected 'ai', got 'default'. Fix: third guard in `hydrateAndResolveAppMode` — skip the boot write when `userPref === null && user.defaultPersona`. Defer to `useResolvedAppMode`, which fetches the persona doc and can compute the correct candidate against the full chain. When userPref IS set it wins over persona under the unified precedence, so writing at boot remains safe in that branch. All 69 existing Jest tests still pass. * fix(appMode): provisional boot-session; resolver overrides with persona-based candidate Rung 4 (persona beats tenant default) failed because AuthProvider's boot resolver writes a sessionStorage tuple *before* the persona doc is fetched, and `useResolvedAppMode`'s `if (validSession) return` locked in that pre-persona value forever. Deferring the boot write broke fast-boot rendering (the resolver's async chain — persona-doc fetch + registrySettled + install-gate — leaves the store in DEFAULT long enough to time out UI assertions). Fix: tag each tuple write with a `source` field: - 'manual' — UI toggle (default; sticky) - 'resolver' — useResolvedAppMode's authoritative write (sticky) - 'boot' — AuthProvider's pre-persona provisional write (overrideable once the resolver has persona context) The resolver only bails on non-'boot' sessions; a 'boot' tuple is cleared and the full precedence chain runs. `readSession` now preserves the field on the parse path so the guard is not a silent no-op. Boot writes do not update the cross-tab hint — a provisional value should not leak to sibling tabs as if the user chose it. Also revert the earlier Guard 3 in AuthProvider that tried to defer the boot write when the user has a persona but no server-side pref — that broke 6+ downstream tests by leaving the store in DEFAULT long enough for install-gated UI to time out. Adds a Jest regression test that seeds a `source: 'boot'` tuple, mocks the persona doc to return AI, and asserts the resolver overrides the provisional tuple with the persona-based candidate. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(appMode): preserve store during resolver's boot-provisional override The provisional-session fix cleared the boot tuple via `clearAppModeSessionOnly()` before the resolver's `writeAppMode(candidate)`. That helper calls `useAppModeStore.reset()` which sets currentMode to DEFAULT_APP_MODE. Zustand notifies subscribers synchronously, so between `.reset()` and the resolver's `setMode(candidate)`, one render fires with currentMode = DEFAULT. That's enough to route a non-default URL (/ai-automations, /observability/*, admin app-mode settings, any AI-only path) through the Classic route tree's `path='*'` catch-all and redirect to /404 — or, for pages the Classic tree lacks entirely, hang on the "loading" state past the test timeout. Fix: add `removeAppModeSession()` which drops ONLY the sessionStorage tuple and leaves the store untouched. The resolver's boot-override branch uses it in place of `clearAppModeSessionOnly()`. `writeAppMode(candidate)` then transitions the store from boot's value directly to the resolver's value with no intermediate reset — subscribers see a single mode change, not two. `clearAppModeSessionOnly()` stays as-is for the stale-mode-cleanup branch where resetting the store to DEFAULT is the intended behaviour (session was invalid, no candidate to write). Fixes AppMode Playwright tests on the Collate PR that navigated to AI routes or the admin app-mode settings and timed out on the transient DEFAULT render. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
6 of 8 tasks
This branch was previously deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to #31316. That PR taught `hydrateAndResolveAppMode` to skip its boot-time `writeAppMode` when a session tuple already exists — fine for a returning tab or an in-tab toggle. But the newly-opened tab path (cmd+click from an AI tab, or any browser-issued "new tab in the same context") starts with an empty session tuple and a fresh `omAppModeHint` in localStorage. My previous fix passed straight through that guard and wrote `DEFAULT_APP_MODE` to the session tuple, which then satisfied `useResolvedAppMode`'s "valid session" check — the resolver returned early and never consulted the hint. Result: the sibling tab's active mode failed to carry, breaking exactly the cross-tab-inheritance behaviour the hint exists for.
Fix
Extend the guard to skip the boot-time `writeAppMode` when either signal is present:
`useResolvedAppMode` already handles the hint-adoption path correctly once the tuple isn't in the way — it writes the hint's mode if it's registered, waits for the registry when it isn't, and falls through cleanly if the hint isn't fresh. Fresh boot with neither signal is unchanged.
Fixes these Collate Playwright regressions that #31316 alone did not resolve:
A stale (TTL-expired) hint continues to fall through to the default — `isAppModeHintFresh` returns false and the guard doesn't fire — so `AppModeResolver.spec.ts:227` ("stale hint does not carry AI") remains green.
Test plan
🤖 Generated with Claude Code