Stop a missing sandbox token from failing the whole deploy - #830
Conversation
The production deploy of #819 failed at config load, before any resource was touched, so nothing deployed at all: ConfigError: SchemaError(Expected string at ["SANDBOX_INTERNAL_SERVICE_TOKEN"]) `requireSecretEntry` in the sandbox Worker's props was meant to refuse a sandbox that would answer 401 to every call, which is right. What it actually did was fail `alchemy deploy` for the entire stage on a secret nobody had provisioned yet, taking every other Worker with it. The decision moves up to the stack, where it can be made without failing: `secretIsSet` reports presence rather than reading the value, and the sandbox Worker is declared only when the stage runs one AND the token is there. Without it the deploy proceeds, the sandbox is not provisioned, and the api already logs that the repository sandbox is unavailable. The Worker's own props keep requiring the token, which is now unreachable with it absent. Setting SANDBOX_INTERNAL_SERVICE_TOKEN for prd/stg is what turns the feature on. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
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 (3)
Included review availability: Your plan provides up to 4 included reviews per hour; 1 remains after this review. 📝 WalkthroughWalkthroughThe change adds a non-failing secret-presence helper and uses it to make sandbox deployment conditional on ChangesSandbox token gating
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant AlchemyRun
participant secretIsSet
participant MapleSandbox
AlchemyRun->>secretIsSet: check SANDBOX_INTERNAL_SERVICE_TOKEN
secretIsSet-->>AlchemyRun: return configured boolean
alt token is configured
AlchemyRun->>MapleSandbox: create sandbox
else token is absent
AlchemyRun-->>AlchemyRun: log warning and skip sandbox
end
Merge Risk: ⚪ Minimal · up to Missing or blank sandbox tokens now skip only sandbox creation while allowing other stage resources to deploy. The behavior is covered by tests and no merge-blocking risk remains. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
The production deploy of #819 failed at config load, before any resource was touched, so nothing deployed:
requireSecretEntryin the sandbox Worker's props was meant to refuse a sandbox that would answer 401 to every call. That intent is right. The effect was wrong: it failedalchemy deployfor the entire stage on a secret nobody had provisioned yet, so every other Worker was blocked from updating too.The decision moves up to the stack, where it can be made without failing.
secretIsSetreports presence without reading the value, and the sandbox Worker is declared only when the stage runs one and the token is set. Without it the rest of the stage deploys normally, no sandbox is provisioned, and the api logs that the repository sandbox is unavailable — which it already did for exactly this case. The Worker's own props still require the token; that path is now unreachable while it is absent.Setting
SANDBOX_INTERNAL_SERVICE_TOKENfor prd and stg is what turns the feature on.Verified: typecheck and lint clean, infra tests pass including two new cases pinning that
secretIsSettreats blank as absent and never fails whererequireSecretEntrydoes, and a dev-stage plan is unchanged.Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by CodeRabbit