feat(terminal): let users choose their default shell - #6125

Closed
simon-curtis wants to merge 10 commits into
pingdotgg:mainfrom
simon-curtis:terminal/default-shell
Closed

feat(terminal): let users choose their default shell#6125
simon-curtis wants to merge 10 commits into
pingdotgg:mainfrom
simon-curtis:terminal/default-shell

Conversation

@simon-curtis

@simon-curtissimon-curtis commented Aug 11, 2026

Copy link
Copy Markdown

What Changed

Terminal sessions always chose their shell implicitly, so users who preferred PowerShell, Git Bash, WSL, or another shell could not make it the default.

Settings now includes a Default terminal shell field. New terminal sessions read the current setting, preserve Windows executable paths whether quoted or unquoted, extract executable tokens from POSIX shell command lines, resolve bare Windows commands against the session environment, and retain the existing fallback candidates when the configured command cannot be started.

Why

A coding environment's terminal should open the shell the user actually works in. Making the choice explicit removes platform-dependent surprises while preserving safe fallback behavior and applying setting changes without restarting the server.

Verification

  • Focused terminal and settings suites passed: 88/88 tests.
  • Targeted lint passed for the changed server files.
  • Typechecking passed for t3; prior checks also passed for @t3tools/web and @t3tools/contracts.

UI Changes

Settings gains a Default terminal shell field. Before/after screenshots are not available from this environment.

Checklist

  • This PR is small and focused
  • I explained what changed and why
  • I included before/after screenshots for the Settings change
  • Animation or interaction changes are not applicable

model: gpt-5.6-sol
harness: Codex in T3 Code


Note

Medium Risk
Changes which executable starts PTY sessions and adds Windows PATH resolution per session env; misconfiguration or PATH overrides could pick unexpected shells, though fallbacks remain.

Overview
Adds a Default terminal shell setting (defaultTerminalShell, empty = platform default) in server settings and General settings UI, with search/reset support.

TerminalManager now wires in ServerSettingsService, subscribes to settings changes before reading the initial value, and uses the live value when spawning new sessions (no server restart). Shell resolution is tightened: ~ expansion, quoted executable extraction on POSIX, -NoLogo for Windows pwsh/powershell (including bare aliases), and PATH/PATHEXT lookup for bare Windows command names against each session’s spawn environment—while keeping existing fallback candidates when spawn fails. The terminal layer is provided ServerSettingsLayerLive in server.ts.

Contracts decode patches with trimmed strings; tests cover subscription ordering and the new resolution paths.

Reviewed by Cursor Bugbot for commit 4bb5d7d. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Add configurable default terminal shell setting with live updates

  • Introduces defaultTerminalShell (TrimmedString, defaults to "") in ServerSettings and ServerSettingsPatch in settings.ts
  • TerminalManager now subscribes to ServerSettingsService, reads the initial configured shell, and spawns a scoped fiber to keep the local configuredShell in sync as settings change; shell resolution runs against each session's effective environment instead of the base env
  • normalizeShellCommand expands home-relative paths and handles quoted executables; on Windows, shellCandidateFromCommand appends -NoLogo to pwsh/powershell, and resolveShellCandidates resolves bare command names against effective PATH (with PATHEXT) via resolveCommandPath
  • Frontend adds a DraftInput for the setting in GeneralSettingsPanel, includes it in useSettingsRestore reset logic, and registers a search entry in settingsSearch.ts
  • Risk: resolveShellCandidates now requires FileSystem and Path services and performs PATH lookups on Windows before spawning; misconfigured PATH per-session env can change which executable is selected, and bare names previously spawned without PATH resolution

Macroscope summarized 4bb5d7d.

@coderabbitai

coderabbitaiBot commented Aug 11, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 9ab6f837-f5fd-4c94-a380-be7da102498b

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

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

@github-actionsgithub-actionsBot added size:M 30-99 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list. labels Aug 11, 2026
Comment threadapps/server/src/terminal/Manager.ts
@macroscopeapp

macroscopeappBot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This PR adds a user-facing default-shell setting spanning contracts, settings UI, live server configuration, and terminal process spawning, including platform-specific path resolution and fallback behavior. That is a significant runtime capability change rather than a small isolated tweak, so the terminal/settings integration warrants human review.

You can add or adjust custom eligibility rules. Learn more.

@simon-curtis
simon-curtisforce-pushed the terminal/default-shell branch from 5386fd9 to 0c17f61CompareAugust 13, 2026 10:42
Comment threadapps/web/src/components/settings/SettingsPanels.tsx Outdated
@simon-curtis
simon-curtis marked this pull request as draft August 13, 2026 11:15

@CDVolvikCDVolvik 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.

Windows-shaped tests on the PR files against current main: Manager.test.ts + settings.test.ts 89/89.

What I care about here:

  • subscribe-to-settings beforegetSettings (test pins the order)
  • resolveCommandPath + PATHEXT so jz finds jz.EXE
  • quoted "C:\Program Files\…\bash.exe" is unwrapped before spawn
  • session PATH wins over the process PATH when both have the same name

Empty default + trim is in the contract tests. Fallback to built-in PowerShell remains.

Small mismatch: the Electron placeholder is pwsh.exe. #6260 is the other open Windows-shell PR, and it prefers 5.1 (powershell.exe) over optional pwsh. The placeholder will nudge people at the one this repo is trying to stop probing first.

MERGEABLE. I did not open a real terminal session.

@simon-curtis
simon-curtisforce-pushed the terminal/default-shell branch from 0c17f61 to 137463cCompareAugust 17, 2026 17:04
@simon-curtis

simon-curtis commented Aug 17, 2026

Copy link
Copy Markdown
Author

Review update pushed in e31182709:

  • configured shell input now preserves quoted paths everywhere, keeps unquoted Windows paths intact, and extracts the executable from POSIX command lines such as fish -l;
  • restored focused coverage that pins settings subscription before the initial snapshot;
  • retained Windows PATH/PATHEXT, quoted-path, and per-session PATH coverage.

Verification: focused terminal/settings suites 88/88, targeted lint passed, and the t3 typecheck passed.

I also checked related open PRs. #5268 and #6073 are alternative shell-selection implementations; #6260 overlaps the Windows fallback ordering; #6338 and #4827 touch the same terminal-start block. I kept those separate so this PR remains one shell-agnostic concern against current main.

Comment threadapps/server/src/terminal/Manager.ts Outdated
@simon-curtis
simon-curtisforce-pushed the terminal/default-shell branch from cf76f21 to b972c06CompareAugust 17, 2026 17:49
@simon-curtis
simon-curtis marked this pull request as ready for review August 17, 2026 17:52
Comment threadapps/server/src/terminal/Manager.ts Outdated
@simon-curtis
simon-curtisforce-pushed the terminal/default-shell branch from b972c06 to e311827CompareAugust 17, 2026 18:27
Comment threadapps/server/src/terminal/Manager.ts
@simon-curtis
simon-curtisforce-pushed the terminal/default-shell branch 4 times, most recently from 2d9605b to 03ad822CompareAugust 25, 2026 08:47

@cursorcursorBot 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.

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Want fixes drafted automatically? Bugbot Autofix can create code changes for findings. A team admin can enable Autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 03ad822e19b29ed3a68e615a659271e24d0f2fef. Configure here.

Comment threadapps/server/src/terminal/Manager.ts
@simon-curtis
simon-curtisforce-pushed the terminal/default-shell branch 2 times, most recently from 9e38f97 to 8772631CompareAugust 25, 2026 10:28
@github-actionsgithub-actionsBot added size:L 100-499 changed lines (additions + deletions). and removed size:M 30-99 changed lines (additions + deletions). labels Aug 25, 2026
Comment threadapps/server/src/terminal/Manager.ts Outdated
@simon-curtis
simon-curtisforce-pushed the terminal/default-shell branch from 1a8d434 to 4189825CompareAugust 26, 2026 08:27
@simon-curtis
simon-curtisforce-pushed the terminal/default-shell branch from 4189825 to 4bb5d7dCompareAugust 27, 2026 08:29
@t3dotgg

Copy link
Copy Markdown
Member

Note

🤖 GPT-5.6 Sol responding on behalf of Theo

We're closing this PR as we clean up the T3 Code backlog. Thank you for taking the time to put this together.

#5268 is the retained default shell implementation and limits choices to installed shells reported by the host. This branch accepts arbitrary command text and adds a larger cross-platform parsing contract that we do not need.

If you believe we closed this in error, please reopen the PR and leave a comment explaining what we missed.

@t3dotggt3dotgg closed this Aug 28, 2026
@simon-curtis
simon-curtis deleted the terminal/default-shell branch August 28, 2026 12:39
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L100-499 changed lines (additions + deletions).vouch:unvouchedPR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@simon-curtis@t3dotgg@CDVolvik
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

