[dawsontoth]

Stacked on #1854. The order is #1813 → #1815 → #1816 → #1854 → this PR, and the base is claude/1329-scope-auth-checks-to-org. Retarget to stage once the stack below lands. It pairs with #1878, which is stacked on the same base and shares no lines with this PR.

Closes #1546. It takes all three: #1854, #1878 and this PR. Merge this one last, or drop the Closes line.

⊙ Problem

#1566 stopped the 403 polling loop and took most of #1546 with it. 403s on / and /sign-in fell from 159 a day to a few a day. I measured what's left in Datadog RUM over the last 14 days of production, about 1,200 sessions. Nearly all of the residue is one shape: someone opens a bookmark, or reloads a tab, after their Central Manager session has expired.

74 of the 90 CM 403s land within 10 s of that session's GET /User/current → 401, and 71 of them within 1 s. These are requests sent at page load, alongside the remembered-sign-in check, on a session that is already dead. Central manager answers a request with no session with 403, not 401 (checkUser → notAllowed(), filed as HarperFast/central-manager#938), so the 401 interceptor never sees them.

The matching 403 error events come to 80, across about 36 sessions:

• 59 are remembered-cluster probes. fix(auth): check a remembered cluster sign-in only when its organization is shown #1854 removes them.
• 11 are the org route's data failing before the check answers. The route's beforeLoad 403 is toasted ("Forbidden", and the toast stays up on the sign-in page) and sent to RUM through console.error. Show a failed page load only on its error page, without a duplicate toast or RUM report #1878 makes a failure a route load is awaiting report only through that route's error page.
• 10 are that error page rendering for a frame before the guard redirects. They come in as custom RUM errors from onCaughtError. With Describe 403 and 404 route failures in words, and tell a non-member they have no access to an organization #1804 that page tells the user they "don't have access to this organization", which is wrong for an expired session. With Show a failed page load only on its error page, without a duplicate toast or RUM report #1878 the error page becomes the only report, so it matters more that it does not render. This PR fixes these.
• A few more came from pages still mounted after a mid-session 401. Their queries rebuilt after the sign-out's cache clear and fetched again on the dead session (GET /Cluster and /HarperVersions, 220 ms after a /SystemStatus 401). This PR fixes these too.

Route preloads (beforeLoad → preloadRoute), which the issue blamed for part of the volume, account for 0 of these events in production over those 14 days.

❓ Your call: is the residue worth a change, given that #1854 already removes ~70% of the remaining events? I think yes. What's left (~20 sessions in 14 days, about 1.7%) shows the user wrong information, a permission error page, and the change is a three-line render gate. The alternative is to close #1546 once #1854 and #1878 land, and accept the error-page frame. Fully reversible.

💡 Solution

Dashboard renders its spinner, not its outlet, whenever there is no signed-in user. Before this, it only did so while the user was loading. Every dashboard route needs a user, and the dashboardLayout guard redirects otherwise, so this only changes the moment just before that redirect. That moment is exactly when the route error page rendered for an expired session, and when still-mounted pages re-rendered with no user, rebuilt the queries the sign-out had just cleared, and fetched again on the dead session.

With #1878, an expired session's deep link ends on sign-in with no error page, no toast and no RUM report. A signed-in non-member still gets the error page. I checked that by merging #1878's head (474b549) into this branch locally. This PR's mounted test passes with toast and 403-report assertions added, #1878's routeLoadFailureReporting.test.tsx and ensureRouteQueryData.test.ts pass, and tsc -b is clean on the merged tree.

⚖️ Alternatives

• Make the route guard wait for the sign-in check before any route data loads. That stops the requests from firing at all, but it puts them in series with /User/current for every signed-in cold load. From 7 days of RUM resource timings it adds +85 ms p50 / +262 ms p75 on org pages (n=32) and +287 ms p50 / +680 ms p75 on cluster pages (n=33). About 93% of those loads have a live session, so they would pay that to spare the other ~7% an error.
• Have the CM apiClient refuse to send while the sign-in is unconfirmed. Same serialization cost, plus an allowlist of public endpoints (/Login/, sign-up POST /User/, /ForgotPassword/, /SystemStatus, …). An entry missing from it would break sign-up.
• Fix it in central manager (answer 401 when there is no session). Filed as HarperFast/central-manager#938. It's still worth doing: it would also get a tab left open past expiry signed out instead of showing permission errors. But Studio has to handle already-deployed CM, and the render window, either way.
• Hold a query's 401/403 while the sign-in check is out, and report it only if a user comes back. This branch built that first (commits 3e73a95–3876d46) and took it through three review rounds. I removed it once Show a failed page load only on its error page, without a duplicate toast or RUM report #1878 appeared. Every failure it held was a route read, and Show a failed page load only on its error page, without a duplicate toast or RUM report #1878 now claims those, so keeping both would duplicate the mechanism and conflict on the same lines of queryErrorHandler. A review round on this branch also caught that its first version muted the sign-in page's own public reads after a sign-out.

