Repository navigation
fix(terminal): submit startup commands with Enter on every platform - #23672
Conversation
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: 📥 CommitsReviewing files that changed from the base of the PR and between b1c322a7f0cf2b0ec093bff7fbe89b8c64d1b54c and 2d2529b956e4009c048c431a2aedfbab2fa3df95. 📒 Files selected for processing (4)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 5 remain after this review. 📝 WalkthroughWalkthroughStartup command submissions now use Priority: ➖ Normal Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to Startup commands consistently use carriage return while retaining readiness and bracketed-paste behavior. No concrete merge-blocking issue remains; merge after normal checks pass. Architecture SummaryArchitecture risk: 🔵 Low · up to The change affects 1 system. Changed systems: Architecture concerns Review detailsSystems and components
Before / after behavior
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
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 |
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
The CR switch itself is correct, but the branch leaves its own test suite red: src/main/ipc/pty-login-shell-startup-commands.test.ts still asserts LF and now fails in two tests. That file is CI-sharded (config/scripts/ci-shard-timings.json:2146), so this blocks merge.
Reviewed changes
- Single submit byte:
buildStartupCommandSubmissionno longer takes a caller-suppliedsubmit; it always terminates with CR and replaces any trailing\r\n/\r/\non the command with one CR (src/shared/startup-command-submission.ts). - Callers simplified: the daemon session-create path, the local shell-ready provider, the relay handler and the SSH background delivery all drop their platform checks / explicit
submitand rely on the builder. - Tests realigned to expect CR across the daemon, local-provider, relay and shared-builder paths, plus new LF/CR/CRLF normalization cases.
⚠️ pty-login-shell-startup-commands.test.ts was not updated, so two tests fail
The PR updates every startup test it touched, but misses this file, which exercises the same writeStartupCommandWhenShellReady -> buildStartupCommandSubmission path through the pty:spawn IPC handler. Both assertions still expect LF and fail against this branch.
I ran the changed files locally: node_modules/.bin/vitest run …pty-login-shell-startup-commands.test.ts reports 2 failed | 8 passed, with expected 'claude\n' / printf "hello"\n receiving claude\r / printf "hello"\r. The other seven startup-related test files pass (101 passed).
Technical details
# Missed LF expectations in pty-login-shell-startup-commands.test.ts
## Affected sites
- `src/main/ipc/pty-login-shell-startup-commands.test.ts:236` — `does not write the startup command before the shell-ready marker arrives` expects `toHaveBeenCalledWith('claude\n')`; actual is `'claude\r'`.
- `src/main/ipc/pty-login-shell-startup-commands.test.ts:399` — `keeps the conservative max wait for non-agent startup commands` expects `toHaveBeenCalledWith('printf "hello"\n')`; actual is `'printf "hello"\r'`.
## Required outcome
- Both expectations updated to CR, matching the rest of the PR.
## Suggested approach (optional)
- Not strictly required, but worth adding this file to the set the author ran; the PR body says "the changed tests (93 pass)", which did not include it.ℹ️ Nitpicks
- fish is the only shell flagged bracketed-paste-safe whose line editor differs from readline/zle, and it is untested here (the author noted fish, Linux, PowerShell and nushell were not verified). A quick fish smoke test of a single-line and a multiline startup command would close the one path this PR cannot infer from the zsh/bash results.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
This run re-reviewed the commits pushed since the prior pullfrog review (1782094).
- Missed test expectations corrected:
pty-login-shell-startup-commands.test.ts:236,399andpty-spawn-env-codex-resume-provenance.test.ts:281now assert the CR terminator, resolving the prior review's blocking finding. No production code changed in this commit. - Suite verified green: all 9 startup-related test files pass locally (
116 passed), including the previously-redpty-login-shell-startup-commands.test.ts.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
|
Thanks! Fixed in 4669cd254c: updated both LF expectations in On fish: I tested it with fish 4.9, using the builder's real output for a single-line command and a multiline bracketed-paste command. With default bindings both LF and CR run. With |
4669cd2 to
5e96651
Compare
 |