feat(terminal): let users choose their default shell - #6125

Closed
simon-curtis wants to merge 10 commits into
pingdotgg:mainfrom
simon-curtis:terminal/default-shell
Closed

feat(terminal): let users choose their default shell#6125
simon-curtis wants to merge 10 commits into
pingdotgg:mainfrom
simon-curtis:terminal/default-shell

Conversation

@simon-curtis

@simon-curtissimon-curtis commented Aug 11, 2026

Copy link
Copy Markdown

What Changed

Terminal sessions always chose their shell implicitly, so users who preferred PowerShell, Git Bash, WSL, or another shell could not make it the default.

Settings now includes a Default terminal shell field. New terminal sessions read the current setting, preserve Windows executable paths whether quoted or unquoted, extract executable tokens from POSIX shell command lines, resolve bare Windows commands against the session environment, and retain the existing fallback candidates when the configured command cannot be started.

Why

A coding environment's terminal should open the shell the user actually works in. Making the choice explicit removes platform-dependent surprises while preserving safe fallback behavior and applying setting changes without restarting the server.

Verification

  • Focused terminal and settings suites passed: 88/88 tests.
  • Targeted lint passed for the changed server files.
  • Typechecking passed for t3; prior checks also passed for @t3tools/web and @t3tools/contracts.

UI Changes

Settings gains a Default terminal shell field. Before/after screenshots are not available from this environment.

Checklist

  • This PR is small and focused
  • I explained what changed and why
  • I included before/after screenshots for the Settings change
  • Animation or interaction changes are not applicable

model: gpt-5.6-sol
harness: Codex in T3 Code


Note

Medium Risk
Changes which executable starts PTY sessions and adds Windows PATH resolution per session env; misconfiguration or PATH overrides could pick unexpected shells, though fallbacks remain.

Overview
Adds a Default terminal shell setting (defaultTerminalShell, empty = platform default) in server settings and General settings UI, with search/reset support.

TerminalManager now wires in ServerSettingsService, subscribes to settings changes before reading the initial value, and uses the live value when spawning new sessions (no server restart). Shell resolution is tightened: ~ expansion, quoted executable extraction on POSIX, -NoLogo for Windows pwsh/powershell (including bare aliases), and PATH/PATHEXT lookup for bare Windows command names against each session’s spawn environment—while keeping existing fallback candidates when spawn fails. The terminal layer is provided ServerSettingsLayerLive in server.ts.

Contracts decode patches with trimmed strings; tests cover subscription ordering and the new resolution paths.

Reviewed by Cursor Bugbot for commit 4bb5d7d. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Add configurable default terminal shell setting with live updates

  • Introduces defaultTerminalShell (TrimmedString, defaults to "") in ServerSettings and ServerSettingsPatch in settings.ts
  • TerminalManager now subscribes to ServerSettingsService, reads the initial configured shell, and spawns a scoped fiber to keep the local configuredShell in sync as settings change; shell resolution runs against each session's effective environment instead of the base env
  • normalizeShellCommand expands home-relative paths and handles quoted executables; on Windows, shellCandidateFromCommand appends -NoLogo to pwsh/powershell, and resolveShellCandidates resolves bare command names against effective PATH (with PATHEXT) via resolveCommandPath
  • Frontend adds a DraftInput for the setting in GeneralSettingsPanel, includes it in useSettingsRestore reset logic, and registers a search entry in settingsSearch.ts
  • Risk: resolveShellCandidates now requires FileSystem and Path services and performs PATH lookups on Windows before spawning; misconfigured PATH per-session env can change which executable is selected, and bare names previously spawned without PATH resolution

Macroscope summarized 4bb5d7d.

@coderabbitai

coderabbitaiBot commented Aug 11, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 9ab6f837-f5fd-4c94-a380-be7da102498b

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

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

@github-actionsgithub-actionsBot added size:M 30-99 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list. labels Aug 11, 2026
Comment threadapps/server/src/terminal/Manager.ts
@macroscopeapp

macroscopeappBot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This PR adds a user-facing default-shell setting spanning contracts, settings UI, live server configuration, and terminal process spawning, including platform-specific path resolution and fallback behavior. That is a significant runtime capability change rather than a small isolated tweak, so the terminal/settings integration warrants human review.

You can add or adjust custom eligibility rules. Learn more.

@simon-curtis
simon-curtisforce-pushed the terminal/default-shell branch from 5386fd9 to 0c17f61CompareAugust 13, 2026 10:42
Comment threadapps/web/src/components/settings/SettingsPanels.tsx Outdated
@simon-curtis
simon-curtis marked this pull request as draft August 13, 2026 11:15

@CDVolvikCDVolvik 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.

Windows-shaped tests on the PR files against current main: Manager.test.ts + settings.test.ts 89/89.

What I care about here:

  • subscribe-to-settings beforegetSettings (test pins the order)
  • resolveCommandPath + PATHEXT so jz finds jz.EXE
  • quoted "C:\Program Files\…\bash.exe" is unwrapped before spawn
  • session PATH wins over the process PATH when both have the same name

Empty default + trim is in the contract tests. Fallback to built-in PowerShell remains.

Small mismatch: the Electron placeholder is pwsh.exe. #6260 is the other open Windows-shell PR, and it prefers 5.1 (powershell.exe) over optional pwsh. The placeholder will nudge people at the one this repo is trying to stop probing first.

MERGEABLE. I did not open a real terminal session.

@simon-curtis
simon-curtisforce-pushed the terminal/default-shell branch from 0c17f61 to 137463cCompareAugust 17, 2026 17:04
@simon-curtis

simon-curtis commented Aug 17, 2026

Copy link
Copy Markdown
Author

Review update pushed in e31182709:

  • configured shell input now preserves quoted paths everywhere, keeps unquoted Windows paths intact, and extracts the executable from POSIX command lines such as fish -l;
  • restored focused coverage that pins settings subscription before the initial snapshot;
  • retained Windows PATH/PATHEXT, quoted-path, and per-session PATH coverage.

Verification: focused terminal/settings suites 88/88, targeted lint passed, and the t3 typecheck passed.

I also checked related open PRs. #5268 and #6073 are alternative shell-selection implementations; #6260 overlaps the Windows fallback ordering; #6338 and #4827 touch the same terminal-start block. I kept those separate so this PR remains one shell-agnostic concern against current main.

Comment threadapps/server/src/terminal/Manager.ts Outdated
@simon-curtis
simon-curtisforce-pushed the terminal/default-shell branch from cf76f21 to b972c06CompareAugust 17, 2026 17:49
@simon-curtis
simon-curtis marked this pull request as ready for review August 17, 2026 17:52
Comment threadapps/server/src/terminal/Manager.ts Outdated
@simon-curtis
simon-curtisforce-pushed the terminal/default-shell branch from b972c06 to e311827CompareAugust 17, 2026 18:27
Comment threadapps/server/src/terminal/Manager.ts
@simon-curtis
simon-curtisforce-pushed the terminal/default-shell branch 4 times, most recently from 2d9605b to 03ad822CompareAugust 25, 2026 08:47

@cursorcursorBot 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.

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Want fixes drafted automatically? Bugbot Autofix can create code changes for findings. A team admin can enable Autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 03ad822e19b29ed3a68e615a659271e24d0f2fef. Configure here.

Comment threadapps/server/src/terminal/Manager.ts
@simon-curtis
simon-curtisforce-pushed the terminal/default-shell branch 2 times, most recently from 9e38f97 to 8772631CompareAugust 25, 2026 10:28
@github-actionsgithub-actionsBot added size:L 100-499 changed lines (additions + deletions). and removed size:M 30-99 changed lines (additions + deletions). labels Aug 25, 2026
Comment threadapps/server/src/terminal/Manager.ts Outdated
@simon-curtis
simon-curtisforce-pushed the terminal/default-shell branch from 1a8d434 to 4189825CompareAugust 26, 2026 08:27
@simon-curtis
simon-curtisforce-pushed the terminal/default-shell branch from 4189825 to 4bb5d7dCompareAugust 27, 2026 08:29
@t3dotgg

Copy link
Copy Markdown
Member

Note

🤖 GPT-5.6 Sol responding on behalf of Theo

We're closing this PR as we clean up the T3 Code backlog. Thank you for taking the time to put this together.

#5268 is the retained default shell implementation and limits choices to installed shells reported by the host. This branch accepts arbitrary command text and adds a larger cross-platform parsing contract that we do not need.

If you believe we closed this in error, please reopen the PR and leave a comment explaining what we missed.

@t3dotggt3dotgg closed this Aug 28, 2026
@simon-curtis
simon-curtis deleted the terminal/default-shell branch August 28, 2026 12:39
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L100-499 changed lines (additions + deletions).vouch:unvouchedPR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@simon-curtis@t3dotgg@CDVolvik
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

