Repository navigation
test(rooms): keep revocation cases deterministic under load - #2184
milind-soni merged 2 commits into
Conversation
…r load The two in-flight revocation cases raced a 3-5s scripted window and could go red under pair load (pair 20260929-221744). Hold the child on a gate file after its MCP calls, land the access change while it runs, keep it held until the periodic revalidation of the running child enforces the withhold, then release the gate, with a 30s poll budget and a 60s case timeout. Co-Authored-By: GLM-5.3 <noreply@z.ai> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@pauldupuyjr is attempting to deploy a commit to the SupaMaus Team on Vercel. A member of the Team first needs to authorize it. |
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 33 minutes. View limit detailsLimit details: You’ve used all 8 included reviews currently available. Review configuration: ⚙️ Run configuration
📒 Files selected for processing (1)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughThe room coordination end-to-end tests now gate recipient results and release them after the child node fails. The affected tests also use longer polling and overall timeout limits. ChangesRoom coordination test synchronization
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Other Suggested reviewers: Merge Risk: ⚪ Minimal · up to The change is limited to end-to-end test synchronization, and the timeout path has bounded cleanup. The supplied test and check results are green; no material merge-blocking risk remains. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 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 |
|
Reviewed the gate against the scripted agent and running-child revocation flow. Merged current main locally: the three targeted cases pass, and the complete room-coordination file passes all 40 tests. Assertions remain intact; only the timing window is replaced with observable state. Ready for a refreshed CI run after shared baseline repair #2528. |
What changed
server/room-coordination.e2e.test.tsonly. The three in-flight revocation cases ("withholds a cross-team result if the owner revokes access while it runs" and both variants of "withholds an in-flight result after team %s and recreation") now hold the delegated child on agateFileinstead of a fixeddelayMswindow:server/testing/room-handoff-agent.tsalready has (the same pattern as the existing queued-dispatch and section-change cases).releaseAfterWithhold, waits until the child is markedfailed, then releases the gate.Every assertion is unchanged. No production code changes, and the scripted agent doesn't change.
Why
The child was scripted to stay "running" for 3 s (5 s in the team cases). The test had to see "running", land the access change, and have the periodic revalidation of the running child withhold the result, all inside that window. Under load the 15 s poll for "running" could time out before the child started (
expected undefined to be running). It also happened when two suites shared one machine. A fixed window either expires too soon, or lets the child complete before revalidation runs.Releasing the gate right after the access change isn't enough either. That variant fails deterministically with
expected 'completed' to be 'failed', because the withhold is enforced by revalidation, not at the moment of the PATCH. Holding the gate until the child isfailedremoves both timing dependencies.The longer budgets only matter when a case fails. In passing runs each revocation case still takes about 3 s, the same as before.
How it was verified
pnpm typecheck,pnpm lintandpnpm i18n:checkpass, andvitest run server/room-coordination.e2e.test.tspasses on its own.failedbefore release is needed.Checklist
pnpm typecheckandpnpm testpass locally (Linux; the full suite ran twice as above)dist-server/edits (it's build output)shell: true/ cmd.exe string-buildingSummary by CodeRabbit