Repository navigation
fix(host-service): stage long agent launch commands as a script to dodge MAX_CANON truncation - #6210
Conversation
…X_CANON truncation (#5092) queueInitialCommand types the agent launch command into the PTY after the OSC 133;A marker, but that marker fires from precmd — before the line editor switches the TTY to raw mode. Input written in that gap queues under the kernel's canonical-mode line discipline, which silently drops every byte past MAX_CANON (1024 on macOS). A long --prompt lost its closing quote, wedging the shell at zsh's `quote>` continuation prompt with the agent never launched. Commands over 512 bytes are now written to a 0600 temp script and only a short `. <path>` line is typed (source keeps the interactive shell context; `source` for fish, which dropped `.` in fish 4). The script self-deletes on its first line so cleanup never waits on a long-running agent. Short commands keep the existing type-directly path, and the shell-ready wait + delayed-Enter semantics are unchanged. Covers every initialCommand producer (agents, terminal-agents, setup, command-terminal, teardown) — they all funnel through createTerminalSessionInternal -> queueInitialCommand.
📝 WalkthroughWalkthroughThe terminal service stages initial commands over 512 UTF-8 bytes in temporary self-deleting scripts. It selects shell-specific sourcing, removes cancelled scripts, cleans stale files, falls back to direct typing on staging failure, and adds end-to-end coverage. ChangesInitial command delivery
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant TerminalSession
participant TemporaryLaunchScript
participant PTY
TerminalSession->>TemporaryLaunchScript: Stage oversized initial command
TerminalSession->>PTY: Type shell-specific source command
PTY->>TemporaryLaunchScript: Execute staged command
TemporaryLaunchScript->>TemporaryLaunchScript: Delete script
TerminalSession->>TemporaryLaunchScript: Delete script if cancelled
Possibly related PRs
🚥 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 |
Greptile SummaryThe PR avoids canonical-mode truncation by staging oversized initial terminal commands in restrictive, self-deleting temporary scripts while preserving direct PTY delivery for short commands.
Confidence Score: 4/5The PR appears safe to merge, with a non-blocking cleanup gap for prompt-bearing scripts if the host service terminates during the short pre-execution window. The normal execution and session-cancellation paths remove staged scripts, but process termination before the delayed Enter loses all in-memory cleanup and can leave the complete launch command in temporary storage. Files Needing Attention: packages/host-service/src/terminal/terminal.ts
|
| Filename | Overview |
|---|---|
| packages/host-service/src/terminal/terminal.ts | Stages oversized commands safely and preserves shell selection, but abrupt process termination can leave prompt-bearing scripts in temporary storage. |
| packages/host-service/src/terminal/terminal.initial-command.node-test.ts | Provides focused real-PTY regression coverage for successful long-command staging, self-deletion, and direct delivery of short commands. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Initial command] --> B{UTF-8 length over 512 bytes?}
B -- No --> C[Write command directly to PTY]
B -- Yes --> D[Create exclusive 0600 temp script]
D --> E[Write short source line to PTY]
C --> F[Delayed Enter]
E --> F
F --> G{Session still active?}
G -- Yes --> H[Execute command; script self-deletes]
G -- No --> I[Best-effort unlink]
Prompt To Fix All With AI
### Issue 1
packages/host-service/src/terminal/terminal.ts:901-905
**Prompt-bearing scripts can persist**
If the host-service process terminates after staging an oversized agent command but before the delayed Enter runs, neither cleanup path executes, leaving the complete command and potentially sensitive user prompt in a `superset-launch-*` file until external temporary-directory cleanup removes it.
**How this was verified:** The prompt is written into the staged file, while both deletion paths require the same process to remain alive until the timer or sourced script executes.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Reviews (1): Last reviewed commit: "fix(host-service): stage long initialCom..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| writeFileSync( | ||
| scriptPath, | ||
| `command rm -f -- ${quotedPath}\n${commandText}\n`, | ||
| { mode: 0o600, flag: "wx" }, | ||
| ); |
There was a problem hiding this comment.
If the host-service process terminates after staging an oversized agent command but before the delayed Enter runs, neither cleanup path executes, leaving the complete command and potentially sensitive user prompt in a superset-launch-* file until external temporary-directory cleanup removes it.
How this was verified: The prompt is written into the staged file, while both deletion paths require the same process to remain alive until the timer or sourced script executes.
Knowledge Base Used: pty-daemon
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/host-service/src/terminal/terminal.ts
Line: 901-905
Comment:
**Prompt-bearing scripts can persist**
If the host-service process terminates after staging an oversized agent command but before the delayed Enter runs, neither cleanup path executes, leaving the complete command and potentially sensitive user prompt in a `superset-launch-*` file until external temporary-directory cleanup removes it.
**How this was verified:** The prompt is written into the staged file, while both deletion paths require the same process to remain alive until the timer or sourced script executes.
**Knowledge Base Used:** [pty-daemon](https://app.greptile.com/personal-org-6810/-/custom-context/knowledge-base/superset-sh/superset/-/docs/pty-daemon.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.There was a problem hiding this comment.
Valid — a host-service crash between staging and the delayed Enter skips both cleanup paths (self-delete on execution, pre-Enter unlink on teardown), stranding the prompt-bearing file. Addressed in d73651d: an age-gated (1h) boot sweep removes stale superset-launch-* files from tmpdir; the gate keeps a concurrently-running instance's just-staged script safe (normal script lifetime is <1s). Verified live: a backdated script is removed at module init while a fresh one survives. Residual exposure is same-user-only (0600) and bounded to the crash→next-boot window; note the prompt is equally visible in the launched agent's argv while it runs.
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
A crash between staging and the delayed Enter skips both cleanup paths (self-delete + pre-Enter unlink), leaving the prompt on disk. Age-gated 1h so concurrent instances' just-staged scripts are never touched. Addresses greptile review on #6210.
|
CI note: the Test job failure is pre-existing on main, not from this PR. It fails in |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found and verified against the latest diff
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/host-service/src/terminal/terminal.initial-command.node-test.ts">
<violation number="1" location="packages/host-service/src/terminal/terminal.initial-command.node-test.ts:59">
P3: The new test covers only the /bin/sh happy path; the fish-shell `source` variant, the staging-failure fallback (tmpdir unwritable -> type full command), and the pre-Enter session-end cleanup path added in terminal.ts all ship untested. Adding a fish-shell case (the `source` vs `.` keyword is a distinct branch) and confirming the fallback still types the full line would lock in the non-default behaviors this PR introduced.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| process.env.HOST_SERVICE_VERSION = "0.0.0-initcmd-e2e"; | ||
| process.env.NODE_ENV = "development"; | ||
|
|
||
| __setAccountShellForTesting("/bin/sh"); |
There was a problem hiding this comment.
P3: The new test covers only the /bin/sh happy path; the fish-shell source variant, the staging-failure fallback (tmpdir unwritable -> type full command), and the pre-Enter session-end cleanup path added in terminal.ts all ship untested. Adding a fish-shell case (the source vs . keyword is a distinct branch) and confirming the fallback still types the full line would lock in the non-default behaviors this PR introduced.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/host-service/src/terminal/terminal.initial-command.node-test.ts, line 59:
<comment>The new test covers only the /bin/sh happy path; the fish-shell `source` variant, the staging-failure fallback (tmpdir unwritable -> type full command), and the pre-Enter session-end cleanup path added in terminal.ts all ship untested. Adding a fish-shell case (the `source` vs `.` keyword is a distinct branch) and confirming the fallback still types the full line would lock in the non-default behaviors this PR introduced.</comment>
<file context>
@@ -0,0 +1,167 @@
+ process.env.HOST_SERVICE_VERSION = "0.0.0-initcmd-e2e";
+ process.env.NODE_ENV = "development";
+
+ __setAccountShellForTesting("/bin/sh");
+ initTerminalBaseEnv({
+ PATH: process.env.PATH ?? "/usr/bin:/bin",
</file context>
There was a problem hiding this comment.
Addressed in 44c3441 — added node-tests for the staging-failure fallback (TMPDIR pointed at a nonexistent dir; 550-byte command still round-trips via direct typing) and the pre-Enter teardown cleanup. The fallback test is mutation-verified (fails when the catch returns the staged line). The cleanup test earned its keep immediately: it caught a real leak — dispose unsubscribes daemon callbacks before the pty dies, so exited/cancelled never flip and the Enter-timer guard missed plain disposes, stranding the script and writing Enter into a dead pty. The guard now also treats eviction from the session registry as defunct. Skipped a fish-shell case deliberately: fish isn't on CI runners, so it'd be a permanently-skipped test; the keyword choice is a single data branch (source vs .).
There was a problem hiding this comment.
All reported issues were addressed across 1 file (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
…fallback + cleanup paths Addresses cubic review on #6210, which surfaced a real leak: dispose unsubscribes daemon callbacks before the pty dies, so onExit never sets exited and cancelShellReady no-ops for non-pending states — the Enter timer's guard missed plain disposes entirely, leaking the staged script (and writing Enter into a dead pty). Guard now also treats eviction from the session registry as defunct. Also: warn (not swallow) when the boot sweep fails, and cover the staging-failure fallback and pre-Enter cleanup with node-tests. The fallback test was mutation-verified; the cleanup test is what caught the dispose leak.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/host-service/src/terminal/terminal.initial-command.node-test.ts`:
- Around line 166-191: Update the test around createTerminalSessionInternal to
make the staged command write a sentinel file after its self-delete step, then
wait through the delayed-Enter interval after disposeSessionAndWait and assert
the sentinel remains absent. Keep the existing staged-file cleanup assertion,
and use a unique temporary-path sentinel tied to terminalId.
- Around line 140-162: Update the finally block in the
createTerminalSessionInternal test to restore TMPDIR correctly: when origTmpdir
is undefined, delete process.env.TMPDIR; otherwise restore its original value.
Preserve the existing session cleanup via disposeSessionAndWait.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 06777a67-9d55-4ada-89e4-0d46d7e84cfe
📒 Files selected for processing (2)
packages/host-service/src/terminal/terminal.initial-command.node-test.tspackages/host-service/src/terminal/terminal.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/host-service/src/terminal/terminal.ts
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Restore an unset TMPDIR by deleting it (assigning undefined stores the string "undefined"), and add a sentinel to the pre-Enter cleanup test so it proves the staged command never executed — file absence alone can't distinguish the teardown unlink from run-and-self-delete.
Links
Summary
--prompt(>~1KB) silently truncated the typed launch command and wedged the workspace shell at zsh'squote>continuation prompt — the agent never started even though the dispatch reported ok.queueInitialCommandnow stages commands over 512 bytes as a self-deleting0600temp script and types only a short. <path>source line; short commands keep the existing type-directly path.Why / Context
queueInitialCommanddelivers the agent launch command by typing it into the shell's PTY after the OSC 133;A shell-ready marker. That marker fires fromprecmd, before the line editor switches the TTY to raw mode, so the typed bytes queue under the kernel's canonical-mode line discipline — which caps an input line at MAX_CANON (1024 bytes on macOS) and silently discards the rest. A shell-quoted'claude' '--dangerously-skip-permissions' '<long prompt>'line loses its closing quote; when the delayed Enter lands, zsh sits atquote>forever.Reproduced live via
superset workspaces create --agent claude --prompt "$(python3 -c 'print("x"*2500)')"against v1.18.x: the typed line survived at exactly 1024 bytes (981 of 2500 prompt chars), shell wedged atquote>, no claude process.How It Works
MAX_TYPED_INITIAL_COMMAND_BYTES = 512threshold (conservative — other platforms/line disciplines can be tighter than 1024).$TMPDIR/superset-launch-<terminalId>-<rand>.sh(mode0600,wxflag so it never clobbers) and the typed line becomes. '<path>'— sourcing preserves the interactive shell context so the agent process behaves exactly as if typed. Fish getssource(fish 4 removed.); every other launch shell (zsh/bash/sh/ksh) uses POSIX..command rm -f -- <path>: the shell keeps reading from the already-open fd, so cleanup happens immediately and never waits on a long-running agent. If the session is torn down before the delayed Enter fires, the un-run script is unlinked (asyncrm, best-effort).initialCommandproducers (agents, terminal-agents, setup-terminal, command-terminal, teardown) funnel throughcreateTerminalSessionInternal→queueInitialCommand, so this one site covers them all.Manual QA Checklist
superset workspaces create --agent claude --prompt <2500 chars>→ terminal read shows the typed line cut at exactly 1024 bytes, shell atquote>, no claude processworkspaces.createcall withagents:[{agent:"claude", prompt:"x".repeat(2500)}]→ claude TUI launches with the full 2500-char prompt and responds to it;terminal_agent_bindingsreachesStopsuperset-launch-*leftovers in tmpdir after launch)Testing
terminal.initial-command.node-test.ts(same real-PTY harness asterminal.send-snapshot.node-test.ts): queues a >2KB initialCommand into a real/bin/shsession and asserts the command round-trips byte-for-byte, the staged script self-deletes, and short commands are typed verbatim. Verified it catches the regression: fails (wedged shell, timeout) against unfixedterminal.ts, passes with the fix.bun test(host-service): 950 passnode --test terminal.adoption.node-test.ts: 20/20bun run lint/bun run typecheck: cleanKnown Limitations
terminals send/writeInputfollow-up path is intentionally unchanged: by the time follow-ups arrive the foreground program has the TTY in raw mode, so MAX_CANON doesn't apply. Sending >1KB single-line input to a program sitting in canonical mode (bareread,cat) still truncates — same as any real terminal.terminal.send-snapshot.node-test.ts's three tRPC-caller subtests fail under--experimental-strip-typeson baseline too (extensionless-import loader limitation documented in that file's header) — unrelated, use the tsx loader for those.Summary by cubic
Prevents long agent launch commands from being truncated by the TTY line discipline by staging them as a temp script and typing a short source line instead. Also fixes a cleanup leak on dispose-before-Enter and adds a boot-time sweep for stale scripts.
Bug Fixes
initialCommand>512 bytes to a0600temp script in$TMPDIR, then type. '<path>'(orsourcefor fish); the script self-deletes on its first line to preserve interactive context without leaks.superset-launch-*files older than 1h on start.createTerminalSessionInternal→queueInitialCommand.Testing
Written for commit dc350d7. Summary will update on new commits.
Summary by CodeRabbit