fix(sandbox): don't install session keep-alive in non-interactive create#118
Open
tcerqueira wants to merge 2 commits into
Open
fix(sandbox): don't install session keep-alive in non-interactive create#118tcerqueira wants to merge 2 commits into
tcerqueira wants to merge 2 commits into
Conversation
`sandbox create` defaults to `--timeout session`, which keeps the sandbox alive only while this process (the primary client) stays connected and blocks on a SIGINT keep-alive. In non-interactive / CI mode that loop never terminated: `create --json --non-interactive` hung forever, never emitting JSON nor exiting. Reject the default session timeout up front in non-interactive mode (before creating an orphan) with a USAGE / NON_INTERACTIVE_REQUIRED envelope that points to an explicit `--timeout`, since a session-scoped sandbox would be destroyed the moment the process exits. Gate the SIGINT keep-alive (and the ssh fallback) on interactivity, route status chrome to stderr, and enrich the `--json` create payload with org and timeout.
…mmands Standardize the output contract so stdout carries ONLY data payloads (the --json result, or in human mode the list table / piped value) while every success / confirmation / status message goes to stderr, in both --json and human modes. sandbox/mod.ts: - create: "Created sandbox with id" and "Exposed port ... to ..." -> stderr (the exposeHttp branch no longer needs a --json special-case) - kill: "Sandbox ... killed successfully." -> stderr - deploy: "Successfully deployed ..." -> stderr - ssh helper: the "ssh <conn>" echo and the connect-info fallback -> stderr sandbox/volumes.ts, sandbox/snapshot.ts: - delete: "Successfully deleted volume/snapshot ..." -> stderr config.ts (shared spine, reached by every sandbox command): - "Created configuration file at ..." and the interactive "Selected organization/application" confirmations -> stderr. The config-file message in particular was leaking onto stdout and corrupting `sandbox create --json` whenever it ran in a directory without an existing deno.json(c). Data outputs (sandbox/volume/snapshot ids, list tables, extend result, exec output) stay on stdout unchanged.
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.
Problem
sandbox create --json --non-interactivehangs forever in CI (rough edge from #112).createdefaults to--timeout session. Asessiontimeout keeps the sandbox alive only while the creating process (the primary client) stays connected; the CLI implements this by installing a SIGINT keep-alive and blocking until Ctrl+C. That keep-alive had no TTY /--non-interactiveguard, while the JSON-emit-and-exit path only ran for non-session timeouts. So with the default session timeout in non-interactive mode the process installed the keep-alive and blocked indefinitely, never emitting JSON nor exiting.Fix
isNonInteractive), reject the defaultsessiontimeout up front — before creating an orphan sandbox — with aUSAGE/NON_INTERACTIVE_REQUIREDenvelope (exit 2) whose hint points to an explicit--timeout(e.g.--timeout 15m). A session-scoped sandbox would be destroyed the instant this process exits, so silently returning would hand back a dead id; requiring an explicit, self-sufficient timeout is the correct and least-surprising CI semantics.--jsoncreate payload withorgandtimeout.Interactive behavior (keep-alive until Ctrl+C) is unchanged.
Tests
tests/agent.test.ts):sandbox create --json --non-interactivewith the default session timeout now fails fast with a clean stdout and aNON_INTERACTIVE_REQUIREDenvelope (exit 2) — the test would hang without the fix.deno fmt --check,deno lint,deno checkall pass.