Skip to content

fix(auth): move OAuth account controls below consent actions - #7575

Merged
waleedlatif1 merged 1 commit into
stagingfrom
codex/oauth-consent-account-footer
Sep 8, 2026
Merged

fix(auth): move OAuth account controls below consent actions#7575
waleedlatif1 merged 1 commit into
stagingfrom
codex/oauth-consent-account-footer

Conversation

@waleedlatif1

Copy link
Copy Markdown
Collaborator

Summary

  • Move the account and switch-account controls below the consent actions.
  • Remove the Sim CLI destination sentence while preserving destination visibility for other clients.

Type of Change

  • Bug fix

Testing

All 12 consent and OAuth hook tests passed. Full repository lint, all 46 audits, the block registry check, and docs-manifest validation passed.

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@vercel

vercel Bot commented Sep 8, 2026

Copy link
Copy Markdown

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

1 Skipped Deployment
Project Deployment Actions Updated
docs Skipped Skipped Sep 8, 2026 12:48am UTC

Request Review

@waleedlatif1
waleedlatif1 merged commit f52da15 into staging Sep 8, 2026
25 checks passed
@waleedlatif1
waleedlatif1 deleted the codex/oauth-consent-account-footer branch September 8, 2026 00:48
@greptile-apps

greptile-apps Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR reorders the OAuth consent controls and suppresses the redundant loopback destination sentence for Sim CLI authorization.

  • Moves the current-account and switch-account controls below Allow and Deny.
  • Preserves destination rendering for non-CLI OAuth clients.
  • Updates the CLI consent test to verify control order and destination suppression.

Confidence Score: 4/5

The PR appears safe to merge, with a non-blocking test-coverage gap for preserved non-CLI destination visibility.

The changed rendering logic correctly distinguishes the exact Sim CLI client and preserves existing pending and error behavior; only the new non-CLI conditional branch lacks direct regression coverage.

Files Needing Attention: apps/sim/app/(auth)/oauth/consent/consent-view.test.tsx

Important Files Changed

Filename Overview
apps/sim/app/(auth)/oauth/consent/consent-view.tsx Reorders account controls beneath consent actions and restricts destination text to non-CLI clients.
apps/sim/app/(auth)/oauth/consent/consent-view.test.tsx Covers the revised CLI behavior and control order, but lacks a positive non-CLI destination test.

Reviews (1): Last reviewed commit: "fix(auth): move OAuth account controls b..." | Re-trigger Greptile

expect(container.querySelector('h1')?.textContent).toBe('Authorize Sim CLI')
expect(container.textContent).toContain('Continuing as test@example.com.')
expect(container.textContent).toContain('Returns to this computer.')
expect(container.textContent).not.toContain('Returns to this computer.')

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.

P2 Non-CLI Path Lacks Coverage

The new assertion covers only the CLI suppression path because the fixture always uses clientId='sim-cli'. The new non-CLI branch has no positive test confirming that Returns to {destination} remains visible, so a future regression in that requirement could pass unnoticed. Add a non-CLI case with a valid redirect URI and assert that its destination is shown.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

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.

1 participant