Deep link into profile subscription from a URL

[burieberry]

Closes CS-12461

Adds ?openProfileSettings=subscription, which opens the settings modal and brings the subscription section into view. This will be linked to from the boxel-home pricing page. The param is added to HOST_APP_QUERY_PARAMS in runtime-common/host-routing-validation.ts so it survives routing.

Where it's consumed, and why not the route

In SubmodeLayout's constructor rather than the index route. A visitor arriving from a marketing page is logged out: the route's model hook returns early to render <Auth />, and by the time login completes that hook has already run. SubmodeLayout is only constructed once logged in, and start({ refreshRoutes: true }) re-enters the route after login, so the component naturally exists at exactly the right moment. No login-specific branching.

Two details worth knowing:

• The param is nulled on consumption, the way matrix/auth.gts does for sid. The index route's model hook re-runs on every schedulePersist(), so a param left in the URL would reopen the modal the user just dismissed. It also keeps it out of a URL someone might share.
• Both writes are deferred to afterRender. Doing them in the constructor throws a backtracking-rerender assertion, since they land on state the render pass has already read. auth.gts avoids this by nulling from an action; stack-item.gts uses the same scheduleOnce pattern.

Subscription renders after the name and email fields in a modal with a fixed height, so opening it alone would land the visitor with their target below the fold — hence the scroll.

Tests

Three acceptance tests in the account popover module:

• the deep link opens settings on the subscription section, and the param does not survive into the URL
• a dismissed modal stays dismissed across a state persist — the schedulePersist() hazard above
• the full logged-out path: the param outlives the login form, then after logging in the modal opens on the subscription section and the param is consumed

The logged-out test needs the mock matrix client to accept a password login, so tests/helpers/mock-matrix/_client.ts grew that support.

25/25 in the full operator-mode acceptance file, types and lint clean.

🤖 Generated with Claude Code

[github-actions]

Preview deployments

• Host staging preview

• Host production preview

Host Test Results

    1 files  ± 0      1 suites  ±0   3h 4m 45s ⏱️ - 4m 37s
3 953 tests +67  3 939 ✅ +67  14 💤 ±0  0 ❌ ±0 
3 972 runs  +67  3 958 ✅ +67  14 💤 ±0  0 ❌ ±0 

Results for commit f4de439. ± Comparison against earlier commit 8fa95f1.

Realm Server Test Results

    1 files  ±    0      1 suites  ±0   14m 34s ⏱️ - 1m 5s
2 086 tests ±    0  2 086 ✅ ±    0  0 💤 ±0  0 ❌ ±0 
3 641 runs  +1 476  3 641 ✅ +1 476  0 💤 ±0  0 ❌ ±0 

Results for commit f4de439. ± Comparison against earlier commit 8fa95f1.

[richardhjtan]

[Claude Code 🤖] [data-test-profile-subscription-section] exists whenever the modal exists, so this assertion adds nothing to the line above it — and the test's name claims something neither line checks. Non-blocking, but this is the only coverage the section-targeting has.

Mechanism. In profile-settings-modal.gts the marker sits on an unconditional wrapper:

<div
  class='profile-settings-subscription'
  {{scrollIntoViewIfRequested this.subscriptionRequested}}
  data-test-profile-subscription-section
>
  <ProfileSubscription />
</div>

