Skip to content

feat(terminal): add Cmder as a Windows terminal shell - #22462

Open
b1nhm1nh wants to merge 6 commits into
stablyai:mainfrom
b1nhm1nh:feat/cmder-shell
Open

b1nhm1nh wants to merge 6 commits into
stablyai:mainfrom
b1nhm1nh:feat/cmder-shell

Conversation

@b1nhm1nh

@b1nhm1nh b1nhm1nh commented Sep 23, 2026

Copy link
Copy Markdown

ELI5

Many Windows developers use Cmder: the normal Command Prompt plus a setup script that adds a git-aware prompt, better line editing (Clink), Unix tools and personal aliases. Orca could only open plain Command Prompt, so all of that was missing. This PR adds Cmder as a shell you can pick for Orca terminals on Windows.

What Changed

Before: the Windows shell choices were PowerShell, Command Prompt, Git Bash and WSL. Cmder users got plain cmd.exe with no aliases, prompt or Clink.

After: when Cmder is installed, Cmder appears in Settings → Terminal → Windows Shell, in the first-run onboarding shell step, in the new-tab "+" menu, and as orca terminal create --shell cmder. This also works on a Windows machine reached over SSH when that machine has Cmder. A Cmder terminal opens in the project folder with Cmder fully set up. If Cmder is missing, the pane quietly falls back to plain cmd instead of failing to open. If Cmder was saved as the default and later uninstalled, the option stays visible but disabled (as Git Bash does) so the user can see what is selected and change it.