feat(terminal): let users choose their default shell - #6125

Closed
simon-curtis wants to merge 10 commits into
pingdotgg:mainfrom
simon-curtis:terminal/default-shell
Closed

feat(terminal): let users choose their default shell#6125
simon-curtis wants to merge 10 commits into
pingdotgg:mainfrom
simon-curtis:terminal/default-shell

Conversation

@simon-curtis

@simon-curtissimon-curtis commented Aug 11, 2026

Copy link
Copy Markdown

What Changed

Terminal sessions always chose their shell implicitly, so users who preferred PowerShell, Git Bash, WSL, or another shell could not make it the default.

Settings now includes a Default terminal shell field. New terminal sessions read the current setting, preserve Windows executable paths whether quoted or unquoted, extract executable tokens from POSIX shell command lines, resolve bare Windows commands against the session environment, and retain the existing fallback candidates when the configured command cannot be started.

Why

A coding environment's terminal should open the shell the user actually works in. Making the choice explicit removes platform-dependent surprises while preserving safe fallback behavior and applying setting changes without restarting the server.

Verification

  • Focused terminal and settings suites passed: 88/88 tests.
  • Targeted lint passed for the changed server files.
  • Typechecking passed for t3; prior checks also passed for @t3tools/web and @t3tools/contracts.

UI Changes

Settings gains a Default terminal shell field. Before/after screenshots are not available from this environment.

Checklist

  • This PR is small and focused
  • I explained what changed and why
  • I included before/after screenshots for the Settings change
  • Animation or interaction changes are not applicable

model: gpt-5.6-sol
harness: Codex in T3 Code


Note

Medium Risk
Changes which executable starts PTY sessions and adds Windows PATH resolution per session env; misconfiguration or PATH overrides could pick unexpected shells, though fallbacks remain.

Overview
Adds a Default terminal shell setting (defaultTerminalShell, empty = platform default) in server settings and General settings UI, with search/reset support.

TerminalManager now wires in ServerSettingsService, subscribes to settings changes before reading the initial value, and uses the live value when spawning new sessions (no server restart). Shell resolution is tightened: ~ expansion, quoted executable extraction on POSIX, -NoLogo for Windows pwsh/powershell (including bare aliases), and PATH/PATHEXT lookup for bare Windows command names against each session’s spawn environment—while keeping existing fallback candidates when spawn fails. The terminal layer is provided ServerSettingsLayerLive in server.ts.

Contracts decode patches with trimmed strings; tests cover subscription ordering and the new resolution paths.

Reviewed by Cursor Bugbot for commit 4bb5d7d. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Add configurable default terminal shell setting with live updates

  • Introduces defaultTerminalShell (TrimmedString, defaults to "") in ServerSettings and ServerSettingsPatch in settings.ts
  • TerminalManager now subscribes to ServerSettingsService, reads the initial configured shell, and spawns a scoped fiber to keep the local configuredShell in sync as settings change; shell resolution runs against each session's effective environment instead of the base env
  • normalizeShellCommand expands home-relative paths and handles quoted executables; on Windows, shellCandidateFromCommand appends -NoLogo to pwsh/powershell, and resolveShellCandidates resolves bare command names against effective PATH (with PATHEXT) via resolveCommandPath
  • Frontend adds a DraftInput for the setting in GeneralSettingsPanel, includes it in useSettingsRestore reset logic, and registers a search entry in settingsSearch.ts
  • Risk: resolveShellCandidates now requires FileSystem and Path services and performs PATH lookups on Windows before spawning; misconfigured PATH per-session env can change which executable is selected, and bare names previously spawned without PATH resolution

Macroscope summarized 4bb5d7d.

@coderabbitai

coderabbitaiBot commented Aug 11, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 9ab6f837-f5fd-4c94-a380-be7da102498b

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

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

@github-actionsgithub-actionsBot added size:M 30-99 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list. labels Aug 11, 2026
Comment threadapps/server/src/terminal/Manager.ts
@macroscopeapp

macroscopeappBot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This PR adds a user-facing default-shell setting spanning contracts, settings UI, live server configuration, and terminal process spawning, including platform-specific path resolution and fallback behavior. That is a significant runtime capability change rather than a small isolated tweak, so the terminal/settings integration warrants human review.

You can add or adjust custom eligibility rules. Learn more.

@simon-curtis
simon-curtisforce-pushed the terminal/default-shell branch from 5386fd9 to 0c17f61CompareAugust 13, 2026 10:42
Comment threadapps/web/src/components/settings/SettingsPanels.tsx Outdated
@simon-curtis
simon-curtis marked this pull request as draft August 13, 2026 11:15

@CDVolvikCDVolvik 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.

Windows-shaped tests on the PR files against current main: Manager.test.ts + settings.test.ts 89/89.

What I care about here:

  • subscribe-to-settings beforegetSettings (test pins the order)
  • resolveCommandPath + PATHEXT so jz finds jz.EXE
  • quoted "C:\Program Files\…\bash.exe" is unwrapped before spawn
  • session PATH wins over the process PATH when both have the same name

Empty default + trim is in the contract tests. Fallback to built-in PowerShell remains.

Small mismatch: the Electron placeholder is pwsh.exe. #6260 is the other open Windows-shell PR, and it prefers 5.1 (powershell.exe) over optional pwsh. The placeholder will nudge people at the one this repo is trying to stop probing first.

MERGEABLE. I did not open a real terminal session.

@simon-curtis
simon-curtisforce-pushed the terminal/default-shell branch from 0c17f61 to 137463cCompareAugust 17, 2026 17:04
@simon-curtis

simon-curtis commented Aug 17, 2026

Copy link
Copy Markdown
Author

Review update pushed in e31182709:

  • configured shell input now preserves quoted paths everywhere, keeps unquoted Windows paths intact, and extracts the executable from POSIX command lines such as fish -l;
  • restored focused coverage that pins settings subscription before the initial snapshot;
  • retained Windows PATH/PATHEXT, quoted-path, and per-session PATH coverage.

Verification: focused terminal/settings suites 88/88, targeted lint passed, and the t3 typecheck passed.

I also checked related open PRs. #5268 and #6073 are alternative shell-selection implementations; #6260 overlaps the Windows fallback ordering; #6338 and #4827 touch the same terminal-start block. I kept those separate so this PR remains one shell-agnostic concern against current main.

Comment threadapps/server/src/terminal/Manager.ts Outdated
@simon-curtis
simon-curtisforce-pushed the terminal/default-shell branch from cf76f21 to b972c06CompareAugust 17, 2026 17:49
@simon-curtis
simon-curtis marked this pull request as ready for review August 17, 2026 17:52
Comment threadapps/server/src/terminal/Manager.ts Outdated
@simon-curtis
simon-curtisforce-pushed the terminal/default-shell branch from b972c06 to e311827CompareAugust 17, 2026 18:27
Comment threadapps/server/src/terminal/Manager.ts
@simon-curtis
simon-curtisforce-pushed the terminal/default-shell branch 4 times, most recently from 2d9605b to 03ad822CompareAugust 25, 2026 08:47

@cursorcursorBot 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.

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Want fixes drafted automatically? Bugbot Autofix can create code changes for findings. A team admin can enable Autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 03ad822e19b29ed3a68e615a659271e24d0f2fef. Configure here.

Comment threadapps/server/src/terminal/Manager.ts
@simon-curtis
simon-curtisforce-pushed the terminal/default-shell branch 2 times, most recently from 9e38f97 to 8772631CompareAugust 25, 2026 10:28
@github-actionsgithub-actionsBot added size:L 100-499 changed lines (additions + deletions). and removed size:M 30-99 changed lines (additions + deletions). labels Aug 25, 2026
Comment threadapps/server/src/terminal/Manager.ts Outdated
@simon-curtis
simon-curtisforce-pushed the terminal/default-shell branch from 1a8d434 to 4189825CompareAugust 26, 2026 08:27
@simon-curtis
simon-curtisforce-pushed the terminal/default-shell branch from 4189825 to 4bb5d7dCompareAugust 27, 2026 08:29
@t3dotgg

Copy link
Copy Markdown
Member

Note

🤖 GPT-5.6 Sol responding on behalf of Theo

We're closing this PR as we clean up the T3 Code backlog. Thank you for taking the time to put this together.

#5268 is the retained default shell implementation and limits choices to installed shells reported by the host. This branch accepts arbitrary command text and adds a larger cross-platform parsing contract that we do not need.

If you believe we closed this in error, please reopen the PR and leave a comment explaining what we missed.

@t3dotggt3dotgg closed this Aug 28, 2026
@simon-curtis
simon-curtis deleted the terminal/default-shell branch August 28, 2026 12:39
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L100-499 changed lines (additions + deletions).vouch:unvouchedPR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@simon-curtis@t3dotgg@CDVolvik
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

feat(terminal): let users choose their default shell - #6125

Closed
simon-curtis wants to merge 10 commits into
pingdotgg:mainfrom
simon-curtis:terminal/default-shell
Closed

feat(terminal): let users choose their default shell#6125
simon-curtis wants to merge 10 commits into
pingdotgg:mainfrom
simon-curtis:terminal/default-shell

Conversation

@simon-curtis

@simon-curtissimon-curtis commented Aug 11, 2026

Copy link
Copy Markdown

What Changed

Terminal sessions always chose their shell implicitly, so users who preferred PowerShell, Git Bash, WSL, or another shell could not make it the default.

Settings now includes a Default terminal shell field. New terminal sessions read the current setting, preserve Windows executable paths whether quoted or unquoted, extract executable tokens from POSIX shell command lines, resolve bare Windows commands against the session environment, and retain the existing fallback candidates when the configured command cannot be started.

Why

A coding environment's terminal should open the shell the user actually works in. Making the choice explicit removes platform-dependent surprises while preserving safe fallback behavior and applying setting changes without restarting the server.

Verification

  • Focused terminal and settings suites passed: 88/88 tests.
  • Targeted lint passed for the changed server files.
  • Typechecking passed for t3; prior checks also passed for @t3tools/web and @t3tools/contracts.

UI Changes

Settings gains a Default terminal shell field. Before/after screenshots are not available from this environment.

Checklist

  • This PR is small and focused
  • I explained what changed and why
  • I included before/after screenshots for the Settings change
  • Animation or interaction changes are not applicable

model: gpt-5.6-sol
harness: Codex in T3 Code


Note

Medium Risk
Changes which executable starts PTY sessions and adds Windows PATH resolution per session env; misconfiguration or PATH overrides could pick unexpected shells, though fallbacks remain.

Overview
Adds a Default terminal shell setting (defaultTerminalShell, empty = platform default) in server settings and General settings UI, with search/reset support.

TerminalManager now wires in ServerSettingsService, subscribes to settings changes before reading the initial value, and uses the live value when spawning new sessions (no server restart). Shell resolution is tightened: ~ expansion, quoted executable extraction on POSIX, -NoLogo for Windows pwsh/powershell (including bare aliases), and PATH/PATHEXT lookup for bare Windows command names against each session’s spawn environment—while keeping existing fallback candidates when spawn fails. The terminal layer is provided ServerSettingsLayerLive in server.ts.

Contracts decode patches with trimmed strings; tests cover subscription ordering and the new resolution paths.

Reviewed by Cursor Bugbot for commit 4bb5d7d. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Add configurable default terminal shell setting with live updates

  • Introduces defaultTerminalShell (TrimmedString, defaults to "") in ServerSettings and ServerSettingsPatch in settings.ts
  • TerminalManager now subscribes to ServerSettingsService, reads the initial configured shell, and spawns a scoped fiber to keep the local configuredShell in sync as settings change; shell resolution runs against each session's effective environment instead of the base env
  • normalizeShellCommand expands home-relative paths and handles quoted executables; on Windows, shellCandidateFromCommand appends -NoLogo to pwsh/powershell, and resolveShellCandidates resolves bare command names against effective PATH (with PATHEXT) via resolveCommandPath
  • Frontend adds a DraftInput for the setting in GeneralSettingsPanel, includes it in useSettingsRestore reset logic, and registers a search entry in settingsSearch.ts
  • Risk: resolveShellCandidates now requires FileSystem and Path services and performs PATH lookups on Windows before spawning; misconfigured PATH per-session env can change which executable is selected, and bare names previously spawned without PATH resolution

Macroscope summarized 4bb5d7d.

@coderabbitai

coderabbitaiBot commented Aug 11, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 9ab6f837-f5fd-4c94-a380-be7da102498b

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

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

@github-actionsgithub-actionsBot added size:M 30-99 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list. labels Aug 11, 2026
Comment threadapps/server/src/terminal/Manager.ts
@macroscopeapp

macroscopeappBot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This PR adds a user-facing default-shell setting spanning contracts, settings UI, live server configuration, and terminal process spawning, including platform-specific path resolution and fallback behavior. That is a significant runtime capability change rather than a small isolated tweak, so the terminal/settings integration warrants human review.

You can add or adjust custom eligibility rules. Learn more.

@simon-curtis
simon-curtisforce-pushed the terminal/default-shell branch from 5386fd9 to 0c17f61CompareAugust 13, 2026 10:42
Comment threadapps/web/src/components/settings/SettingsPanels.tsx Outdated
@simon-curtis
simon-curtis marked this pull request as draft August 13, 2026 11:15

@CDVolvikCDVolvik 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.

Windows-shaped tests on the PR files against current main: Manager.test.ts + settings.test.ts 89/89.

What I care about here:

  • subscribe-to-settings beforegetSettings (test pins the order)
  • resolveCommandPath + PATHEXT so jz finds jz.EXE
  • quoted "C:\Program Files\…\bash.exe" is unwrapped before spawn
  • session PATH wins over the process PATH when both have the same name

Empty default + trim is in the contract tests. Fallback to built-in PowerShell remains.

Small mismatch: the Electron placeholder is pwsh.exe. #6260 is the other open Windows-shell PR, and it prefers 5.1 (powershell.exe) over optional pwsh. The placeholder will nudge people at the one this repo is trying to stop probing first.

MERGEABLE. I did not open a real terminal session.

@simon-curtis
simon-curtisforce-pushed the terminal/default-shell branch from 0c17f61 to 137463cCompareAugust 17, 2026 17:04
@simon-curtis

simon-curtis commented Aug 17, 2026

Copy link
Copy Markdown
Author

Review update pushed in e31182709:

  • configured shell input now preserves quoted paths everywhere, keeps unquoted Windows paths intact, and extracts the executable from POSIX command lines such as fish -l;
  • restored focused coverage that pins settings subscription before the initial snapshot;
  • retained Windows PATH/PATHEXT, quoted-path, and per-session PATH coverage.

Verification: focused terminal/settings suites 88/88, targeted lint passed, and the t3 typecheck passed.

I also checked related open PRs. #5268 and #6073 are alternative shell-selection implementations; #6260 overlaps the Windows fallback ordering; #6338 and #4827 touch the same terminal-start block. I kept those separate so this PR remains one shell-agnostic concern against current main.

Comment threadapps/server/src/terminal/Manager.ts Outdated
@simon-curtis
simon-curtisforce-pushed the terminal/default-shell branch from cf76f21 to b972c06CompareAugust 17, 2026 17:49
@simon-curtis
simon-curtis marked this pull request as ready for review August 17, 2026 17:52
Comment threadapps/server/src/terminal/Manager.ts Outdated
@simon-curtis
simon-curtisforce-pushed the terminal/default-shell branch from b972c06 to e311827CompareAugust 17, 2026 18:27
Comment threadapps/server/src/terminal/Manager.ts
@simon-curtis
simon-curtisforce-pushed the terminal/default-shell branch 4 times, most recently from 2d9605b to 03ad822CompareAugust 25, 2026 08:47

@cursorcursorBot 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.

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Want fixes drafted automatically? Bugbot Autofix can create code changes for findings. A team admin can enable Autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 03ad822e19b29ed3a68e615a659271e24d0f2fef. Configure here.

Comment threadapps/server/src/terminal/Manager.ts
@simon-curtis
simon-curtisforce-pushed the terminal/default-shell branch 2 times, most recently from 9e38f97 to 8772631CompareAugust 25, 2026 10:28
@github-actionsgithub-actionsBot added size:L 100-499 changed lines (additions + deletions). and removed size:M 30-99 changed lines (additions + deletions). labels Aug 25, 2026
Comment threadapps/server/src/terminal/Manager.ts Outdated
@simon-curtis
simon-curtisforce-pushed the terminal/default-shell branch from 1a8d434 to 4189825CompareAugust 26, 2026 08:27
@simon-curtis
simon-curtisforce-pushed the terminal/default-shell branch from 4189825 to 4bb5d7dCompareAugust 27, 2026 08:29
@t3dotgg

Copy link
Copy Markdown
Member

Note

🤖 GPT-5.6 Sol responding on behalf of Theo

We're closing this PR as we clean up the T3 Code backlog. Thank you for taking the time to put this together.

#5268 is the retained default shell implementation and limits choices to installed shells reported by the host. This branch accepts arbitrary command text and adds a larger cross-platform parsing contract that we do not need.

If you believe we closed this in error, please reopen the PR and leave a comment explaining what we missed.

@t3dotggt3dotgg closed this Aug 28, 2026
@simon-curtis
simon-curtis deleted the terminal/default-shell branch August 28, 2026 12:39
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L100-499 changed lines (additions + deletions).vouch:unvouchedPR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@simon-curtis@t3dotgg@CDVolvik
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

feat(terminal): let users choose their default shell - #6125

Closed
simon-curtis wants to merge 10 commits into
pingdotgg:mainfrom
simon-curtis:terminal/default-shell
Closed

feat(terminal): let users choose their default shell#6125
simon-curtis wants to merge 10 commits into
pingdotgg:mainfrom
simon-curtis:terminal/default-shell

Conversation

@simon-curtis

@simon-curtissimon-curtis commented Aug 11, 2026

Copy link
Copy Markdown

What Changed

Terminal sessions always chose their shell implicitly, so users who preferred PowerShell, Git Bash, WSL, or another shell could not make it the default.

Settings now includes a Default terminal shell field. New terminal sessions read the current setting, preserve Windows executable paths whether quoted or unquoted, extract executable tokens from POSIX shell command lines, resolve bare Windows commands against the session environment, and retain the existing fallback candidates when the configured command cannot be started.

Why

A coding environment's terminal should open the shell the user actually works in. Making the choice explicit removes platform-dependent surprises while preserving safe fallback behavior and applying setting changes without restarting the server.

Verification

  • Focused terminal and settings suites passed: 88/88 tests.
  • Targeted lint passed for the changed server files.
  • Typechecking passed for t3; prior checks also passed for @t3tools/web and @t3tools/contracts.

UI Changes

Settings gains a Default terminal shell field. Before/after screenshots are not available from this environment.

Checklist

  • This PR is small and focused
  • I explained what changed and why
  • I included before/after screenshots for the Settings change
  • Animation or interaction changes are not applicable

model: gpt-5.6-sol
harness: Codex in T3 Code


Note

Medium Risk
Changes which executable starts PTY sessions and adds Windows PATH resolution per session env; misconfiguration or PATH overrides could pick unexpected shells, though fallbacks remain.

Overview
Adds a Default terminal shell setting (defaultTerminalShell, empty = platform default) in server settings and General settings UI, with search/reset support.

TerminalManager now wires in ServerSettingsService, subscribes to settings changes before reading the initial value, and uses the live value when spawning new sessions (no server restart). Shell resolution is tightened: ~ expansion, quoted executable extraction on POSIX, -NoLogo for Windows pwsh/powershell (including bare aliases), and PATH/PATHEXT lookup for bare Windows command names against each session’s spawn environment—while keeping existing fallback candidates when spawn fails. The terminal layer is provided ServerSettingsLayerLive in server.ts.

Contracts decode patches with trimmed strings; tests cover subscription ordering and the new resolution paths.

Reviewed by Cursor Bugbot for commit 4bb5d7d. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Add configurable default terminal shell setting with live updates

  • Introduces defaultTerminalShell (TrimmedString, defaults to "") in ServerSettings and ServerSettingsPatch in settings.ts
  • TerminalManager now subscribes to ServerSettingsService, reads the initial configured shell, and spawns a scoped fiber to keep the local configuredShell in sync as settings change; shell resolution runs against each session's effective environment instead of the base env
  • normalizeShellCommand expands home-relative paths and handles quoted executables; on Windows, shellCandidateFromCommand appends -NoLogo to pwsh/powershell, and resolveShellCandidates resolves bare command names against effective PATH (with PATHEXT) via resolveCommandPath
  • Frontend adds a DraftInput for the setting in GeneralSettingsPanel, includes it in useSettingsRestore reset logic, and registers a search entry in settingsSearch.ts
  • Risk: resolveShellCandidates now requires FileSystem and Path services and performs PATH lookups on Windows before spawning; misconfigured PATH per-session env can change which executable is selected, and bare names previously spawned without PATH resolution

Macroscope summarized 4bb5d7d.

@coderabbitai

coderabbitaiBot commented Aug 11, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 9ab6f837-f5fd-4c94-a380-be7da102498b

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

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

@github-actionsgithub-actionsBot added size:M 30-99 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list. labels Aug 11, 2026
Comment threadapps/server/src/terminal/Manager.ts
@macroscopeapp

macroscopeappBot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This PR adds a user-facing default-shell setting spanning contracts, settings UI, live server configuration, and terminal process spawning, including platform-specific path resolution and fallback behavior. That is a significant runtime capability change rather than a small isolated tweak, so the terminal/settings integration warrants human review.

You can add or adjust custom eligibility rules. Learn more.

@simon-curtis
simon-curtisforce-pushed the terminal/default-shell branch from 5386fd9 to 0c17f61CompareAugust 13, 2026 10:42
Comment threadapps/web/src/components/settings/SettingsPanels.tsx Outdated
@simon-curtis
simon-curtis marked this pull request as draft August 13, 2026 11:15

@CDVolvikCDVolvik 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.

Windows-shaped tests on the PR files against current main: Manager.test.ts + settings.test.ts 89/89.

What I care about here:

  • subscribe-to-settings beforegetSettings (test pins the order)
  • resolveCommandPath + PATHEXT so jz finds jz.EXE
  • quoted "C:\Program Files\…\bash.exe" is unwrapped before spawn
  • session PATH wins over the process PATH when both have the same name

Empty default + trim is in the contract tests. Fallback to built-in PowerShell remains.

Small mismatch: the Electron placeholder is pwsh.exe. #6260 is the other open Windows-shell PR, and it prefers 5.1 (powershell.exe) over optional pwsh. The placeholder will nudge people at the one this repo is trying to stop probing first.

MERGEABLE. I did not open a real terminal session.

@simon-curtis
simon-curtisforce-pushed the terminal/default-shell branch from 0c17f61 to 137463cCompareAugust 17, 2026 17:04
@simon-curtis

simon-curtis commented Aug 17, 2026

Copy link
Copy Markdown
Author

Review update pushed in e31182709:

  • configured shell input now preserves quoted paths everywhere, keeps unquoted Windows paths intact, and extracts the executable from POSIX command lines such as fish -l;
  • restored focused coverage that pins settings subscription before the initial snapshot;
  • retained Windows PATH/PATHEXT, quoted-path, and per-session PATH coverage.

Verification: focused terminal/settings suites 88/88, targeted lint passed, and the t3 typecheck passed.

I also checked related open PRs. #5268 and #6073 are alternative shell-selection implementations; #6260 overlaps the Windows fallback ordering; #6338 and #4827 touch the same terminal-start block. I kept those separate so this PR remains one shell-agnostic concern against current main.

Comment threadapps/server/src/terminal/Manager.ts Outdated
@simon-curtis
simon-curtisforce-pushed the terminal/default-shell branch from cf76f21 to b972c06CompareAugust 17, 2026 17:49
@simon-curtis
simon-curtis marked this pull request as ready for review August 17, 2026 17:52
Comment threadapps/server/src/terminal/Manager.ts Outdated
@simon-curtis
simon-curtisforce-pushed the terminal/default-shell branch from b972c06 to e311827CompareAugust 17, 2026 18:27
Comment threadapps/server/src/terminal/Manager.ts
@simon-curtis
simon-curtisforce-pushed the terminal/default-shell branch 4 times, most recently from 2d9605b to 03ad822CompareAugust 25, 2026 08:47

@cursorcursorBot 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.

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Want fixes drafted automatically? Bugbot Autofix can create code changes for findings. A team admin can enable Autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 03ad822e19b29ed3a68e615a659271e24d0f2fef. Configure here.

Comment threadapps/server/src/terminal/Manager.ts
@simon-curtis
simon-curtisforce-pushed the terminal/default-shell branch 2 times, most recently from 9e38f97 to 8772631CompareAugust 25, 2026 10:28
@github-actionsgithub-actionsBot added size:L 100-499 changed lines (additions + deletions). and removed size:M 30-99 changed lines (additions + deletions). labels Aug 25, 2026
Comment threadapps/server/src/terminal/Manager.ts Outdated
@simon-curtis
simon-curtisforce-pushed the terminal/default-shell branch from 1a8d434 to 4189825CompareAugust 26, 2026 08:27
@simon-curtis
simon-curtisforce-pushed the terminal/default-shell branch from 4189825 to 4bb5d7dCompareAugust 27, 2026 08:29
@t3dotgg

Copy link
Copy Markdown
Member

Note

🤖 GPT-5.6 Sol responding on behalf of Theo

We're closing this PR as we clean up the T3 Code backlog. Thank you for taking the time to put this together.

#5268 is the retained default shell implementation and limits choices to installed shells reported by the host. This branch accepts arbitrary command text and adds a larger cross-platform parsing contract that we do not need.

If you believe we closed this in error, please reopen the PR and leave a comment explaining what we missed.

@t3dotggt3dotgg closed this Aug 28, 2026
@simon-curtis
simon-curtis deleted the terminal/default-shell branch August 28, 2026 12:39
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L100-499 changed lines (additions + deletions).vouch:unvouchedPR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@simon-curtis@t3dotgg@CDVolvik
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

