Filter app runtime env vars from terminal spawn environment - #44

Merged
juliusmarminge merged 1 commit into
mainfrom
codething/2e2908ea
Feb 14, 2026
Merged

Filter app runtime env vars from terminal spawn environment#44
juliusmarminge merged 1 commit into
mainfrom
codething/2e2908ea

Conversation

@juliusmarminge

@juliusmarmingejuliusmarminge commented Feb 14, 2026

Copy link
Copy Markdown
Member

Summary

  • Filter terminal spawn environment variables to exclude app/runtime keys that can interfere with shell sessions.
  • Add explicit exclusion logic for PORT, ELECTRON_RENDERER_PORT, ELECTRON_RUN_AS_NODE, and any keys prefixed with T3CODE_ or VITE_.
  • Keep unrelated environment variables intact when launching terminal sessions.
  • Add a regression test verifying blocked keys are removed and non-blocked keys are preserved.

Testing

  • Not run (not executed in this PR context).
  • Added unit test: apps/server/src/terminalManager.test.ts (filters app runtime env variables from terminal sessions) to verify:
    • PORT, T3CODE_PORT, and VITE_DEV_SERVER_URL are excluded from terminal spawn env.
    • Non-app variable TEST_TERMINAL_KEEP is preserved.

Open with Devin

Summary by CodeRabbit

Release Notes

  • Bug Fixes
    • Improved terminal environment isolation by filtering out framework and application-specific environment variables from terminal sessions. This prevents potential conflicts and enhances security when spawning new terminal instances.

- build terminal spawn env from a filtered copy of process env
- exclude `PORT`, `T3CODE_*`, `VITE_*`, and Electron runtime vars
- add test coverage to verify filtered and preserved env keys
@coderabbitai

coderabbitaiBot commented Feb 14, 2026

Copy link
Copy Markdown

Walkthrough

Implements environment variable filtering for spawned terminal sessions. A blocklist of sensitive variables (PORT, ELECTRON_RENDERER_PORT, ELECTRON_RUN_AS_NODE) and framework-specific prefixes (T3CODE_, VITE_) are excluded from the shell environment. New helper functions sanitize the environment before terminal spawn, with comprehensive test coverage validating the filtering behavior.

Changes

Cohort / File(s)Summary
Terminal Environment Filtering
apps/server/src/terminalManager.ts
Added shouldExcludeTerminalEnvKey and createTerminalSpawnEnv helper functions to filter environment variables. Implemented blocklist for PORT, ELECTRON_RENDERER_PORT, ELECTRON_RUN_AS_NODE and exclusion of variables with T3CODE_ or VITE_ prefixes. Integrated filtered environment into shell spawn within startSession.
Terminal Environment Filtering Tests
apps/server/src/terminalManager.test.ts
New test case verifies that blocked environment variables (PORT, T3CODE_PORT, VITE_DEV_SERVER_URL) are excluded from spawned terminal sessions while permitted variables (TEST_TERMINAL_KEEP) are preserved. Includes environment snapshot/restore helpers.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 2
❌ Failed checks (2 warnings)
Check nameStatusExplanationResolution
Docstring Coverage⚠️ WarningDocstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.Write docstrings for the functions missing them to satisfy the coverage threshold.
Merge Conflict Detection⚠️ Warning❌ Merge conflicts detected (5 files):

⚔️ TODO.md (content)
⚔️ apps/server/src/terminalManager.test.ts (content)
⚔️ apps/server/src/terminalManager.ts (content)
⚔️ apps/web/src/components/ChatView.tsx (content)
⚔️ apps/web/src/components/Sidebar.tsx (content)

