site deploy: finalize through the session id the create returned - #602
site deploy: finalize through the session id the create returned#602netanelgilad wants to merge 4 commits into
Conversation
Two deploys of one commit share a deployment id — it is derived from the commit — so they also share the upload session it addresses, and the loser of a create race finalizes against whatever the winner wrote last. The platform now hands back a per-attempt session_id at create; passing it to finalize resolves this run's own uploads. Both directions stay compatible: session_id is optional on the create response, so a CLI newer than its platform sends nothing and gets the commit-derived session, and an older CLI (the one pinned in today's sandbox images) is unaffected. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G1E31AZvYBa86zpcYJFD1g
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G1E31AZvYBa86zpcYJFD1g
🚀 Package Preview Available!Install this PR's preview build with npm: npm i @base44-preview/cli@0.1.9-pr.602.a7206e5Prefer not to change any import paths? Install using npm alias so your code still imports npm i "base44@npm:@base44-preview/cli@0.1.9-pr.602.a7206e5"Or add it to your {
"dependencies": {
"base44": "npm:@base44-preview/cli@0.1.9-pr.602.a7206e5"
}
}
Preview published to npm registry — try new features instantly! |
The optionality was protecting nobody. The static lane is gated by BASE44_STATIC_DEPLOYMENTS, which only the sandbox publish path sets, and nothing else in the platform calls the deployments API — so there is no client that both reaches this code and lacks a session. A platform that opens none is a mismatched deploy, and failing on it beats silently finalizing into the commit-derived session two siblings share. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G1E31AZvYBa86zpcYJFD1g
| { | ||
| body: formData, | ||
| timeout: 180_000, | ||
| // Resolves this attempt's uploads rather than a concurrent deploy's: |
There was a problem hiding this comment.
Remove the comment
There was a problem hiding this comment.
Removed. searchParams: { session_id: sessionId } reads for itself, and the why belongs in the commit message, not on the line.
Generated by Claude Code
|
|
||
| interface DeploymentCreateResponse { | ||
| deployment_id: string; | ||
| /** This attempt's upload session. Optional here so a spec can mock a |
There was a problem hiding this comment.
No need to test a scenario like that, it won't happen the platform will always return it
There was a problem hiding this comment.
Agreed — dropped. session_id is required on the testkit type again, and the fails clearly when the platform opens no upload session spec is gone. The only reason it was optional was to mock that case, so both went together.
One thing I did beyond the comment, push back if you disagree: with the negative case gone, finalizes through the session the create handed back was setup-identical to sends no PUTs and still finalizes when every asset is already stored and differed only in its one assertion. Rather than leave a near-duplicate, I folded expect(t.api.finalizeQueries[0]).toEqual({ session_id: SESSION_ID }) into the happy-path deploy test and deleted the standalone one. Same coverage, one fewer test — 14 passing.
Generated by Claude Code
The platform always opens a session, so there was nothing to tolerate — the testkit type is required again and the negative spec is gone. The remaining session assertion folds into the happy-path deploy test rather than standing as a near-duplicate of it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G1E31AZvYBa86zpcYJFD1g
Summary
Pairs with base44-dev/apper#20445. A deployment id is derived from the commit, so two deploys of one commit address the same server-side upload session — the loser of a create race finalizes against whatever the winner wrote last. The platform now returns a per-attempt
session_idat create; this passes it back at finalize so the run resolves its own uploads.CreateDeploymentResponseSchemagains requiredsession_id→sessionId.finalizeStaticDeployment/postFinalizetake the session id and always send it as?session_id=.deployStaticSitethreadscreated.sessionIdthrough.docs/deployments.mdupdated per rule 9.Why
session_idis required, not optionalAn earlier revision made it optional so a CLI newer than its platform would degrade gracefully. That was protecting nobody, and the check is concrete:
BASE44_STATIC_DEPLOYMENTS, set in exactly one place across the platform —backend/app/user_apps/sandbox/sandbox_build_operations.py— the sandbox publish path.sandbox-cli-static-publishflag, which is currently closed.The platform always opens a session, so there is nothing to tolerate. Public
base44 site deployusers are unaffected either way: with the gate off they take the legacy tar.gz path and never touch this code.Testing
bun run test static_site_deployments— 14 passed. The session assertion lives in the happy-path deploy test (expect(t.api.finalizeQueries[0]).toEqual({ session_id: SESSION_ID })) rather than in a test of its own, since a standalone one was setup-identical to an existing case.Making
session_idrequired also forced every mocked create in the spec to carry one, which is the schema doing its job.TestAPIServernow captures finalize query strings infinalizeQueries— it only captured multipart fields before.Full-suite notes, both verified pre-existing on clean
main, not introduced here:bun run typecheckreports 6 errors insrc/cli/dev/dev-server/function-bundler.ts— identical count with the change stashed.bun run testfails 25 tests indev.spec.ts/exec.spec.ts;denoisn't installed in this environment and those specs drive Deno subprocesses.exec.specalone fails 15/17 on stashedmaintoo.