Skip to content

test(grants): a pending revoke error and confirm do not survive a 403 - #3281

Merged
lilyshen0722 merged 1 commit into
mainfrom
launch/grants-403-test
Nov 1, 2026
Merged

lilyshen0722 merged 1 commit into
mainfrom
launch/grants-403-test

Conversation

@lilyshen0722

Copy link
Copy Markdown
Contributor

Follow-up to CodeRabbit's note on #3276. The forbidden-reload test there revoked successfully, so the revoke error and the held confirm were already clear before the 403 reload. It would have passed with either clear removed.

The new test fails the revoke so both stay up, loses access (a 403 reload), then gets it back (a clean reload). It checks that the list returns with no alert and no held confirm. A new auth token recreates the api client and reruns the load, the way a session change would.

Reverse-verified: removing setRevokeError('') or setConfirmGrantId(null) from the 403 branch fails it. V2AgentRuntimeGrants: 29 of 29 pass.

🤖 Generated with Claude Code

https://claude.ai/code/session_015AKgpMD6Qh13f7fj6mbGT3

The existing forbidden-reload test revoked successfully, so both states were
already clear before the 403 and it passed with either clear removed. This
one fails the revoke first, loses access, gets it back, and checks neither
the error nor the held confirm returns. Removing either clear fails it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015AKgpMD6Qh13f7fj6mbGT3
@coderabbitai

coderabbitai Bot commented Nov 1, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between 95ee27c and 6a456ae.

Files selected for processing: 1
  • frontend/src/v2/__tests__/V2AgentRuntimeGrants.test.tsx

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 2 remain after this review.

Walkthrough

This change adds a regression test for connector-grant state across failed revocation, a 403 reload, and restored access after a token change.

Changes: Connector grant state

Layer / File(s) Summary
Grant state reset coverage
frontend/src/v2/__tests__/V2AgentRuntimeGrants.test.tsx
The test checks that a failed revoke error and confirmation do not remain after a 403 reload and later successful reload. It also checks that the restored Revoke control is enabled.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Other

Possibly related PRs

Merge Risk: Minimal · up to 6a456

No actionable merge-blocking risk was identified in the regression test.

Pre-merge checks: Passed 4
Check name Status Explanation
Title check Passed The title clearly identifies the new grants test and the pending revoke error and confirmation behavior across a 403 reload.
Description check Passed The description directly explains the test scenario, the expected state clearing, and the reported test validation.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Finishing Touches
Generate docstrings
  • Commit to this branch
  • Create a new PR
Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

@lilyshen0722 lilyshen0722 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Review at 6a456ae, tests only.

  • It reaches the 403 branch. A new token rebuilds useV2Api's headers and then get, api and load, which reruns the effect. The "Owner and admins only" assertion confirms the 403 path ran.
  • It fails without each clear. Removing setRevokeError('') brings the "Could not revoke the grant." alert back. Removing setConfirmGrantId(null) brings "Yes, revoke it" back. Both checked on a copy, with the original restored and checksum-matched.
  • It isn't flaky. Three repeated runs pass, and the full file passes, 29 of 29. Every await is a findBy*. beforeEach resets the axios mocks, so nothing leaks into later tests.

GATE: PASS

@lilyshen0722
lilyshen0722 added this pull request to the merge queue Nov 1, 2026
Merged via the queue into main with commit 350da46 Nov 1, 2026
30 checks passed
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