Consume the profile-settings deep link on the route, not on component construction - #6096
Conversation
… construction `?openProfileSettings` was read once in `SubmodeLayout`'s constructor, which raced two things the component has no relationship to. The index route's model hook redirects whenever the URL carries no `operatorModeState` — exactly the deep link's URL shape. That redirect aborts the transition before `Route#setup` has written the param onto the controller, and rebuilds the param set from the router's own cache, which never learned the value; the request is dropped from the controller and the URL together. Separately, every transition's `setup()` rewrites each declared param onto the controller from its captured values, so a transition that captured the param before it was consumed puts it straight back. Which of those bites depends on where the render that constructs the layout falls between them, and the layout is mounted and unmounted by the authentication gate — so on the logged-out arrival path it moved with shard order and load, and `account popover: a deep link arriving logged out waits for login, then opens the modal` failed intermittently on CI shard 9. The route now consumes it at fixed points in the transition instead: in `model()` from `params` (the controller is not written until after `model()` resolves) ahead of the redirect, and in `setupController` after `setup()`'s write-back. The redirect drops the param explicitly so the router cannot rebuild it. A `[profile-deep-link]` channel records whether each arrival was acted on or held for the login form. The new regression test covers an arrival with the layout already mounted, which the constructor read could never see; it fails without this change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GNSdVEE6LxfSjJ2Nv7aczY
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f4d6ca925b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
[Claude Code 🤖] CI is green on All 69 check runs are success or skipped, including Host Tests (9, 16) — the shard this flake lives on:
For contrast, the run that prompted this fix reported
Worth knowing when you look at it: this diff contains no template or CSS changes at all — it deletes a constructor from Nothing else is outstanding; the Codex finding is answered on its thread. Generated by Claude Code |
3 participants
The flake
Acceptance | operator mode tests > account popover: a deep link arriving logged out waits for login, then opens the modalfails intermittently on host shard 9, on commits unrelated to it. The check-run annotation is always the same assertion:The app is logged in by then and the request is still in the URL — the modal just never opens.
Why
?openProfileSettingswas read once, inSubmodeLayout's constructor. The param is moved by the router; the component is mounted and unmounted by the authentication gate. Neither actor knows about the other, and the request can be lost at two separate points:operatorModeState— exactly the deep link's URL shape. That redirect aborts the transition beforeRoute#setuphas written the param onto the controller, then rebuilds the param set from the router's own cache, which never learned the value. The request is gone from the controller and the URL together.Route#setupputs it back. Every transition rewrites each declared param onto the controller from its captured values, so a transition that captured the param before it was consumed resurrects it afterwards — which is how the modal can open and the param still be left in the URL.Which one bites depends on where the render that constructs the layout falls between them. On the logged-out arrival path the session churns (logout → login → route refresh → redirect), so that ordering moves with load and with the shard's module order, which is itself not stable between runs.
The change
The route consumes the request at fixed points in the transition instead:
model()paramsparams, not the controller —Route#setupdoes not write this transition's params onto the controller until aftermodel()resolvessetupControllerRoute#setup's write-back, so a transition that captured the param before it was consumed cannot put it backAll three sit after the logged-in gate, so a logged-out arrival still leaves the request in the URL to outlive the login form, and the post-login route refresh brings it back with a session to act on. Consumption stays idempotent and still nulls the param, so a dismissed modal does not reopen on the model refresh every
schedulePersist()triggers.The
scheduleOnce('afterRender')dance inSubmodeLayoutgoes away with it — it existed to work around writing state the same render pass had already read, which the route does not have to do.A
[profile-deep-link]channel (test-gated, alongside the existing[login-diag]/[start-phase]ones) records whether each arrival was acted on or held for the login form, so a future failure names which of the two it was rather than being silent.Tests
A new case covers an arrival with the layout already mounted — an in-app navigation rather than a cold boot, which a constructor read can never see. It is a genuine guard, not decoration:
main's sourceElement [data-test-settings-modal] does not exist, the same assertion and message CI reportsaccount popovermoduleAcceptance | operator mode testsfileTypes and lint clean. Run against a locally booted stack (base, skills,
/test/,/node-test/, live prerender).🤖 Generated with Claude Code
https://claude.ai/code/session_01GNSdVEE6LxfSjJ2Nv7aczY
Generated by Claude Code