b1c322a to
83d8a7a
Compare
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
This run re-reviewed the commit pushed since the prior pullfrog review (4669cd2): 83d8a7a flips three stale expectations in the two pty:spawn IPC test files.
- Missed IPC expectations corrected:
pty-login-shell-startup-commands.test.ts:236,399andpty-spawn-env-codex-resume-provenance.test.ts:281now assert the CR terminator instead of LF, resolving the prior blocking finding. No production code changed. - Suite verified green: the two changed files plus
startup-command-submission.test.tspass locally (31 passed), including the previously-redpty-login-shell-startup-commands.test.ts.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
This run re-reviewed the commit pushed since the prior pullfrog review (83d8a7a): 2d2529b normalises interior line breaks on the non-bracketed-paste path.
- Raw-path interior breaks now use CR:
buildStartupCommandSubmissionreplaces interior\r\n/\nwith CR whenbracketedPasteSafeis false, so a multiline startup command onsh/dash/nushell/PowerShell/cmd.exesubmits every line with Enter rather than leaving the same remappable Ctrl+J mid-command (src/shared/startup-command-submission.ts:63). - Bracketed-paste payload untouched: the paste-safe branch still wraps the body verbatim and appends a single trailing CR, so literal newlines inside a quoted prompt survive.
- Tests realigned and extended: a new unit case pins LF/CR/CRLF interior normalization (
startup-command-submission.test.ts:41-47) and another pins that the paste payload keeps its literal newlines (:49-54); the local-provider and daemon integration assertions now expect CR between lines. All changed/relevant startup test files pass locally (74 passed), andoxlintis clean on the commit's files.
The one behavior change worth naming — raw LF previously did not act as Enter on Windows PowerShell, so interior breaks were non-submitting there — is benign: PSReadLine re-inserts a newline for syntactically incomplete input, and complete lines executing line-by-line is normal interactive-shell behavior. No regression found.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
|
Thanks, Kelvin. This is the right fix: CR is what Enter sends, and with Ctrl+J remapped (the #23250 setup), new agent tabs and startup commands now run instead of sitting at the prompt. I checked it live on macOS (before: stuck, after: runs, both single-line and multi-line) and on Linux bash/zsh with Ctrl+J rebound; default shells behave exactly as before. One problem showed up on Windows. Converting interior line breaks to Enter makes Windows PowerShell 5.1 (which Orca runs without PSReadLine) wait at One note on the description: the fix covers commands Orca types into the shell (on the default path that's fully covered), and Windows mostly passes startup commands as launch arguments, so it only sees the typed path in the cases above. |
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
This run re-reviewed the commits pushed since the prior pullfrog review (2d2529b).
- Collapsed multi-line PowerShell literals onto one physical line:
quotePowerShellLiteralnow routes any value containing\r/\nthrough a newquotePowerShellMultilineLiteral, which emits a double-quoted string with backtick escapes instead of a single-quoted string with literal newlines, so an agent prompt containing a line break no longer submits early mid-command on Windows (src/shared/powershell-native-argument.ts:1-20). - Kept the escaping value-preserving:
$, backtick, ASCII"and the typographic double quotes U+201C–U+201E are backtick-escaped,\r/\nbecome`r/`n, and the single-quoted form's typographic single-quote doubling is unnecessary inside a double-quoted string — so the new form evaluates to the same string the old one did, just on one line. - Extended the tests: new unit cases pin the one-line form and its double-quoted specials (
src/shared/powershell-native-argument.test.ts:12-25), the native-argv pre-escape path is carried through the multi-line form (:33), a win32-only native test compares the new form's argv against the legacy form (powershell-native-argument.windows.test.ts), and a Windows agent-launch prompt is pinned to one line (src/shared/tui-agent-startup.test.ts:88-99).
I verified the equivalence independently against the PowerShell parser source (its IsDoubleQuote is exactly " plus U+201C/201D/201E, and a backtick before an unrecognized character returns that character), and by executing the old and new transforms side by side — including through quotePowerShellNativeArgument's (\\*)" pre-escape — with no input where the two forms diverge. The many existing quotePowerShellLiteral call sites (SSH relay deploy/probe scripts, opencode runtime, hook installer values, agent prompts) embed the result in -EncodedCommand or pass single-line values, so the format change is cosmetic for them. All touched and consumer tests pass locally.
deepseek-v4.1-flash (free via Pullfrog for OSS) ｜ 𝕏
LF is Ctrl+J to a line editor, so a shell that remaps it left the command at the prompt. Fixes stablyai#23250 Co-authored-by: Aashish Mahato <145881415+aashish254@users.noreply.github.com>
…ith CR The CR switch only replaced the trailing terminator. On the raw path — a shell with no bracketed paste, so each line break submits its own line — interior LFs stayed LF, which is the same remappable Ctrl+J the fix set out to remove. A multiline startup command in sh/dash/nushell/PowerShell therefore still stalled partway through for a user who rebinds Ctrl+J. Normalise interior breaks to CR on that path only; the bracketed-paste payload keeps its literal newlines.
A multi-line agent prompt was quoted as a single-quoted PowerShell string with raw line breaks. Typed into Windows PowerShell 5.1 without PSReadLine, each break submits a line and the shell waits at `>>` forever. Values with CR/LF now become one double-quoted line with backtick escapes, keeping every byte; single-line values keep the single-quoted form unchanged.
…quoting Restores the escape form the single-quoted regex used on main and applies it to the new double-quoted regex and the native argv test, so the diff against main stays minimal and the characters are visible in review.
bf999bf to
ea9ea8a
Compare
nwparker
left a comment
nwparker
left a comment
There was a problem hiding this comment.
Startup command submission now sends Enter consistently across local, daemon, relay and SSH. Bracketed multiline paste remains exact, raw multiline uses CR, and PowerShell literals remain a single physical shell line. 184 tests, node/web typechecks and quality gate pass; Windows-only real PowerShell test skipped on macOS.
Required PR Checks verify passed on this exact commit: ea9ea8a.
…rompt-on-command-line Main landed its own daemon protocol v40 (#25291), so staging moves to v41 with 40 attachable (the version file, the local build contract and the pinned tests). Main's #23672 submits startup commands with CR and drops the submit option: the daemon and relay keep the staged line under the new signature, and the staging tests expect the sourcing line to end in CR. The renderer SSH startup delivery file stays deleted; main's only change there removed the submit option.
…not ride the line after #23672 (#25734) #23672 quotes a multi-line PowerShell argument as one physical line, so the control's short multi-line prompt now rides the line on a Windows host that can prove the agent, and the unit shard went red. The control now uses a multi-line prompt over the typed budget, which still pastes, and a new test pins that a short multi-line prompt rides the PowerShell line as one physical line.
…ole (step 2 of 7) (#23962) * feat(agent-launch): host-side prompt delivery for agent.launch The host's agent.launch typed any launch prompt into the shell as part of the launch command. A long or multi-line prompt then ran line by line in the shell, and an agent that never showed readiness or crashed at startup had nothing guarding where its text went. agent.launch now carries a prompt on the typed line only when the line stays one line, control-free and at most 512 bytes; otherwise the agent starts clean and the host pastes the prompt once the agent's own ready signal fires (bracketed paste plus its composer marker or a quiet render, read only after the shell's last hand-off, never while the pane's own shell is proven in front), with main's draft-paste bytes and an Enter 50 ms later. Orchestration worker starts wait on tui-idle as before. A replay-safe launch admits and claims its ledger row in one write, Qwen Code gets a second Enter, the desktop and phone share one launch-refusal classifier, and hosts advertise agent.launch.prompt-carry.v1. Split out of #23748, which moves the desktop source-control buttons onto this path. * fix(agent-launch): keep a short-lined multi-line prompt on a local zsh launch line, as main did #24257 moved every multi-line or over-512-byte prompt off the typed launch line and pasted it after readiness. The phone's AI buttons and review notes, whose multi-line prompts main typed whole into zsh, then reached Claude 0.5-3 s later and their RPC reply waited for the paste. The host now names the shell a local macOS or Linux line is typed into, the way the spawn picks it, and a multi-line prompt rides a zsh line when every line is at most 512 bytes and the whole line at most 8 KB. A real-zsh test types such a line through Orca's own ready barrier and startup write, including when a slow user config makes the write land early. Elsewhere the measured unsafe cases keep the paste: bash 3.2 runs multi-line lines piecemeal, fish drops an early multi-line write, and any shell loses a line over 1 KB written early. * test(agent-launch): keep the real-zsh launch-line test out of the Windows lane's gate scan The Windows lane registration check read `const ZSH_PATH = process.platform === 'win32'` (the head of a multi-line ternary) as a Windows-true flag, so `describe.skipIf(!ZSH_PATH)` looked like a Windows-only suite. The file is POSIX-only; the zsh lookup is now a function. * refactor(protocol): move the agent.launch capabilities into their own module Main's protocol-version.ts sits at the 300-line cap, so the prompt-carry capability pushed it over. The four agent.launch capabilities and their doc move to agent-launch-runtime-capability.ts, re-exported by name and spread into RUNTIME_CAPABILITIES at the same position; the advertised lists and every export are unchanged. * refactor(protocol): import the agent.launch capabilities from their own module `export *` from protocol-version left the four names undefined under the mobile recording loader, which resolves a relative import through a Proxy with no own keys, so 37 phone recordings lost agent.launchReplay. Importers now name agent-launch-runtime-capability directly; protocol-version only spreads its list. * refactor(agent-launch): drop the unshipped viewMode field and trusted local caller id Both were inert in step 1 and existed only for step 2. agent.launch will become a public plugin API, so every wire field is permanent once shipped; a top-level viewMode reads as "choose terminal vs chat", which the host decides. Step 2 introduces placement and view intent under a placement object instead. * fix(agent-launch): read Codex's provisional startup from the rule files' hold anchor Main (#24375) moved Codex's provisional-header check into codex.json's provisional_startup hold anchor and deleted codex-terminal-readiness.ts, so the launch readiness hold now asks showsHoldAnchor, as main's own settled check does. * fix(agent-launch): hold rule-file name titles to quiet for a launch, and census the zsh fixture Main (#24375) answers a name-only title from each agent's rule file ahead of the sustained-title lane, so gemini.json's name_title settled a launch readiness wait on the shell's auto-title while Gemini was still booting. A launch now asks quiet of every weak idle verdict, as that lane did. Main's readiness census requires a recorder for every runtime fixture; the zsh prompt recording is a non-agent control. Gemini's synthetic baseline is regenerated for this PR's stated change: a bare gemini title is no longer its rest mark, so name-only rows settle weak, and a fresh working or blocked status is no longer overridden. * fix(launch): stage long or multi-line launch lines so they arrive whole A launch line that is long or has several lines could lose lines or wedge a shell that is still starting, because it was typed into the terminal before the shell could read it whole. The terminal host (local daemon, and the SSH relay) now writes such a line to a script file and types one short line that runs it, for every POSIX shell; a shell that cannot read Orca's quoting runs Orca's own agent lines through /bin/sh. If the script cannot be written, the terminal says so. The SSH background launch path no longer types the line from the renderer; the relay stages it like any other terminal. Bumps the terminal daemon protocol to 40 (main took 39); a v39 daemon keeps its sessions and gets no staged line. Restacked onto #24257 from the version reviewed on top of #23748; carries none of #23748's changes. * fix(launch): run a staged launch line as its own job in fish and ksh fish and ksh run the commands of a sourced file in the shell's own process group, so a staged agent was not a job: Ctrl-Z was dropped (a typed line stops), and the shell-in-front check saw the shell's group in the foreground while the agent ran. fish now runs the script with `eval (string collect < '<path>')`, which runs it as if typed and leaves the shell's job-control mode alone. ksh and mksh run a long Orca-built line through `/bin/sh '<path>'`, a real job; a long command the user wrote is typed as before. The real-shell test now asserts the agent's process group is its own and is the terminal's foreground group. The staging rule also reuses hasControlByte and the 512-byte budget from startup-line-prompt-carry so the carry and staging limits cannot drift. * fix(agent-launch): paste a launch prompt only when the launched agent is proven in front A launch pasted its prompt unless a shell was proven in the terminal's foreground, so any read that could not prove one let the prompt through. After an agent exited at startup, its shell turned bracketed paste on at the next prompt, readiness fired on it, and the prompt was typed into the shell: - macOS: a pane runs its shell under login, so the process-group fence's root was never the shell's group and never proved it; the cached foreground name could also still name the exited process. - Windows Git Bash and WSL: the shell-alone-in-its-job check never answers. Now one fresh read of the terminal's foreground decides: agent, shell or unknown. Only 'agent' lets a write through (paste, Enter, second Enter, reused panes too); 'shell' still drops a ready signal. A Windows host never proves the agent, so there the launch line carries the prompt at any size, as on main. * test(agent-launch): cover the Windows QA stub, a grok override that exits at once * fix(agent-launch): keep the local socket alive while a prompted launch waits for its agent A launch with a prompt now waits up to 60 s for the terminal agent to be ready before it writes the prompt, and reports not-delivered when the agent never is. The local runtime socket closes a connection idle for 30 s unless the request is a long poll, so a launch whose agent exited at startup lost its reply and the caller saw 'runtime closed the connection' instead of not-delivered. Classify a prompted agent.launch and agent.launchReplay as a long poll, as orchestration.workerStart already is for the same wait. * refactor(agent-launch): narrow the launch params by 'in' instead of a cast * fix(agent-launch): find a launched agent behind a wrapper that leads its process group A tcsh or nu launch line runs the agent from /bin/sh '<script>', and a wrapper script that does not exec its agent does the same: the wrapper leads the terminal's foreground process group and the agent is a member of it. The fresh foreground read names the group's leader, sh, so a prompted launch was refused or pasted late (M4Air tcsh: 2 of 4 not delivered, 2 pasted ~9 s late). Before that read, take the host's process-group observation as positive proof when it names the launched agent among the foreground group's members and is younger than a ready signal's quiet window. It never proves a shell. * fix(launch): type a plain Orca agent line as is in tcsh and nu In tcsh and nu, every Orca-built agent line ran through `/bin/sh '<script>'`, so the agent was a child of sh in sh's process group. When the prompt was pasted after the agent was ready, the paste guard read sh in front and, on a loaded Mac, reported the prompt not delivered (4/4 live runs in tcsh). Now only a line that needs it goes through `/bin/sh`: one over 512 bytes, with a control byte, or with a character those shells would not read literally (`!`, backslash, `"`, `$`, backtick). A plain line such as `claude '--dangerously-skip-permissions'` is typed as on main, so the agent is the shell's own job. * fix(agent-launch): judge the foreground-group proof by when its capture began, not how long ps took The age the host stamps on a process-group observation runs from the start of its whole-machine ps, so on a loaded Mac a capture begun after the read was asked for still read as older than 1 s and the proof was dropped. Count an observation whose capture began after the read was asked for, less the window a shared capture is reused across. * test(agent-launch): keep the crash-guard live test out of the Windows lane's gate scan The Windows-lane registration scan read the const assigned from a platform check as a Windows-only gate, though the suite runs everywhere but Windows; find zsh in a function instead, as the real-zsh typed-line test does. Under load the fresh foreground scan can fail to answer, which lets the shell's prompt settle readiness (2 of 4 paired runs). The guard still refuses that write, so assert the refused write, the property that must always hold. * perf(agent-launch): read a local pane's foreground from its own terminal, not the whole process table The foreground read that gates every launch paste ran the daemon's inspectProcess capture and then a fresh scan, each a whole-machine ps; the fresh one also waits for any capture already running before it starts its own. Measured here at load 5: 1.2 s a read (M4Air QA: 3.4-5.0 s, and worker starts 17.6-32 s against main's 9-12 s at load 25-84). On a local macOS or Linux host, take the pane's root pid from the provider's session inventory and run one ps limited to that pane's terminal. Its foreground process group decides: the launched agent or any non-shell member is the agent (a wrapper that did not exec its agent leads the group), a group of shells alone is the shell. Same pane, same verdict: 2.7 ms a read. SSH hosts keep the relay's observation and name. * test(mobile): re-measure the web app's script sweep after agent.launch's capabilities moved out of protocol-version The mobile web bundle check failed at 124 assets against a ceiling of 123. Main already sat exactly on that ceiling: its sweep table read 69 scripts at 16 routes while the tree builds 73, the whole margin of 4. This branch imports the agent.launch capabilities from their own module, so protocol-version is no longer pulled into the root layout and four other routes. That moves which routes share which modules, and the Qoder capability module, imported by protocol-version and the AI-vault resume path, no longer shares an importer set with anything, so it gets a chunk of its own: 74 scripts. The fence says to re-derive the bound rather than raise it, so the sweep is re-measured on this head (every prefix of the sorted route list). The worst route now adds 10 scripts (session), not 9, which moves the pinned shell crossing from 32 to 30 routes; main re-measured on its own lands on the same crossing. * fix(agent-launch): a worker's brief needs its agent found in front, and Grok's start answers on its composer A paired-server worker start whose agent exited at startup typed its brief into the server's shell, which ran it: the idle wait can settle on a shell back at its prompt, and the brief was written with no foreground read. Both worker-start paths now check before each brief write, as a launch prompt is checked: on a host that can find the agent in front it must be there; on one that cannot (Windows) a shell proven in front still refuses, and anything else writes as before. A Grok worker start waited ~10 s more than main: its only rest signal is its bare name, which a launch holds to quiet output, and Grok animates its logo for ten seconds after its composer glyph. A worker start for an agent whose rest signal is its bare name and whose composer draws a marker (Grok, DSH, mimo-code) now also answers on that marker, whichever comes first. * test(startup-staging): expect the CR that submits a typed launch line since #23672
…ch-prompt-argv-carry Conflicts resolved against #23962's head this branch had merged (83de487), so each keeps main's squashed #23962 plus this branch's change: - daemon protocol v42 (launch files), v41 (#23962's staged startup lines) now in PREVIOUS; build compatibility contract and pinned version tests follow. - agent-draft-readiness keeps main's OpenCode agent-row grace (#25101) and this branch's bracketed-paste-off revocation of the quiet window. - startup-line-prompt-carry.test.ts stays deleted; its rule lives in launch-prompt-carry. Semantic follow-ups from #23672 (CR submission, one-line PowerShell quoting): - windowsLaunchLineVerdict reads line breaks from the prompt too: PowerShell now quotes a multi-line prompt onto one physical line, which hid the measured multi-line rows (a 9 KB multi-line prompt read as exact) and the shim's line-break cut. - Typed launch lines end in CR; raw multi-line lines submit each break with CR; the PowerShell multi-line expectations use the one-line backtick form. - A duplicated pair of #23962 staging tests left by the auto-merge is removed. - agent-paste-draft: the two identical launch-paste tails share sendLaunchPaste; tui-agent-config writes devin on one line like jcode (main sat at the line cap).
ELI5
When Orca starts a command in a new terminal, it types the command and then "presses Enter". On macOS and Linux it was actually pressing Ctrl+J. Most shells treat Ctrl+J like Enter, but some people change what Ctrl+J does. For them the command just sat at the prompt until they pressed Enter themselves. Now Orca presses the real Enter key everywhere, including between the lines of a command that spans several lines.
What Changed
Before: on macOS and Linux, quick commands, new agent tabs, automations, setup scripts and
orca terminal create --commandtyped the command but did not run it if the user's shell remaps Ctrl+J (for example, vi mode with Ctrl+J set to "insert a new line").After: the command runs right away.
How: every path that starts a command in a new terminal goes through
buildStartupCommandSubmission. It used to take the "Enter" character from its callers, and three of them picked LF (\n, which is Ctrl+J) on macOS/Linux and CR (\r, which is Enter) on Windows. Now the builder always ends the command with CR, and the callers no longer choose. If a command already ends with a line break, I replace it with CR so it isn't sent twice.A command can also span several lines. There are two ways Orca delivers one:
sh,dash, nushell, PowerShell,cmd.exe) has no paste mode, so each line break inside the command is itself an "Enter". Those are now CR too — otherwise a multi-line command would still stall partway through for exactly the user this PR is for.Why
CR is what the Enter key sends, so any shell that responds to Enter responds to it. Windows already used CR, the SSH background path already used CR, and Orca's other "press Enter" code (
orca terminal send --enter, quick-command insert, agent prompt submit) already sends CR. I could have just changed the three platform checks, but keeping the choice in one place stops them from drifting apart again.Linked Issue
Fixes #23250
Refs #23156. It has the same symptom, but a default zsh runs the command fine with the old character, and the reporter hasn't said whether they remap Ctrl+J, so I can't confirm it's the same cause.
Visual Proof
N/A. Nothing in the UI changes. The only difference is which character Orca sends to the shell after a startup command, so the proof is the shell behaviour under Testing.
Testing
pnpm tc,oxlint,pnpm run check:code-quality:changedand every changed test file (116 pass).I ran
echo RAN-$((6*7))(fish:echo RAN-(math 6 \* 7)) in real shells on macOS (zsh 5.9, bash 3.2, fish 4.9), ending it with each character:"\C-j": self-insertin.inputrc)bind ctrl-j 'commandline -i \n'), single-line and multilineReview
Agent skill upstream boundary
docs/reference/agent-skill-sharing-upstream-boundary.mdand copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.Notes
completedwhen the command never ran. That needs its own fix.Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)