Skip to content

fix(functional-tests): drop login_hint from the prompt=none grant test - #21359

Merged
nshirley merged 1 commit into
mainfrom
agent-31f814
Oct 1, 2026
Merged

nshirley merged 1 commit into
mainfrom
agent-31f814

Conversation

@fxa-agent

@fxa-agent fxa-agent Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Because

  • The prompt=none silent grant test in unverifiedSession.spec.ts fails on every production smoke run. 123done shows unauthorized_client.
  • When a prompt=none request has a login_hint, fxa-settings requires the client to be on the prompt_none allowlist. The production 123done client is not on it. The stage client is.

This pull request

  • Opens 123done without login_hint before the test clicks the prompt=none button. The test signs in first, so the silent grant uses the current session and does not need the hint.

Issue that this pull request solves

Closes:

Checklist

Put an x in the boxes that apply

  • My commit is GPG signed.
  • If applicable, I have modified or added tests which pass locally.
  • I have added necessary documentation (if appropriate).
  • I have verified that my changes render correctly in RTL (if appropriate).
  • I have manually reviewed all AI generated code.

How to review (Optional)

  • Key files/areas to focus on: unverifiedSession.spec.ts, the prompt=none test.
  • Suggested review order: n/a, one file.
  • Risky or complex parts: none. oauthPromptNone.spec.ts still covers login_hint checks on local and stage.

Screenshots (Optional)

n/a

Other information (Optional)

  • Production: an engineer ran the test against production. It failed with login_hint and passed without it.
  • Local: not verified. On the sandbox runner, the settings dev server did not compile, so the test stopped at the first Email first click, before the changed line. CI runs this test on the local stack.
  • 123done does not need a change.
  • One reviewer call: is it fine to drop the hint here, or should ops add the production 123done client to the prompt_none allowlist instead?

@fxa-agent fxa-agent Bot added the auto label Sep 30, 2026
@LZoog
LZoog marked this pull request as ready for review September 30, 2026 20:37
@LZoog
LZoog requested a review from a team as a code owner September 30, 2026 20:37
Copilot AI lite review requested due to automatic review settings September 30, 2026 20:37

Copilot AI 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.

Copilot review overview

🟢 Approval recommended

The change is low risk; the remaining documentation nit is non-blocking.

Review effort: Lite
Findings: 1 Low severity

Open (1)
What changed in this PR

Fixes the production prompt=none smoke test by relying on the existing signed-in session instead of login_hint.

Changes:

  • Removes login_hint from the test navigation.
  • Retains silent-grant and verification assertions.
File Description
packages/​functional-tests/​tests/​signin/​unverifiedSession.spec.ts Updates the prompt=none flow to use the current session.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.


const query = new URLSearchParams({ login_hint: credentials.email });
await page.goto(`${target.relierUrl}/?${query.toString()}`);
await relier.goto();
@nshirley

nshirley commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

force push was to rebase with main

Because:

* The prompt=none silent grant test fails on every production smoke run with unauthorized_client.
* A prompt=none request with login_hint needs the client on the prompt_none allowlist, and the production 123done client is not on it.

This commit:

* Opens 123done without login_hint before the prompt=none click, so the silent grant uses the current session.
@nshirley
nshirley merged commit dad409c into main Oct 1, 2026
20 checks passed
@nshirley
nshirley deleted the agent-31f814 branch October 1, 2026 19:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants