fix: bound optional provider pricing requests - #1630
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. |
|
Codex review: needs real behavior proof before merge. Reviewed September 3, 2026, 12:07 PM ET / 16:07 UTC. ClawSweeper reviewWhat this changesThis PR adds five-second deadlines to optional AWS and Hetzner price lookups, updates cost-rate documentation, and adds Worker regression coverage for stalled provider responses. Merge readiness⛔ Blocked before merge - 8 items remain Keep open: current main still awaits AWS pricing without a deadline, so the work remains necessary, but the proposed AWS timeout cannot release callers when Node credential resolution never settles and the branch is currently conflicted with main. Priority: P1 Review scores
Verification
How this fits togetherThe Worker coordinator asks provider adapters for optional hourly prices before lease admission, activation, and recovery, then combines them with fallback rates for cost records. These price lookups must not delay provisioning workflows when provider metadata is unavailable. flowchart LR
A[Lease or workspace request] --> B[Worker coordinator]
B --> C[Provider price lookup]
C --> D{Price response arrives}
D -->|Yes| E[Cost calculation]
D -->|Timeout or failure| F[Fallback rate]
E --> G[Lease admission or activation]
F --> G
Before merge
Findings
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Copy recommended automerge instructionTechnical reviewBest possible solution: Rebase onto current main and make optional AWS pricing return the existing fallback after five seconds even when credential resolution never settles, with regression coverage through a Fleet caller and redacted runtime proof of fallback continuation. Do we have a high-confidence way to reproduce the issue? Yes, source-reproducible: configure an AWS credential provider that never resolves, request a spot quote, and advance five seconds. The abort signal cannot interrupt the outstanding credential await, while Fleet remains awaiting the quote. Is this the best way to solve the issue? No. Propagating an abort signal to AWS HTTP is useful, but the deadline must also release the optional-price caller when credential resolution never settles while preventing any post-timeout provider request. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against a2ee4be96759. LabelsLabel changes:
Label justifications:
EvidenceAcceptance criteria:
What I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
|
Optional provider pricing is awaited before lease reservation, final activation, and workspace recovery. A stalled pricing response can therefore block these flows indefinitely, even though they already use fallback rates when a quote fails.
This change gives AWS and Hetzner pricing HTTP requests a five-second lifetime, including response-body reads. It aborts the owned request and awaits settlement before the existing fallback continues. AWS quotes use a separate EC2/STS client so their deadline cannot cancel another operation's cached identity check; only these optional quotes disable automatic retries. Ordinary provider requests, identity validation, provisioning deadlines, and cost precedence remain unchanged. Node AWS credential resolution stays owned by the SDK chain, and an expired quote starts no HTTP request after credentials return.
The adjacent usage example now uses the flat cost-override keys actually accepted by the existing parser.
Validation:
cx53infsn1, returning$0.06/hourin 898 ms without allocating a resource.The candidate AWS API path has not been verified against real AWS credentials. The local cancellation test is HTTP transport proof, not live AWS proof. No coordinator deployment was performed; merge is on hold pending the maintainer's live-proof decision. This does not claim to fix an observed long-running coordinator request whose blocked server stack was not available, nor the separate fixed-ID cancellation race.
Production delta: +45 lines for request-lifetime ownership; tests: +368; docs/changelog: +11. No config, storage/schema, protocol, or dependency changes. Exact-lockfile CI is required; local tests used the existing prepared dependency installation.