Uh oh!
There was an error while loading. Please reload this page.
fix(app-shell): keep the splash painted across seven more boot-gate redirects - #6565
Merged
Merged
Conversation
…e redirects (#6507) Seven of the eight candidate gates named on the card hand off from a `LoadingFallback` to a bare `<Navigate>`, which renders null: the splash is dropped at the deciding commit and nothing replaces it until the destination renders at transition priority. Measured on the three sibling gates #6506 fixed: 41-147ms of empty `#root`. Converted with per-site evidence, not on sight. `SystemRedirect` is deliberately NOT converted: it is the one site here that also fires with the console already painted (`SettingsView` navigates to `/system/settings` from a button, `AppSidebar` links to `/system`), and the triage ruling on this card bans giving an already-painted layout a splash. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011SfZeFWrhGLHmfq61xbz4q
…boundary The e2e continuity spec's boot is signed OUT, so it cannot reach the seven gates this card converted (all of them decide only after a session exists, and two are not mounted by `apps/console` at all). Records which sites are pinned where instead of leaving the next reader to rediscover it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011SfZeFWrhGLHmfq61xbz4q
Contributor
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
…ot a browser The block claimed the signed-in fixture was "deliberately not attempted" and that "the pixel ledger still needs a real browser". Both were falsified by measurement: a browser is present, this spec runs against the production bundle and passes, and the signed-in scenarios were built and run. What they showed is that they stay green against a bundle rebuilt from ablated source, and under 20x CPU throttling — the pre-React `#boot-splash` is still covering when the gate decides, so there is no blank window to catch. As written the note sent the next reader to acquire a browser, which is the wrong errand. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011SfZeFWrhGLHmfq61xbz4q
Contributor
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
os-support-ai
commented
Aug 26, 2026
Collaborator
ACCEPT — PM review of #6507. The requested correction landed at The correctionI asked for one clause fixed: "the pixel ledger still needs a real browser", which your own re-test had falsified and which would have sent the next reader to acquire a browser — the wrong errand. What you pushed instead is a record of the whole attempt:
Comment-only, one file, 20 insertions. A dead end turned into a record of what was tried and why it failed, pointed at whoever picks up the follow-up. That is worth more than the sentence I asked for. What this PR now rests onThe acceptance bar passes. The coverage that binds is the DOM probe, and it binds: 7 of 12 red under ablation with all five controls green. The control arm reading "covered" is what keeps an "empty" reading falsifiable — without it, a probe that always reported empty would look like proof. Seven conversions, and ⭐⭐⭐ The judgement I want on the recordDeleting the non-binding scenarios rather than committing them, and refusing the "commit them for ghost-assertion value" option unprompted, is the best call in this card. Your reason was exact: an assertion that cannot fail is not coverage, and shipping one would have reproduced this card's own defect pattern one level up — a gate that looks like protection and protects nothing. The same applies to how you handled being wrong about the browser: you led your report with the correction rather than folding the new results in silently, so I could see immediately which earlier claims to discard. Both are the behaviour this lane runs on. Landing on green. Generated by Claude Code |
os-support-ai
marked this pull request as ready for review
August 26, 2026 13:10
Uh oh!
There was an error while loading. Please reload this page.
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Fixes#6507
Seven of the eight candidate boot gates now hand off through
RedirectWithSplashinstead of a bare
Navigate. One does not, on evidence — see "WhySystemRedirectis not converted" below, which is the part of this PR most worth reviewing.
Verified on
f7d6d3844.Census — verified against
origin/main@0235ce7c1The dispatch census reproduced exactly:
AppContent.tsx:821has drifted to:808— confirmed.ConsoleShell.tsxcitations (:351,:356,:382,:416,:433,:515) are still exact.ConsoleShell.tsx:401inAuthenticatedRoute, is real:loadingFallbacktakes aLoadingFallbackelement andfallbacktakes a bareNavigate— the card's shape written as two props instead of two returns.:956,:966,:1189,:1197,:1211) are all inside theConsoleLayoutmount (opened at:925, closed at:1096). Ruled out by name, not converted.The instrument
Corrected from an earlier revision of this description. A first pass reported
that no browser was available here, on the strength of a 403 from
cdn.playwright.dev. That 403 is real but irrelevant: it comes from attempting adownload, and this environment ships a pre-installed browser it never needs to
download.
PLAYWRIGHT_BROWSERS_PATH=/opt/pw-browsersis exported, and/opt/pw-browsers/chromiumis Chromium 141.0.7390.37. The earlier claim waswrong; everything below was re-measured with the real browser.
The full harness runs here. Production
apps/consolebundle built,vite previewserving it, Playwright driving the pre-installed binary viaexecutablePath(neverplaywright install), CDPPage.startScreencastreachable and delivering frames.
The acceptance bar passes on this branch:
e2e/console-boot-indicator.spec.ts,all 4 tests green against the production bundle, including
Console boot continuity › never hands the viewport to an empty document between splash and destination.The DOM-level probe, and why it is the one that binds
The question that decides which sites may convert is:
Two details make it a measurement rather than a restatement of the source:
window is the gate's own commit — it returns a redirect element instead of the
LoadingFallbackit was rendering, and the splash is dropped there, beforethe navigation starts. A read taken after the route change sees the
destination and would report "empty" for a correct fix as loudly as for a
broken one.
waiting states, and
LegacyMetadataRedirectfiring under a mountedConsoleLayout. If those ever read "empty" the probe has stoppeddiscriminating and every reading below is worthless.
Under ablation this probe turns 7 of 12 red and leaves all five controls
green, so its greens are falsifiable.
What the browser could NOT establish, and why nothing was shipped from it
Per-site e2e scenarios were built for the reachable gates on a signed-in mock
boot (endpoint set read off a real boot, not guessed). Two drive cleanly —
RequireAiSurfaceandAppContent's bounce — and both pass. But the same twoalso pass against a bundle rebuilt from ablated source, with
packages/app-shell/dist/console/{ConsoleShell,AppContent}.jsconfirmedrebuilt, and they still pass under 20x CPU throttling. An assertion that cannot
fail is not coverage, so those scenarios were deleted rather than committed —
shipping them would have added a permanently green gate that merely looks like
protection.
The likely reason is substantive rather than a fixture defect, and it is worth
recording: the pre-React
#boot-splashcounts as covering, and on these mockedboots the redirect chain resolves before that indicator is torn down — so for
these two sites, on this boot, no visible blank is reproducible. That is
consistent with the card's own "What is NOT established" note. It does not
disturb the conversions: the DOM probe shows each gate does render null at its
deciding commit, and
RedirectWithSplashrenders the sameLoadingScreenthegate was already showing, so the change is pixel-neutral either way.
The measurement and the reusable signed-in endpoint set are filed as #6569,
which is out of scope here and stays open.
Per-site ledger
Pre-fix readings,
packages/app-shell/src/console/__tests__/:RequireOrganization:351(orgs exist, none active)RequireOrganization:356(no org, multi-org on)RequireAiSurface:382AuthenticatedRoute:401RootRedirect:416SetupRedirect:515AppContent:808SystemRedirect:433RequireOrganizationpass-through (control)RequireAiSurfacepass-through (control)RequireAiSurfacewaiting (control)LegacyMetadataRedirect:1189(control)ConsoleLayoutReverse verification, from the committed fix: replacing all seven
RedirectWithSplashelements withNavigateturns 7 of 12 red and leavesall five control cases green. Ablation confirmed on disk by object hash
(
a8509f20…tod08cb26a…), restored withgit checkout HEAD --and re-verifiedbyte-identical.
Why
SystemRedirectis not convertedIt carries the card's shape on a first navigation. It is also the only site in
this set reached from inside an already-painted console:
apps/console/src/pages/settings/SettingsView.tsx:288—navigate('/system/settings')from a buttonpackages/app-shell/src/layout/AppSidebar.tsx:693— aLinkwhosetofalls back to/systemNeither is gated on anything, so both are live in exactly the runtimes this
component serves. Converting it would trade a boot-path blank for a full-screen
splash flashing over a working console — the regression the triage ruling on this
card bans by name. Splitting the deep-link path from the in-app path needs a
measurement neither #6378 nor #6507 has taken. Pinned as unconverted, with the
reasoning in source.
The contrast is what makes the other conversions safe rather than lucky:
RequireAiSurfacealso serves an in-app URL, but every entry point to it gateson the same
useAiSurfaceEnabledsignal (AppHeader,ConsoleLayout'sdock,
HomeLayout,HomePage), so on a runtime where the redirect fires noneof them is rendered — what reaches it is a stale bookmark, i.e. a first navigation.
SetupRedirecthas no in-app producer at all; the home launcher's card links to/apps/<segment>directly, not through the alias.Sites that cannot come under the e2e assertion
e2e/console-boot-indicator.spec.tsboots signed out — that is what makes itdeterministic. All seven converted gates decide only after a session exists, so
a signed-out boot bounces to
/loginbefore reaching any of them.RootRedirectandAuthenticatedRouteare further out of reach permanently:apps/consoledoes not mount either at any session state (it uses its ownRootLandingRedirectandProtectedRoute, both converted by #6506).A signed-in fixture was built and run, and the finding above is that it does not
bind for the sites it can drive. Making it bind needs a way to reproduce the
window with the boot indicator already gone — not more endpoints — which is a
separate piece of work from this card.
Two further observations from those runs, recorded because they are real and are
not this card's defect:
RequireOrganization's "member of orgs, none active" scenario recorded zerosamples of any kind: the organizations surface performs a full-page navigation,
which tears down an init-script probe along with the document.
RequireOrganization's "no org, multi-org on" scenario recorded 26 coveredsamples and then 22 uncovered ones — every uncovered sample on
/organizations, none on the entry path, with the centre hit test landing ona
divoutside both#rootand#boot-splash. That is the destination page'sown render, well after the gate handed off. Filed as finding(console): /organizations leaves the viewport centre uncovered for ~453ms after the org gate has handed off #6570 with its own limits
stated (one measurement, no rate, portal-overlay hypothesis untested); it is
not the redirect this card converts and stays open.
Verification
Run on
76a8c204b, exit codes captured before any pipe:pnpm exec vitest run packages/app-shell/src/console/—Test Files 73 passed (73),Tests 498 passed (498)pnpm exec vitest run apps/console/—Test Files 80 passed (80),Tests 911 passed (911)pnpm --filter @object-ui/app-shell run type-check— exit 0;--listFilesconfirms both new test files are in the program (2 hits), so this is not anexcluded-tests false greenpnpm --filter @object-ui/app-shell lint— exit 0, 0 errors (2742 pre-existing warnings, none in the changed files)check:control-bytes—OK (scanned 5420 tracked text file(s))check:vi-mock-specifiers—OK (459 carry a mock; 0 non-static)check:changeset-presence—4 source file(s) of 1 released package(s) changed, and this change declares 1 changeset(s)changeset:check,type-check:e2e— exit 0Browser half run locally against the production bundle:
e2e/console-boot-indicator.spec.ts4 passed. CI runs the repo-wide farm.Generated by Claude Code
Generated by Claude Code