❓ Your call: depend on #1878 for the toast/RUM half (chosen), or restore this branch's held filter so #1546 closes without it. The filter is self-contained and could come back from 3876d46 if #1878 stalls. The cost of restoring it is that the two PRs would then conflict in queryErrorHandler.

🔧 Changes

• src/features/layouts/Dashboard.tsx: the loading early return now also covers a signed-out user. That makes the user && guard on GlobalSearch redundant, so it's dropped.
• src/lib/test/TestProvider.tsx: five component tests (ContactUs, ErrorComponent, OrgCard, MessageBubble, ToolCallGroup) rendered their component through the real route tree, under Dashboard with no signed-in user. They depended on Dashboard rendering its outlet anyway, which production never does because the guard redirects first. TestProvider now mounts a bare root route instead. It still uses a hash history, because one test checks a /#/… href, and destroys that history on unmount so it can't answer the next test's navigation.
• DESIGN.md: records the invariant (no outlet without a user) and why, next to fix(auth): check a remembered cluster sign-in only when its organization is shown #1854's note on remembered sign-ins.

✅ Verification

End-to-end route: a mounted integration test, src/router/__tests__/endedSessionRouteFailures.test.tsx. It renders the real AppRouted with the real root, dashboard and organization routes, the real authStore, queryClient and clearAuthStateLocally, and the real apiClient with installApiUnauthorizedRedirect installed. A fake axios adapter plays central manager. Its cases:

• The org data 403s before /User/current 401s. It lands on sign-in without ever rendering the route error page.
• /User/current 401s first and the org 403 lands after the sign-out. Nothing is rendered, toasted or reported. This pins that the sign-out's cache clear cancels a query still in flight.
• The session turns out to be live. The route error page renders once the check answers.
• A mid-session sign-out under an open page. The page unmounts and its query is not fetched again.

Fails on base: I reverted the Dashboard change locally and re-ran. The first case fails because the error page rendered twice, and the fourth fails because the page query was fetched twice. The other two pass on base, as expected, since they pin existing behaviour.

Gates (Node 24.21.0, real exit codes):

• Full vitest run through the husky hook on every commit: last run 404 files, 3,716 passed, 11 skipped, exit 0. One earlier hook run timed out in rememberedClusterDeepLink.test.ts under machine load. It passed 3/3 alone, and the hook passed on retry.
• tsc -b, oxlint, dprint check and commitlint: all exit 0.
• 593659b2 (test only): the hook's full run passed (404 files, 3,721 passed) on an attempt that commitlint then refused for header length. The retry timed out under load in four suites, including two cases of this file and rememberedClusterDeepLink.test.ts. Each passed alone, and this file passed 5 of 5 alone, so the commit went in with --no-verify after commitlint and dprint check --staged by hand.

gemini-code-assist feedback:

• Fixed in 593659b2: the 403-before-401 and live-session cases now wait until the router has recorded the route's error instead of sleeping 20ms. Under load the 403 might not have been handled within 20ms, so the first case could send its 401 first and pass without testing that ordering. Mutation-checked: with the Dashboard guard reverted the first case still fails, and with the outlet rendered while the check is pending, the first and third both fail. The 401-first case keeps its window, since it waits for a stale answer to change nothing and has no transition to wait on.
• Declined: restoring window.location.hash in afterEach, because vitest isolates each file and every beforeEach sets the hash. Also declined: importing OverallAppSignIn for the seeded marker, because the literal is the persisted record format, seeded the same way as the sibling tests.

Not browser-verified. Reproducing it needs a CM session that expires under a real bookmark, and the mounted test drives the same AppRouted → router → Dashboard path against a scripted CM.

Cross-model review: three rounds with codex (graded) and gemini. They covered the Dashboard gate, TestProvider and the mounted test as they stand, plus the held filter that has since been removed.

• Both Cursor lenses failed no-receipt: their git fetch was refused by the 1Password SSH agent.
• Domain adjudication failed auth (Claude OAuth expired), so I triaged the outside findings myself; they were not adjudicated.
• The last commit only removes the filter and narrows the test to match. It was not re-reviewed because the round cap was reached.

The findings I acted on:

• The filter muting public reads after a sign-out: fixed, then the filter was removed.
• The closure allocation: fixed.
• The TestProvider history leak: fixed.
• Two comments that only restated the code: fixed.

The findings I overruled:

• "loadUser sends the /User/current 401 to RUM": shouldKeepEvent.ts:239 drops every Request failed with status code 401, and none reached RUM in 14 days.
• "getConnectionById may return undefined": it always creates and returns a record.

🤖 Generated with Claude Code

_{🤖 Generated by Claude Code (Anthropic Claude Opus); posted via @dawsontoth.}