Mechanism:

  • New shell name cmder joins the existing allow-list (src/shared/windows-terminal-shell.ts), so the CLI, the terminal.create RPC and the SSH relay all accept it from one place. Commands typed into it are quoted as cmd.
  • src/main/cmder.ts finds the install: an optional hidden setting (terminalWindowsCmderPath), then CMDER_ROOT (set by Cmder's docs and the Scoop package), then standard folders (Program Files, %LOCALAPPDATA%, %USERPROFILE%, Scoop, C:\tools, C:\cmder). It is found when vendor\init.bat exists.
  • The hidden setting travels to terminal processes as ORCA_CMDER_ROOT in the spawn environment (cleared when the setting is emptied), so the out-of-process daemon and the in-process fallback resolve the same folder as the main process.
  • Both terminal spawn paths (the in-process provider and the out-of-process daemon) turn cmder into cmd.exe /K "chcp 65001 > nul & call <init.bat> & …", reusing the existing cmd.exe launch builder (resolveWindowsShellLaunchArgs).
  • The init.bat path reaches cmd.exe through environment variables (%ORCA_CMDER_INIT_QUOTE%%ORCA_CMDER_INIT%%ORCA_CMDER_INIT_QUOTE%), the same technique the existing Codex preflight uses, because node-pty backslash-escapes literal quotes in argv and cmd.exe cannot read them. This keeps install paths with spaces working without putting user text on the command line.
  • Inherited CMDER_CONFIGURED / CMDER_INIT_* are cleared first. Otherwise, when Orca itself is launched from a Cmder console, init.bat sees them and skips its setup.
  • A new availability probe (cmder:isAvailable IPC and host.cmder.isAvailable RPC) decides whether Settings, onboarding and the "+" menu show Cmder. For SSH, the relay's existing Windows capability check (preflight.detectWindowsTerminalCapabilities) now also reports cmderAvailable. The field is optional everywhere, so older hosts and relays simply report no Cmder.
  • The SSH relay on a Windows host maps cmder to cmd.exe plus init.bat for both new spawns and revived panes, instead of trying to run a program named cmder. It finds Cmder using that host's own environment, including a CMDER_ROOT passed with the pane.
  • Cmder tabs reuse the existing CMD icon; the design-system check disallows new hard-coded icon colours.

Why

  • A dedicated shell entry rather than asking users to set cmd's AutoRun registry key: AutoRun applies to every cmd.exe on the machine (scripts, build tools, CI helpers), which is invasive and breaks things. Scoping the setup to Orca's own panes avoids that.
  • Env-var quoting rather than a literal quoted path: node-pty escapes argv quotes as \", which cmd.exe doesn't understand. The env-var approach is already proven in this file for the Codex preflight.
  • Auto-detection with a hidden override rather than a visible path field: CMDER_ROOT plus standard folders covered every setup we tried, so a visible field was noise. The hidden setting remains for unusual installs. It applies to this machine only; a remote Windows host finds its own Cmder.
  • Fallback to plain cmd rather than an error: a settings value can outlive an uninstall, and a pane that won't open is worse than one without Cmder's extras.

Linked Issue

Fixes #22461

Visual Proof

Settings → Terminal → Windows Shell, on a machine with Cmder installed.

Before: PowerShell, Command Prompt and Git Bash only.

before-windows-shell

After: Cmder appears as an extra option (selected here). It is only shown when Cmder is installed (or already saved as the default); no other layout or style changes.

after-windows-shell-cmder

The new-tab "+" menu and the onboarding shell step list Cmder under the same rule. Behaviour is also covered by a real launch test: src/main/cmder.smoke.node-pty.test.ts starts a real terminal with the same launch args and asserts init.bat ran and the working folder was kept (runs only when Cmder is installed).

Testing

Tested on Windows 11 (LTSC 2024, build 26100) with Cmder 1.3.25 at C:\programs\cmder, CMDER_ROOT set. Not tested on macOS/Linux (the feature is Windows-only and every new code path is guarded by process.platform === 'win32'). SSH was exercised only through unit tests of the relay launch config, not against a live Windows SSH host.

  • I manually tested these changes locally
    • pnpm build:unpack succeeds, including the mobile web bundle.
    • In the running app, Cmder shows in Settings and the "+" menu, and new Cmder panes open with the Clink prompt, git branch and user aliases in the project folder.
    • Panes restored after restarting the app keep their Cmder setup.
    • Resource Manager: an idle Cmder pane uses about 20–30 MB versus about 75 MB for PowerShell 7, with no ongoing CPU after startup.
  • Automated tests added/updated
    • src/main/cmder.test.ts: discovery order, quoting/trailing separators, platform guard, env setup, clearing inherited Cmder state, dropping a stale carried folder when the setting is cleared.
    • windows-shell-args.test.ts: the exact /K chain with Cmder init plus a startup command.
    • local-pty-provider-windows-shell-launch.test.ts: cmder default spawns cmd.exe with init.bat before the Codex preflight, with the right env; the in-process fallback honours the carried ORCA_CMDER_ROOT.
    • pty-shell-launch.test.ts (relay): cmder maps to cmd.exe with a valid launch config; on Windows, a pane-supplied CMDER_ROOT resolves to the real init.bat args.
    • preflight-handler.test.ts (relay): the SSH Windows capability check reports cmderAvailable.
    • windows-terminal-capabilities.test.ts: the local probe reports Cmder; results keep their exact shape.
    • windows-terminal-shell.test.ts: allow-list includes cmder.
    • cmder.smoke.node-pty.test.ts: real terminal launch (skips when Cmder isn't installed).

Checks run locally: typecheck passes; oxlint clean on changed files; pnpm run check:code-quality:changed passes (design system, React Doctor, casting rationale); generate:rpc-params-catalog rerun for the new RPC. 274 related test files (1,984 tests) pass.

Some suites (for example src/main/ipc/pty, host-capabilities, provider-dispatch) fail to load on this Windows machine because of vite SSR temp-file EPERM errors; they fail the same way on an untouched main checkout, so I've left that to CI.

AI Disclosure

Written with Claude Code (Claude Opus 5.5), reviewed and tested by me.

Review

Agent skill upstream boundary

  • Not applicable, or this change follows docs/reference/agent-skill-sharing-upstream-boundary.md and copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Notes

  • Security: no user text is placed on a command line. The shell name must be on the fixed allow-list; the init.bat path travels only through environment variables. The only new input is a local setting (hidden) or CMDER_ROOT, i.e. the same trust as a custom shell path. The new probe answers yes/no only.
  • Cross-platform: Windows-only; every new branch checks win32, and non-Windows callers get null/no-op.
  • Remote SSH: a Windows SSH host resolves and reports Cmder on that host, not the client; hosts without it (or relays too old to report it) show no Cmder and fall back to cmd.
  • Mobile / mixed versions: host.cmder.isAvailable is added to the mobile allow-list; clients treat a missing method as "no Cmder", and cmderAvailable is optional, so old hosts and new clients (and the reverse) keep working.
  • Performance: one file-exists check per Cmder pane spawn and per availability probe.

Checklist

  • This PR is small and focused
  • I explained what changed and why (ELI5, the user-facing before/after, the mechanism, and why over the alternatives)
  • Before/after screenshots or videos attached for UI changes, or N/A with reason
  • Self-reviewed for correctness, security, and performance
  • Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
  • pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred)

🤖 Generated with Claude Code

@coderabbitai

coderabbitai Bot commented Sep 23, 2026

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

⛔ Files ignored due to path filters (1)
  • src/shared/rpc-contract/rpc-params-catalog.generated.ts is excluded by !**/*.generated.*
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: 033aad9e-9aa3-4fb9-a8ba-8506de21fa37

📥 Commits

Reviewing files that changed from the base of the PR and between 58508c3 and 2f32f25.

⛔ Files ignored due to path filters (1)
  • src/shared/rpc-contract/rpc-params-catalog.generated.ts is excluded by !**/*.generated.*

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: 7973cf7f-9970-461d-a7a8-344e146cc2ce

📥 Commits

Reviewing files that changed from the base of the PR and between e2beef6 and 58508c3.

📒 Files selected for processing (2)
  • src/relay/pty-handler-revive.test.ts
  • src/relay/pty-handler.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 6 remain after this review.


📝 Walkthrough

Walkthrough

Adds Cmder as a Windows terminal shell. The change discovers Cmder installations, applies vendor\init.bat, preserves the requested working directory, and falls back to plain cmd.exe when Cmder is unavailable. It adds local and relay launch support, availability IPC and RPC methods, settings persistence, capability reporting, renderer shell options, onboarding support, telemetry mapping, icons, and tests.

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to 58508

Cmder panes retain their installation root when revived. No actionable issue remains in the supplied change evidence.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The PR implements the Cmder shell option, orca terminal create --shell cmder, discovery through CMDER_ROOT and standard folders, vendor\\init.bat startup in the project folder, plain cmd.exe fa… Hide Cmder whenever it is unavailable, including when Cmder is the saved shell. Preserve a separate way to change an unavailable saved value if required by the product design, and add or update the unavailable saved-shell test.
Docstring Coverage ⚠️ Warning Docstring coverage is 18.18% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 44 functions across 45 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Out of Scope Changes check ✅ Passed The changes support Cmder discovery, launch, availability reporting, shell selection, onboarding, remote capability checks, pane revival, and automated tests for [#22461]. The summary does not establi…
Title check ✅ Passed The title clearly identifies the main change: adding Cmder as a Windows terminal shell.
Description check ✅ Passed The description follows the repository template and covers the user-facing behavior, implementation, rationale, linked issue, visual proof, testing, AI disclosure, compatibility considerations, and ch…
Full details: Linked Issues check

Explanation

The PR implements the Cmder shell option, orca terminal create --shell cmder, discovery through CMDER_ROOT and standard folders, vendor\init.bat startup in the project folder, plain cmd.exe fallback, availability reporting, remote capability handling, and pane revival. It adds automated coverage for discovery, startup arguments, fallback, local and relay launch paths, capability reporting, and revived panes [#22461]. The issue requires Cmder to be shown only when Cmder is installed. The PR still shows a saved Cmder option when Cmder is unavailable, although it disables the option.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3


ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: c7552715-6f53-4b42-81be-73cb3336fb0c

📥 Commits

Reviewing files that changed from the base of the PR and between 8d6fec5 and f72e68a.

⛔ Files ignored due to path filters (1)
  • src/shared/rpc-contract/rpc-params-catalog.generated.ts is excluded by !**/*.generated.*
📒 Files selected for processing (40)
  • src/main/cmder.smoke.node-pty.test.ts
  • src/main/cmder.test.ts
  • src/main/cmder.ts
  • src/main/daemon/pty-subprocess/shell-launch-plan.ts
  • src/main/ipc/app.ts
  • src/main/ipc/pty/ipc/spawn-options.ts
  • src/main/ipc/pty/runtime/spawn-options.ts
  • src/main/providers/local-pty-launch-plan.ts
  • src/main/providers/local-pty-provider-windows-shell-launch.test.ts
  • src/main/providers/local-pty-windows-spawn-environment.ts
  • src/main/providers/windows-shell-args.test.ts
  • src/main/providers/windows-shell-args.ts
  • src/main/runtime/rpc/methods/host-capabilities.ts
  • src/main/runtime/runtime-rpc/runtime-rpc-mobile-method-allowlist.ts
  • src/main/startup/main-process-ready-foundation.ts
  • src/preload/api-types.ts
  • src/preload/api/git-bash-bridge.ts
  • src/preload/api/runtime-api.ts
  • src/preload/index.ts
  • src/relay/pty-handler.ts
  • src/relay/pty-shell-launch.test.ts
  • src/relay/pty-shell-launch.ts
  • src/renderer/src/components/onboarding/WindowsTerminalStep.tsx
  • src/renderer/src/components/onboarding/windows-terminal-onboarding-telemetry.ts
  • src/renderer/src/components/settings/TerminalPane.tsx
  • src/renderer/src/components/settings/TerminalWindowsShellSection.tsx
  • src/renderer/src/components/settings/settings-interface-primary-section-renderers.tsx
  • src/renderer/src/components/tab-bar/shell-icons.tsx
  • src/renderer/src/components/tab-bar/tab-bar-windows-shell-options.ts
  • src/renderer/src/components/tab-bar/use-tab-bar-create-menu-controller.ts
  • src/renderer/src/lib/windows-terminal-capabilities.test.ts
  • src/renderer/src/lib/windows-terminal-capabilities.ts
  • src/renderer/src/lib/windows-terminal-capability-read.ts
  • src/renderer/src/lib/windows-terminal-capability-reprobe.ts
  • src/renderer/src/store/terminals/terminal-workspace-routing.ts
  • src/renderer/src/web/web-preload-api.ts
  • src/shared/default-global-settings.ts
  • src/shared/global-settings-types.ts
  • src/shared/windows-terminal-shell.test.ts
  • src/shared/windows-terminal-shell.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.

Comment thread src/main/cmder.ts
Comment thread src/relay/pty-shell-launch.ts Outdated
Comment thread src/renderer/src/components/onboarding/WindowsTerminalStep.tsx

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ No critical issues — one feature gap and a couple of rough edges are worth a look.

Reviewed changes

  • Shell sentinel and plumbing — adds WINDOWS_CMDER_SHELL = 'cmder' to the built-in shells, the override canonical-name map, the startup-family resolver (→ cmd), and the CLI/RPC shell allow-list.
  • Discoverysrc/main/cmder.ts resolves a root from the hidden terminalWindowsCmderPath setting, ORCA_CMDER_ROOT, CMDER_ROOT, then guessed Program Files / LocalAppData / Scoop / C:\tools / C:\cmder, requiring vendor\init.bat.
  • Launch wiring — local provider, daemon subprocess, and SSH relay all map cmder to cmd.exe and prepend call %ORCA_CMDER_INIT% via env-quoted path; inherited CMDER_* state is stripped; the configured folder is carried to the daemon as ORCA_CMDER_ROOT.
  • Renderer surfaces — Settings shell section, tab-bar "+" menu, onboarding normalization, telemetry bucket, CMD icon, and the new cmderAvailable capability field / host.cmder.isAvailable RPC.

ℹ️ The configured Cmder folder cannot reach a remote host

terminalWindowsCmderPath is only injected as ORCA_CMDER_ROOT on non-connection spawns, and the relay strips ORCA_CMDER_ROOT before resolving, while the relay root resolver reads the relay daemon's own process.env. That is correct for a client-local path, but it means a remote Windows host whose Cmder sits outside the guessed folders always falls back to plain cmd.exe with no signal to the user. Worth deciding whether the override should be host-scoped or whether relay auto-detection is considered sufficient.

Technical details
# Configured Cmder folder does not reach a remote Windows host

## Affected sites
- `src/relay/pty-shell-launch.ts:65``getRelayCmderLaunchConfig` calls `resolveCmderRoot()` with no `env`, so it reads only the relay process env + module state; the per-spawn `spawnEnv` is ignored.
- `src/relay/pty-handler.ts:1928` / `:3059``stripInheritedCmderState(spawnEnv)` deletes `ORCA_CMDER_ROOT` before the config is built.
- `src/main/ipc/pty/ipc/spawn-options.ts:123`, `src/main/ipc/pty/runtime/spawn-options.ts:153``applyConfiguredCmderRootEnv` is gated on `!connectionId`, so the setting is never sent to a relay.

## Required outcome
- Decide whether a remote Windows host should honor a user-specified Cmder folder. If yes, the setting needs a host-scoped path (`hostSettingOverrides`) or the relay must be told which root to use; if no, relay auto-detection (`CMDER_ROOT` on the host env or the guessed folders) is the documented contract.

## Open questions for the human
- Is the hidden `terminalWindowsCmderPath` intended to be local-only, or should it configure the active host?

ℹ️ Nitpicks

  • WindowsTerminalStep.tsx now preserves terminalWindowsShell: 'cmder' in normalizeWindowsShell, but the onboarding shell cards still have no Cmder option, so such a profile shows no card selected in onboarding.
  • The new relay test only asserts the empty { args: [], env: {} } config on a machine without Cmder, so the args.length > 0 branch (the actual init.bat arg string) is never exercised in CI; the same string is covered by windows-shell-args.test.ts.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

Comment thread src/renderer/src/lib/windows-terminal-capability-read.ts
@b1nhm1nh

Copy link
Copy Markdown
Author

Thanks for the reviews. Addressed in 404a3c8; notes on the review-level points:

  • Out-of-scope formatting (app.ts, runtime-api.ts): reverted; only the Cmder lines remain.
  • Cmder disabled vs hidden in Settings: Cmder is hidden when not installed. It only shows (disabled) when it is already the saved default, so the current value stays visible — the same rule Git Bash uses.
  • Configured Cmder folder on a remote host (Pullfrog): intentionally local-only. A Windows SSH host resolves Cmder from its own CMDER_ROOT or the standard install folders, and with this commit it also reports availability through the SSH preflight, so the option appears only when that host has Cmder.
  • Onboarding nitpick / relay test coverage: onboarding now has a Cmder card, and the relay test exercises the real init.bat branch on Windows.
  • Docstring coverage: the repo's guidelines ask for brief, non-obvious comments only, so I kept to that.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes

  • Addressed prior feedbackapplyConfiguredCmderRootEnv now drops a carried ORCA_CMDER_ROOT before deciding, so clearing the hidden folder setting can no longer leave a stale path ahead of CMDER_ROOT; the relay resolves the Cmder root from the pane spawn env ({ ...process.env, ...spawnEnv }) instead of the relay process env alone.
  • Windows SSH hosts now report Cmder availability — the relay's detectWindowsTerminalCapabilities returns cmderAvailable, carried by the shared RemoteWindowsTerminalCapabilities type and mirrored in the preload API as optional, so old relays read as "no Cmder" and the Settings/tab-bar gates offer Cmder on a remote Windows host.
  • Onboarding Cmder cardWindowsTerminalStep renders a Cmder option from capabilities.cmderAvailable, keeping a saved-but-missing selection visible and disabled exactly as the Git Bash option does.
  • Tests — new coverage in cmder.test.ts (stale-key removal), preflight-handler.test.ts (relay capability shape), and pty-shell-launch.test.ts (spawn-env root, win32-gated); formatting-only edits in app.ts and runtime-api.ts reverted.

Pullfrog  | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to GitHub limitations.

⚠️ Outside diff range comments (1)

🟡 Minor · Resolve Cmder from the spawn environment on the local fallback. · local-pty-launch-plan.ts:127-129

src/main/providers/local-pty-launch-plan.ts:127-129
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Resolve Cmder from the spawn environment on the local fallback.

When orcad cannot start its daemon, it continues with the in-process LocalPtyProvider. Its PTY handlers use terminalWindowsShell to select Cmder and place terminalWindowsCmderPath in args.env.ORCA_CMDER_ROOT. This resolver ignores args.env, and orcad does not initialize configuredCmderRoot. A custom root outside the resolver’s other candidates can be missed, so cmd.exe can start without Cmder’s vendor\init.bat.

Suggested fix
-  const cmderRoot = resolveWindowsCmderShellRoot(shellFamily)
+  const cmderRoot = resolveWindowsCmderShellRoot(shellFamily, {
+    env: { ...process.env, ...args.env }
+  })

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: 80565d5b-bb9c-4dc9-a9dd-8dbb0eb3e2dc

📥 Commits

Reviewing files that changed from the base of the PR and between f72e68a and 404a3c8.

📒 Files selected for processing (11)
  • src/main/cmder.test.ts
  • src/main/cmder.ts
  • src/main/ipc/app.ts
  • src/main/ipc/preflight-remote-windows-terminal-capabilities.ts
  • src/preload/api/preflight-api.ts
  • src/preload/api/runtime-api.ts
  • src/relay/preflight-handler.test.ts
  • src/relay/preflight-handler.ts
  • src/relay/pty-shell-launch.test.ts
  • src/relay/pty-shell-launch.ts
  • src/renderer/src/components/onboarding/WindowsTerminalStep.tsx
🚧 Files skipped from review as they are similar to previous changes (6)
  • src/renderer/src/components/onboarding/WindowsTerminalStep.tsx
  • src/main/cmder.test.ts
  • src/relay/pty-shell-launch.test.ts
  • src/preload/api/runtime-api.ts
  • src/relay/pty-shell-launch.ts
  • src/main/cmder.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review.

@b1nhm1nh

Copy link
Copy Markdown
Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Sep 23, 2026

Copy link
Copy Markdown
Contributor
⚠️ Action not completed

Already reviewed the last commit. Use @coderabbitai full review to rerun a review of the entire changeset.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@b1nhm1nh

Copy link
Copy Markdown
Author

@coderabbitai re the outside-diff comment on local-pty-launch-plan.ts:127-129 (in-process fallback ignoring args.env): fixed in e2beef6. resolveWindowsCmderShellRoot now gets { ...process.env, ...args.env }, so the setting-carried ORCA_CMDER_ROOT is honoured there as it already was on the daemon path. Added a test in local-pty-provider-windows-shell-launch.test.ts.

On the remaining Linked Issues warning: a saved Cmder default stays visible (disabled) when Cmder is missing so the user can see what is selected and change it, the same rule Git Bash and WSL use in Settings and onboarding. Keeping that for consistency.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes

  • Resolved Cmder from the spawn environment on the in-process fallbackcreateWindowsLocalPtyLaunchPlan now calls resolveWindowsCmderShellRoot(shellFamily, { env: { ...process.env, ...args.env } }), so orcad's fallback honors the ORCA_CMDER_ROOT the setting carries in the spawn env even though that process never calls setConfiguredCmderRoot. This closes the gap where a Cmder root outside the guessed folders would have fallen back to plain cmd.exe.
  • Added regression coverage — a provider test passing ORCA_CMDER_ROOT through spawn({ env }) asserts CMDER_ROOT / ORCA_CMDER_INIT land on the cmd.exe spawn. It fails without the production change, so it is real coverage rather than a tautology.

Pullfrog  | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

@b1nhm1nh

Copy link
Copy Markdown
Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Sep 23, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Full review finished.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: aae111f7-e090-4365-bd8c-80e6b44e4a10

📥 Commits

Reviewing files that changed from the base of the PR and between 8d6fec5 and e2beef6.

⛔ Files ignored due to path filters (1)
  • src/shared/rpc-contract/rpc-params-catalog.generated.ts is excluded by !**/*.generated.*
📒 Files selected for processing (44)
  • src/main/cmder.smoke.node-pty.test.ts
  • src/main/cmder.test.ts
  • src/main/cmder.ts
  • src/main/daemon/pty-subprocess/shell-launch-plan.ts
  • src/main/ipc/app.ts
  • src/main/ipc/preflight-remote-windows-terminal-capabilities.ts
  • src/main/ipc/pty/ipc/spawn-options.ts
  • src/main/ipc/pty/runtime/spawn-options.ts
  • src/main/providers/local-pty-launch-plan.ts
  • src/main/providers/local-pty-provider-windows-shell-launch.test.ts
  • src/main/providers/local-pty-windows-spawn-environment.ts
  • src/main/providers/windows-shell-args.test.ts
  • src/main/providers/windows-shell-args.ts
  • src/main/runtime/rpc/methods/host-capabilities.ts
  • src/main/runtime/runtime-rpc/runtime-rpc-mobile-method-allowlist.ts
  • src/main/startup/main-process-ready-foundation.ts
  • src/preload/api-types.ts
  • src/preload/api/git-bash-bridge.ts
  • src/preload/api/preflight-api.ts
  • src/preload/api/runtime-api.ts
  • src/preload/index.ts
  • src/relay/preflight-handler.test.ts
  • src/relay/preflight-handler.ts
  • src/relay/pty-handler.ts
  • src/relay/pty-shell-launch.test.ts
  • src/relay/pty-shell-launch.ts
  • src/renderer/src/components/onboarding/WindowsTerminalStep.tsx
  • src/renderer/src/components/onboarding/windows-terminal-onboarding-telemetry.ts
  • src/renderer/src/components/settings/TerminalPane.tsx
  • src/renderer/src/components/settings/TerminalWindowsShellSection.tsx
  • src/renderer/src/components/settings/settings-interface-primary-section-renderers.tsx
  • src/renderer/src/components/tab-bar/shell-icons.tsx
  • src/renderer/src/components/tab-bar/tab-bar-windows-shell-options.ts
  • src/renderer/src/components/tab-bar/use-tab-bar-create-menu-controller.ts
  • src/renderer/src/lib/windows-terminal-capabilities.test.ts
  • src/renderer/src/lib/windows-terminal-capabilities.ts
  • src/renderer/src/lib/windows-terminal-capability-read.ts
  • src/renderer/src/lib/windows-terminal-capability-reprobe.ts
  • src/renderer/src/store/terminals/terminal-workspace-routing.ts
  • src/renderer/src/web/web-preload-api.ts
  • src/shared/default-global-settings.ts
  • src/shared/global-settings-types.ts
  • src/shared/windows-terminal-shell.test.ts
  • src/shared/windows-terminal-shell.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 6 remain after this review.

Comment thread src/relay/pty-handler.ts
@coderabbitai

coderabbitai Bot commented Sep 23, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail
ast-grep outline src/main/providers/local-pty-launch-plan.ts --items all --match 'resolve|Cmder' || true
ast-grep outline src/main/providers/local-pty-provider-windows-shell-launch.test.ts --items all --match 'Cmder|cmder' || true
ast-grep outline src/renderer/src/components/settings/TerminalWindowsShellSection.tsx --items all || true
ast-grep outline src/renderer/src/components/onboarding/WindowsTerminalStep.tsx --items all || true
rg -n -C5 'resolveWindowsCmderShellRoot|ORCA_CMDER_ROOT|cmderAvailable|WINDOWS_CMDER_SHELL|gitBashAvailable|wslAvailable' src/main/providers/local-pty-launch-plan.ts src/main/providers/local-pty-provider-windows-shell-launch.test.ts src/renderer/src/components/settings/TerminalWindowsShellSection.tsx src/renderer/src/components/onboarding/WindowsTerminalStep.tsx

Length of output: 26636


🏁 Script executed:

#!/bin/bash
set -euo pipefail
sed -n '680,720p' src/main/providers/local-pty-provider-windows-shell-launch.test.ts
ast-grep outline src/main/cmder.ts --items all --match 'resolveWindowsCmderShellRoot|resolveCmderRoot|getCmderRootCandidates' || true
rg -n -C5 'function resolveWindowsCmderShellRoot|function resolveCmderRoot|ORCA_CMDER_ROOT_ENV' src/main/cmder.ts

Length of output: 5135


@b1nhm1nh, verification now confirms the fix.

The test sets ORCA_CMDER_ROOT in the spawn environment. It verifies that the in-process fallback starts cmd.exe with CMDER_ROOT and ORCA_CMDER_INIT resolved from that value. The resolver prioritizes ORCA_CMDER_ROOT when it resolves Cmder candidates.

This addresses the args.env fallback finding. I did not run the test.


If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

You are interacting with an AI system.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes

  • Persisted the Cmder root across relay revivecmderRoot now rides ManagedPty and SerializedPtyEntry, so a pane whose CMDER_ROOT arrived only in the spawn environment is re-resolved on revive instead of falling back to plain cmd.exe. Discovery still requires vendor\init.bat, so a stale root degrades to plain cmd, and the resolved root is re-stored after revive for a second round trip.
  • Regression coverage — a Windows-only revive test in pty-handler-revive.test.ts spawns cmder from a temp root, serializes, revives, and asserts the revived cmd.exe receives ORCA_CMDER_INIT pointing at that root's vendor\init.bat. It fails without the production change.

Pullfrog  | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

b1nhm1nh and others added 6 commits September 24, 2026 16:06
Adds a `cmder` shell choice (settings, new-tab menu, `terminal create --shell cmder`)
that spawns cmd.exe and calls Cmder's vendor\init.bat before any startup command,
so panes get Clink, Cmder aliases, and the git prompt while staying in the
requested cwd. Cmder is found via CMDER_ROOT or common install dirs; hosts
without it degrade to plain cmd. The init path is quoted through env vars because
node-pty backslash-escapes literal argv quotes that cmd.exe cannot read.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
- Strip inherited CMDER_CONFIGURED/CMDER_INIT_* so Orca launched from a Cmder
  console still runs init.bat in new panes.
- SSH relay on Windows spawns cmd.exe + init.bat for the `cmder` override
  instead of trying to exec a file named `cmder` (fresh spawn and revive).
- Onboarding keeps a saved `cmder` default instead of resetting to PowerShell.
- New Settings > Windows Shell > Cmder Folder (terminalWindowsCmderPath);
  empty auto-detects. Carried to the daemon as ORCA_CMDER_ROOT and stripped
  before the shell sees it.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Auto-detection (CMDER_ROOT, then standard install dirs) covers the common
setups, so the extra Settings row was noise. terminalWindowsCmderPath stays
as a hidden override and is still honored by every spawn path.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
- Drop a carried ORCA_CMDER_ROOT when the Cmder folder setting is cleared.
- Resolve the relay Cmder root from the pane spawn env, not only the relay process env.
- Report cmderAvailable through the SSH preflight so Windows SSH hosts offer Cmder.
- Add a Cmder card to onboarding, disabled when a saved Cmder default is not installed.
- Revert formatting-only edits in app.ts and runtime-api.ts.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
orcad's LocalPtyProvider fallback never sets configuredCmderRoot, so read the
setting-carried ORCA_CMDER_ROOT from args.env like the daemon path does.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
A remote Cmder pane whose CMDER_ROOT came only with its spawn request revived
as plain cmd.exe, because serialize() did not save the root. Store it with the
pane and feed it back as CMDER_ROOT on revive; discovery still requires
vendor\init.bat to exist there.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature]: Cmder as a Windows terminal shell

1 participant