refactor(providers): use shared random suffix helper - #2140
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. Reviewed September 12, 2026, 9:11 AM ET / 13:11 UTC (Revision 6). ClawSweeper reviewWhat this changesFive provider adapters call the shared random suffix generator directly and remove their equivalent local wrappers. Merge readiness✅ Ready for maintainer review Keep open as a useful, behavior-preserving cleanup with no actionable findings. The wrappers remain on the reviewed main revision, and the related merged cleanup addresses a different helper. Priority: P3 Review scores
Verification
How this fits togetherCrabbox provider adapters translate CLI run requests into remote sandbox operations. The shared suffix generator supplies randomness for sandbox names and Crownest creation request identifiers. flowchart LR
A[CLI run request] --> B[Provider adapter]
B --> C[Shared suffix generator]
C --> D[Sandbox name]
C --> E[Creation request identifier]
D --> F[Provider API]
E --> F
Before mergeNone. Agent review detailsSecurityNone. Review metrics
Technical reviewBest possible solution: Use the existing shared generator directly while preserving each adapter's naming rules and request timing. Do we have a high-confidence way to reproduce the issue? Not applicable: this refactor reports no broken runtime behavior, and the complete diff shows equivalent helper calls. Is this the best way to solve the issue? Yes. Removing trivial forwarding wrappers is a narrow simplification that retains the existing implementation and provider boundaries. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against b26f057a6cf2. LabelsLabel justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (5 earlier review cycles)
|
2e49acf to
ab7ad91
Compare
ab7ad91 to
f2abd6c
Compare
f2abd6c to
eea65a0
Compare
What Problem This Solves
Several provider adapters duplicated the same random suffix wrapper, making naming behavior harder to audit consistently.
Why This Change Was Made
Use the shared internal
shared.RandomSuffixhelper at the existing call sites and remove equivalent provider-local wrappers. Generation behavior and call timing are unchanged.User Impact
No user-visible behavior changes. Random suffix generation now uses one shared implementation across the updated providers.
Evidence
go vet ./...go build -trimpath ./cmd/crabbox