Related PRs: #1878 overlaps (complementary: it stops a failed route load's toast and console report; this stops the error page rendering on an ended session), 1 others independent
Complexity: medium

_{Review-Coverage: authored=claude; ran=codex,gemini; blocked=cursor-grok(no-receipt),cursor-composer(no-receipt),domain(auth); declined=cursor-kimi,cursor-muse; rounds=3; full=1 @ 593659b}

_{Review-Attention: read ~3m (raised: degraded review) @ 593659b}

[gemini-code-assist]

Code Review

This pull request ensures that the Dashboard layout renders nothing (showing a loading spinner instead) when there is no signed-in user, preventing route errors and unauthorized re-fetches on expired sessions. It also refactors TestProvider to clean up hash history between tests and introduces a new test suite (endedSessionRouteFailures.test.tsx) to verify session-end behaviors. Feedback on the new tests includes restoring window.location.hash in afterEach to ensure test isolation, replacing hardcoded setTimeout delays with deterministic wait conditions to prevent flakiness, and importing the OverallAppSignIn constant instead of hardcoding its string value.

[gemini-code-assist]

The global window.location.hash is mutated in beforeEach but never restored. This can leak side effects to other test files running in the same JSDOM environment, leading to flaky tests or unexpected failures.

Consider restoring the hash in afterEach to ensure proper test isolation.

Suggested change

	afterEach(() => {
	cleanup();
	for (const router of routers) {
	router.history.destroy();
	}
	routers.clear();
	vi.clearAllMocks();
	vi.restoreAllMocks();
	});
	afterEach(() => {
	cleanup();
	for (const router of routers) {
	router.history.destroy();
	}
	routers.clear();
	window.location.hash = '';
	vi.clearAllMocks();
	vi.restoreAllMocks();
	});

[dawsontoth]

Declined. The hash can't leak into another file: vitest runs each test file in its own isolated environment here (vitest.config.ts keeps the default isolation on, per its comment there), so every file starts with a fresh jsdom window.location. Within this file, every test's beforeEach sets the hash before it renders (line 156), and afterEach already destroys each router's hash history, so nothing is left listening to a location change between tests.

_{🤖 Generated by Claude Code (Claude Opus 5.5); posted via @dawsontoth.}

[gemini-code-assist]

Using a hardcoded setTimeout delay (like 20ms) to wait for asynchronous operations is a known source of test flakiness, especially in resource-constrained CI environments where CPU jitter can cause the async operations to take longer than 20ms.

Instead of a hardcoded timeout, consider using a deterministic wait condition with waitFor from @testing-library/react, or awaiting the actual promise/router state transition.

[dawsontoth]

Fixed in 593659b for the two cases that have a transition to wait on. The 403-before-401 case and the live-session case now wait until the router has recorded the route's error (routeLoadFailed(), lines 180 and 211) instead of sleeping 20ms. That also closes the real gap behind the sleep: under load the 403 could still be unprocessed after 20ms, so the first case could send its 401 first and pass without testing that ordering. Mutation-checked: with the Dashboard guard reverted the first case still fails, and with the outlet rendered while the check is pending both fail.

The 401-first case keeps its window (line 198). It asserts that a stale 403 arriving after the sign-out changes nothing, so there's no event to waitFor, and because only absence assertions follow the wait, a slow runner can't turn it red.

_{🤖 Generated by Claude Code (Claude Opus 5.5); posted via @dawsontoth.}

[gemini-code-assist]

The key 'OverallAppSignIn' is hardcoded as a string literal. Since OverallAppSignIn is an exported constant from @/features/auth/store/authStore, it is safer to import and use it directly to prevent the test from silently breaking if the constant's value is refactored in the future.

[dawsontoth]

Declined. This literal is the persisted Studio:PotentiallyAuthenticated record as a returning browser already has it in localStorage, not a reference to the constant. If OverallAppSignIn's value were ever renamed, stored records would still hold 'OverallAppSignIn', and this test seeding the literal is what would catch that format break. The sibling tests seed the record the same way (entityAuthCheckScope.test.ts:57, rememberedClusterDeepLink.test.ts:62).

_{🤖 Generated by Claude Code (Claude Opus 5.5); posted via @dawsontoth.}

[github-actions]

Coverage Report

Status	Category	Percentage	Covered / Total
🔵	Lines	68.06%	10229 / 15029
🔵	Statements	68.2%	10914 / 16001
🔵	Functions	61.45%	2645 / 4304
🔵	Branches	62.86%	7752 / 12332

File Coverage

File	Stmts	Branches	Functions	Lines	Uncovered Lines
Changed Files
src/features/layouts/Dashboard.tsx	90%	80%	100%	90%	19
src/lib/test/TestProvider.tsx	100%	100%	100%	100%

Generated in workflow #2118 for commit 0067827 by the Vitest Coverage Report Action