test(oauth): exercise the account-removal path the quorum test is named for - #3600
Conversation
lidge-jun
added
the
chore
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Maintainer admin merge (ruleset bypass recorded per MAINTAINERS.md): test-only follow-up to #3530; 7/0 locally with a RED probe; per maintainer instruction the final dev-tip CI run is the acceptance gate for this batch. wp5 E0 of the 260905 open-work closeout. |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Team Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe test now mirrors the account deletion flow. It removes the account, clears session affinity, and verifies that ChangesQuorum cache invalidation
Estimated code review effort: 2 (Simple) | ~10 minutes ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d73d1bca04
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| expect(await removeAccount("anthropic", ids[1]!)).toBe(true); | ||
| clearAnthropicSessionAffinityForAccount(ids[1]!); |
There was a problem hiding this comment.
If the Anthropic branch in oauth-account-routes.ts stops calling clearAnthropicSessionAffinityForAccount, this test still passes because it manually performs both the store removal and cache invalidation. The existing DELETE API test only verifies that the roster shrinks, so the production regression—completed account deletion leaving the cached quorum true for the TTL—would remain undetected. Invoke the DELETE handler/endpoint here, or add the quorum assertion to the API test, rather than duplicating its implementation.
Useful? React with 👍 / 👎.
|
✅ Deterministic PR hygiene checks passed. |
1 participant
Summary
Follow-up to #3530. The restored contract test
removing an account invalidates immediately, not after the TTLnever removed an account: it cleared session affinity, marked the store unread, and asserted only that the store was re-read. With two accounts still in the roster the predicate could not observe the transition the test is named for, so deleting theremoveAccountcall from the DELETE route would have left it green. The test now mirrors the route order (removeAccountfirst, thenclearAnthropicSessionAffinityForAccount) and asserts quorum is actuallyfalseafterward.Unit:
devlog/_plan/260905_open_work_closeout/(050 E0, 051).Verification
bun test tests/routing/anthropic-quorum-cache.test.ts— 7 pass / 0 fail. RED probe: with the newremoveAccountline commented out the newtoBe(false)assertion fails (6 pass / 1 fail), proving the assertion observes the roster reduction; the sibling "manual account selection invalidates immediately" test still passes with two accounts intact.bun run typecheck— exit 0.Checklist
devSummary by CodeRabbit