feat(web): cloud environments (part 2: setup scripts) - #424
Open
akhileshrangani4 wants to merge 16 commits into
Open
akhileshrangani4 wants to merge 16 commits into
akhileshrangani4 wants to merge 16 commits into
Conversation
…cap test Reject environment variable names outside the POSIX name rule (^[A-Za-z_][A-Za-z0-9_]*$) at the API boundary (PATCH /api/environments/[id] and PATCH /api/user/repo-env), and make encryptEnvironmentVariables skip invalid keys as a second line of defense. Job env keys reach a shell export line unquoted, so an invalid key was a metacharacter injection point, not just a validation gap. Also add a multi-byte case to the setup-script size-cap test: an all-ASCII oversized string can't tell a correct Buffer.byteLength check apart from a wrong .length check, so it didn't actually pin the byte-cap behavior. Minor: quote the hardcoded paths in writeSetupScript's shell commands to match the uploadFilesToSandbox precedent.
…ccess handleSaveEnvVars awaited both PATCHes without checking response.ok, and EnvironmentVariablesModal treated a non-throwing onSave as success and closed. Once /api/user/repo-env started rejecting invalid variable names, that gap went from dormant to live: a bad key closed the modal as though the save worked, with nothing actually saved. patchJsonOrThrow now throws with the server's error message on a non-2xx response, and runSaveEnvVars keeps the modal open and shows that message on a rejected save, matching how the standalone Environment editor already handles this. Also close the third route the same key validation missed: PATCH /api/chats/[chatId]/env now validates keys against the same POSIX rule before encrypting, with accept/reject tests.
createSandboxForChat now writes the environment's setup script into every new sandbox and starts it as a detached job when the script is non-empty. The resulting SetupRunRecord is persisted on the chat, and ensure-sandbox moves the chat to setting_up only while a job is genuinely running. SetupRunRecord.handle is now optional: a script-less environment gets a record for hash tracking but no fabricated JobHandle, since there is no job to poll or attach to. isSetupRunRecord enforces that a handle-less record can never claim state "running".
…cing setup createSandboxForChat now wraps everything after daytona.create in a try/catch: a throw anywhere in that span (clone, branch setup, writing or starting the setup script) deletes the sandbox it just created and rethrows the original error unchanged, so a failed create no longer leaves an untracked sandbox behind. Added a runSetupScript option, defaulting to true, and scheduled.ts now passes false. The cron has no setting_up-equivalent gating yet, so starting a setup job there would let the agent run against a tree that is mid-install; scheduled runs keep getting the environment's variables, just not its setup script, until that follow-up lands. Also drops a stray em dash from a setup-script.test.ts comment.
A chat whose sandbox started a setup job now returns setting_up instead of running the turn. The user's message is persisted before that return, so a client that disconnects does not lose it and the cron backstop has something to dispatch. The turn-start path moves out of the messages POST handler into runQueuedTurnForChat, so exactly one path starts an agent turn: the handler drives it for a normal send, and dispatchQueuedTurn drives it for a turn held by setup. The move is verbatim apart from setupFailureNote, which is prepended only to the string handed to the agent, never to the persisted user message. Both the /setup SSE endpoint and the agent-lifecycle cron can observe the same job exit, so the setting_up transition is an updateMany guarded on the current status and only the caller that changed a row starts the turn. Setup failure never hard-blocks: the agent runs anyway, told what broke and where the script lives.
Claiming a finished setup run used to flip the chat to ready before starting the turn. Nothing looks at a ready chat: the cron's interactive monitor wants running plus a backgroundSessionId, its setup phase wants setting_up, and /api/agent/stop returns early without a backgroundSessionId. An invocation killed during turn startup, which spans history, MCP, skill discovery and session creation, therefore stranded the chat with a persisted message, no reply and no way out. The claim now only stamps claimedAt into setupRun and leaves the chat in setting_up, so it stays 409-busy to a second tab, persistTurn moves it to running when the turn really starts, and the cron re-claims a claim older than five minutes. Verified against Postgres: an unclaimed run is claimable, a live claim is not, a stale one is, and two concurrent observers still produce one winner. Also: the dispatcher and cron phase 5 start the sandbox before using it, since a held turn skipped ensureSandboxForChat and autoStopInterval is five minutes; phase 5 marks a chat errored when its sandbox is gone instead of re-failing forever; the agent-switch replay excludes the queued message, which the earlier persist made visible to it; and the no-handle case leaves the SSE endpoint as a done event rather than a JSON body an EventSource client cannot read.
The setup dispatch phase marked a chat errored on any throw from daytona.get, not just a 404. Because the phase iterates every setting_up chat, a Daytona 5xx, a network timeout, or a rotated API key would have errored all of them in one tick and discarded every queued turn in the system. isSandboxGoneError, alongside the existing statusCode precedent in lib/sandbox.ts, now separates the two. Only a genuinely absent sandbox is terminal; everything else rethrows into the per-chat catch, so the chat stays setting_up and the next tick retries. That matches what the adjacent ensureSandboxStarted call already did. Phase 5 moves into _lib/setup-dispatch.ts, following the convention the route already documents and its four other phases already follow, so both give-up branches can be tested directly. Removing the guard fails exactly the two transient-failure tests.
Adds syncSetupScript, called best-effort after both turn-completion paths (the SSE stream handler and the agent-lifecycle cron), so a fix an agent makes to a broken setup script persists for future chats on that repo. Only the script column is ever written back; a diverged stored script (a user edit during the turn) is left alone rather than overwritten.
Adds SetupBlock (live progress off the setup SSE stream, collapses on success, expands with the log on failure or a broken stream) and SetupScriptUpdatedNotice (diff view + one-level revert) to ChatPanel, plus the revert-script endpoint they need. Extends EnvironmentDTO with setupScriptPrevious, which nothing previously exposed to the client.
…estamps The notice's visibility was gated on Environment.updatedAt vs Chat.updatedAt, but sync-back always writes the environment before the chat's own row is bumped, so the comparison had the ordering backwards and could never fire. Replace it with an explicit scriptUpdateNotice marker that sync-setup-script stamps onto the chat itself and that rides the SSE complete event straight to a watching client, with no query invalidation required. The notice now fetches its diff bodies on demand instead of holding them for every chat, and can be dismissed (keyed per edit, so a later edit is never swallowed by an earlier dismissal). Also stop SetupBlock from reporting a broken SSE connection as a failed script: reconnect with a bounded backoff first, and only give up with an honest "connection lost, setup may still be running" once retries are exhausted.
Adds two actions to the environment editor: "Run setup" validates the saved script in a throwaway sandbox built from the environment's real settings (repo, network mode, variables), streaming its output and deleting the sandbox on every exit path. "Set up with agent" opens a normal chat seeded to have the agent write, run, and iterate on the script until the project builds, then navigates there so the chat is never created somewhere the user can't find it. Extracts SetupBlock's log-panel markup into a shared SetupLogPanel so the new run doesn't duplicate that rendering. Splits the pure setup path/prompt helpers into lib/setup-paths.ts (re-exported from lib/setup-script.ts) so the client-side prompt builder doesn't pull Node's crypto or sandbox-jobs into the browser bundle. Also corrects the setup-script description in the editor (it does run, as of Task 3) and removes a leftover em dash.
…ranch Caps a validation run's setup-script timeout well under the route's maxDuration and gives the throwaway sandbox a short auto-delete window, so a run that would previously outlive its own invocation no longer strands a sandbox until Daytona's four-day backstop. Resolves the repo's real default branch instead of assuming main. Requires Sec-Fetch-Site: same-origin on the run-setup GET so a crafted cross-site link cannot create and bill a sandbox on a signed-in victim's account. Also reorders the assisted-setup prompt consumption in ChatPanel to check agent/model before consuming the staged prompt, so a partially-populated chat can't silently destroy it.
Both comments asserted that a jobs.status() poll counts as Daytona sandbox activity as if it were confirmed fact. Nobody has run this against a real sandbox yet, so reword both to say what is assumed versus what is verified, and point at the staging checklist that covers it.
…e turns misfiring The POST /messages held-turn body was a shape no client knew about: the optimistic-send path read uploadedFiles off it, threw inside the cache updater, and painted a red error on the first send of every chat whose environment has a setup script. SendMessageResponse is now a discriminated union with a setting_up variant, so the compiler makes every consumer say which one it is handling. The held body carries sandboxId, branch and previewUrlPattern so the cache is right before the turn starts, and drops setupRun, which nothing read. The dispatcher parks the chat in setting_up, skips startStreaming, and removes the optimistic assistant placeholder, which the server never persists and whose id the dispatcher does not reuse. When the setup stream finishes, the chat is reloaded with the server's own status and session rather than being forced to ready, so the dispatched turn is picked up immediately instead of after the next chat-list poll. Also: - the /setup route reads the job log from byte 0 on every connection, so the client replaces its buffer on each connection's first chunk instead of appending a full replay to it - a queued turn is refused if the chat's newest conversational row is an assistant message: on a recreated sandbox a dispatch killed before the message was persisted could otherwise re-run an answered turn - the /setup route starts the sandbox before polling it, as cron phase 5 already does, so a tab reopened after autoStopInterval does not report a lost connection for a job that is merely parked
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part 2 of cloud environments. Part 1 (#423) let you save a setup script. This one actually runs it.
Stacks on #423, so that one needs to go in first.
how it works
The script gets written into the sandbox as a real file at
/home/daytona/.backgrounder/setup.sh, outside the repo clone so no push flow picks it up, and started as a detached job with@background-agents/sandbox-jobs. It isn't awaited. an npm install routinely outlives the request that kicked it off.The chat goes into a new
setting_upstate and the first turn waits.GET /api/chats/[chatId]/setupstreams the log and starts the turn when the job exits. If nobody's watching, the per-minute cron picks it up instead. Both of them can see the same exit, so the transition is a status-guardedupdateManyand only whoever actually changed a row starts the turn.If the script fails the turn still runs, the agent just gets told what broke. And since the script is a real file the agent can fix it, which syncs back to the environment with one level of undo. that also gives us "Set up with agent" basically for free, it's a normal chat pointed at that file with a seed prompt.
the file thing is the whole design
Writing it as a file instead of piping a string is what makes the agent able to fix or author a script with no new tool to learn or forget. Everything else falls out of that.
Sync-back only ever reads the script back. never variables, never network mode, never the name. an agent that can write a file shouldn't be able to write a secret into the DB, and the query doesn't even load the variables.
stuff worth knowing before you review
scheduled runs skip setup on purpose. they'd have started a job nobody waits for, so the agent would run against a half-installed tree. gave
createSandboxForChatan opt-out and the cron passes it, so scheduled behaviour is identical to today. proper fix needs the same waiting machinery the chat path has, didn't want to write a second copy of it.Run setup uses a throwaway sandbox with a 3 minute script cap (under the route's 5 minute ceiling) and a 15 minute auto-delete, so a hard-killed invocation leaks minutes instead of the default 4 days.
that route is a GET that creates a sandbox. EventSource can't send a body. it checks
Sec-Fetch-Site: same-originso a crafted link can't spend your quota, which is defence in depth and not a CSRF token. real fix is POST-to-start plus GET-to-stream, didn't do it here. also Safari before 16.4 doesn't send that header so those users get a 403 on that button.env var names are validated now (
^[A-Za-z_][A-Za-z0-9_]*$). sandbox-jobs interpolates the key straight into anexportline and nothing was checking it. not an escalation since it's your own vars in your own sandbox, but an invalid name used to fail as a weird shell error instead of a clear 400.not verified
i haven't tested this against a real sandbox yet. still need to run it with a Daytona API key. so the server emitting a held response off a live job, the real SSE stream, and a real dispatch on job exit are all unproven so far. the client seam is verified with intercepted routes, that's it.
There's a 7 item staging checklist in the spec covering what I still need to run. The one I'd actually worry about is whether the cron's per-minute poll counts as Daytona activity against
autoStopInterval: 5. if it doesn't, a long script with nobody watching gets its sandbox stopped out from under it. two code comments used to assert it did, they now say it's an assumption.Also
e2e/setup-scripts.spec.tscan't pass in our harness even with a key, the test-auth user has no linked GitHub account so the clone dies before any of this runs.known gaps
testing
556 unit tests, typecheck clean, production build clean.