Skip to content

feat: bind canonical managed-auth submissions - #20

Open
masnwilliams wants to merge 9 commits into
mainfrom
hypeship/canonical-awaiting-input
Open

feat: bind canonical managed-auth submissions#20
masnwilliams wants to merge 9 commits into
mainfrom
hypeship/canonical-awaiting-input

Conversation

@masnwilliams

@masnwilliams masnwilliams commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

summary

  • prefer canonical managed-auth fields and choices when present while retaining legacy response and submission fallbacks
  • preserve exact canonical field and choice IDs through rendering and submission
  • bind canonical submissions to the state’s interaction_id and reject incomplete canonical state instead of falling back to legacy inputs
  • preserve structured API error codes, then refresh state and reconnect SSE after stale_interaction
  • carry legacy-only field hints and MFA destinations into the canonical UI projection during the compatibility window

testing

  • bun run test
  • bun run typecheck
  • bun run build
  • bun run format:check
  • cd packages/managed-auth-react && npm pack --dry-run

backend dependency

Deploy kernel/kernel#3185 and kernel/kernel#3380 before publishing this client update.

Read top-level fields/choices from managed-auth state events and responses when
present, mapping them into the existing form rendering model. Keep falling back
to legacy discovered_fields/pending_sso_buttons/mfa_options/sign_in_options so
older API responses remain supported during the deprecation window.

Co-authored-by: Cursor <cursoragent@cursor.com>
@vercel

vercel Bot commented Jun 25, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
managed-auth-react-demo Ready Ready Preview Aug 14, 2026 5:01pm

Comment thread packages/managed-auth-react/src/session/useManagedAuthSession.ts Outdated
Comment thread packages/managed-auth-react/src/session/useManagedAuthSession.ts Outdated
When canonical fields/choices are present, submit field_values by canonical field
ID and selected_choice_id by canonical choice ID. Continue supporting legacy
fields, SSO selectors, MFA option IDs, and sign-in option IDs when canonical data
is absent.

Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Comment thread packages/managed-auth-react/src/session/useManagedAuthSession.ts Outdated
Comment thread packages/managed-auth-react/src/session/useManagedAuthSession.ts Outdated
Comment thread packages/managed-auth-react/src/session/useManagedAuthSession.ts Outdated
Co-authored-by: Cursor <cursoragent@cursor.com>
State events are complete snapshots: the server omits fields/choices when
the current step has none, so retaining the prior values leaks a previous
step's inputs (e.g. login SSO buttons rendering on the MFA step).
Comment thread packages/managed-auth-react/src/session/useManagedAuthSession.ts Outdated
Comment thread packages/managed-auth-react/src/session/useManagedAuthSession.ts Outdated

@dcruzeneil2 dcruzeneil2 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The final shape is close and the hard parts are right: canonical-first with legacy fallback is the correct migration posture, the client only sends canonical submit payloads after the server has sent canonical data first (so rollout ordering is safe), and the snapshot-replace fix in mergeStateEvent landed on the correct semantics. CI is green.

I checked the types and submit payloads here against the upcoming API behavior and most of it lines up: the field and choice type enums match, field_values keyed by canonical field id round-trips correctly, and SSO submit via selected_choice_id works. Two things don't line up, and one behavioral gap will show up as soon as the API starts emitting canonical data. Requesting changes on the first two, the rest are non-blocking.

Blocking

  1. The submit strategy is split down the middle, and the MFA half loses information. Fields and SSO now submit canonically, but MFA and sign-in selections rendered from canonical choices still go through the legacy helpers. For MFA this is lossy: mfaOptionsFromCanonical squashes choice.id through normalizeMFAChoiceId, and since MFAOption has no id, the original choice id is gone by submit time. Anything unrecognized collapses to "other", so two unfamiliar MFA methods become indistinguishable to the server. This happens to work right now because of how the server currently builds these ids, but that's a coincidence, not a contract. The fix I'd like to see: add an optional id to MFAOption (and SignInOption) and submit selected_choice_id whenever the option came from canonical data, same as submitSSO already does. If we'd rather not do that yet, then revert fields/SSO to legacy submits too so the switch happens as one coherent change. Half-and-half is the one state we shouldn't ship. Also worth noting the PR description still says everything submits through legacy helpers, which stopped being true a few commits ago.

  2. ManagedAuthChoice.display_text and context don't exist in the API contract. The fallback chain in signInOptionsFromCanonical (description ?? context ?? display_text) can never fire, and the type definitions will convince the next reader that the wire carries fields it doesn't. Please drop both, or hold this until they actually exist on the API side if that's the plan.