feat(terminal): let users choose their default shell - #6125

Closed
simon-curtis wants to merge 10 commits into
pingdotgg:mainfrom
simon-curtis:terminal/default-shell
Closed

feat(terminal): let users choose their default shell#6125
simon-curtis wants to merge 10 commits into
pingdotgg:mainfrom
simon-curtis:terminal/default-shell

Conversation

@simon-curtis

@simon-curtissimon-curtis commented Aug 11, 2026

Copy link
Copy Markdown

What Changed

Terminal sessions always chose their shell implicitly, so users who preferred PowerShell, Git Bash, WSL, or another shell could not make it the default.

Settings now includes a Default terminal shell field. New terminal sessions read the current setting, preserve Windows executable paths whether quoted or unquoted, extract executable tokens from POSIX shell command lines, resolve bare Windows commands against the session environment, and retain the existing fallback candidates when the configured command cannot be started.

Why

A coding environment's terminal should open the shell the user actually works in. Making the choice explicit removes platform-dependent surprises while preserving safe fallback behavior and applying setting changes without restarting the server.

Verification

  • Focused terminal and settings suites passed: 88/88 tests.
  • Targeted lint passed for the changed server files.
  • Typechecking passed for t3; prior checks also passed for @t3tools/web and @t3tools/contracts.

UI Changes

Settings gains a Default terminal shell field. Before/after screenshots are not available from this environment.

Checklist

  • This PR is small and focused
  • I explained what changed and why
  • I included before/after screenshots for the Settings change
  • Animation or interaction changes are not applicable

model: gpt-5.6-sol
harness: Codex in T3 Code


Note

Medium Risk
Changes which executable starts PTY sessions and adds Windows PATH resolution per session env; misconfiguration or PATH overrides could pick unexpected shells, though fallbacks remain.

Overview
Adds a Default terminal shell setting (defaultTerminalShell, empty = platform default) in server settings and General settings UI, with search/reset support.

TerminalManager now wires in ServerSettingsService, subscribes to settings changes before reading the initial value, and uses the live value when spawning new sessions (no server restart). Shell resolution is tightened: ~ expansion, quoted executable extraction on POSIX, -NoLogo for Windows pwsh/powershell (including bare aliases), and PATH/PATHEXT lookup for bare Windows command names against each session’s spawn environment—while keeping existing fallback candidates when spawn fails. The terminal layer is provided ServerSettingsLayerLive in server.ts.

Contracts decode patches with trimmed strings; tests cover subscription ordering and the new resolution paths.

Reviewed by Cursor Bugbot for commit 4bb5d7d. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Add configurable default terminal shell setting with live updates

  • Introduces defaultTerminalShell (TrimmedString, defaults to "") in ServerSettings and ServerSettingsPatch in settings.ts
  • TerminalManager now subscribes to ServerSettingsService, reads the initial configured shell, and spawns a scoped fiber to keep the local configuredShell in sync as settings change; shell resolution runs against each session's effective environment instead of the base env
  • normalizeShellCommand expands home-relative paths and handles quoted executables; on Windows, shellCandidateFromCommand appends -NoLogo to pwsh/powershell, and resolveShellCandidates resolves bare command names against effective PATH (with PATHEXT) via resolveCommandPath
  • Frontend adds a DraftInput for the setting in GeneralSettingsPanel, includes it in useSettingsRestore reset logic, and registers a search entry in settingsSearch.ts
  • Risk: resolveShellCandidates now requires FileSystem and Path services and performs PATH lookups on Windows before spawning; misconfigured PATH per-session env can change which executable is selected, and bare names previously spawned without PATH resolution

Macroscope summarized 4bb5d7d.

@coderabbitai

coderabbitaiBot commented Aug 11, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 9ab6f837-f5fd-4c94-a380-be7da102498b

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

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

@github-actionsgithub-actionsBot added size:M 30-99 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list. labels Aug 11, 2026
Comment threadapps/server/src/terminal/Manager.ts
@macroscopeapp

macroscopeappBot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This PR adds a user-facing default-shell setting spanning contracts, settings UI, live server configuration, and terminal process spawning, including platform-specific path resolution and fallback behavior. That is a significant runtime capability change rather than a small isolated tweak, so the terminal/settings integration warrants human review.

You can add or adjust custom eligibility rules. Learn more.

@simon-curtis
simon-curtisforce-pushed the terminal/default-shell branch from 5386fd9 to 0c17f61CompareAugust 13, 2026 10:42
Comment threadapps/web/src/components/settings/SettingsPanels.tsx Outdated
@simon-curtis
simon-curtis marked this pull request as draft August 13, 2026 11:15

@CDVolvikCDVolvik 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.

Windows-shaped tests on the PR files against current main: Manager.test.ts + settings.test.ts 89/89.

What I care about here:

  • subscribe-to-settings beforegetSettings (test pins the order)
  • resolveCommandPath + PATHEXT so jz finds jz.EXE
  • quoted "C:\Program Files\…\bash.exe" is unwrapped before spawn
  • session PATH wins over the process PATH when both have the same name

Empty default + trim is in the contract tests. Fallback to built-in PowerShell remains.

Small mismatch: the Electron placeholder is pwsh.exe. #6260 is the other open Windows-shell PR, and it prefers 5.1 (powershell.exe) over optional pwsh. The placeholder will nudge people at the one this repo is trying to stop probing first.

MERGEABLE. I did not open a real terminal session.

@simon-curtis
simon-curtisforce-pushed the terminal/default-shell branch from 0c17f61 to 137463cCompareAugust 17, 2026 17:04
@simon-curtis

simon-curtis commented Aug 17, 2026

Copy link
Copy Markdown
Author

Review update pushed in e31182709:

  • configured shell input now preserves quoted paths everywhere, keeps unquoted Windows paths intact, and extracts the executable from POSIX command lines such as fish -l;
  • restored focused coverage that pins settings subscription before the initial snapshot;
  • retained Windows PATH/PATHEXT, quoted-path, and per-session PATH coverage.

Verification: focused terminal/settings suites 88/88, targeted lint passed, and the t3 typecheck passed.

I also checked related open PRs. #5268 and #6073 are alternative shell-selection implementations; #6260 overlaps the Windows fallback ordering; #6338 and #4827 touch the same terminal-start block. I kept those separate so this PR remains one shell-agnostic concern against current main.

Comment threadapps/server/src/terminal/Manager.ts Outdated
@simon-curtis
simon-curtisforce-pushed the terminal/default-shell branch from cf76f21 to b972c06CompareAugust 17, 2026 17:49
@simon-curtis
simon-curtis marked this pull request as ready for review August 17, 2026 17:52
Comment threadapps/server/src/terminal/Manager.ts Outdated
@simon-curtis
simon-curtisforce-pushed the terminal/default-shell branch from b972c06 to e311827CompareAugust 17, 2026 18:27
Comment threadapps/server/src/terminal/Manager.ts
@simon-curtis
simon-curtisforce-pushed the terminal/default-shell branch 4 times, most recently from 2d9605b to 03ad822CompareAugust 25, 2026 08:47

@cursorcursorBot 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.

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Want fixes drafted automatically? Bugbot Autofix can create code changes for findings. A team admin can enable Autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 03ad822e19b29ed3a68e615a659271e24d0f2fef. Configure here.

Comment threadapps/server/src/terminal/Manager.ts
@simon-curtis
simon-curtisforce-pushed the terminal/default-shell branch 2 times, most recently from 9e38f97 to 8772631CompareAugust 25, 2026 10:28
@github-actionsgithub-actionsBot added size:L 100-499 changed lines (additions + deletions). and removed size:M 30-99 changed lines (additions + deletions). labels Aug 25, 2026
Comment threadapps/server/src/terminal/Manager.ts Outdated
@simon-curtis
simon-curtisforce-pushed the terminal/default-shell branch from 1a8d434 to 4189825CompareAugust 26, 2026 08:27
@simon-curtis
simon-curtisforce-pushed the terminal/default-shell branch from 4189825 to 4bb5d7dCompareAugust 27, 2026 08:29
@t3dotgg

Copy link
Copy Markdown
Member

Note

🤖 GPT-5.6 Sol responding on behalf of Theo

We're closing this PR as we clean up the T3 Code backlog. Thank you for taking the time to put this together.

#5268 is the retained default shell implementation and limits choices to installed shells reported by the host. This branch accepts arbitrary command text and adds a larger cross-platform parsing contract that we do not need.

If you believe we closed this in error, please reopen the PR and leave a comment explaining what we missed.

@t3dotggt3dotgg closed this Aug 28, 2026
@simon-curtis
simon-curtis deleted the terminal/default-shell branch August 28, 2026 12:39
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L100-499 changed lines (additions + deletions).vouch:unvouchedPR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@simon-curtis@t3dotgg@CDVolvik
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