No {{#if}}. The div renders on every path that renders the modal, including a plain "open settings from the profile popover". So assert.dom('[data-test-profile-subscription-section]').exists() is implied by the [data-test-settings-modal] assertion immediately above and carries no additional information.

What would have to break for these tests to fail. I checked three ways to disable the feature's section half:

1. Make subscriptionRequested always return false → the modifier no-ops → all three tests still pass.
2. Call openProfileSettings(undefined) instead of passing 'subscription' through in submode-layout.gts → all three tests still pass.
3. Delete the scrollIntoViewIfRequested modifier from the template entirely → all three tests still pass.

So ?openProfileSettings=subscription is currently tested as "opens the settings modal", which is also what ?openProfileSettings=anything does. The behavior that distinguishes this feature is untested, and the test titles — "opens settings on the subscription section" here, and "the modal opens on the subscription section" in the logged-out test — assert more than the assertions do. That's the expensive kind of green: the next person to refactor the modal has no signal that they broke the deep link's point.

What to assert instead. The state that actually drives it is deterministic and already exposed:

assert.strictEqual(
  getService('operator-mode-state-service').profileSettingsSection,
  'subscription',
  'the modal opened targeting the subscription section',
);

That fails under all three mutations above. Scroll position itself is the wrong thing to assert on in an acceptance test — layout-dependent and flaky — so the service state is the right proxy: it's what subscriptionRequested reads and the only input the modifier has.

The same substitution applies to the assertion in the logged-out test further down, where it's doing the same non-work.

One more assertion worth having, since it's the pair to this one and costs a line: after opening settings the ordinary way ([data-test-profile-icon-button] → settings), profileSettingsSection should be undefined. That pins the negative — a plain open doesn't jump to subscription — which is the property that makes the deep link a deep link rather than a redundant one.

Scope: regression in what the tests establish, introduced with them. Non-blocking.

[burieberry]

[Claude Code 🤖] Done — both tests now assert profileSettingsSection === 'subscription' on the service instead of the unconditional DOM marker, and the plain-open test pins the negative (undefined after opening via the popover). All three of your mutations now fail the suite.

[richardhjtan]

[Claude Code 🤖] An unrecognized value opens the settings modal at the top rather than being ignored, and nothing says whether that's the intent. Non-blocking; the ask is a decision plus one test.

Mechanism. IndexController types the param as string | null, and the narrowing here is requested === 'subscription' ? 'subscription' : undefined. The section is discarded for anything else, but openProfileSettings(undefined) still runs — so it still sets profileSettingsOpen = true. Concretely:

URL	Result
?openProfileSettings=subscription	settings open, scrolled to subscription
?openProfileSettings=email	settings open, top of modal
?openProfileSettings=1	settings open, top of modal
?openProfileSettings=	nothing (falsy)

Why it matters more than it looks. Per the description this param is linked from the boxel-home pricing page — a surface that ships independently of the host app. Two plausible futures make the fallback load-bearing: someone links ?openProfileSettings=billing guessing the name, or a future rename leaves the old link live. Right now both land the visitor in profile settings at the top, which is a reasonable outcome — but it's reasonable by accident, and the opposite choice (ignore what you don't recognize, so a stale link is inert) is equally defensible. The code doesn't say which was chosen.

Two ways out. Keep it and make it explicit — the ProfileSettingsSection type is already the natural place:

// Any recognized value targets its section; an unrecognized one still opens
// settings, so a stale or mistyped link from an external page degrades to
// the modal's default view rather than doing nothing.
const PROFILE_SETTINGS_SECTIONS: ProfileSettingsSection[] = ['subscription'];
let section = PROFILE_SETTINGS_SECTIONS.find((s) => s === requested);
this.operatorModeStateService.openProfileSettings(section);

Or narrow it, if a stale link should be inert: if (!section) return; before opening — noting the param is already nulled above, so an unrecognized value is still consumed rather than left in the URL.

I'd take the first: opening settings is a harmless, useful fallback for a link the app doesn't control, and the named list makes adding the next section a one-line change with the validation already in place.

Either way the branch wants a test — ?openProfileSettings=nonsense is the case a marketing-page typo produces, and it's currently the only path through this function with no coverage.

Scope: follow-up, non-blocking.

[burieberry]

[Claude Code 🤖] Decision made and documented: an unrecognized value deliberately degrades to opening settings at the default view, since the param is linked from external pages we don't ship with. Added a comment saying so at the call site. Skipped the named-list refactor and the ?openProfileSettings=nonsense test for now — with one recognized section the list would have a single entry; worth revisiting when a second section arrives.

[richardhjtan]

[Claude Code 🤖] Confirmation — the section can't leak into a later open. I went looking for that bug specifically and it isn't reachable. Nothing to change; recording the enumeration because the invariant lives in two files and the next editor won't have it.

The hazard: profileSettingsSection is sticky service state set by a one-shot URL param. If any close path skipped the reset, a consumed 'subscription' would survive, and the next plain "open settings" from the profile popover would silently jump to the subscription section — a bug that only shows up two user actions after its cause.

It can't happen, because toggleProfileSettings is the only way the modal closes. Every writer of profileSettingsOpen outside this service:

• submode-layout.gts — @action toggleProfileSettings() delegates here; passed to ProfileInfoPopover and to ProfileSettingsModal as @toggleProfileSettings.
• profile-settings-modal.gts — @onClose={{@toggleProfileSettings}} on the modal, plus the internal call after a successful save.
• ai-assistant/message/index.gts — @action={{this.operatorModeStateService.toggleProfileSettings}}, straight to this method.

No call site assigns profileSettingsOpen = false directly, and toggleProfileSettings clears the section on every transition to closed. resetState() clears it too, so a logout/teardown can't strand it either.

The condition that would break it: any future close path that sets profileSettingsOpen = false without going through toggleProfileSettings — an Escape-key handler, a route transition that force-closes modals, a "close all overlays" action. If that arrives, the durable fix is to move the reset out of the toggle and make it a consequence of the flag itself (clear profileSettingsSection wherever profileSettingsOpen becomes false, or derive profileSettingsSection so it can't outlive an open modal) rather than adding a second reset at the new call site.

One small asymmetry worth naming while it's cheap: openProfileSettings(section?) always assigns the section, so calling it with no argument on an already-open modal clears a section that was set moments earlier. That's the right behavior for the deep link and nothing else calls it today — just note that it's an assignment, not a merge, if a second caller ever appears.

Scope: confirmation, non-blocking.

[burieberry]

[Claude Code 🤖] Thanks for recording the enumeration — agreed the invariant holds, and the note about deriving the reset from the flag if a second close path ever appears is the right future fix. No change made.

[richardhjtan]

[Claude Code 🤖] The scroll is single-shot, and the section's height is still growing when it fires. Cosmetic, optional — flagging mainly because a function-based modifier taking a boolean looks like it keeps the element aligned, and it doesn't.

Mechanism. The modifier runs on install and re-runs only when a tracked value it consumed changes. requested is true at install and never changes within the modal's lifetime — the modal unmounts when settings close — so scrollIntoView fires exactly once, at insert.

At that moment ProfileSubscription has rendered its structure but not its data: WithSubscriptionData yields values that render a LoadingIndicator while the fetch is in flight, and two things below are still absent — the Manage Plan button (gated on this.billingService.subscriptionData.plan) and the {{#each this.billingService.extraCreditsPricingFormatted}} rows. So the section grows after the scroll.

Growth below the target doesn't move a block: 'start' alignment, so this is mostly fine. The one case where it isn't: the subscription wrapper is the last element in the form, so aligning its top to the container top requires a viewport's worth of content beneath it. Before the async content lands there may not be, and the browser clamps the scroll to the maximum offset — leaving the section visible but not top-aligned, and nothing re-aligns it once the content arrives. Which is a perfectly acceptable outcome for the actual goal ("the visitor lands on the subscription section rather than below the fold"), just not the one the code reads as promising.

If exact alignment matters, the fix is to give the modifier a reason to re-run — consuming the loading flag it depends on, so it re-aligns once the data lands:

const scrollIntoViewIfRequested = modifier(
  (element: HTMLElement, [requested, _settled]: [boolean, unknown]) => {
    if (requested) {
      element.scrollIntoView({ block: 'start' });
    }
  },
);

…invoked with the subscription-data loading state as the second argument. If it doesn't matter, leaving it is right and the comment above the modifier is the place to say so — one clause ("aligned once on insert; the section is last in the form so the browser may clamp") stops the next reader re-deriving all of the above.

Separate, and genuinely nice: folding .profile-settings-subscription into the existing .profile-field + .profile-field margin rule rather than inventing a second spacing value is the right instinct — the section now spaces itself like every other field.

Scope: follow-up, non-blocking.

[burieberry]

[Claude Code 🤖] Leaving as-is: the clamped case still lands the visitor on the subscription section, which is the actual goal — exact top-alignment isn't worth injecting billingService into the modal, and the loading flag wouldn't cover the extra-credits rows anyway.

[richardhjtan]

[Claude Code 🤖] The comment's claim is accurate, and the mock newly makes a pre-existing inconsistency in this file reachable. No change asked for in this PR — the new test doesn't trip it, and the note is for whoever writes the next login test.

The claim checks out. clientOpts.userId is exactly the lever the identity getter reads: get loggedInAs() { return this.clientOpts.userId ?? this.sdkOpts.loggedInAs; }. So assigning it does mirror what module setup establishes, and credentials ({ userId: this.loggedInAs ?? null }) follows along.

The inconsistency. getOpenIdToken resolves the same identity in the opposite order:

let accessToken =
  this.sdkOpts.loggedInAs ?? this.clientOpts.userId ?? 'mock-matrix-user';

sdkOpts.loggedInAs first, clientOpts.userId second — the reverse of the getter. Before this PR the two orders agreed in practice, because clientOpts.userId was only ever set at setup to the same value. Now a test can set it at runtime, so the two disagree the moment a test logs in as someone other than the module's configured loggedInAs: credentials.userId reports the new user while the OpenID token is still minted for the old one.

The new test doesn't hit it — it logs in as testuser, which resolves to the same @testuser:localhost the module already uses, so both orders return the same string. It's live for the next test that logs in as a second identity, and it would present as a realm-auth failure with no obvious connection to the login call.

Also worth knowing about the mock as written: _password is unused, so every credential succeeds. Fine for driving the form, but it means a failed-login path can't be tested through it, and a test that expects a rejection would silently pass through to a logged-in state. If that's ever needed, the smallest version is to reject when the password doesn't match a configurable expected value, defaulting to accept-all so existing callers are unaffected.

Scope: pre-existing, newly reachable — follow-up, non-blocking.

[burieberry]

[Claude Code 🤖] Fixed — getOpenIdToken now resolves through the loggedInAs getter, so the token identity always agrees with credentials, including after a runtime loginWithPassword. Left the accept-all password behavior; the comment on loginWithPassword documents it.