fix(pty): strip inherited TERMINFO from tab child env

[charliek]

Summary

Roost forces TERM=xterm-256color on both UIs but let the launching terminal's TERMINFO pass through to tab children. When the app is launched from a terminal with a private terminfo DB (e.g. Ghostty, whose DB ships only ghostty/xterm-ghostty), strict $TERMINFO-only readers (some Go/Rust TUI terminfo loaders that don't fall back to /usr/share/terminfo the way ncurses does) find no entry for the TERM Roost advertises and can degrade to monochrome.

Found while investigating a reported single-color-TUI regression (ultimately not reproducible in any UI — this is the env-hygiene hardening that investigation surfaced; see docs/development/iced-migration-roadmap.md M0 on poc/iced).

• PtySupervisor.swift: remove TERMINFO in the child env; extract env construction into a pure childEnvironment() so the contract is testable without spawning a PTY (the PTY-touching Swift tests are disabled under swift-testing).
• roost-linux daemon/pty.rs: cmd.env_remove("TERMINFO") in the same spot TERM is forced.

Testing

• New ChildEnvironmentTests (TERMINFO stripped; TERM/COLORTERM/Roost contract forced) — 688 Swift tests pass.
• pty_injects_roost_env_vars extended: sets TERMINFO in the parent, asserts the spawned child's env output lacks it — cargo test -p roost-linux green.

🤖 Generated with Claude Code

https://claude.ai/code/session_01WLKWsLV45DAk61xG6Utj3e

Summary by CodeRabbit

• Bug Fixes
  • Fixed terminal sessions to ignore inherited TERMINFO settings that could cause incorrect terminal behavior.
  • Ensured terminal sessions consistently use the expected terminal configuration and required environment settings.
  • Preserved existing shell startup behavior for zsh and bash.

[coderabbitai]

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 40 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 3f0777b0-27f8-440b-b8c1-881659dae6e4

📥 Commits

Reviewing files that changed from the base of the PR and between 28d9f81 and 6969bbe.

📒 Files selected for processing (2)

• crates/roost-linux/tests/pty_env_terminfo.rs
• mac/Sources/Roost/PtySupervisor.swift

📝 Walkthrough

Walkthrough

PTY child environment construction now removes inherited TERMINFO on Linux and macOS. macOS centralizes terminal, Roost, resource, and shell bootstrap variables. Tests verify the sanitized environment and required values.

Changes

PTY environment handling

Layer / File(s)	Summary
Child environment construction crates/roost-linux/src/daemon/pty.rs, mac/Sources/Roost/PtySupervisor.swift	build_command removes inherited TERMINFO at crates/roost-linux/src/daemon/pty.rs:705. macOS uses childEnvironment to assemble terminal, Roost, resource, and shell bootstrap variables, then builds envp at mac/Sources/Roost/PtySupervisor.swift:548.
Environment behavior validation crates/roost-linux/tests/pty_smoke.rs, mac/Tests/RoostTests/ChildEnvironmentTests.swift	Linux tests set inherited TERMINFO and verify its removal. macOS tests verify TERMINFO removal and required TERM, COLORTERM, ROOST_TAB_ID, ROOST_SOCKET, and TERM_PROGRAM values.

Estimated code review effort: 3 (Moderate) | ~20 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: removing inherited TERMINFO from PTY child environments.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/pty-terminfo-env

---

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

[coderabbitai]

Caution

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

⚠️ Outside diff range comments (1)

crates/roost-linux/tests/pty_smoke.rs (1)

112-146: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Serialize and restore the environment mutation.

crates/roost-linux/tests/pty_smoke.rs:119 mutates the process environment while multi-threaded tests run. Other tests call PtySupervisor::spawn, which reads environment variables in crates/roost-linux/src/daemon/pty.rs:681-757; POSIX environment reads and writes are not thread-safe. Serialize this test with those tests and restore or remove the previous TERMINFO value with a cleanup guard.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/roost-linux/tests/pty_smoke.rs` around lines 112 - 146, Update
pty_injects_roost_env_vars to serialize its environment mutation with other
PtySupervisor::spawn callers, using the test’s existing synchronization
mechanism if available. Capture the prior TERMINFO value before
std::env::set_var and install cleanup that restores it or removes TERMINFO when
it was previously unset, including on failure or panic.

🧹 Nitpick comments (1)

mac/Sources/Roost/PtySupervisor.swift (1)

542-547: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Remove the leftover shouldBashBootstrap doc comment stacked above buildEnv.

mac/Sources/Roost/PtySupervisor.swift:542-547 still carries the doc comment that used to describe the inline shouldBashBootstrap function. That function moved to file scope at mac/Sources/Roost/PtySupervisor.swift:567-577, with its own copy of the same comment at lines 567-572. The old copy is now orphaned directly above buildEnv's doc comment (lines 548-549), which reads as if it explains buildEnv.

Delete the duplicate block so buildEnv only carries its own doc comment.

🧹 Proposed cleanup

-    /// Whether to bash-auto-bootstrap `resolvedArgv`: the pure predicate
-    /// (`bashAutobootstrap`) AND the shipped roost.bash being present at
-    /// `resourcesDir`. `--posix` and the ENV injection must be applied
-    /// together — a `--posix` shell with no ENV script to source would be
-    /// stuck in POSIX mode with no startup recreation — so `buildArgv` and
-    /// `buildEnv` both gate on this.
     /// Build the NULL-terminated envp array. Inherits the
     /// parent's environment then overlays Roost's injected vars.
     private func buildEnv(tabID: Int64, socketPath: String, argv: [String])

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@mac/Sources/Roost/PtySupervisor.swift` around lines 542 - 547, Remove the
orphaned duplicate shouldBashBootstrap documentation block immediately before
buildEnv in PtySupervisor.swift, leaving only buildEnv’s own doc comment there.
Preserve the identical documentation attached to the file-scope
shouldBashBootstrap function.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@crates/roost-linux/tests/pty_smoke.rs`:
- Around line 112-146: Update pty_injects_roost_env_vars to serialize its
environment mutation with other PtySupervisor::spawn callers, using the test’s
existing synchronization mechanism if available. Capture the prior TERMINFO
value before std::env::set_var and install cleanup that restores it or removes
TERMINFO when it was previously unset, including on failure or panic.

---

Nitpick comments:
In `@mac/Sources/Roost/PtySupervisor.swift`:
- Around line 542-547: Remove the orphaned duplicate shouldBashBootstrap
documentation block immediately before buildEnv in PtySupervisor.swift, leaving
only buildEnv’s own doc comment there. Preserve the identical documentation
attached to the file-scope shouldBashBootstrap function.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: fd8af035-428e-40a4-aeea-9263ab2d0485

📥 Commits

Reviewing files that changed from the base of the PR and between 3ffa7fa and 28d9f81.

📒 Files selected for processing (4)

• crates/roost-linux/src/daemon/pty.rs
• crates/roost-linux/tests/pty_smoke.rs
• mac/Sources/Roost/PtySupervisor.swift
• mac/Tests/RoostTests/ChildEnvironmentTests.swift