feat(terminal): let users choose their default shell - #6125

Closed
simon-curtis wants to merge 10 commits into
pingdotgg:mainfrom
simon-curtis:terminal/default-shell
Closed

feat(terminal): let users choose their default shell#6125
simon-curtis wants to merge 10 commits into
pingdotgg:mainfrom
simon-curtis:terminal/default-shell

Conversation

@simon-curtis

@simon-curtissimon-curtis commented Aug 11, 2026

Copy link
Copy Markdown

What Changed

Terminal sessions always chose their shell implicitly, so users who preferred PowerShell, Git Bash, WSL, or another shell could not make it the default.

Settings now includes a Default terminal shell field. New terminal sessions read the current setting, preserve Windows executable paths whether quoted or unquoted, extract executable tokens from POSIX shell command lines, resolve bare Windows commands against the session environment, and retain the existing fallback candidates when the configured command cannot be started.

Why

A coding environment's terminal should open the shell the user actually works in. Making the choice explicit removes platform-dependent surprises while preserving safe fallback behavior and applying setting changes without restarting the server.

Verification

  • Focused terminal and settings suites passed: 88/88 tests.
  • Targeted lint passed for the changed server files.
  • Typechecking passed for t3; prior checks also passed for @t3tools/web and @t3tools/contracts.

UI Changes

Settings gains a Default terminal shell field. Before/after screenshots are not available from this environment.

Checklist

  • This PR is small and focused
  • I explained what changed and why
  • I included before/after screenshots for the Settings change
  • Animation or interaction changes are not applicable

model: gpt-5.6-sol
harness: Codex in T3 Code


Note

Medium Risk
Changes which executable starts PTY sessions and adds Windows PATH resolution per session env; misconfiguration or PATH overrides could pick unexpected shells, though fallbacks remain.

Overview
Adds a Default terminal shell setting (defaultTerminalShell, empty = platform default) in server settings and General settings UI, with search/reset support.

TerminalManager now wires in ServerSettingsService, subscribes to settings changes before reading the initial value, and uses the live value when spawning new sessions (no server restart). Shell resolution is tightened: ~ expansion, quoted executable extraction on POSIX, -NoLogo for Windows pwsh/powershell (including bare aliases), and PATH/PATHEXT lookup for bare Windows command names against each session’s spawn environment—while keeping existing fallback candidates when spawn fails. The terminal layer is provided ServerSettingsLayerLive in server.ts.

Contracts decode patches with trimmed strings; tests cover subscription ordering and the new resolution paths.

Reviewed by Cursor Bugbot for commit 4bb5d7d. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Add configurable default terminal shell setting with live updates

  • Introduces defaultTerminalShell (TrimmedString, defaults to "") in ServerSettings and ServerSettingsPatch in settings.ts
  • TerminalManager now subscribes to ServerSettingsService, reads the initial configured shell, and spawns a scoped fiber to keep the local configuredShell in sync as settings change; shell resolution runs against each session's effective environment instead of the base env
  • normalizeShellCommand expands home-relative paths and handles quoted executables; on Windows, shellCandidateFromCommand appends -NoLogo to pwsh/powershell, and resolveShellCandidates resolves bare command names against effective PATH (with PATHEXT) via resolveCommandPath
  • Frontend adds a DraftInput for the setting in GeneralSettingsPanel, includes it in useSettingsRestore reset logic, and registers a search entry in settingsSearch.ts
  • Risk: resolveShellCandidates now requires FileSystem and Path services and performs PATH lookups on Windows before spawning; misconfigured PATH per-session env can change which executable is selected, and bare names previously spawned without PATH resolution

Macroscope summarized 4bb5d7d.

@coderabbitai

coderabbitaiBot commented Aug 11, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 9ab6f837-f5fd-4c94-a380-be7da102498b

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

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

@github-actionsgithub-actionsBot added size:M 30-99 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list. labels Aug 11, 2026
Comment threadapps/server/src/terminal/Manager.ts
@macroscopeapp

macroscopeappBot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This PR adds a user-facing default-shell setting spanning contracts, settings UI, live server configuration, and terminal process spawning, including platform-specific path resolution and fallback behavior. That is a significant runtime capability change rather than a small isolated tweak, so the terminal/settings integration warrants human review.

You can add or adjust custom eligibility rules. Learn more.

@simon-curtis
simon-curtisforce-pushed the terminal/default-shell branch from 5386fd9 to 0c17f61CompareAugust 13, 2026 10:42
Comment threadapps/web/src/components/settings/SettingsPanels.tsx Outdated
@simon-curtis
simon-curtis marked this pull request as draft August 13, 2026 11:15

@CDVolvikCDVolvik 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.

Windows-shaped tests on the PR files against current main: Manager.test.ts + settings.test.ts 89/89.

What I care about here:

  • subscribe-to-settings beforegetSettings (test pins the order)
  • resolveCommandPath + PATHEXT so jz finds jz.EXE
  • quoted "C:\Program Files\…\bash.exe" is unwrapped before spawn
  • session PATH wins over the process PATH when both have the same name

Empty default + trim is in the contract tests. Fallback to built-in PowerShell remains.

Small mismatch: the Electron placeholder is pwsh.exe. #6260 is the other open Windows-shell PR, and it prefers 5.1 (powershell.exe) over optional pwsh. The placeholder will nudge people at the one this repo is trying to stop probing first.

MERGEABLE. I did not open a real terminal session.

@simon-curtis
simon-curtisforce-pushed the terminal/default-shell branch from 0c17f61 to 137463cCompareAugust 17, 2026 17:04
@simon-curtis

simon-curtis commented Aug 17, 2026

Copy link
Copy Markdown
Author

Review update pushed in e31182709:

  • configured shell input now preserves quoted paths everywhere, keeps unquoted Windows paths intact, and extracts the executable from POSIX command lines such as fish -l;
  • restored focused coverage that pins settings subscription before the initial snapshot;
  • retained Windows PATH/PATHEXT, quoted-path, and per-session PATH coverage.

Verification: focused terminal/settings suites 88/88, targeted lint passed, and the t3 typecheck passed.

I also checked related open PRs. #5268 and #6073 are alternative shell-selection implementations; #6260 overlaps the Windows fallback ordering; #6338 and #4827 touch the same terminal-start block. I kept those separate so this PR remains one shell-agnostic concern against current main.

Comment threadapps/server/src/terminal/Manager.ts Outdated
@simon-curtis
simon-curtisforce-pushed the terminal/default-shell branch from cf76f21 to b972c06CompareAugust 17, 2026 17:49
@simon-curtis
simon-curtis marked this pull request as ready for review August 17, 2026 17:52
Comment threadapps/server/src/terminal/Manager.ts Outdated
@simon-curtis
simon-curtisforce-pushed the terminal/default-shell branch from b972c06 to e311827CompareAugust 17, 2026 18:27
Comment threadapps/server/src/terminal/Manager.ts
@simon-curtis
simon-curtisforce-pushed the terminal/default-shell branch 4 times, most recently from 2d9605b to 03ad822CompareAugust 25, 2026 08:47

@cursorcursorBot 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.

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Want fixes drafted automatically? Bugbot Autofix can create code changes for findings. A team admin can enable Autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 03ad822e19b29ed3a68e615a659271e24d0f2fef. Configure here.

Comment threadapps/server/src/terminal/Manager.ts
@simon-curtis
simon-curtisforce-pushed the terminal/default-shell branch 2 times, most recently from 9e38f97 to 8772631CompareAugust 25, 2026 10:28
@github-actionsgithub-actionsBot added size:L 100-499 changed lines (additions + deletions). and removed size:M 30-99 changed lines (additions + deletions). labels Aug 25, 2026
Comment threadapps/server/src/terminal/Manager.ts Outdated
@simon-curtis
simon-curtisforce-pushed the terminal/default-shell branch from 1a8d434 to 4189825CompareAugust 26, 2026 08:27
@simon-curtis
simon-curtisforce-pushed the terminal/default-shell branch from 4189825 to 4bb5d7dCompareAugust 27, 2026 08:29
@t3dotgg

Copy link
Copy Markdown
Member

Note

🤖 GPT-5.6 Sol responding on behalf of Theo

We're closing this PR as we clean up the T3 Code backlog. Thank you for taking the time to put this together.

#5268 is the retained default shell implementation and limits choices to installed shells reported by the host. This branch accepts arbitrary command text and adds a larger cross-platform parsing contract that we do not need.

If you believe we closed this in error, please reopen the PR and leave a comment explaining what we missed.

@t3dotggt3dotgg closed this Aug 28, 2026
@simon-curtis
simon-curtis deleted the terminal/default-shell branch August 28, 2026 12:39
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L100-499 changed lines (additions + deletions).vouch:unvouchedPR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@simon-curtis@t3dotgg@CDVolvik
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

