[Lucenx9]

Summary

A login shell that writes enough startup diagnostics to fill stderr's pipe can fail to provide its environment to the desktop app. The probe reads stdout and waits for exit but never consumes stderr, so a noisy startup script can stall until the existing timeout and leave shell-only CLI directories out of PATH.

Ignore the unused stderr stream so noisy login-shell startup can complete. Preserve the current environment-capture command, markers, exit-status validation, deadlines and fallback. Keep only the desktop change and its real-process regression after main removed the unrelated web fixture.

- stderr: "pipe"
+ stderr: "ignore"

A small obvious reliability bug under CONTRIBUTING.md's focused-fix exception: one production option and a regression for that failure. No public settings, API, dependencies or UI changes. Targeted all-state issue/PR and discussion searches found no equivalent fix. Related #21771 handles the preparation flag after a failed probe; #18219 changes startup sequencing.

Evidence

Integrated with main at 22dc1dabc6d3b1616d0fa5017268c086a77887bc, preserving its intentional test deletions. Verified on Debian 13 x86-64 with Node 24.15.0. The current project declares Node 26.12.1; CI validates its configured runtime.

• From apps/desktop/: ../../node_modules/.bin/vp test run src/shell/DesktopShellEnvironment.test.ts --maxWorkers=1: 13 passed. The regression uses the real Node spawner and a child that writes 2 MiB to stderr, requiring the shell-only PATH and no fallback probe.
• Changed-file format/lint with one thread and scoped no-emit typecheck passed. No repo-wide checks were run. Effect emitted suggestions only in unchanged dependencies.
• Final independent standards and requirements reviews with GPT-6.1 Sol via Codex found no actionable issues.

No GUI or personal shell profile was exercised. Native macOS and Windows were not run.

Merge danger

Door: two-way. This code-only change can be reverted.

Blast radius: Desktop.

Implemented with GPT-6.1 Sol via Codex in T3 Code; reviewed with GPT Daybreak Blue via Codex.

Updated and reviewed with GPT-6.1 Sol via the Codex harness in T3 Code.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-19T20:43:14.460597Z	9a9d51f	PR opened

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[macroscopeapp]

Approvability

Verdict: Approved at 2932d24

Macroscope's review found this PR approvable — This narrowly fixes desktop shell-environment capture by preventing unused stderr backpressure from blocking noisy probes, while preserving stdout, exit validation, timeouts, and fallback behavior. A targeted real-process regression test covers the affected scenario.

Notes:

• All code in this push has already been reviewed. Approvability was decided on eligibility alone.

^{You can add or adjust custom eligibility rules. Learn more.}

[coderabbitai]

No actionable comments were generated in the recent review.

Recent review info

Run configuration

• Source mode: Auto
• Repository: pingdotgg/t3code
• Branch: main
• File: .coderabbit.config.ts
• Commit: 80a2351
• Layers: Repository TS config
• Review profile: CHILL
• Plan: Advanced
• Run ID: 197fcdca-eb4d-4268-97ff-306915dfc6ab

Commits

Reviewing files that changed from the base of the PR and between 2932d24 and 3ab7170.

Files selected for processing: 1

• apps/desktop/src/shell/DesktopShellEnvironment.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 1 remain after this review.

Walkthrough

The shell command runner now ignores child-process stderr while capturing stdout. A Linux regression test uses a real process that writes 2 MiB to stderr before emitting a shell-only PATH. The test checks that the path is installed and only /test-shell is invoked.

Changes: Shell environment startup

Layer / File(s)	Summary
Ignore stderr and verify shell startup apps/desktop/src/shell/DesktopShellEnvironment.ts, apps/desktop/src/shell/DesktopShellEnvironment.test.ts	runCommandOutput ignores child-process stderr. The test helper accepts a process spawner, and a Linux test verifies PATH capture after large stderr output.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Possibly related PRs

• pingdotgg/t3code#24054: Changes shell command stderr handling and adds real-process regression coverage.
• pingdotgg/t3code#22286: Addresses desktop shell-environment capture that can leave PATH incomplete.

Suggested labels: bugfix

Suggested reviewers: juliusmarminge

Merge Risk: Minimal · up to 3ab71

The change removes stderr backpressure without affecting the shell PATH capture or command failure handling, so it is ready to merge.

Architecture risk: Low · up to 7436b

The change affects 2 systems.

Changed systems: apps/desktop, apps/web

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

• observed — apps/desktop (service) was modified; 2 changed files map to changed impact.
• observed — apps/web (ui) was modified; 1 changed file maps to changed impact.

