Repository navigation
Reuse the gateway setup in PostgreSQL credential-withdrawal tests - #2790
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
|
Codex review: needs maintainer review before merge. What this changesThis PR replaces ten repeated worker configurations with the existing gateway setup helper. Example: A credential-withdrawal test starts its worker with a 50 ms convergence timeout.
Review scores
ProductKind: Test only · Worth it: Yes · Fix scope: Complete Merge readiness✅ Ready for maintainer review Keep this PR open: it is a focused, behavior-preserving test cleanup with no actionable findings, and repository policy reserves closure for maintainer triage. Priority: P3 Before mergeNone. FindingsNone. Agent review detailsHow this fits togetherPostgreSQL integration fixtures start the real controller worker with simulated Compute and gateway Drivers, then inspect durable work, authorization outcomes, and withdrawal state. flowchart LR
A[Withdrawal test] --> B[Existing gateway setup helper]
B --> C[Worker fixture]
C --> D[Controller worker]
D --> E[PostgreSQL state]
D --> F[Simulated Compute Driver]
Technical reviewBest possible solution: Reuse the existing helper while retaining each scenario’s Compute behavior and the event-recording case’s explicit options. Do we have a high-confidence way to reproduce the issue? No product bug is claimed; source inspection verifies that the helper supplies the exact options removed from each call site. Is this the best way to solve the issue? The existing helper is the smallest complete owner for this repeated setup, and the diff preserves scenarios, assertions, and special event capture. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against ee9391867036. Provenance checked
TestingProof path: in-process harness. SecurityNone. EvidenceWhat I checked:
Likely related people:
Review metrics
LabelsLabel changes:
Label justifications:
Rating scale6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior. WorkflowClawSweeper edits this one comment on every review. Comment Reviewed October 17, 2026, 6:09 PM ET / 22:09 UTC. |
Ten worker starts repeated the gateway driver and 50 ms convergence setting already provided by
startWithGateway. They now pass just the Compute behavior each test needs. The case that also records worker events keeps its explicit setup.All 36 tests in the file passed before and after with PostgreSQL 18.6. In a separate copy, making the first withdrawal report
revokedinstead ofpendingfailed the existing retry assertion: one attempt instead of two. These are worker tests with a simulated gateway, not a live credential operation.