feat(terminal): let users choose their default shell - #6125

Closed
simon-curtis wants to merge 10 commits into
pingdotgg:mainfrom
simon-curtis:terminal/default-shell
Closed

feat(terminal): let users choose their default shell#6125
simon-curtis wants to merge 10 commits into
pingdotgg:mainfrom
simon-curtis:terminal/default-shell

Conversation

@simon-curtis

@simon-curtissimon-curtis commented Aug 11, 2026

Copy link
Copy Markdown

What Changed

Terminal sessions always chose their shell implicitly, so users who preferred PowerShell, Git Bash, WSL, or another shell could not make it the default.

Settings now includes a Default terminal shell field. New terminal sessions read the current setting, preserve Windows executable paths whether quoted or unquoted, extract executable tokens from POSIX shell command lines, resolve bare Windows commands against the session environment, and retain the existing fallback candidates when the configured command cannot be started.

Why

A coding environment's terminal should open the shell the user actually works in. Making the choice explicit removes platform-dependent surprises while preserving safe fallback behavior and applying setting changes without restarting the server.

Verification

  • Focused terminal and settings suites passed: 88/88 tests.
  • Targeted lint passed for the changed server files.
  • Typechecking passed for t3; prior checks also passed for @t3tools/web and @t3tools/contracts.

UI Changes

Settings gains a Default terminal shell field. Before/after screenshots are not available from this environment.

Checklist

  • This PR is small and focused
  • I explained what changed and why
  • I included before/after screenshots for the Settings change
  • Animation or interaction changes are not applicable

model: gpt-5.6-sol
harness: Codex in T3 Code


Note

Medium Risk
Changes which executable starts PTY sessions and adds Windows PATH resolution per session env; misconfiguration or PATH overrides could pick unexpected shells, though fallbacks remain.

Overview
Adds a Default terminal shell setting (defaultTerminalShell, empty = platform default) in server settings and General settings UI, with search/reset support.

TerminalManager now wires in ServerSettingsService, subscribes to settings changes before reading the initial value, and uses the live value when spawning new sessions (no server restart). Shell resolution is tightened: ~ expansion, quoted executable extraction on POSIX, -NoLogo for Windows pwsh/powershell (including bare aliases), and PATH/PATHEXT lookup for bare Windows command names against each session’s spawn environment—while keeping existing fallback candidates when spawn fails. The terminal layer is provided ServerSettingsLayerLive in server.ts.

Contracts decode patches with trimmed strings; tests cover subscription ordering and the new resolution paths.

Reviewed by Cursor Bugbot for commit 4bb5d7d. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Add configurable default terminal shell setting with live updates

  • Introduces defaultTerminalShell (TrimmedString, defaults to "") in ServerSettings and ServerSettingsPatch in settings.ts
  • TerminalManager now subscribes to ServerSettingsService, reads the initial configured shell, and spawns a scoped fiber to keep the local configuredShell in sync as settings change; shell resolution runs against each session's effective environment instead of the base env
  • normalizeShellCommand expands home-relative paths and handles quoted executables; on Windows, shellCandidateFromCommand appends -NoLogo to pwsh/powershell, and resolveShellCandidates resolves bare command names against effective PATH (with PATHEXT) via resolveCommandPath
  • Frontend adds a DraftInput for the setting in GeneralSettingsPanel, includes it in useSettingsRestore reset logic, and registers a search entry in settingsSearch.ts
  • Risk: resolveShellCandidates now requires FileSystem and Path services and performs PATH lookups on Windows before spawning; misconfigured PATH per-session env can change which executable is selected, and bare names previously spawned without PATH resolution

Macroscope summarized 4bb5d7d.

@coderabbitai

coderabbitaiBot commented Aug 11, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 9ab6f837-f5fd-4c94-a380-be7da102498b

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

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

@github-actionsgithub-actionsBot added size:M 30-99 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list. labels Aug 11, 2026
Comment threadapps/server/src/terminal/Manager.ts
@macroscopeapp

macroscopeappBot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This PR adds a user-facing default-shell setting spanning contracts, settings UI, live server configuration, and terminal process spawning, including platform-specific path resolution and fallback behavior. That is a significant runtime capability change rather than a small isolated tweak, so the terminal/settings integration warrants human review.

You can add or adjust custom eligibility rules. Learn more.

@simon-curtis
simon-curtisforce-pushed the terminal/default-shell branch from 5386fd9 to 0c17f61CompareAugust 13, 2026 10:42
Comment threadapps/web/src/components/settings/SettingsPanels.tsx Outdated
@simon-curtis
simon-curtis marked this pull request as draft August 13, 2026 11:15

@CDVolvikCDVolvik 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.

Windows-shaped tests on the PR files against current main: Manager.test.ts + settings.test.ts 89/89.

What I care about here:

  • subscribe-to-settings beforegetSettings (test pins the order)
  • resolveCommandPath + PATHEXT so jz finds jz.EXE
  • quoted "C:\Program Files\…\bash.exe" is unwrapped before spawn
  • session PATH wins over the process PATH when both have the same name

Empty default + trim is in the contract tests. Fallback to built-in PowerShell remains.

Small mismatch: the Electron placeholder is pwsh.exe. #6260 is the other open Windows-shell PR, and it prefers 5.1 (powershell.exe) over optional pwsh. The placeholder will nudge people at the one this repo is trying to stop probing first.

MERGEABLE. I did not open a real terminal session.

@simon-curtis
simon-curtisforce-pushed the terminal/default-shell branch from 0c17f61 to 137463cCompareAugust 17, 2026 17:04
@simon-curtis

simon-curtis commented Aug 17, 2026

Copy link
Copy Markdown
Author

Review update pushed in e31182709:

  • configured shell input now preserves quoted paths everywhere, keeps unquoted Windows paths intact, and extracts the executable from POSIX command lines such as fish -l;
  • restored focused coverage that pins settings subscription before the initial snapshot;
  • retained Windows PATH/PATHEXT, quoted-path, and per-session PATH coverage.

Verification: focused terminal/settings suites 88/88, targeted lint passed, and the t3 typecheck passed.

I also checked related open PRs. #5268 and #6073 are alternative shell-selection implementations; #6260 overlaps the Windows fallback ordering; #6338 and #4827 touch the same terminal-start block. I kept those separate so this PR remains one shell-agnostic concern against current main.

Comment threadapps/server/src/terminal/Manager.ts Outdated
@simon-curtis
simon-curtisforce-pushed the terminal/default-shell branch from cf76f21 to b972c06CompareAugust 17, 2026 17:49
@simon-curtis
simon-curtis marked this pull request as ready for review August 17, 2026 17:52
Comment threadapps/server/src/terminal/Manager.ts Outdated
@simon-curtis
simon-curtisforce-pushed the terminal/default-shell branch from b972c06 to e311827CompareAugust 17, 2026 18:27
Comment threadapps/server/src/terminal/Manager.ts
@simon-curtis
simon-curtisforce-pushed the terminal/default-shell branch 4 times, most recently from 2d9605b to 03ad822CompareAugust 25, 2026 08:47

@cursorcursorBot 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.

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Want fixes drafted automatically? Bugbot Autofix can create code changes for findings. A team admin can enable Autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 03ad822e19b29ed3a68e615a659271e24d0f2fef. Configure here.

Comment threadapps/server/src/terminal/Manager.ts
@simon-curtis
simon-curtisforce-pushed the terminal/default-shell branch 2 times, most recently from 9e38f97 to 8772631CompareAugust 25, 2026 10:28
@github-actionsgithub-actionsBot added size:L 100-499 changed lines (additions + deletions). and removed size:M 30-99 changed lines (additions + deletions). labels Aug 25, 2026
Comment threadapps/server/src/terminal/Manager.ts Outdated
@simon-curtis
simon-curtisforce-pushed the terminal/default-shell branch from 1a8d434 to 4189825CompareAugust 26, 2026 08:27
@simon-curtis
simon-curtisforce-pushed the terminal/default-shell branch from 4189825 to 4bb5d7dCompareAugust 27, 2026 08:29
@t3dotgg

Copy link
Copy Markdown
Member

Note

🤖 GPT-5.6 Sol responding on behalf of Theo

We're closing this PR as we clean up the T3 Code backlog. Thank you for taking the time to put this together.

#5268 is the retained default shell implementation and limits choices to installed shells reported by the host. This branch accepts arbitrary command text and adds a larger cross-platform parsing contract that we do not need.

If you believe we closed this in error, please reopen the PR and leave a comment explaining what we missed.

@t3dotggt3dotgg closed this Aug 28, 2026
@simon-curtis
simon-curtis deleted the terminal/default-shell branch August 28, 2026 12:39
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L100-499 changed lines (additions + deletions).vouch:unvouchedPR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@simon-curtis@t3dotgg@CDVolvik