Before / after behavior

• observed — Modified behavior in apps/desktop/src/shell/DesktopShellEnvironment.test.ts: The test imports HostProcess to obtain the executable path used by the spawned shell process.
• observed — Modified behavior in apps/desktop/src/shell/DesktopShellEnvironment.test.ts: runShellEnvironment input now accepts an optional child-process spawner service.
• observed — Modified behavior in apps/desktop/src/shell/DesktopShellEnvironment.test.ts: The helper delegates spawning to the supplied processSpawner when present; otherwise, it continues through the existing failure and handler paths.
• observed — Modified behavior in apps/desktop/src/shell/DesktopShellEnvironment.test.ts: Adds a test that starts the configured shell through a real spawner, writes 2 MiB to stderr, then emits a marked PATH on stdout. It asserts that the environment receives the emitted path and that only the configured shell command was invoked.

Pre-merge checks: 4

Check name	Status	Explanation
Title check		The title is concise, conventional, and clearly identifies the primary desktop fix: preventing noisy shell probes from blocking PATH capture.
Description check		The description explains the problem, fix, scope, approval exception, verification results, limitations, and review context. It uses different headings from the template, but it includes the required …
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Description check

Explanation

The description explains the problem, fix, scope, approval exception, verification results, limitations, and review context. It uses different headings from the template, but it includes the required information and is sufficiently complete.

Finishing Touches

Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Dismissing prior approval to re-evaluate 7436b20

Dismissing prior approval to re-evaluate 2932d24

[maria-rcks]

Note

Written by gpt-6.1-sol on behalf of Maria

merged current main (80a2351fcb90) into this branch, preserving all contributor commits. the original implementation already has the smallest correct fix: ignore unused stderr while retaining stdout capture, exit validation, deadlines and fallback. no production or test changes were needed beyond the merge; the diff remains two files, +39/-1.

verified through blacksmith-testbox:

• with main's stderr: "pipe", the real-process 2 mib regression fails to capture /shell-only/bin; the other 13 tests pass.
• with stderr: "ignore", all 14 focused tests pass.
• vp run check:changed github/main passes format, lint, both knip passes, affected desktop/scripts typechecks and the 14 tests.
• two independent code reviews found no issues.

unverified: packaged electron startup, personal shell profiles, native macos and windows, and a real provider turn. this changes desktop process plumbing without changing the ui, so no screenshots apply. ci on the new merge head is pending.

[maria-rcks]

Note

Written by gpt-6.1-sol on behalf of Maria

Verified in a real dev environment

verified unchanged head 3ab7170a9977dabb2634d2c2f04c6aa7af0c381c on this linux boat vm.

• reproduced on github/main (6d46336476) in a separate worktree after vp i. a real login-shell executable wrote 2 mib to stderr before running the actual environment capture. desktop logged its 5,000 ms timeout and used a fallback shell. its trace recorded 5,009.979 ms for the failed probe.
• checked out the exact pr head, installed dependencies, and ran the real desktop dev app. the same noisy shell completed capture in 38.145 ms without timeout or fallback. the desktop-owned backend inherited its shell-only path directory. these measurements come from the actual startup path.
• created the project and thread through the app. a real codex gpt-6.1-sol turn in electron executed the requested command and returned the exact pr head with exit code 0. a follow-up sent through google chrome, driven by the repo playwright dependency against that same desktop-owned backend, found /usr/bin/git and returned PR21913_FOLLOWUP_OK, exit code 0.
• ordinary bash startup remained usable. a configured shell exiting 7 still logged the failure, fell back successfully, and reopened the persisted provider conversation after restart.
• blacksmith passed all 14 existing shell-environment tests and vp run check:changed against pr base 80a2351fcb909034e65a512dfabeb3cdfd367762: format, lint, both knip passes, affected typechecks, and focused tests. current pr checks have no failing or pending checks.

short runtime transcript

inspected the recording, trimmed loading frames, and checked playback and seeking after upload. the incidental .t3-pr21913 diff in the conversation is verification state captured by checkpoints, not source changes. no source changes or pushes were needed. all task dev processes were stopped.

Not verified

macos and windows shell probes, packaged release startup, native ios/android, and other providers were not exercised. the default linux gnome-libsecret path stalled automation here; integrated desktop/provider checks used the supported --password-store=basic option. default linux secret-store behavior remains unverified. the controlled noisy shell reproduces stderr backpressure; no personal shell profile was changed. there is no visual change requiring matching before/after screenshots.