These conflicts must be resolved before merging into main.
Resolve conflicts locally and push changes to this branch.
✅ Passed checks (2 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Title check✅ PassedThe title 'Filter app runtime env vars from terminal spawn environment' directly and clearly summarizes the main change: filtering environment variables from terminal spawn environments.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch codething/2e2908ea
⚔️ Resolve merge conflicts (beta)
  • Auto-commit resolved conflicts to branch codething/2e2908ea
  • Create stacked PR with resolved conflicts
  • Post resolved changes as copyable diffs in a comment

No actionable comments were generated in the recent review. 🎉

🧹 Recent nitpick comments
apps/server/src/terminalManager.test.ts (1)

462-465: Consider extending test coverage for remaining blocklist items.

The test validates the filtering pattern well. For completeness, you could optionally add assertions for the other blocklist entries (ELECTRON_RENDERER_PORT, ELECTRON_RUN_AS_NODE) to ensure full coverage of the explicit blocklist.

💡 Optional: Extended assertions
 setEnv("PORT", "5173");
+ setEnv("ELECTRON_RENDERER_PORT", "9000");+ setEnv("ELECTRON_RUN_AS_NODE", "1");
setEnv("T3CODE_PORT", "3773");
setEnv("VITE_DEV_SERVER_URL", "http://localhost:5173");
setEnv("TEST_TERMINAL_KEEP", "keep-me");
try {
const { manager, ptyAdapter } = makeManager();
await manager.open(openInput());
const spawnInput = ptyAdapter.spawnInputs[0];
expect(spawnInput).toBeDefined();
if (!spawnInput) return;
expect(spawnInput.env.PORT).toBeUndefined();
+ expect(spawnInput.env.ELECTRON_RENDERER_PORT).toBeUndefined();+ expect(spawnInput.env.ELECTRON_RUN_AS_NODE).toBeUndefined();
expect(spawnInput.env.T3CODE_PORT).toBeUndefined();

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

@macroscopeapp

Copy link
Copy Markdown
Contributor

Filter terminal spawn environment in TerminalManager.open to exclude PORT, ELECTRON_RENDERER_PORT, ELECTRON_RUN_AS_NODE, and variables starting with T3CODE_ or VITE_ per the PR motivation in terminalManager.ts

Add terminal env filtering via TERMINAL_ENV_BLOCKLIST, shouldExcludeTerminalEnvKey, and createTerminalSpawnEnv, and update TerminalManager.open to pass the sanitized env to ptyAdapter.spawn. A new test verifies exclusion and retention behavior in terminalManager.test.ts.

📍Where to Start

Start with TerminalManager.open in terminalManager.ts and trace into createTerminalSpawnEnv and shouldExcludeTerminalEnvKey.


Macroscope summarized ecea657.

@greptile-apps

Copy link
Copy Markdown

Greptile Overview

Greptile Summary

Adds environment variable filtering to terminal spawn operations to prevent app runtime configuration from interfering with shell sessions.

  • Introduces shouldExcludeTerminalEnvKey function that filters out PORT, ELECTRON_RENDERER_PORT, ELECTRON_RUN_AS_NODE, and variables prefixed with T3CODE_ or VITE_
  • Adds createTerminalSpawnEnv helper that creates a clean environment by excluding filtered keys
  • Modified startSession in apps/server/src/terminalManager.ts:496 to use filtered environment instead of process.env directly
  • Includes regression test with proper environment restoration to verify filtering behavior

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk
  • Clean implementation with focused scope, comprehensive test coverage, and no breaking changes. The filtering logic is straightforward and addresses a specific issue without affecting existing functionality.
  • No files require special attention

Important Files Changed

FilenameOverview
apps/server/src/terminalManager.tsAdded environment variable filtering logic to prevent app runtime variables from leaking into terminal sessions. Implementation is clean and well-tested.
apps/server/src/terminalManager.test.tsAdded comprehensive test verifying that blocked environment variables are excluded while non-app variables are preserved during terminal spawn.

Flowchart

flowchart TD
A[startSession called] --> B[createTerminalSpawnEnv called with process.env]
B --> C{For each env key/value}
C --> D{value === undefined?}
D -->|Yes| E[Skip]
D -->|No| F{shouldExcludeTerminalEnvKey}
F --> G{Starts with T3CODE_?}
G -->|Yes| E
G -->|No| H{Starts with VITE_?}
H -->|Yes| E
H -->|No| I{In TERMINAL_ENV_BLOCKLIST?}
I -->|Yes - PORT, ELECTRON_RENDERER_PORT, ELECTRON_RUN_AS_NODE| E
I -->|No| J[Include in spawnEnv]
E --> K{More keys?}
J --> K
K -->|Yes| C
K -->|No| L[Return filtered spawnEnv]
L --> M[ptyAdapter.spawn with filtered env]
Loading

Last reviewed commit: ecea657

@juliusmarminge
juliusmarminge merged commit 4b4abcd into mainFeb 14, 2026
4 checks passed
DavidIlie added a commit to DavidIlie/t3code that referenced this pull request Mar 13, 2026
…ker gating
Port upstream commits 9bb9023..b36888e:
- Handle branch selection across main and secondary worktrees (pingdotgg#44)
- Preserve fork PR upstreams when preparing local and worktree threads (pingdotgg#45)
Adds resolveBranchSelectionTarget for unified checkout cwd/worktree decisions,
GitCore helpers for remote management (ensureRemote, fetchRemoteBranch,
setBranchUpstream), GitHub CLI cross-repo PR metadata parsing, and
GitManager fork head materialization with upstream tracking.
aorwall added a commit to aorwall/t3code that referenced this pull request Aug 6, 2026
* Revert "feat: add a split-screen-horizontally toggle for the right panel (pingdotgg#41)"
This reverts commit 0ea9e96. The ask was to remove a horizontal split,
not to add one, so the toggle should not have shipped.
Every file pingdotgg#41 touched is back to its pre-pingdotgg#41 content and its three new
files are gone: rightPanelOrientation.ts, useResizablePanelHeight.ts and
its test. `git diff fd3b851 -- apps packages` is empty. The fork delta
here is nil again, so the Split Screen Delta section, its inventory row,
its path policy rows and its convergence row go with it.
Kept from pingdotgg#41, because they stand on their own and are not about the
toggle: the upstream-remote ownership check in Path policy, the .plans/**
row split it turned up, the messageOrigin.ts verification, and AGENTS.md's
rule that fork code says so in a comment where it sits.
Verification: web typecheck and lint clean, vp fmt --check clean, 83 tests
across apps/web/src/hooks and apps/web/src/components/preview.
Model: Claude Opus 5. Harness: Claude Code.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(web): drop the terminal-drawer toggle from the chat header
The chat's panel layout controls carry two buttons: one opens the right
panel, the other splits the screen horizontally by opening a terminal
across the bottom. The fork keeps one way to reach a terminal — the right
panel's terminal surface — so the second button is a split nobody asked
for beside a panel that already does the job.
Hides it the fork way rather than deleting upstream's code: a new
`terminalDrawerToggle` flag in `apps/web/src/fork/features.ts` and one
gate expression in `PanelLayoutControls.tsx`. Upstream's props, the
drawer and the `terminal.toggle` keybinding are untouched, so the drawer
still opens from the keyboard and deleting the flag brings the button
back.
Model: Claude Opus 5. Harness: Claude Code.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* docs(fork): the log keeps what is known, not what was undone
The 2026-08-03 entry led with a toggle that was added and then reverted
in the same day. Nothing in the tree carries it, so a future merge learns
nothing from reading about it. What that day actually produced is the
upstream-remote ownership check and what it found, which is what the
entry says now.
Model: Claude Opus 5. Harness: Claude Code.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
aorwall added a commit to aorwall/t3code that referenced this pull request Aug 29, 2026
The chat's upper-right corner lost the terminal-drawer button in pingdotgg#44, and
the right panel's sandbox pill had taken up residence at the right end of
the tab bar beside it — the same strip upstream reserves for its layout
toggles. Getting the button back is one half; the other is not putting it
next to a fork element that was crowding it.
The gate goes rather than flips, the way features.ts says to turn a flag
on, so PanelLayoutControls.tsx is upstream's byte for byte again and the
corner carries the terminal drawer, the right-panel toggle and maximize.
The sandbox pill moves into the panel body, where the surfaces it governs
are opened: under the launcher's Browser/Terminal/Files cards, and under
the disabled state's reason, where it stops being a status somewhere else
and becomes the Start button next to the explanation. Compact there, since
the heading has already said what is wrong. A ready sandbox's dot is green
now — amber beside "Sandbox running" was survivable in the tab bar and
would have been the loudest thing in the launcher.
Trade-off, recorded in the merge log rather than solved: with the pill in
the body, Stop is out of reach while a surface is open and the sandbox is
running. Losing a sandbox disables the surfaces and brings the control
back with the reason, so the way in is never the one that goes missing.
A fork-only test pins the placement, since it is a position rather than a
symbol and a merge could carry the hunk back into the tab bar without a
type error.
Model: Claude Opus 5. Harness: Claude Code.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@juliusmarminge
, '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

Filter app runtime env vars from terminal spawn environment - #44

Merged
juliusmarminge merged 1 commit into
mainfrom
codething/2e2908ea
Feb 14, 2026
Merged

Filter app runtime env vars from terminal spawn environment#44
juliusmarminge merged 1 commit into
mainfrom
codething/2e2908ea

Conversation

@juliusmarminge

@juliusmarmingejuliusmarminge commented Feb 14, 2026

Copy link
Copy Markdown
Member

Summary

  • Filter terminal spawn environment variables to exclude app/runtime keys that can interfere with shell sessions.
  • Add explicit exclusion logic for PORT, ELECTRON_RENDERER_PORT, ELECTRON_RUN_AS_NODE, and any keys prefixed with T3CODE_ or VITE_.
  • Keep unrelated environment variables intact when launching terminal sessions.
  • Add a regression test verifying blocked keys are removed and non-blocked keys are preserved.

Testing

  • Not run (not executed in this PR context).
  • Added unit test: apps/server/src/terminalManager.test.ts (filters app runtime env variables from terminal sessions) to verify:
    • PORT, T3CODE_PORT, and VITE_DEV_SERVER_URL are excluded from terminal spawn env.
    • Non-app variable TEST_TERMINAL_KEEP is preserved.

Open with Devin

Summary by CodeRabbit

Release Notes

  • Bug Fixes
    • Improved terminal environment isolation by filtering out framework and application-specific environment variables from terminal sessions. This prevents potential conflicts and enhances security when spawning new terminal instances.

- build terminal spawn env from a filtered copy of process env
- exclude `PORT`, `T3CODE_*`, `VITE_*`, and Electron runtime vars
- add test coverage to verify filtered and preserved env keys
@coderabbitai

coderabbitaiBot commented Feb 14, 2026

Copy link
Copy Markdown

Walkthrough

Implements environment variable filtering for spawned terminal sessions. A blocklist of sensitive variables (PORT, ELECTRON_RENDERER_PORT, ELECTRON_RUN_AS_NODE) and framework-specific prefixes (T3CODE_, VITE_) are excluded from the shell environment. New helper functions sanitize the environment before terminal spawn, with comprehensive test coverage validating the filtering behavior.

Changes

Cohort / File(s)Summary
Terminal Environment Filtering
apps/server/src/terminalManager.ts
Added shouldExcludeTerminalEnvKey and createTerminalSpawnEnv helper functions to filter environment variables. Implemented blocklist for PORT, ELECTRON_RENDERER_PORT, ELECTRON_RUN_AS_NODE and exclusion of variables with T3CODE_ or VITE_ prefixes. Integrated filtered environment into shell spawn within startSession.
Terminal Environment Filtering Tests
apps/server/src/terminalManager.test.ts
New test case verifies that blocked environment variables (PORT, T3CODE_PORT, VITE_DEV_SERVER_URL) are excluded from spawned terminal sessions while permitted variables (TEST_TERMINAL_KEEP) are preserved. Includes environment snapshot/restore helpers.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 2
❌ Failed checks (2 warnings)
Check nameStatusExplanationResolution
Docstring Coverage⚠️ WarningDocstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.Write docstrings for the functions missing them to satisfy the coverage threshold.
Merge Conflict Detection⚠️ Warning❌ Merge conflicts detected (5 files):

⚔️ TODO.md (content)
⚔️ apps/server/src/terminalManager.test.ts (content)
⚔️ apps/server/src/terminalManager.ts (content)
⚔️ apps/web/src/components/ChatView.tsx (content)
⚔️ apps/web/src/components/Sidebar.tsx (content)

These conflicts must be resolved before merging into main.
Resolve conflicts locally and push changes to this branch.
✅ Passed checks (2 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Title check✅ PassedThe title 'Filter app runtime env vars from terminal spawn environment' directly and clearly summarizes the main change: filtering environment variables from terminal spawn environments.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch codething/2e2908ea
⚔️ Resolve merge conflicts (beta)
  • Auto-commit resolved conflicts to branch codething/2e2908ea
  • Create stacked PR with resolved conflicts
  • Post resolved changes as copyable diffs in a comment

No actionable comments were generated in the recent review. 🎉

🧹 Recent nitpick comments
apps/server/src/terminalManager.test.ts (1)

462-465: Consider extending test coverage for remaining blocklist items.

The test validates the filtering pattern well. For completeness, you could optionally add assertions for the other blocklist entries (ELECTRON_RENDERER_PORT, ELECTRON_RUN_AS_NODE) to ensure full coverage of the explicit blocklist.

💡 Optional: Extended assertions
 setEnv("PORT", "5173");
+ setEnv("ELECTRON_RENDERER_PORT", "9000");+ setEnv("ELECTRON_RUN_AS_NODE", "1");
setEnv("T3CODE_PORT", "3773");
setEnv("VITE_DEV_SERVER_URL", "http://localhost:5173");
setEnv("TEST_TERMINAL_KEEP", "keep-me");
try {
const { manager, ptyAdapter } = makeManager();
await manager.open(openInput());
const spawnInput = ptyAdapter.spawnInputs[0];
expect(spawnInput).toBeDefined();
if (!spawnInput) return;
expect(spawnInput.env.PORT).toBeUndefined();
+ expect(spawnInput.env.ELECTRON_RENDERER_PORT).toBeUndefined();+ expect(spawnInput.env.ELECTRON_RUN_AS_NODE).toBeUndefined();
expect(spawnInput.env.T3CODE_PORT).toBeUndefined();

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

@macroscopeapp

Copy link
Copy Markdown
Contributor

Filter terminal spawn environment in TerminalManager.open to exclude PORT, ELECTRON_RENDERER_PORT, ELECTRON_RUN_AS_NODE, and variables starting with T3CODE_ or VITE_ per the PR motivation in terminalManager.ts

Add terminal env filtering via TERMINAL_ENV_BLOCKLIST, shouldExcludeTerminalEnvKey, and createTerminalSpawnEnv, and update TerminalManager.open to pass the sanitized env to ptyAdapter.spawn. A new test verifies exclusion and retention behavior in terminalManager.test.ts.

📍Where to Start

Start with TerminalManager.open in terminalManager.ts and trace into createTerminalSpawnEnv and shouldExcludeTerminalEnvKey.


Macroscope summarized ecea657.

@greptile-apps

Copy link
Copy Markdown

Greptile Overview

Greptile Summary

Adds environment variable filtering to terminal spawn operations to prevent app runtime configuration from interfering with shell sessions.

  • Introduces shouldExcludeTerminalEnvKey function that filters out PORT, ELECTRON_RENDERER_PORT, ELECTRON_RUN_AS_NODE, and variables prefixed with T3CODE_ or VITE_
  • Adds createTerminalSpawnEnv helper that creates a clean environment by excluding filtered keys
  • Modified startSession in apps/server/src/terminalManager.ts:496 to use filtered environment instead of process.env directly
  • Includes regression test with proper environment restoration to verify filtering behavior

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk
  • Clean implementation with focused scope, comprehensive test coverage, and no breaking changes. The filtering logic is straightforward and addresses a specific issue without affecting existing functionality.
  • No files require special attention

Important Files Changed

FilenameOverview
apps/server/src/terminalManager.tsAdded environment variable filtering logic to prevent app runtime variables from leaking into terminal sessions. Implementation is clean and well-tested.
apps/server/src/terminalManager.test.tsAdded comprehensive test verifying that blocked environment variables are excluded while non-app variables are preserved during terminal spawn.

Flowchart

flowchart TD
A[startSession called] --> B[createTerminalSpawnEnv called with process.env]
B --> C{For each env key/value}
C --> D{value === undefined?}
D -->|Yes| E[Skip]
D -->|No| F{shouldExcludeTerminalEnvKey}
F --> G{Starts with T3CODE_?}
G -->|Yes| E
G -->|No| H{Starts with VITE_?}
H -->|Yes| E
H -->|No| I{In TERMINAL_ENV_BLOCKLIST?}
I -->|Yes - PORT, ELECTRON_RENDERER_PORT, ELECTRON_RUN_AS_NODE| E
I -->|No| J[Include in spawnEnv]
E --> K{More keys?}
J --> K
K -->|Yes| C
K -->|No| L[Return filtered spawnEnv]
L --> M[ptyAdapter.spawn with filtered env]
Loading

Last reviewed commit: ecea657

@juliusmarminge
juliusmarminge merged commit 4b4abcd into mainFeb 14, 2026
4 checks passed
DavidIlie added a commit to DavidIlie/t3code that referenced this pull request Mar 13, 2026
…ker gating
Port upstream commits 9bb9023..b36888e:
- Handle branch selection across main and secondary worktrees (pingdotgg#44)
- Preserve fork PR upstreams when preparing local and worktree threads (pingdotgg#45)
Adds resolveBranchSelectionTarget for unified checkout cwd/worktree decisions,
GitCore helpers for remote management (ensureRemote, fetchRemoteBranch,
setBranchUpstream), GitHub CLI cross-repo PR metadata parsing, and
GitManager fork head materialization with upstream tracking.
aorwall added a commit to aorwall/t3code that referenced this pull request Aug 6, 2026
* Revert "feat: add a split-screen-horizontally toggle for the right panel (pingdotgg#41)"
This reverts commit 0ea9e96. The ask was to remove a horizontal split,
not to add one, so the toggle should not have shipped.
Every file pingdotgg#41 touched is back to its pre-pingdotgg#41 content and its three new
files are gone: rightPanelOrientation.ts, useResizablePanelHeight.ts and
its test. `git diff fd3b851 -- apps packages` is empty. The fork delta
here is nil again, so the Split Screen Delta section, its inventory row,
its path policy rows and its convergence row go with it.
Kept from pingdotgg#41, because they stand on their own and are not about the
toggle: the upstream-remote ownership check in Path policy, the .plans/**
row split it turned up, the messageOrigin.ts verification, and AGENTS.md's
rule that fork code says so in a comment where it sits.
Verification: web typecheck and lint clean, vp fmt --check clean, 83 tests
across apps/web/src/hooks and apps/web/src/components/preview.
Model: Claude Opus 5. Harness: Claude Code.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(web): drop the terminal-drawer toggle from the chat header
The chat's panel layout controls carry two buttons: one opens the right
panel, the other splits the screen horizontally by opening a terminal
across the bottom. The fork keeps one way to reach a terminal — the right
panel's terminal surface — so the second button is a split nobody asked
for beside a panel that already does the job.
Hides it the fork way rather than deleting upstream's code: a new
`terminalDrawerToggle` flag in `apps/web/src/fork/features.ts` and one
gate expression in `PanelLayoutControls.tsx`. Upstream's props, the
drawer and the `terminal.toggle` keybinding are untouched, so the drawer
still opens from the keyboard and deleting the flag brings the button
back.
Model: Claude Opus 5. Harness: Claude Code.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* docs(fork): the log keeps what is known, not what was undone
The 2026-08-03 entry led with a toggle that was added and then reverted
in the same day. Nothing in the tree carries it, so a future merge learns
nothing from reading about it. What that day actually produced is the
upstream-remote ownership check and what it found, which is what the
entry says now.
Model: Claude Opus 5. Harness: Claude Code.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
aorwall added a commit to aorwall/t3code that referenced this pull request Aug 29, 2026
The chat's upper-right corner lost the terminal-drawer button in pingdotgg#44, and
the right panel's sandbox pill had taken up residence at the right end of
the tab bar beside it — the same strip upstream reserves for its layout
toggles. Getting the button back is one half; the other is not putting it
next to a fork element that was crowding it.
The gate goes rather than flips, the way features.ts says to turn a flag
on, so PanelLayoutControls.tsx is upstream's byte for byte again and the
corner carries the terminal drawer, the right-panel toggle and maximize.
The sandbox pill moves into the panel body, where the surfaces it governs
are opened: under the launcher's Browser/Terminal/Files cards, and under
the disabled state's reason, where it stops being a status somewhere else
and becomes the Start button next to the explanation. Compact there, since
the heading has already said what is wrong. A ready sandbox's dot is green
now — amber beside "Sandbox running" was survivable in the tab bar and
would have been the loudest thing in the launcher.
Trade-off, recorded in the merge log rather than solved: with the pill in
the body, Stop is out of reach while a surface is open and the sandbox is
running. Losing a sandbox disables the surfaces and brings the control
back with the reason, so the way in is never the one that goes missing.
A fork-only test pins the placement, since it is a position rather than a
symbol and a merge could carry the hunk back into the tab bar without a
type error.
Model: Claude Opus 5. Harness: Claude Code.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@juliusmarminge
, '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

Filter app runtime env vars from terminal spawn environment - #44

Merged
juliusmarminge merged 1 commit into
mainfrom
codething/2e2908ea
Feb 14, 2026
Merged

Filter app runtime env vars from terminal spawn environment#44
juliusmarminge merged 1 commit into
mainfrom
codething/2e2908ea

Conversation

@juliusmarminge

@juliusmarmingejuliusmarminge commented Feb 14, 2026

Copy link
Copy Markdown
Member

Summary

  • Filter terminal spawn environment variables to exclude app/runtime keys that can interfere with shell sessions.
  • Add explicit exclusion logic for PORT, ELECTRON_RENDERER_PORT, ELECTRON_RUN_AS_NODE, and any keys prefixed with T3CODE_ or VITE_.
  • Keep unrelated environment variables intact when launching terminal sessions.
  • Add a regression test verifying blocked keys are removed and non-blocked keys are preserved.

Testing

  • Not run (not executed in this PR context).
  • Added unit test: apps/server/src/terminalManager.test.ts (filters app runtime env variables from terminal sessions) to verify:
    • PORT, T3CODE_PORT, and VITE_DEV_SERVER_URL are excluded from terminal spawn env.
    • Non-app variable TEST_TERMINAL_KEEP is preserved.

Open with Devin

Summary by CodeRabbit

Release Notes

  • Bug Fixes
    • Improved terminal environment isolation by filtering out framework and application-specific environment variables from terminal sessions. This prevents potential conflicts and enhances security when spawning new terminal instances.

- build terminal spawn env from a filtered copy of process env
- exclude `PORT`, `T3CODE_*`, `VITE_*`, and Electron runtime vars
- add test coverage to verify filtered and preserved env keys
@coderabbitai

coderabbitaiBot commented Feb 14, 2026

Copy link
Copy Markdown

Walkthrough

Implements environment variable filtering for spawned terminal sessions. A blocklist of sensitive variables (PORT, ELECTRON_RENDERER_PORT, ELECTRON_RUN_AS_NODE) and framework-specific prefixes (T3CODE_, VITE_) are excluded from the shell environment. New helper functions sanitize the environment before terminal spawn, with comprehensive test coverage validating the filtering behavior.

Changes

Cohort / File(s)Summary
Terminal Environment Filtering
apps/server/src/terminalManager.ts
Added shouldExcludeTerminalEnvKey and createTerminalSpawnEnv helper functions to filter environment variables. Implemented blocklist for PORT, ELECTRON_RENDERER_PORT, ELECTRON_RUN_AS_NODE and exclusion of variables with T3CODE_ or VITE_ prefixes. Integrated filtered environment into shell spawn within startSession.
Terminal Environment Filtering Tests
apps/server/src/terminalManager.test.ts
New test case verifies that blocked environment variables (PORT, T3CODE_PORT, VITE_DEV_SERVER_URL) are excluded from spawned terminal sessions while permitted variables (TEST_TERMINAL_KEEP) are preserved. Includes environment snapshot/restore helpers.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 2
❌ Failed checks (2 warnings)
Check nameStatusExplanationResolution
Docstring Coverage⚠️ WarningDocstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.Write docstrings for the functions missing them to satisfy the coverage threshold.
Merge Conflict Detection⚠️ Warning❌ Merge conflicts detected (5 files):

⚔️ TODO.md (content)
⚔️ apps/server/src/terminalManager.test.ts (content)
⚔️ apps/server/src/terminalManager.ts (content)
⚔️ apps/web/src/components/ChatView.tsx (content)
⚔️ apps/web/src/components/Sidebar.tsx (content)

These conflicts must be resolved before merging into main.
Resolve conflicts locally and push changes to this branch.
✅ Passed checks (2 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Title check✅ PassedThe title 'Filter app runtime env vars from terminal spawn environment' directly and clearly summarizes the main change: filtering environment variables from terminal spawn environments.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch codething/2e2908ea
⚔️ Resolve merge conflicts (beta)
  • Auto-commit resolved conflicts to branch codething/2e2908ea
  • Create stacked PR with resolved conflicts
  • Post resolved changes as copyable diffs in a comment

No actionable comments were generated in the recent review. 🎉

🧹 Recent nitpick comments
apps/server/src/terminalManager.test.ts (1)

462-465: Consider extending test coverage for remaining blocklist items.

The test validates the filtering pattern well. For completeness, you could optionally add assertions for the other blocklist entries (ELECTRON_RENDERER_PORT, ELECTRON_RUN_AS_NODE) to ensure full coverage of the explicit blocklist.

💡 Optional: Extended assertions
 setEnv("PORT", "5173");
+ setEnv("ELECTRON_RENDERER_PORT", "9000");+ setEnv("ELECTRON_RUN_AS_NODE", "1");
setEnv("T3CODE_PORT", "3773");
setEnv("VITE_DEV_SERVER_URL", "http://localhost:5173");
setEnv("TEST_TERMINAL_KEEP", "keep-me");
try {
const { manager, ptyAdapter } = makeManager();
await manager.open(openInput());
const spawnInput = ptyAdapter.spawnInputs[0];
expect(spawnInput).toBeDefined();
if (!spawnInput) return;
expect(spawnInput.env.PORT).toBeUndefined();
+ expect(spawnInput.env.ELECTRON_RENDERER_PORT).toBeUndefined();+ expect(spawnInput.env.ELECTRON_RUN_AS_NODE).toBeUndefined();
expect(spawnInput.env.T3CODE_PORT).toBeUndefined();

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

@macroscopeapp

Copy link
Copy Markdown
Contributor

Filter terminal spawn environment in TerminalManager.open to exclude PORT, ELECTRON_RENDERER_PORT, ELECTRON_RUN_AS_NODE, and variables starting with T3CODE_ or VITE_ per the PR motivation in terminalManager.ts

Add terminal env filtering via TERMINAL_ENV_BLOCKLIST, shouldExcludeTerminalEnvKey, and createTerminalSpawnEnv, and update TerminalManager.open to pass the sanitized env to ptyAdapter.spawn. A new test verifies exclusion and retention behavior in terminalManager.test.ts.

📍Where to Start

Start with TerminalManager.open in terminalManager.ts and trace into createTerminalSpawnEnv and shouldExcludeTerminalEnvKey.


Macroscope summarized ecea657.

@greptile-apps

Copy link
Copy Markdown

Greptile Overview

Greptile Summary

Adds environment variable filtering to terminal spawn operations to prevent app runtime configuration from interfering with shell sessions.

  • Introduces shouldExcludeTerminalEnvKey function that filters out PORT, ELECTRON_RENDERER_PORT, ELECTRON_RUN_AS_NODE, and variables prefixed with T3CODE_ or VITE_
  • Adds createTerminalSpawnEnv helper that creates a clean environment by excluding filtered keys
  • Modified startSession in apps/server/src/terminalManager.ts:496 to use filtered environment instead of process.env directly
  • Includes regression test with proper environment restoration to verify filtering behavior

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk
  • Clean implementation with focused scope, comprehensive test coverage, and no breaking changes. The filtering logic is straightforward and addresses a specific issue without affecting existing functionality.
  • No files require special attention

Important Files Changed

FilenameOverview
apps/server/src/terminalManager.tsAdded environment variable filtering logic to prevent app runtime variables from leaking into terminal sessions. Implementation is clean and well-tested.
apps/server/src/terminalManager.test.tsAdded comprehensive test verifying that blocked environment variables are excluded while non-app variables are preserved during terminal spawn.

Flowchart

flowchart TD
A[startSession called] --> B[createTerminalSpawnEnv called with process.env]
B --> C{For each env key/value}
C --> D{value === undefined?}
D -->|Yes| E[Skip]
D -->|No| F{shouldExcludeTerminalEnvKey}
F --> G{Starts with T3CODE_?}
G -->|Yes| E
G -->|No| H{Starts with VITE_?}
H -->|Yes| E
H -->|No| I{In TERMINAL_ENV_BLOCKLIST?}
I -->|Yes - PORT, ELECTRON_RENDERER_PORT, ELECTRON_RUN_AS_NODE| E
I -->|No| J[Include in spawnEnv]
E --> K{More keys?}
J --> K
K -->|Yes| C
K -->|No| L[Return filtered spawnEnv]
L --> M[ptyAdapter.spawn with filtered env]
Loading

Last reviewed commit: ecea657

@juliusmarminge
juliusmarminge merged commit 4b4abcd into mainFeb 14, 2026
4 checks passed
DavidIlie added a commit to DavidIlie/t3code that referenced this pull request Mar 13, 2026
…ker gating
Port upstream commits 9bb9023..b36888e:
- Handle branch selection across main and secondary worktrees (pingdotgg#44)
- Preserve fork PR upstreams when preparing local and worktree threads (pingdotgg#45)
Adds resolveBranchSelectionTarget for unified checkout cwd/worktree decisions,
GitCore helpers for remote management (ensureRemote, fetchRemoteBranch,
setBranchUpstream), GitHub CLI cross-repo PR metadata parsing, and
GitManager fork head materialization with upstream tracking.
aorwall added a commit to aorwall/t3code that referenced this pull request Aug 6, 2026
* Revert "feat: add a split-screen-horizontally toggle for the right panel (pingdotgg#41)"
This reverts commit 0ea9e96. The ask was to remove a horizontal split,
not to add one, so the toggle should not have shipped.
Every file pingdotgg#41 touched is back to its pre-pingdotgg#41 content and its three new
files are gone: rightPanelOrientation.ts, useResizablePanelHeight.ts and
its test. `git diff fd3b851 -- apps packages` is empty. The fork delta
here is nil again, so the Split Screen Delta section, its inventory row,
its path policy rows and its convergence row go with it.
Kept from pingdotgg#41, because they stand on their own and are not about the
toggle: the upstream-remote ownership check in Path policy, the .plans/**
row split it turned up, the messageOrigin.ts verification, and AGENTS.md's
rule that fork code says so in a comment where it sits.
Verification: web typecheck and lint clean, vp fmt --check clean, 83 tests
across apps/web/src/hooks and apps/web/src/components/preview.
Model: Claude Opus 5. Harness: Claude Code.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(web): drop the terminal-drawer toggle from the chat header
The chat's panel layout controls carry two buttons: one opens the right
panel, the other splits the screen horizontally by opening a terminal
across the bottom. The fork keeps one way to reach a terminal — the right
panel's terminal surface — so the second button is a split nobody asked
for beside a panel that already does the job.
Hides it the fork way rather than deleting upstream's code: a new
`terminalDrawerToggle` flag in `apps/web/src/fork/features.ts` and one
gate expression in `PanelLayoutControls.tsx`. Upstream's props, the
drawer and the `terminal.toggle` keybinding are untouched, so the drawer
still opens from the keyboard and deleting the flag brings the button
back.
Model: Claude Opus 5. Harness: Claude Code.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* docs(fork): the log keeps what is known, not what was undone
The 2026-08-03 entry led with a toggle that was added and then reverted
in the same day. Nothing in the tree carries it, so a future merge learns
nothing from reading about it. What that day actually produced is the
upstream-remote ownership check and what it found, which is what the
entry says now.
Model: Claude Opus 5. Harness: Claude Code.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
aorwall added a commit to aorwall/t3code that referenced this pull request Aug 29, 2026
The chat's upper-right corner lost the terminal-drawer button in pingdotgg#44, and
the right panel's sandbox pill had taken up residence at the right end of
the tab bar beside it — the same strip upstream reserves for its layout
toggles. Getting the button back is one half; the other is not putting it
next to a fork element that was crowding it.
The gate goes rather than flips, the way features.ts says to turn a flag
on, so PanelLayoutControls.tsx is upstream's byte for byte again and the
corner carries the terminal drawer, the right-panel toggle and maximize.
The sandbox pill moves into the panel body, where the surfaces it governs
are opened: under the launcher's Browser/Terminal/Files cards, and under
the disabled state's reason, where it stops being a status somewhere else
and becomes the Start button next to the explanation. Compact there, since
the heading has already said what is wrong. A ready sandbox's dot is green
now — amber beside "Sandbox running" was survivable in the tab bar and
would have been the loudest thing in the launcher.
Trade-off, recorded in the merge log rather than solved: with the pill in
the body, Stop is out of reach while a surface is open and the sandbox is
running. Losing a sandbox disables the surfaces and brings the control
back with the reason, so the way in is never the one that goes missing.
A fork-only test pins the placement, since it is a position rather than a
symbol and a merge could carry the hunk back into the tab bar without a
type error.
Model: Claude Opus 5. Harness: Claude Code.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@juliusmarminge
, '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

Filter app runtime env vars from terminal spawn environment - #44

Merged
juliusmarminge merged 1 commit into
mainfrom
codething/2e2908ea
Feb 14, 2026
Merged

Filter app runtime env vars from terminal spawn environment#44
juliusmarminge merged 1 commit into
mainfrom
codething/2e2908ea

Conversation

@juliusmarminge

@juliusmarmingejuliusmarminge commented Feb 14, 2026

Copy link
Copy Markdown
Member

Summary

  • Filter terminal spawn environment variables to exclude app/runtime keys that can interfere with shell sessions.
  • Add explicit exclusion logic for PORT, ELECTRON_RENDERER_PORT, ELECTRON_RUN_AS_NODE, and any keys prefixed with T3CODE_ or VITE_.
  • Keep unrelated environment variables intact when launching terminal sessions.
  • Add a regression test verifying blocked keys are removed and non-blocked keys are preserved.

Testing

  • Not run (not executed in this PR context).
  • Added unit test: apps/server/src/terminalManager.test.ts (filters app runtime env variables from terminal sessions) to verify:
    • PORT, T3CODE_PORT, and VITE_DEV_SERVER_URL are excluded from terminal spawn env.
    • Non-app variable TEST_TERMINAL_KEEP is preserved.

Open with Devin

Summary by CodeRabbit

Release Notes

  • Bug Fixes
    • Improved terminal environment isolation by filtering out framework and application-specific environment variables from terminal sessions. This prevents potential conflicts and enhances security when spawning new terminal instances.

- build terminal spawn env from a filtered copy of process env
- exclude `PORT`, `T3CODE_*`, `VITE_*`, and Electron runtime vars
- add test coverage to verify filtered and preserved env keys
@coderabbitai

coderabbitaiBot commented Feb 14, 2026

Copy link
Copy Markdown

Walkthrough

Implements environment variable filtering for spawned terminal sessions. A blocklist of sensitive variables (PORT, ELECTRON_RENDERER_PORT, ELECTRON_RUN_AS_NODE) and framework-specific prefixes (T3CODE_, VITE_) are excluded from the shell environment. New helper functions sanitize the environment before terminal spawn, with comprehensive test coverage validating the filtering behavior.

Changes

Cohort / File(s)Summary
Terminal Environment Filtering
apps/server/src/terminalManager.ts
Added shouldExcludeTerminalEnvKey and createTerminalSpawnEnv helper functions to filter environment variables. Implemented blocklist for PORT, ELECTRON_RENDERER_PORT, ELECTRON_RUN_AS_NODE and exclusion of variables with T3CODE_ or VITE_ prefixes. Integrated filtered environment into shell spawn within startSession.
Terminal Environment Filtering Tests
apps/server/src/terminalManager.test.ts
New test case verifies that blocked environment variables (PORT, T3CODE_PORT, VITE_DEV_SERVER_URL) are excluded from spawned terminal sessions while permitted variables (TEST_TERMINAL_KEEP) are preserved. Includes environment snapshot/restore helpers.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 2
❌ Failed checks (2 warnings)
Check nameStatusExplanationResolution
Docstring Coverage⚠️ WarningDocstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.Write docstrings for the functions missing them to satisfy the coverage threshold.
Merge Conflict Detection⚠️ Warning❌ Merge conflicts detected (5 files):

⚔️ TODO.md (content)
⚔️ apps/server/src/terminalManager.test.ts (content)
⚔️ apps/server/src/terminalManager.ts (content)
⚔️ apps/web/src/components/ChatView.tsx (content)
⚔️ apps/web/src/components/Sidebar.tsx (content)

These conflicts must be resolved before merging into main.
Resolve conflicts locally and push changes to this branch.
✅ Passed checks (2 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Title check✅ PassedThe title 'Filter app runtime env vars from terminal spawn environment' directly and clearly summarizes the main change: filtering environment variables from terminal spawn environments.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch codething/2e2908ea
⚔️ Resolve merge conflicts (beta)
  • Auto-commit resolved conflicts to branch codething/2e2908ea
  • Create stacked PR with resolved conflicts
  • Post resolved changes as copyable diffs in a comment

No actionable comments were generated in the recent review. 🎉

🧹 Recent nitpick comments
apps/server/src/terminalManager.test.ts (1)

462-465: Consider extending test coverage for remaining blocklist items.

The test validates the filtering pattern well. For completeness, you could optionally add assertions for the other blocklist entries (ELECTRON_RENDERER_PORT, ELECTRON_RUN_AS_NODE) to ensure full coverage of the explicit blocklist.

💡 Optional: Extended assertions
 setEnv("PORT", "5173");
+ setEnv("ELECTRON_RENDERER_PORT", "9000");+ setEnv("ELECTRON_RUN_AS_NODE", "1");
setEnv("T3CODE_PORT", "3773");
setEnv("VITE_DEV_SERVER_URL", "http://localhost:5173");
setEnv("TEST_TERMINAL_KEEP", "keep-me");
try {
const { manager, ptyAdapter } = makeManager();
await manager.open(openInput());
const spawnInput = ptyAdapter.spawnInputs[0];
expect(spawnInput).toBeDefined();
if (!spawnInput) return;
expect(spawnInput.env.PORT).toBeUndefined();
+ expect(spawnInput.env.ELECTRON_RENDERER_PORT).toBeUndefined();+ expect(spawnInput.env.ELECTRON_RUN_AS_NODE).toBeUndefined();
expect(spawnInput.env.T3CODE_PORT).toBeUndefined();

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

@macroscopeapp

Copy link
Copy Markdown
Contributor

Filter terminal spawn environment in TerminalManager.open to exclude PORT, ELECTRON_RENDERER_PORT, ELECTRON_RUN_AS_NODE, and variables starting with T3CODE_ or VITE_ per the PR motivation in terminalManager.ts

Add terminal env filtering via TERMINAL_ENV_BLOCKLIST, shouldExcludeTerminalEnvKey, and createTerminalSpawnEnv, and update TerminalManager.open to pass the sanitized env to ptyAdapter.spawn. A new test verifies exclusion and retention behavior in terminalManager.test.ts.

📍Where to Start

Start with TerminalManager.open in terminalManager.ts and trace into createTerminalSpawnEnv and shouldExcludeTerminalEnvKey.


Macroscope summarized ecea657.

@greptile-apps

Copy link
Copy Markdown

Greptile Overview

Greptile Summary

Adds environment variable filtering to terminal spawn operations to prevent app runtime configuration from interfering with shell sessions.

  • Introduces shouldExcludeTerminalEnvKey function that filters out PORT, ELECTRON_RENDERER_PORT, ELECTRON_RUN_AS_NODE, and variables prefixed with T3CODE_ or VITE_
  • Adds createTerminalSpawnEnv helper that creates a clean environment by excluding filtered keys
  • Modified startSession in apps/server/src/terminalManager.ts:496 to use filtered environment instead of process.env directly
  • Includes regression test with proper environment restoration to verify filtering behavior

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk
  • Clean implementation with focused scope, comprehensive test coverage, and no breaking changes. The filtering logic is straightforward and addresses a specific issue without affecting existing functionality.
  • No files require special attention

Important Files Changed

FilenameOverview
apps/server/src/terminalManager.tsAdded environment variable filtering logic to prevent app runtime variables from leaking into terminal sessions. Implementation is clean and well-tested.
apps/server/src/terminalManager.test.tsAdded comprehensive test verifying that blocked environment variables are excluded while non-app variables are preserved during terminal spawn.

Flowchart

flowchart TD
A[startSession called] --> B[createTerminalSpawnEnv called with process.env]
B --> C{For each env key/value}
C --> D{value === undefined?}
D -->|Yes| E[Skip]
D -->|No| F{shouldExcludeTerminalEnvKey}
F --> G{Starts with T3CODE_?}
G -->|Yes| E
G -->|No| H{Starts with VITE_?}
H -->|Yes| E
H -->|No| I{In TERMINAL_ENV_BLOCKLIST?}
I -->|Yes - PORT, ELECTRON_RENDERER_PORT, ELECTRON_RUN_AS_NODE| E
I -->|No| J[Include in spawnEnv]
E --> K{More keys?}
J --> K
K -->|Yes| C
K -->|No| L[Return filtered spawnEnv]
L --> M[ptyAdapter.spawn with filtered env]
Loading

Last reviewed commit: ecea657

@juliusmarminge
juliusmarminge merged commit 4b4abcd into mainFeb 14, 2026
4 checks passed
DavidIlie added a commit to DavidIlie/t3code that referenced this pull request Mar 13, 2026
…ker gating
Port upstream commits 9bb9023..b36888e:
- Handle branch selection across main and secondary worktrees (pingdotgg#44)
- Preserve fork PR upstreams when preparing local and worktree threads (pingdotgg#45)
Adds resolveBranchSelectionTarget for unified checkout cwd/worktree decisions,
GitCore helpers for remote management (ensureRemote, fetchRemoteBranch,
setBranchUpstream), GitHub CLI cross-repo PR metadata parsing, and
GitManager fork head materialization with upstream tracking.
aorwall added a commit to aorwall/t3code that referenced this pull request Aug 6, 2026
* Revert "feat: add a split-screen-horizontally toggle for the right panel (pingdotgg#41)"
This reverts commit 0ea9e96. The ask was to remove a horizontal split,
not to add one, so the toggle should not have shipped.
Every file pingdotgg#41 touched is back to its pre-pingdotgg#41 content and its three new
files are gone: rightPanelOrientation.ts, useResizablePanelHeight.ts and
its test. `git diff fd3b851 -- apps packages` is empty. The fork delta
here is nil again, so the Split Screen Delta section, its inventory row,
its path policy rows and its convergence row go with it.
Kept from pingdotgg#41, because they stand on their own and are not about the
toggle: the upstream-remote ownership check in Path policy, the .plans/**
row split it turned up, the messageOrigin.ts verification, and AGENTS.md's
rule that fork code says so in a comment where it sits.
Verification: web typecheck and lint clean, vp fmt --check clean, 83 tests
across apps/web/src/hooks and apps/web/src/components/preview.
Model: Claude Opus 5. Harness: Claude Code.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(web): drop the terminal-drawer toggle from the chat header
The chat's panel layout controls carry two buttons: one opens the right
panel, the other splits the screen horizontally by opening a terminal
across the bottom. The fork keeps one way to reach a terminal — the right
panel's terminal surface — so the second button is a split nobody asked
for beside a panel that already does the job.
Hides it the fork way rather than deleting upstream's code: a new
`terminalDrawerToggle` flag in `apps/web/src/fork/features.ts` and one
gate expression in `PanelLayoutControls.tsx`. Upstream's props, the
drawer and the `terminal.toggle` keybinding are untouched, so the drawer
still opens from the keyboard and deleting the flag brings the button
back.
Model: Claude Opus 5. Harness: Claude Code.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* docs(fork): the log keeps what is known, not what was undone
The 2026-08-03 entry led with a toggle that was added and then reverted
in the same day. Nothing in the tree carries it, so a future merge learns
nothing from reading about it. What that day actually produced is the
upstream-remote ownership check and what it found, which is what the
entry says now.
Model: Claude Opus 5. Harness: Claude Code.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
aorwall added a commit to aorwall/t3code that referenced this pull request Aug 29, 2026
The chat's upper-right corner lost the terminal-drawer button in pingdotgg#44, and
the right panel's sandbox pill had taken up residence at the right end of
the tab bar beside it — the same strip upstream reserves for its layout
toggles. Getting the button back is one half; the other is not putting it
next to a fork element that was crowding it.
The gate goes rather than flips, the way features.ts says to turn a flag
on, so PanelLayoutControls.tsx is upstream's byte for byte again and the
corner carries the terminal drawer, the right-panel toggle and maximize.
The sandbox pill moves into the panel body, where the surfaces it governs
are opened: under the launcher's Browser/Terminal/Files cards, and under
the disabled state's reason, where it stops being a status somewhere else
and becomes the Start button next to the explanation. Compact there, since
the heading has already said what is wrong. A ready sandbox's dot is green
now — amber beside "Sandbox running" was survivable in the tab bar and
would have been the loudest thing in the launcher.
Trade-off, recorded in the merge log rather than solved: with the pill in
the body, Stop is out of reach while a surface is open and the sandbox is
running. Losing a sandbox disables the surfaces and brings the control
back with the reason, so the way in is never the one that goes missing.
A fork-only test pins the placement, since it is a position rather than a
symbol and a merge could carry the hunk back into the tab bar without a
type error.
Model: Claude Opus 5. Harness: Claude Code.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@juliusmarminge
, '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

Filter app runtime env vars from terminal spawn environment - #44

Merged
juliusmarminge merged 1 commit into
mainfrom
codething/2e2908ea
Feb 14, 2026
Merged

Filter app runtime env vars from terminal spawn environment#44
juliusmarminge merged 1 commit into
mainfrom
codething/2e2908ea

Conversation

@juliusmarminge

@juliusmarmingejuliusmarminge commented Feb 14, 2026

Copy link
Copy Markdown
Member

Summary

  • Filter terminal spawn environment variables to exclude app/runtime keys that can interfere with shell sessions.
  • Add explicit exclusion logic for PORT, ELECTRON_RENDERER_PORT, ELECTRON_RUN_AS_NODE, and any keys prefixed with T3CODE_ or VITE_.
  • Keep unrelated environment variables intact when launching terminal sessions.
  • Add a regression test verifying blocked keys are removed and non-blocked keys are preserved.

Testing

  • Not run (not executed in this PR context).
  • Added unit test: apps/server/src/terminalManager.test.ts (filters app runtime env variables from terminal sessions) to verify:
    • PORT, T3CODE_PORT, and VITE_DEV_SERVER_URL are excluded from terminal spawn env.
    • Non-app variable TEST_TERMINAL_KEEP is preserved.

Open with Devin

Summary by CodeRabbit

Release Notes

  • Bug Fixes
    • Improved terminal environment isolation by filtering out framework and application-specific environment variables from terminal sessions. This prevents potential conflicts and enhances security when spawning new terminal instances.

- build terminal spawn env from a filtered copy of process env
- exclude `PORT`, `T3CODE_*`, `VITE_*`, and Electron runtime vars
- add test coverage to verify filtered and preserved env keys
@coderabbitai

coderabbitaiBot commented Feb 14, 2026

Copy link
Copy Markdown

Walkthrough

Implements environment variable filtering for spawned terminal sessions. A blocklist of sensitive variables (PORT, ELECTRON_RENDERER_PORT, ELECTRON_RUN_AS_NODE) and framework-specific prefixes (T3CODE_, VITE_) are excluded from the shell environment. New helper functions sanitize the environment before terminal spawn, with comprehensive test coverage validating the filtering behavior.

Changes

Cohort / File(s)Summary
Terminal Environment Filtering
apps/server/src/terminalManager.ts
Added shouldExcludeTerminalEnvKey and createTerminalSpawnEnv helper functions to filter environment variables. Implemented blocklist for PORT, ELECTRON_RENDERER_PORT, ELECTRON_RUN_AS_NODE and exclusion of variables with T3CODE_ or VITE_ prefixes. Integrated filtered environment into shell spawn within startSession.
Terminal Environment Filtering Tests
apps/server/src/terminalManager.test.ts
New test case verifies that blocked environment variables (PORT, T3CODE_PORT, VITE_DEV_SERVER_URL) are excluded from spawned terminal sessions while permitted variables (TEST_TERMINAL_KEEP) are preserved. Includes environment snapshot/restore helpers.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 2
❌ Failed checks (2 warnings)
Check nameStatusExplanationResolution
Docstring Coverage⚠️ WarningDocstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.Write docstrings for the functions missing them to satisfy the coverage threshold.
Merge Conflict Detection⚠️ Warning❌ Merge conflicts detected (5 files):

⚔️ TODO.md (content)
⚔️ apps/server/src/terminalManager.test.ts (content)
⚔️ apps/server/src/terminalManager.ts (content)
⚔️ apps/web/src/components/ChatView.tsx (content)
⚔️ apps/web/src/components/Sidebar.tsx (content)

These conflicts must be resolved before merging into main.
Resolve conflicts locally and push changes to this branch.
✅ Passed checks (2 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Title check✅ PassedThe title 'Filter app runtime env vars from terminal spawn environment' directly and clearly summarizes the main change: filtering environment variables from terminal spawn environments.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch codething/2e2908ea
⚔️ Resolve merge conflicts (beta)
  • Auto-commit resolved conflicts to branch codething/2e2908ea
  • Create stacked PR with resolved conflicts
  • Post resolved changes as copyable diffs in a comment

No actionable comments were generated in the recent review. 🎉

🧹 Recent nitpick comments
apps/server/src/terminalManager.test.ts (1)

462-465: Consider extending test coverage for remaining blocklist items.

The test validates the filtering pattern well. For completeness, you could optionally add assertions for the other blocklist entries (ELECTRON_RENDERER_PORT, ELECTRON_RUN_AS_NODE) to ensure full coverage of the explicit blocklist.

💡 Optional: Extended assertions
 setEnv("PORT", "5173");
+ setEnv("ELECTRON_RENDERER_PORT", "9000");+ setEnv("ELECTRON_RUN_AS_NODE", "1");
setEnv("T3CODE_PORT", "3773");
setEnv("VITE_DEV_SERVER_URL", "http://localhost:5173");
setEnv("TEST_TERMINAL_KEEP", "keep-me");
try {
const { manager, ptyAdapter } = makeManager();
await manager.open(openInput());
const spawnInput = ptyAdapter.spawnInputs[0];
expect(spawnInput).toBeDefined();
if (!spawnInput) return;
expect(spawnInput.env.PORT).toBeUndefined();
+ expect(spawnInput.env.ELECTRON_RENDERER_PORT).toBeUndefined();+ expect(spawnInput.env.ELECTRON_RUN_AS_NODE).toBeUndefined();
expect(spawnInput.env.T3CODE_PORT).toBeUndefined();

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

@macroscopeapp

Copy link
Copy Markdown
Contributor

Filter terminal spawn environment in TerminalManager.open to exclude PORT, ELECTRON_RENDERER_PORT, ELECTRON_RUN_AS_NODE, and variables starting with T3CODE_ or VITE_ per the PR motivation in terminalManager.ts

Add terminal env filtering via TERMINAL_ENV_BLOCKLIST, shouldExcludeTerminalEnvKey, and createTerminalSpawnEnv, and update TerminalManager.open to pass the sanitized env to ptyAdapter.spawn. A new test verifies exclusion and retention behavior in terminalManager.test.ts.

📍Where to Start

Start with TerminalManager.open in terminalManager.ts and trace into createTerminalSpawnEnv and shouldExcludeTerminalEnvKey.


Macroscope summarized ecea657.

@greptile-apps

Copy link
Copy Markdown

Greptile Overview

Greptile Summary

Adds environment variable filtering to terminal spawn operations to prevent app runtime configuration from interfering with shell sessions.

  • Introduces shouldExcludeTerminalEnvKey function that filters out PORT, ELECTRON_RENDERER_PORT, ELECTRON_RUN_AS_NODE, and variables prefixed with T3CODE_ or VITE_
  • Adds createTerminalSpawnEnv helper that creates a clean environment by excluding filtered keys
  • Modified startSession in apps/server/src/terminalManager.ts:496 to use filtered environment instead of process.env directly
  • Includes regression test with proper environment restoration to verify filtering behavior

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk
  • Clean implementation with focused scope, comprehensive test coverage, and no breaking changes. The filtering logic is straightforward and addresses a specific issue without affecting existing functionality.
  • No files require special attention

Important Files Changed

FilenameOverview
apps/server/src/terminalManager.tsAdded environment variable filtering logic to prevent app runtime variables from leaking into terminal sessions. Implementation is clean and well-tested.
apps/server/src/terminalManager.test.tsAdded comprehensive test verifying that blocked environment variables are excluded while non-app variables are preserved during terminal spawn.

Flowchart

flowchart TD
A[startSession called] --> B[createTerminalSpawnEnv called with process.env]
B --> C{For each env key/value}
C --> D{value === undefined?}
D -->|Yes| E[Skip]
D -->|No| F{shouldExcludeTerminalEnvKey}
F --> G{Starts with T3CODE_?}
G -->|Yes| E
G -->|No| H{Starts with VITE_?}
H -->|Yes| E
H -->|No| I{In TERMINAL_ENV_BLOCKLIST?}
I -->|Yes - PORT, ELECTRON_RENDERER_PORT, ELECTRON_RUN_AS_NODE| E
I -->|No| J[Include in spawnEnv]
E --> K{More keys?}
J --> K
K -->|Yes| C
K -->|No| L[Return filtered spawnEnv]
L --> M[ptyAdapter.spawn with filtered env]
Loading

Last reviewed commit: ecea657

@juliusmarminge
juliusmarminge merged commit 4b4abcd into mainFeb 14, 2026
4 checks passed
DavidIlie added a commit to DavidIlie/t3code that referenced this pull request Mar 13, 2026
…ker gating
Port upstream commits 9bb9023..b36888e:
- Handle branch selection across main and secondary worktrees (pingdotgg#44)
- Preserve fork PR upstreams when preparing local and worktree threads (pingdotgg#45)
Adds resolveBranchSelectionTarget for unified checkout cwd/worktree decisions,
GitCore helpers for remote management (ensureRemote, fetchRemoteBranch,
setBranchUpstream), GitHub CLI cross-repo PR metadata parsing, and
GitManager fork head materialization with upstream tracking.
aorwall added a commit to aorwall/t3code that referenced this pull request Aug 6, 2026
* Revert "feat: add a split-screen-horizontally toggle for the right panel (pingdotgg#41)"
This reverts commit 0ea9e96. The ask was to remove a horizontal split,
not to add one, so the toggle should not have shipped.
Every file pingdotgg#41 touched is back to its pre-pingdotgg#41 content and its three new
files are gone: rightPanelOrientation.ts, useResizablePanelHeight.ts and
its test. `git diff fd3b851 -- apps packages` is empty. The fork delta
here is nil again, so the Split Screen Delta section, its inventory row,
its path policy rows and its convergence row go with it.
Kept from pingdotgg#41, because they stand on their own and are not about the
toggle: the upstream-remote ownership check in Path policy, the .plans/**
row split it turned up, the messageOrigin.ts verification, and AGENTS.md's
rule that fork code says so in a comment where it sits.
Verification: web typecheck and lint clean, vp fmt --check clean, 83 tests
across apps/web/src/hooks and apps/web/src/components/preview.
Model: Claude Opus 5. Harness: Claude Code.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(web): drop the terminal-drawer toggle from the chat header
The chat's panel layout controls carry two buttons: one opens the right
panel, the other splits the screen horizontally by opening a terminal
across the bottom. The fork keeps one way to reach a terminal — the right
panel's terminal surface — so the second button is a split nobody asked
for beside a panel that already does the job.
Hides it the fork way rather than deleting upstream's code: a new
`terminalDrawerToggle` flag in `apps/web/src/fork/features.ts` and one
gate expression in `PanelLayoutControls.tsx`. Upstream's props, the
drawer and the `terminal.toggle` keybinding are untouched, so the drawer
still opens from the keyboard and deleting the flag brings the button
back.
Model: Claude Opus 5. Harness: Claude Code.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* docs(fork): the log keeps what is known, not what was undone
The 2026-08-03 entry led with a toggle that was added and then reverted
in the same day. Nothing in the tree carries it, so a future merge learns
nothing from reading about it. What that day actually produced is the
upstream-remote ownership check and what it found, which is what the
entry says now.
Model: Claude Opus 5. Harness: Claude Code.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
aorwall added a commit to aorwall/t3code that referenced this pull request Aug 29, 2026
The chat's upper-right corner lost the terminal-drawer button in pingdotgg#44, and
the right panel's sandbox pill had taken up residence at the right end of
the tab bar beside it — the same strip upstream reserves for its layout
toggles. Getting the button back is one half; the other is not putting it
next to a fork element that was crowding it.
The gate goes rather than flips, the way features.ts says to turn a flag
on, so PanelLayoutControls.tsx is upstream's byte for byte again and the
corner carries the terminal drawer, the right-panel toggle and maximize.
The sandbox pill moves into the panel body, where the surfaces it governs
are opened: under the launcher's Browser/Terminal/Files cards, and under
the disabled state's reason, where it stops being a status somewhere else
and becomes the Start button next to the explanation. Compact there, since
the heading has already said what is wrong. A ready sandbox's dot is green
now — amber beside "Sandbox running" was survivable in the tab bar and
would have been the loudest thing in the launcher.
Trade-off, recorded in the merge log rather than solved: with the pill in
the body, Stop is out of reach while a surface is open and the sandbox is
running. Losing a sandbox disables the surfaces and brings the control
back with the reason, so the way in is never the one that goes missing.
A fork-only test pins the placement, since it is a position rather than a
symbol and a merge could carry the hunk back into the tab bar without a
type error.
Model: Claude Opus 5. Harness: Claude Code.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@juliusmarminge
, '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

Filter app runtime env vars from terminal spawn environment - #44

Merged
juliusmarminge merged 1 commit into
mainfrom
codething/2e2908ea
Feb 14, 2026
Merged

Filter app runtime env vars from terminal spawn environment#44
juliusmarminge merged 1 commit into
mainfrom
codething/2e2908ea

Conversation

@juliusmarminge

@juliusmarmingejuliusmarminge commented Feb 14, 2026

Copy link
Copy Markdown
Member

Summary

  • Filter terminal spawn environment variables to exclude app/runtime keys that can interfere with shell sessions.
  • Add explicit exclusion logic for PORT, ELECTRON_RENDERER_PORT, ELECTRON_RUN_AS_NODE, and any keys prefixed with T3CODE_ or VITE_.
  • Keep unrelated environment variables intact when launching terminal sessions.
  • Add a regression test verifying blocked keys are removed and non-blocked keys are preserved.

Testing

  • Not run (not executed in this PR context).
  • Added unit test: apps/server/src/terminalManager.test.ts (filters app runtime env variables from terminal sessions) to verify:
    • PORT, T3CODE_PORT, and VITE_DEV_SERVER_URL are excluded from terminal spawn env.
    • Non-app variable TEST_TERMINAL_KEEP is preserved.

Open with Devin

Summary by CodeRabbit

Release Notes

  • Bug Fixes
    • Improved terminal environment isolation by filtering out framework and application-specific environment variables from terminal sessions. This prevents potential conflicts and enhances security when spawning new terminal instances.

- build terminal spawn env from a filtered copy of process env
- exclude `PORT`, `T3CODE_*`, `VITE_*`, and Electron runtime vars
- add test coverage to verify filtered and preserved env keys
@coderabbitai

coderabbitaiBot commented Feb 14, 2026

Copy link
Copy Markdown

Walkthrough

Implements environment variable filtering for spawned terminal sessions. A blocklist of sensitive variables (PORT, ELECTRON_RENDERER_PORT, ELECTRON_RUN_AS_NODE) and framework-specific prefixes (T3CODE_, VITE_) are excluded from the shell environment. New helper functions sanitize the environment before terminal spawn, with comprehensive test coverage validating the filtering behavior.

Changes

Cohort / File(s)Summary
Terminal Environment Filtering
apps/server/src/terminalManager.ts
Added shouldExcludeTerminalEnvKey and createTerminalSpawnEnv helper functions to filter environment variables. Implemented blocklist for PORT, ELECTRON_RENDERER_PORT, ELECTRON_RUN_AS_NODE and exclusion of variables with T3CODE_ or VITE_ prefixes. Integrated filtered environment into shell spawn within startSession.
Terminal Environment Filtering Tests
apps/server/src/terminalManager.test.ts
New test case verifies that blocked environment variables (PORT, T3CODE_PORT, VITE_DEV_SERVER_URL) are excluded from spawned terminal sessions while permitted variables (TEST_TERMINAL_KEEP) are preserved. Includes environment snapshot/restore helpers.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 2
❌ Failed checks (2 warnings)
Check nameStatusExplanationResolution
Docstring Coverage⚠️ WarningDocstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.Write docstrings for the functions missing them to satisfy the coverage threshold.
Merge Conflict Detection⚠️ Warning❌ Merge conflicts detected (5 files):

⚔️ TODO.md (content)
⚔️ apps/server/src/terminalManager.test.ts (content)
⚔️ apps/server/src/terminalManager.ts (content)
⚔️ apps/web/src/components/ChatView.tsx (content)
⚔️ apps/web/src/components/Sidebar.tsx (content)

These conflicts must be resolved before merging into main.
Resolve conflicts locally and push changes to this branch.
✅ Passed checks (2 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Title check✅ PassedThe title 'Filter app runtime env vars from terminal spawn environment' directly and clearly summarizes the main change: filtering environment variables from terminal spawn environments.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch codething/2e2908ea
⚔️ Resolve merge conflicts (beta)
  • Auto-commit resolved conflicts to branch codething/2e2908ea
  • Create stacked PR with resolved conflicts
  • Post resolved changes as copyable diffs in a comment

No actionable comments were generated in the recent review. 🎉

🧹 Recent nitpick comments
apps/server/src/terminalManager.test.ts (1)

462-465: Consider extending test coverage for remaining blocklist items.

The test validates the filtering pattern well. For completeness, you could optionally add assertions for the other blocklist entries (ELECTRON_RENDERER_PORT, ELECTRON_RUN_AS_NODE) to ensure full coverage of the explicit blocklist.

💡 Optional: Extended assertions
 setEnv("PORT", "5173");
+ setEnv("ELECTRON_RENDERER_PORT", "9000");+ setEnv("ELECTRON_RUN_AS_NODE", "1");
setEnv("T3CODE_PORT", "3773");
setEnv("VITE_DEV_SERVER_URL", "http://localhost:5173");
setEnv("TEST_TERMINAL_KEEP", "keep-me");
try {
const { manager, ptyAdapter } = makeManager();
await manager.open(openInput());
const spawnInput = ptyAdapter.spawnInputs[0];
expect(spawnInput).toBeDefined();
if (!spawnInput) return;
expect(spawnInput.env.PORT).toBeUndefined();
+ expect(spawnInput.env.ELECTRON_RENDERER_PORT).toBeUndefined();+ expect(spawnInput.env.ELECTRON_RUN_AS_NODE).toBeUndefined();
expect(spawnInput.env.T3CODE_PORT).toBeUndefined();

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

@macroscopeapp

Copy link
Copy Markdown
Contributor

Filter terminal spawn environment in TerminalManager.open to exclude PORT, ELECTRON_RENDERER_PORT, ELECTRON_RUN_AS_NODE, and variables starting with T3CODE_ or VITE_ per the PR motivation in terminalManager.ts

Add terminal env filtering via TERMINAL_ENV_BLOCKLIST, shouldExcludeTerminalEnvKey, and createTerminalSpawnEnv, and update TerminalManager.open to pass the sanitized env to ptyAdapter.spawn. A new test verifies exclusion and retention behavior in terminalManager.test.ts.

📍Where to Start

Start with TerminalManager.open in terminalManager.ts and trace into createTerminalSpawnEnv and shouldExcludeTerminalEnvKey.


Macroscope summarized ecea657.

@greptile-apps

Copy link
Copy Markdown

Greptile Overview

Greptile Summary

Adds environment variable filtering to terminal spawn operations to prevent app runtime configuration from interfering with shell sessions.

  • Introduces shouldExcludeTerminalEnvKey function that filters out PORT, ELECTRON_RENDERER_PORT, ELECTRON_RUN_AS_NODE, and variables prefixed with T3CODE_ or VITE_
  • Adds createTerminalSpawnEnv helper that creates a clean environment by excluding filtered keys
  • Modified startSession in apps/server/src/terminalManager.ts:496 to use filtered environment instead of process.env directly
  • Includes regression test with proper environment restoration to verify filtering behavior

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk
  • Clean implementation with focused scope, comprehensive test coverage, and no breaking changes. The filtering logic is straightforward and addresses a specific issue without affecting existing functionality.
  • No files require special attention

Important Files Changed

FilenameOverview
apps/server/src/terminalManager.tsAdded environment variable filtering logic to prevent app runtime variables from leaking into terminal sessions. Implementation is clean and well-tested.
apps/server/src/terminalManager.test.tsAdded comprehensive test verifying that blocked environment variables are excluded while non-app variables are preserved during terminal spawn.

Flowchart

flowchart TD
A[startSession called] --> B[createTerminalSpawnEnv called with process.env]
B --> C{For each env key/value}
C --> D{value === undefined?}
D -->|Yes| E[Skip]
D -->|No| F{shouldExcludeTerminalEnvKey}
F --> G{Starts with T3CODE_?}
G -->|Yes| E
G -->|No| H{Starts with VITE_?}
H -->|Yes| E
H -->|No| I{In TERMINAL_ENV_BLOCKLIST?}
I -->|Yes - PORT, ELECTRON_RENDERER_PORT, ELECTRON_RUN_AS_NODE| E
I -->|No| J[Include in spawnEnv]
E --> K{More keys?}
J --> K
K -->|Yes| C
K -->|No| L[Return filtered spawnEnv]
L --> M[ptyAdapter.spawn with filtered env]
Loading

Last reviewed commit: ecea657

@juliusmarminge
juliusmarminge merged commit 4b4abcd into mainFeb 14, 2026
4 checks passed
DavidIlie added a commit to DavidIlie/t3code that referenced this pull request Mar 13, 2026
…ker gating
Port upstream commits 9bb9023..b36888e:
- Handle branch selection across main and secondary worktrees (pingdotgg#44)
- Preserve fork PR upstreams when preparing local and worktree threads (pingdotgg#45)
Adds resolveBranchSelectionTarget for unified checkout cwd/worktree decisions,
GitCore helpers for remote management (ensureRemote, fetchRemoteBranch,
setBranchUpstream), GitHub CLI cross-repo PR metadata parsing, and
GitManager fork head materialization with upstream tracking.
aorwall added a commit to aorwall/t3code that referenced this pull request Aug 6, 2026
* Revert "feat: add a split-screen-horizontally toggle for the right panel (pingdotgg#41)"
This reverts commit 0ea9e96. The ask was to remove a horizontal split,
not to add one, so the toggle should not have shipped.
Every file pingdotgg#41 touched is back to its pre-pingdotgg#41 content and its three new
files are gone: rightPanelOrientation.ts, useResizablePanelHeight.ts and
its test. `git diff fd3b851 -- apps packages` is empty. The fork delta
here is nil again, so the Split Screen Delta section, its inventory row,
its path policy rows and its convergence row go with it.
Kept from pingdotgg#41, because they stand on their own and are not about the
toggle: the upstream-remote ownership check in Path policy, the .plans/**
row split it turned up, the messageOrigin.ts verification, and AGENTS.md's
rule that fork code says so in a comment where it sits.
Verification: web typecheck and lint clean, vp fmt --check clean, 83 tests
across apps/web/src/hooks and apps/web/src/components/preview.
Model: Claude Opus 5. Harness: Claude Code.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(web): drop the terminal-drawer toggle from the chat header
The chat's panel layout controls carry two buttons: one opens the right
panel, the other splits the screen horizontally by opening a terminal
across the bottom. The fork keeps one way to reach a terminal — the right
panel's terminal surface — so the second button is a split nobody asked
for beside a panel that already does the job.
Hides it the fork way rather than deleting upstream's code: a new
`terminalDrawerToggle` flag in `apps/web/src/fork/features.ts` and one
gate expression in `PanelLayoutControls.tsx`. Upstream's props, the
drawer and the `terminal.toggle` keybinding are untouched, so the drawer
still opens from the keyboard and deleting the flag brings the button
back.
Model: Claude Opus 5. Harness: Claude Code.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* docs(fork): the log keeps what is known, not what was undone
The 2026-08-03 entry led with a toggle that was added and then reverted
in the same day. Nothing in the tree carries it, so a future merge learns
nothing from reading about it. What that day actually produced is the
upstream-remote ownership check and what it found, which is what the
entry says now.
Model: Claude Opus 5. Harness: Claude Code.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
aorwall added a commit to aorwall/t3code that referenced this pull request Aug 29, 2026
The chat's upper-right corner lost the terminal-drawer button in pingdotgg#44, and
the right panel's sandbox pill had taken up residence at the right end of
the tab bar beside it — the same strip upstream reserves for its layout
toggles. Getting the button back is one half; the other is not putting it
next to a fork element that was crowding it.
The gate goes rather than flips, the way features.ts says to turn a flag
on, so PanelLayoutControls.tsx is upstream's byte for byte again and the
corner carries the terminal drawer, the right-panel toggle and maximize.
The sandbox pill moves into the panel body, where the surfaces it governs
are opened: under the launcher's Browser/Terminal/Files cards, and under
the disabled state's reason, where it stops being a status somewhere else
and becomes the Start button next to the explanation. Compact there, since
the heading has already said what is wrong. A ready sandbox's dot is green
now — amber beside "Sandbox running" was survivable in the tab bar and
would have been the loudest thing in the launcher.
Trade-off, recorded in the merge log rather than solved: with the pill in
the body, Stop is out of reach while a surface is open and the sandbox is
running. Losing a sandbox disables the surfaces and brings the control
back with the reason, so the way in is never the one that goes missing.
A fork-only test pins the placement, since it is a position rather than a
symbol and a merge could carry the hunk back into the tab bar without a
type error.
Model: Claude Opus 5. Harness: Claude Code.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@juliusmarminge
, '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

Filter app runtime env vars from terminal spawn environment - #44

Merged
juliusmarminge merged 1 commit into
mainfrom
codething/2e2908ea
Feb 14, 2026
Merged

Filter app runtime env vars from terminal spawn environment#44
juliusmarminge merged 1 commit into
mainfrom
codething/2e2908ea

Conversation

@juliusmarminge

@juliusmarmingejuliusmarminge commented Feb 14, 2026

Copy link
Copy Markdown
Member

Summary

  • Filter terminal spawn environment variables to exclude app/runtime keys that can interfere with shell sessions.
  • Add explicit exclusion logic for PORT, ELECTRON_RENDERER_PORT, ELECTRON_RUN_AS_NODE, and any keys prefixed with T3CODE_ or VITE_.
  • Keep unrelated environment variables intact when launching terminal sessions.
  • Add a regression test verifying blocked keys are removed and non-blocked keys are preserved.

Testing

  • Not run (not executed in this PR context).
  • Added unit test: apps/server/src/terminalManager.test.ts (filters app runtime env variables from terminal sessions) to verify:
    • PORT, T3CODE_PORT, and VITE_DEV_SERVER_URL are excluded from terminal spawn env.
    • Non-app variable TEST_TERMINAL_KEEP is preserved.

Open with Devin

Summary by CodeRabbit

Release Notes

  • Bug Fixes
    • Improved terminal environment isolation by filtering out framework and application-specific environment variables from terminal sessions. This prevents potential conflicts and enhances security when spawning new terminal instances.

- build terminal spawn env from a filtered copy of process env
- exclude `PORT`, `T3CODE_*`, `VITE_*`, and Electron runtime vars
- add test coverage to verify filtered and preserved env keys
@coderabbitai

coderabbitaiBot commented Feb 14, 2026

Copy link
Copy Markdown

Walkthrough

Implements environment variable filtering for spawned terminal sessions. A blocklist of sensitive variables (PORT, ELECTRON_RENDERER_PORT, ELECTRON_RUN_AS_NODE) and framework-specific prefixes (T3CODE_, VITE_) are excluded from the shell environment. New helper functions sanitize the environment before terminal spawn, with comprehensive test coverage validating the filtering behavior.

Changes

Cohort / File(s)Summary
Terminal Environment Filtering
apps/server/src/terminalManager.ts
Added shouldExcludeTerminalEnvKey and createTerminalSpawnEnv helper functions to filter environment variables. Implemented blocklist for PORT, ELECTRON_RENDERER_PORT, ELECTRON_RUN_AS_NODE and exclusion of variables with T3CODE_ or VITE_ prefixes. Integrated filtered environment into shell spawn within startSession.
Terminal Environment Filtering Tests
apps/server/src/terminalManager.test.ts
New test case verifies that blocked environment variables (PORT, T3CODE_PORT, VITE_DEV_SERVER_URL) are excluded from spawned terminal sessions while permitted variables (TEST_TERMINAL_KEEP) are preserved. Includes environment snapshot/restore helpers.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 2
❌ Failed checks (2 warnings)
Check nameStatusExplanationResolution
Docstring Coverage⚠️ WarningDocstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.Write docstrings for the functions missing them to satisfy the coverage threshold.
Merge Conflict Detection⚠️ Warning❌ Merge conflicts detected (5 files):

⚔️ TODO.md (content)
⚔️ apps/server/src/terminalManager.test.ts (content)
⚔️ apps/server/src/terminalManager.ts (content)
⚔️ apps/web/src/components/ChatView.tsx (content)
⚔️ apps/web/src/components/Sidebar.tsx (content)

These conflicts must be resolved before merging into main.
Resolve conflicts locally and push changes to this branch.
✅ Passed checks (2 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Title check✅ PassedThe title 'Filter app runtime env vars from terminal spawn environment' directly and clearly summarizes the main change: filtering environment variables from terminal spawn environments.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch codething/2e2908ea
⚔️ Resolve merge conflicts (beta)
  • Auto-commit resolved conflicts to branch codething/2e2908ea
  • Create stacked PR with resolved conflicts
  • Post resolved changes as copyable diffs in a comment

No actionable comments were generated in the recent review. 🎉

🧹 Recent nitpick comments
apps/server/src/terminalManager.test.ts (1)

462-465: Consider extending test coverage for remaining blocklist items.

The test validates the filtering pattern well. For completeness, you could optionally add assertions for the other blocklist entries (ELECTRON_RENDERER_PORT, ELECTRON_RUN_AS_NODE) to ensure full coverage of the explicit blocklist.

💡 Optional: Extended assertions
 setEnv("PORT", "5173");
+ setEnv("ELECTRON_RENDERER_PORT", "9000");+ setEnv("ELECTRON_RUN_AS_NODE", "1");
setEnv("T3CODE_PORT", "3773");
setEnv("VITE_DEV_SERVER_URL", "http://localhost:5173");
setEnv("TEST_TERMINAL_KEEP", "keep-me");
try {
const { manager, ptyAdapter } = makeManager();
await manager.open(openInput());
const spawnInput = ptyAdapter.spawnInputs[0];
expect(spawnInput).toBeDefined();
if (!spawnInput) return;
expect(spawnInput.env.PORT).toBeUndefined();
+ expect(spawnInput.env.ELECTRON_RENDERER_PORT).toBeUndefined();+ expect(spawnInput.env.ELECTRON_RUN_AS_NODE).toBeUndefined();
expect(spawnInput.env.T3CODE_PORT).toBeUndefined();

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

@macroscopeapp

Copy link
Copy Markdown
Contributor

Filter terminal spawn environment in TerminalManager.open to exclude PORT, ELECTRON_RENDERER_PORT, ELECTRON_RUN_AS_NODE, and variables starting with T3CODE_ or VITE_ per the PR motivation in terminalManager.ts

Add terminal env filtering via TERMINAL_ENV_BLOCKLIST, shouldExcludeTerminalEnvKey, and createTerminalSpawnEnv, and update TerminalManager.open to pass the sanitized env to ptyAdapter.spawn. A new test verifies exclusion and retention behavior in terminalManager.test.ts.

📍Where to Start

Start with TerminalManager.open in terminalManager.ts and trace into createTerminalSpawnEnv and shouldExcludeTerminalEnvKey.


Macroscope summarized ecea657.

@greptile-apps

Copy link
Copy Markdown

Greptile Overview

Greptile Summary

Adds environment variable filtering to terminal spawn operations to prevent app runtime configuration from interfering with shell sessions.

  • Introduces shouldExcludeTerminalEnvKey function that filters out PORT, ELECTRON_RENDERER_PORT, ELECTRON_RUN_AS_NODE, and variables prefixed with T3CODE_ or VITE_
  • Adds createTerminalSpawnEnv helper that creates a clean environment by excluding filtered keys
  • Modified startSession in apps/server/src/terminalManager.ts:496 to use filtered environment instead of process.env directly
  • Includes regression test with proper environment restoration to verify filtering behavior

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk
  • Clean implementation with focused scope, comprehensive test coverage, and no breaking changes. The filtering logic is straightforward and addresses a specific issue without affecting existing functionality.
  • No files require special attention

Important Files Changed

FilenameOverview
apps/server/src/terminalManager.tsAdded environment variable filtering logic to prevent app runtime variables from leaking into terminal sessions. Implementation is clean and well-tested.
apps/server/src/terminalManager.test.tsAdded comprehensive test verifying that blocked environment variables are excluded while non-app variables are preserved during terminal spawn.

Flowchart

flowchart TD
A[startSession called] --> B[createTerminalSpawnEnv called with process.env]
B --> C{For each env key/value}
C --> D{value === undefined?}
D -->|Yes| E[Skip]
D -->|No| F{shouldExcludeTerminalEnvKey}
F --> G{Starts with T3CODE_?}
G -->|Yes| E
G -->|No| H{Starts with VITE_?}
H -->|Yes| E
H -->|No| I{In TERMINAL_ENV_BLOCKLIST?}
I -->|Yes - PORT, ELECTRON_RENDERER_PORT, ELECTRON_RUN_AS_NODE| E
I -->|No| J[Include in spawnEnv]
E --> K{More keys?}
J --> K
K -->|Yes| C
K -->|No| L[Return filtered spawnEnv]
L --> M[ptyAdapter.spawn with filtered env]
Loading

Last reviewed commit: ecea657

@juliusmarminge
juliusmarminge merged commit 4b4abcd into mainFeb 14, 2026
4 checks passed
DavidIlie added a commit to DavidIlie/t3code that referenced this pull request Mar 13, 2026
…ker gating
Port upstream commits 9bb9023..b36888e:
- Handle branch selection across main and secondary worktrees (pingdotgg#44)
- Preserve fork PR upstreams when preparing local and worktree threads (pingdotgg#45)
Adds resolveBranchSelectionTarget for unified checkout cwd/worktree decisions,
GitCore helpers for remote management (ensureRemote, fetchRemoteBranch,
setBranchUpstream), GitHub CLI cross-repo PR metadata parsing, and
GitManager fork head materialization with upstream tracking.
aorwall added a commit to aorwall/t3code that referenced this pull request Aug 6, 2026
* Revert "feat: add a split-screen-horizontally toggle for the right panel (pingdotgg#41)"
This reverts commit 0ea9e96. The ask was to remove a horizontal split,
not to add one, so the toggle should not have shipped.
Every file pingdotgg#41 touched is back to its pre-pingdotgg#41 content and its three new
files are gone: rightPanelOrientation.ts, useResizablePanelHeight.ts and
its test. `git diff fd3b851 -- apps packages` is empty. The fork delta
here is nil again, so the Split Screen Delta section, its inventory row,
its path policy rows and its convergence row go with it.
Kept from pingdotgg#41, because they stand on their own and are not about the
toggle: the upstream-remote ownership check in Path policy, the .plans/**
row split it turned up, the messageOrigin.ts verification, and AGENTS.md's
rule that fork code says so in a comment where it sits.
Verification: web typecheck and lint clean, vp fmt --check clean, 83 tests
across apps/web/src/hooks and apps/web/src/components/preview.
Model: Claude Opus 5. Harness: Claude Code.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(web): drop the terminal-drawer toggle from the chat header
The chat's panel layout controls carry two buttons: one opens the right
panel, the other splits the screen horizontally by opening a terminal
across the bottom. The fork keeps one way to reach a terminal — the right
panel's terminal surface — so the second button is a split nobody asked
for beside a panel that already does the job.
Hides it the fork way rather than deleting upstream's code: a new
`terminalDrawerToggle` flag in `apps/web/src/fork/features.ts` and one
gate expression in `PanelLayoutControls.tsx`. Upstream's props, the
drawer and the `terminal.toggle` keybinding are untouched, so the drawer
still opens from the keyboard and deleting the flag brings the button
back.
Model: Claude Opus 5. Harness: Claude Code.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* docs(fork): the log keeps what is known, not what was undone
The 2026-08-03 entry led with a toggle that was added and then reverted
in the same day. Nothing in the tree carries it, so a future merge learns
nothing from reading about it. What that day actually produced is the
upstream-remote ownership check and what it found, which is what the
entry says now.
Model: Claude Opus 5. Harness: Claude Code.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
aorwall added a commit to aorwall/t3code that referenced this pull request Aug 29, 2026
The chat's upper-right corner lost the terminal-drawer button in pingdotgg#44, and
the right panel's sandbox pill had taken up residence at the right end of
the tab bar beside it — the same strip upstream reserves for its layout
toggles. Getting the button back is one half; the other is not putting it
next to a fork element that was crowding it.
The gate goes rather than flips, the way features.ts says to turn a flag
on, so PanelLayoutControls.tsx is upstream's byte for byte again and the
corner carries the terminal drawer, the right-panel toggle and maximize.
The sandbox pill moves into the panel body, where the surfaces it governs
are opened: under the launcher's Browser/Terminal/Files cards, and under
the disabled state's reason, where it stops being a status somewhere else
and becomes the Start button next to the explanation. Compact there, since
the heading has already said what is wrong. A ready sandbox's dot is green
now — amber beside "Sandbox running" was survivable in the tab bar and
would have been the loudest thing in the launcher.
Trade-off, recorded in the merge log rather than solved: with the pill in
the body, Stop is out of reach while a surface is open and the sandbox is
running. Losing a sandbox disables the surfaces and brings the control
back with the reason, so the way in is never the one that goes missing.
A fork-only test pins the placement, since it is a position rather than a
symbol and a merge could carry the hunk back into the tab bar without a
type error.
Model: Claude Opus 5. Harness: Claude Code.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@juliusmarminge
, '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

Filter app runtime env vars from terminal spawn environment - #44

Merged
juliusmarminge merged 1 commit into
mainfrom
codething/2e2908ea
Feb 14, 2026
Merged

Filter app runtime env vars from terminal spawn environment#44
juliusmarminge merged 1 commit into
mainfrom
codething/2e2908ea

Conversation

@juliusmarminge

@juliusmarmingejuliusmarminge commented Feb 14, 2026

Copy link
Copy Markdown
Member

Summary

  • Filter terminal spawn environment variables to exclude app/runtime keys that can interfere with shell sessions.
  • Add explicit exclusion logic for PORT, ELECTRON_RENDERER_PORT, ELECTRON_RUN_AS_NODE, and any keys prefixed with T3CODE_ or VITE_.
  • Keep unrelated environment variables intact when launching terminal sessions.
  • Add a regression test verifying blocked keys are removed and non-blocked keys are preserved.

Testing

  • Not run (not executed in this PR context).
  • Added unit test: apps/server/src/terminalManager.test.ts (filters app runtime env variables from terminal sessions) to verify:
    • PORT, T3CODE_PORT, and VITE_DEV_SERVER_URL are excluded from terminal spawn env.
    • Non-app variable TEST_TERMINAL_KEEP is preserved.

Open with Devin

Summary by CodeRabbit

Release Notes

  • Bug Fixes
    • Improved terminal environment isolation by filtering out framework and application-specific environment variables from terminal sessions. This prevents potential conflicts and enhances security when spawning new terminal instances.

- build terminal spawn env from a filtered copy of process env
- exclude `PORT`, `T3CODE_*`, `VITE_*`, and Electron runtime vars
- add test coverage to verify filtered and preserved env keys
@coderabbitai

coderabbitaiBot commented Feb 14, 2026

Copy link
Copy Markdown

Walkthrough

Implements environment variable filtering for spawned terminal sessions. A blocklist of sensitive variables (PORT, ELECTRON_RENDERER_PORT, ELECTRON_RUN_AS_NODE) and framework-specific prefixes (T3CODE_, VITE_) are excluded from the shell environment. New helper functions sanitize the environment before terminal spawn, with comprehensive test coverage validating the filtering behavior.

Changes

Cohort / File(s)Summary
Terminal Environment Filtering
apps/server/src/terminalManager.ts
Added shouldExcludeTerminalEnvKey and createTerminalSpawnEnv helper functions to filter environment variables. Implemented blocklist for PORT, ELECTRON_RENDERER_PORT, ELECTRON_RUN_AS_NODE and exclusion of variables with T3CODE_ or VITE_ prefixes. Integrated filtered environment into shell spawn within startSession.
Terminal Environment Filtering Tests
apps/server/src/terminalManager.test.ts
New test case verifies that blocked environment variables (PORT, T3CODE_PORT, VITE_DEV_SERVER_URL) are excluded from spawned terminal sessions while permitted variables (TEST_TERMINAL_KEEP) are preserved. Includes environment snapshot/restore helpers.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 2
❌ Failed checks (2 warnings)
Check nameStatusExplanationResolution
Docstring Coverage⚠️ WarningDocstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.Write docstrings for the functions missing them to satisfy the coverage threshold.
Merge Conflict Detection⚠️ Warning❌ Merge conflicts detected (5 files):

⚔️ TODO.md (content)
⚔️ apps/server/src/terminalManager.test.ts (content)
⚔️ apps/server/src/terminalManager.ts (content)
⚔️ apps/web/src/components/ChatView.tsx (content)
⚔️ apps/web/src/components/Sidebar.tsx (content)

These conflicts must be resolved before merging into main.
Resolve conflicts locally and push changes to this branch.
✅ Passed checks (2 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Title check✅ PassedThe title 'Filter app runtime env vars from terminal spawn environment' directly and clearly summarizes the main change: filtering environment variables from terminal spawn environments.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch codething/2e2908ea
⚔️ Resolve merge conflicts (beta)
  • Auto-commit resolved conflicts to branch codething/2e2908ea
  • Create stacked PR with resolved conflicts
  • Post resolved changes as copyable diffs in a comment

No actionable comments were generated in the recent review. 🎉

🧹 Recent nitpick comments
apps/server/src/terminalManager.test.ts (1)

462-465: Consider extending test coverage for remaining blocklist items.

The test validates the filtering pattern well. For completeness, you could optionally add assertions for the other blocklist entries (ELECTRON_RENDERER_PORT, ELECTRON_RUN_AS_NODE) to ensure full coverage of the explicit blocklist.

💡 Optional: Extended assertions
 setEnv("PORT", "5173");
+ setEnv("ELECTRON_RENDERER_PORT", "9000");+ setEnv("ELECTRON_RUN_AS_NODE", "1");
setEnv("T3CODE_PORT", "3773");
setEnv("VITE_DEV_SERVER_URL", "http://localhost:5173");
setEnv("TEST_TERMINAL_KEEP", "keep-me");
try {
const { manager, ptyAdapter } = makeManager();
await manager.open(openInput());
const spawnInput = ptyAdapter.spawnInputs[0];
expect(spawnInput).toBeDefined();
if (!spawnInput) return;
expect(spawnInput.env.PORT).toBeUndefined();
+ expect(spawnInput.env.ELECTRON_RENDERER_PORT).toBeUndefined();+ expect(spawnInput.env.ELECTRON_RUN_AS_NODE).toBeUndefined();
expect(spawnInput.env.T3CODE_PORT).toBeUndefined();

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

@macroscopeapp

Copy link
Copy Markdown
Contributor

Filter terminal spawn environment in TerminalManager.open to exclude PORT, ELECTRON_RENDERER_PORT, ELECTRON_RUN_AS_NODE, and variables starting with T3CODE_ or VITE_ per the PR motivation in terminalManager.ts

Add terminal env filtering via TERMINAL_ENV_BLOCKLIST, shouldExcludeTerminalEnvKey, and createTerminalSpawnEnv, and update TerminalManager.open to pass the sanitized env to ptyAdapter.spawn. A new test verifies exclusion and retention behavior in terminalManager.test.ts.

📍Where to Start

Start with TerminalManager.open in terminalManager.ts and trace into createTerminalSpawnEnv and shouldExcludeTerminalEnvKey.


Macroscope summarized ecea657.

@greptile-apps

Copy link
Copy Markdown

Greptile Overview

Greptile Summary

Adds environment variable filtering to terminal spawn operations to prevent app runtime configuration from interfering with shell sessions.

  • Introduces shouldExcludeTerminalEnvKey function that filters out PORT, ELECTRON_RENDERER_PORT, ELECTRON_RUN_AS_NODE, and variables prefixed with T3CODE_ or VITE_
  • Adds createTerminalSpawnEnv helper that creates a clean environment by excluding filtered keys
  • Modified startSession in apps/server/src/terminalManager.ts:496 to use filtered environment instead of process.env directly
  • Includes regression test with proper environment restoration to verify filtering behavior

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk
  • Clean implementation with focused scope, comprehensive test coverage, and no breaking changes. The filtering logic is straightforward and addresses a specific issue without affecting existing functionality.
  • No files require special attention

Important Files Changed

FilenameOverview
apps/server/src/terminalManager.tsAdded environment variable filtering logic to prevent app runtime variables from leaking into terminal sessions. Implementation is clean and well-tested.
apps/server/src/terminalManager.test.tsAdded comprehensive test verifying that blocked environment variables are excluded while non-app variables are preserved during terminal spawn.

Flowchart

flowchart TD
A[startSession called] --> B[createTerminalSpawnEnv called with process.env]
B --> C{For each env key/value}
C --> D{value === undefined?}
D -->|Yes| E[Skip]
D -->|No| F{shouldExcludeTerminalEnvKey}
F --> G{Starts with T3CODE_?}
G -->|Yes| E
G -->|No| H{Starts with VITE_?}
H -->|Yes| E
H -->|No| I{In TERMINAL_ENV_BLOCKLIST?}
I -->|Yes - PORT, ELECTRON_RENDERER_PORT, ELECTRON_RUN_AS_NODE| E
I -->|No| J[Include in spawnEnv]
E --> K{More keys?}
J --> K
K -->|Yes| C
K -->|No| L[Return filtered spawnEnv]
L --> M[ptyAdapter.spawn with filtered env]
Loading

Last reviewed commit: ecea657

@juliusmarminge
juliusmarminge merged commit 4b4abcd into mainFeb 14, 2026
4 checks passed
DavidIlie added a commit to DavidIlie/t3code that referenced this pull request Mar 13, 2026
…ker gating
Port upstream commits 9bb9023..b36888e:
- Handle branch selection across main and secondary worktrees (pingdotgg#44)
- Preserve fork PR upstreams when preparing local and worktree threads (pingdotgg#45)
Adds resolveBranchSelectionTarget for unified checkout cwd/worktree decisions,
GitCore helpers for remote management (ensureRemote, fetchRemoteBranch,
setBranchUpstream), GitHub CLI cross-repo PR metadata parsing, and
GitManager fork head materialization with upstream tracking.
aorwall added a commit to aorwall/t3code that referenced this pull request Aug 6, 2026
* Revert "feat: add a split-screen-horizontally toggle for the right panel (pingdotgg#41)"
This reverts commit 0ea9e96. The ask was to remove a horizontal split,
not to add one, so the toggle should not have shipped.
Every file pingdotgg#41 touched is back to its pre-pingdotgg#41 content and its three new
files are gone: rightPanelOrientation.ts, useResizablePanelHeight.ts and
its test. `git diff fd3b851 -- apps packages` is empty. The fork delta
here is nil again, so the Split Screen Delta section, its inventory row,
its path policy rows and its convergence row go with it.
Kept from pingdotgg#41, because they stand on their own and are not about the
toggle: the upstream-remote ownership check in Path policy, the .plans/**
row split it turned up, the messageOrigin.ts verification, and AGENTS.md's
rule that fork code says so in a comment where it sits.
Verification: web typecheck and lint clean, vp fmt --check clean, 83 tests
across apps/web/src/hooks and apps/web/src/components/preview.
Model: Claude Opus 5. Harness: Claude Code.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(web): drop the terminal-drawer toggle from the chat header
The chat's panel layout controls carry two buttons: one opens the right
panel, the other splits the screen horizontally by opening a terminal
across the bottom. The fork keeps one way to reach a terminal — the right
panel's terminal surface — so the second button is a split nobody asked
for beside a panel that already does the job.
Hides it the fork way rather than deleting upstream's code: a new
`terminalDrawerToggle` flag in `apps/web/src/fork/features.ts` and one
gate expression in `PanelLayoutControls.tsx`. Upstream's props, the
drawer and the `terminal.toggle` keybinding are untouched, so the drawer
still opens from the keyboard and deleting the flag brings the button
back.
Model: Claude Opus 5. Harness: Claude Code.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* docs(fork): the log keeps what is known, not what was undone
The 2026-08-03 entry led with a toggle that was added and then reverted
in the same day. Nothing in the tree carries it, so a future merge learns
nothing from reading about it. What that day actually produced is the
upstream-remote ownership check and what it found, which is what the
entry says now.
Model: Claude Opus 5. Harness: Claude Code.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
aorwall added a commit to aorwall/t3code that referenced this pull request Aug 29, 2026
The chat's upper-right corner lost the terminal-drawer button in pingdotgg#44, and
the right panel's sandbox pill had taken up residence at the right end of
the tab bar beside it — the same strip upstream reserves for its layout
toggles. Getting the button back is one half; the other is not putting it
next to a fork element that was crowding it.
The gate goes rather than flips, the way features.ts says to turn a flag
on, so PanelLayoutControls.tsx is upstream's byte for byte again and the
corner carries the terminal drawer, the right-panel toggle and maximize.
The sandbox pill moves into the panel body, where the surfaces it governs
are opened: under the launcher's Browser/Terminal/Files cards, and under
the disabled state's reason, where it stops being a status somewhere else
and becomes the Start button next to the explanation. Compact there, since
the heading has already said what is wrong. A ready sandbox's dot is green
now — amber beside "Sandbox running" was survivable in the tab bar and
would have been the loudest thing in the launcher.
Trade-off, recorded in the merge log rather than solved: with the pill in
the body, Stop is out of reach while a surface is open and the sandbox is
running. Losing a sandbox disables the surfaces and brings the control
back with the reason, so the way in is never the one that goes missing.
A fork-only test pins the placement, since it is a position rather than a
symbol and a merge could carry the hunk back into the tab bar without a
type error.
Model: Claude Opus 5. Harness: Claude Code.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@juliusmarminge