Should fix

  1. MFA options rendered from canonical lose the masked destination. Legacy MFAOption.target carries hints like ***-***-5678, the canonical choice has no slot for it, and this PR prefers canonical when present. So the moment the API ships canonical data, MFA steps stop showing the masked phone/email even though the legacy arrays in the same payload still have it. This is deterministic, not an edge case. A reasonable client-side fix while the dual contract exists: carry target over from the matching legacy mfa_options entry during normalization.

  2. The legacy fallback should key off whether canonical data is present, not whether the conversion produced anything. Right now a payload with canonical choices containing only SSO buttons plus a stale legacy mfa_options array would render both, mixing content from two different steps. Not reachable with the current server since both views come from the same source, but the fix is one line per category: if choices is non-null, derive every choice-based category from canonical only, and same for fields.

Nits

  1. The snapshot-vs-merge question took three review rounds to settle and the answer now lives only in a comment. mergeStateEvent and the *FromCanonical helpers are pure functions, so a small test file would lock in "absent key means cleared" before someone re-adds the ?? base.fields fallback because it looks like the safe thing to do.

  2. ManagedAuthResponse is exported and now references ManagedAuthField and ManagedAuthChoice, which aren't exported from index.ts, so consumers can see the shape but can't name it. And given the new public type surface and new submit behavior, this feels like a minor rather than a patch.

Happy to take another look as soon as 1 and 2 are in. Everything else here is small.

Comment thread packages/managed-auth-react/src/session/state.ts
Comment thread packages/managed-auth-react/src/session/submission.ts Outdated

@dcruzeneil2 dcruzeneil2 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving — all four items from my last review are addressed.

Two non-blocking follow-ups:

  1. Canonical fields lose placeholder and hint. fieldsFromCanonical copies id/ref/name/type/label/required and nothing else, but the form also renders placeholder and hint — so both silently disappear whenever a step renders from canonical data. hint is the helper line like "Enter the phone ending in (***) ***-**92"; on a code-entry step it's the only thing telling the user where their code went. Easy to miss in manual testing unless the flow you run actually has hint text on a step. Fix is simple: canonical ref matches the legacy field's name, so you can look up the corresponding discovered_fields entry by ref and carry the two properties over.

  2. The MFA target carryover pairs by position. legacyMFATargets gives the Nth canonical sms choice the Nth legacy sms target. That holds today but isn't guaranteed by the contract — if the orders ever diverge, we'd display one masked number on a button that actually selects a different destination, with no error anywhere. Canonical and legacy entries for the same option share a label, so matching on (type, label) is exact and removes the ordering assumption.

Comment thread packages/managed-auth-react/src/session/state.ts
Comment thread packages/managed-auth-react/src/session/state.ts
@masnwilliams masnwilliams changed the title feat: prefer canonical awaiting-input contract feat: bind canonical managed-auth submissions Aug 14, 2026

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 03aa03a. Configure here.

return;
} catch {
connectStream(jwt);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale refresh ignores session teardown

High Severity

The new stale_interaction resync awaits retrieveManagedAuth and then writes stateRef, calls setState, and may connectStream without checking generationRef (or exchangeRef.active). The other retrieve paths bail out after that await when the session effect has cleaned up. After unmount or a new sessionId/handoffCode, this path can still apply the old snapshot, fire success/error callbacks, and open an SSE connection that nothing will tear down.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 03aa03a. Configure here.

} catch {
connectStream(jwt);
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale refresh skips expiry handling

Medium Severity

If the stale_interaction GET fails, the catch always calls connectStream and then falls through to awaiting_input. resyncAndConnect treats 401/410 on the same GET as an expired session: it sets terminalRef, shows expired UI, and fires onError. Here an expired JWT instead reconnects SSE and puts the user back on the form, so they can keep submitting until a later stream error.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 03aa03a. Configure here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants