fix: remote panes lose TERM_PROGRAM, breaking inline image protocols

[jvm]

Current behavior

Panes opened through herdr --remote <host> have no TERM_PROGRAM. Applications that pick an inline image protocol from it fall back to no-image rendering, so reading an image appears to succeed but displays nothing.

It is not remote-only. Pane children are seeded from a snapshot of the server process environment, and the server is persistent, so its terminal variables belong to whichever terminal started it:

• Remote: the server is started under sshd and has no terminal identity at all.
• Local: a server started under kitty and later attached from a terminal without inline image support still hands panes KITTY_WINDOW_ID, so applications select a protocol the real outer terminal cannot render.

apply_pane_terminal_env already normalizes TERM/COLORTERM for this reason and stops there.

Expected behavior

A pane advertises the terminal that actually displays it, which is the attached client's outer terminal, not the server's.

Reproduction

herdr --remote <host>
# in the remote pane:
printf 'TERM_PROGRAM=%q\n' "$TERM_PROGRAM"   # empty

Impact

Inline images do not render in remote panes. Any application keying off TERM_PROGRAM sees no terminal, or on local sessions sees a stale one.

Change

The client reports its outer terminal in Hello; the server stores it per connection and applies it at pane spawn. There is no other channel to a remote server: the bridge runs ssh -T, requests no pty, and forwards no environment.

Two ordering cases needed handling beyond the handshake:

• Panes are spawned while the app is built, before any client can attach. A restarting server therefore restores its whole session first, which is every pane a remote user lands in. The server is given the identity at launch — inherited from the spawning client locally, named on the bridge command line for remote.
• --no-session has no client, so it seeds from its own environment.

TERM stays xterm-256color; only the outer identity is propagated. With no identity available TERM_PROGRAM is removed rather than left stale.

Commits are separable. The second drops the other identity variables that no longer describe the attached client (KITTY_WINDOW_ID, LC_TERMINAL, ...); it can be dropped independently if you would rather not change what panes inherit.

Things to flag

• PROTOCOL_VERSION 18 -> 19. Unavoidable given the ssh boundary, but it is a breaking change: servers must restart, and mismatched client/server pairs reject each other. Source protocol equalled the latest released tag, so this follows the bump rule in AGENTS.md.
• TERM_PROGRAM=kitty is synthesized. Kitty sets no TERM_PROGRAM, so for kitty this reconstructs a value rather than restoring one. It is what consumers check, and fabricating KITTY_WINDOW_ID would be worse, but it is the one judgement call here and I would rather you see it than find it.
• Existing panes keep the environment they were spawned with; this cannot be changed after the fact.
• I looked for prior art and found Expose host terminal app identity to plugins #849, which proposed a similar client -> Hello -> server path for plugin metadata and was closed. This is scoped to the pane environment bug rather than that feature; say the word if you would still rather it went to Discussions first.

Verification

• just check clean on the pinned 1.96.1 toolchain. Three pre-existing agent-detection failures (live_handoff_keeps_agent_started_pane_after_agent_exits, live_handoff_keeps_unmanaged_agent_name_bound_to_saved_session, pane_info_and_subscriptions_expose_done_agent_status) reproduce identically on an unmodified checkout here.
• Tests added for the client-side identity, the pane environment with and without an attached client, and the bridge command including its shell quoting. Protocol fixtures and the schema artifact updated for the bump.
• Manually checked on macOS + Ghostty against a real Linux remote: the remote pane reports TERM_PROGRAM=ghostty, and an image read in a coding agent renders inline. Also checked locally that panes track the attaching client and that --no-session reports its own terminal.

Environment: herdr 0.7.5, stable, macOS 15 arm64, Ghostty local, Linux x86_64 remote.

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

@jvm, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 11 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: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 1670ea32-e397-452c-8030-7c6d04d7542e

📥 Commits

Reviewing files that changed from the base of the PR and between 3c08fd5 and 0df12e4.

📒 Files selected for processing (1)

• src/server/client_transport.rs

📝 Walkthrough

Walkthrough

The protocol version advances to 19, and client handshakes now carry optional outer terminal identity. The server propagates this identity to foreground pane environments, while monolithic and remote bridge paths initialize or forward it.

Changes

Outer terminal identity

Layer / File(s)	Summary
Terminal identity detection and pane environment src/terminal_notify.rs, src/pane.rs, src/main.rs	Terminal identities are detected, stored, initialized, propagated to panes, and stale identity variables are cleared.
Handshake contract and terminal metadata propagation src/protocol/wire.rs, src/client/mod.rs, src/server/..., tests/..., docs/next/api/herdr-api.schema.json	Protocol version 19 adds term_program to Hello messages and propagates normalized values through client connection events.
Headless foreground synchronization src/server/headless.rs	The headless server updates the stored terminal identity when foreground clients connect, change, or disconnect.
Remote bridge terminal forwarding src/remote/unix.rs	Remote bridge commands forward terminal identity through a shell-quoted environment assignment.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant ClientTransport
  participant HeadlessServer
  participant Pane
  Client->>ClientTransport: Send Hello with term_program
  ClientTransport->>HeadlessServer: Emit ClientConnected with normalized term_program
  HeadlessServer->>Pane: Set outer terminal program
  Pane->>Pane: Apply terminal identity to pane environment

Loading

Possibly related PRs

• herdrdev/herdr#2081: Both changes update protocol version 19 and related schema/tests, while this change adds terminal identity to the handshake.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the primary fix: preserving TERM_PROGRAM for remote panes to restore inline image protocol detection.
Description check	✅ Passed	The description directly explains the bug, expected behavior, implementation, compatibility impact, and verification for terminal identity propagation.
Docstring Coverage	✅ Passed	Docstring coverage is 94.34% which is sufficient. The required threshold is 80.00%.
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 unit tests (beta)

• Create PR with unit tests

---

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

[greptile-apps]

Greptile Summary

The PR propagates the attached client’s terminal identity into newly spawned panes.

• Adds term_program to the client handshake and bumps the wire protocol to 19.
• Tracks terminal identity per connection and synchronizes it with foreground-client transitions.
• Seeds terminal identity before local, remote, and no-session pane startup.
• Normalizes pane terminal variables and removes stale emulator-specific identifiers.
• Updates protocol fixtures, schema data, and terminal-environment tests.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename	Overview
src/client/mod.rs	Adds the detected outer-terminal identity to the client handshake.
src/protocol/wire.rs	Extends Hello with an optional terminal identity and bumps the protocol version.
src/server/client_transport.rs	Validates the reported identity and carries it into the server event stream.
src/server/clients.rs	Stores terminal identity as connection-scoped state.
src/server/headless.rs	Seeds startup identity and synchronizes pane identity with the foreground client lifecycle.
src/pane.rs	Applies the current terminal identity at pane spawn and removes stale terminal-specific variables.
src/remote/unix.rs	Passes terminal identity through the quoted remote bridge command for pre-attachment restoration.
src/terminal_notify.rs	Derives a normalized terminal-program name from explicit environment data or backend detection.

Sequence Diagram

sequenceDiagram
    participant T as Outer terminal
    participant C as Client
    participant S as Server
    participant P as New pane
    T->>C: Launch herdr
    C->>C: Detect terminal identity
    C->>S: Hello(term_program)
    S->>S: Store identity per connection
    S->>S: Select foreground client
    S->>P: Spawn with TERM_PROGRAM
    P-->>T: Render output through client

Loading

_{Reviews (4): Last reviewed commit: "fix: reject malformed client-reported te..." | Re-trigger Greptile}

[coderabbitai]

Actionable comments posted: 1

Caution

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

⚠️ Outside diff range comments (1)

src/server/headless.rs (1)

1052-1101: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Preserve the startup TERM_PROGRAM seed until a real client disconnects. src/server/headless.rs:1052 clears OUTER_TERM_PROGRAM whenever foreground_client_id is None, but handle_api_request_with_shutdown_check_inner() calls this before every API request, including the headless WorkspaceCreate/TabCreate requests from handle_deferred_requests_headless(). That wipes the launch identity seeded in run_server() before those panes are spawned. Move the reset into the actual detach/disconnect path instead of the generic no-client sync.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: b11d115b-fe40-474f-9420-01e02015d9f8

📥 Commits

Reviewing files that changed from the base of the PR and between b411274 and 1df625e.

📒 Files selected for processing (16)

• docs/next/api/herdr-api.schema.json
• src/client/mod.rs
• src/main.rs
• src/pane.rs
• src/protocol/wire.rs
• src/remote/unix.rs
• src/server/autodetect.rs
• src/server/client_transport.rs
• src/server/clients.rs
• src/server/headless.rs
• src/terminal_notify.rs
• tests/api_ping.rs
• tests/cross_area.rs
• tests/multi_client.rs
• tests/server_headless.rs
• tests/support/mod.rs

[jvm]

Pushed a fix for the ubuntu failure: tests/cli/sessions.rs had six more hardcoded protocol: 18 expectations that I missed when bumping the version.

I could not have caught that locally — tests/cli.rs and tests/auto_detect.rs are #![cfg(not(target_os = "macos"))], so the whole CLI suite is invisible on my machine and the macOS job passes without it. I have since run the full suite on a Linux box: 3114/3114 passing, plus cargo fmt --check and clippy --all-targets --locked -D warnings clean on Linux with the pinned 1.96.1 toolchain.

Grepped the macOS-excluded suites for any other assumptions this change could break; the remaining protocol references all derive from support::CURRENT_PROTOCOL. website/preview.json also pins protocol 18, but I left it alone since release CI owns it.

[jvm]

@coderabbitai both findings were real and are fixed in 3c08fd5.

Startup seed cleared before deferred panes spawn (Major). Confirmed: the sync_foreground_client_state() call at src/server/headless.rs:3194 is inside handle_api_request_with_shutdown_check_inner and runs before dispatch, so any API request with no client attached reached the no-client branch and dropped the seed — including the headless workspace/tab creates that spawn panes at startup, which is exactly what the seed exists for. I took your suggestion and moved the reset to promote_latest_remaining_client, the real detach transition (called from the three client-removal paths at 1474/1739/2601). The seed is deliberately not restored on detach: it described whoever started the server, not whoever just left.

Launch seed inherited by pane children (Minor). Fixed as you proposed, with the variable seeded in capture_shell_output and asserted cleared.

Both new tests were verified to fail with their fix reverted, so they pin the behavior rather than just passing. Full suite on Linux: 3117/3117, with fmt --check and clippy --all-targets --locked -D warnings clean on Linux and macOS under the pinned 1.96.1.

[coderabbitai]

Tip

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

@jvm Thanks for the thorough confirmation. The revised lifecycle is correct: preserving the launch seed through startup/deferred pane creation and clearing identity only on an actual client-detach transition addresses the major finding. The child-environment regression coverage addresses the minor finding as well.

🐇 ✅

---

✏️ Learnings added

Learnt from: jvm
URL: https://github.com/herdrdev/herdr/pull/2106

Timestamp: 2026-07-30T22:59:30.674Z
Learning: In `src/server/headless.rs`, the launched outer-terminal identity seed must survive headless startup API requests and deferred pane creation. It is cleared only as part of a real client-removal transition in `promote_latest_remaining_client`; it is not restored after detachment because it represents the terminal that launched the server, not the departing client.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

_{You are interacting with an AI system.}

[coderabbitai]

🧹 Nitpick comments (1)

src/server/client_transport.rs (1)

522-533: 🎯 Functional Correctness | 🔵 Trivial

Consider hardening the term_program filter beyond empty strings.

This is the sole normalization point for client-reported term_program before it flows (per the PR summary) into pane spawn environment. Only empty strings are filtered; NUL bytes or other control characters are passed through unchanged. Since this value is client-controlled (including over herdr --remote) and later used as an env var value for spawned processes, it's worth filtering out NUL bytes (and maybe other control characters) here as well, to avoid depending on downstream spawn-env handling to gracefully reject malformed values.

🛡️ Proposed hardening

-                term_program.filter(|value| !value.is_empty())
+                term_program.filter(|value| !value.is_empty() && !value.contains('\0'))

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: a37b1c48-7bc4-4a19-8d83-8eed37859a64

📥 Commits

Reviewing files that changed from the base of the PR and between 1df625e and 3c08fd5.

📒 Files selected for processing (17)

• docs/next/api/herdr-api.schema.json
• src/client/mod.rs
• src/main.rs
• src/pane.rs
• src/protocol/wire.rs
• src/remote/unix.rs
• src/server/autodetect.rs
• src/server/client_transport.rs
• src/server/clients.rs
• src/server/headless.rs
• src/terminal_notify.rs
• tests/api_ping.rs
• tests/cli/sessions.rs
• tests/cross_area.rs
• tests/multi_client.rs
• tests/server_headless.rs
• tests/support/mod.rs

🚧 Files skipped from review as they are similar to previous changes (2)

• tests/api_ping.rs
• tests/cross_area.rs

[jvm]

@coderabbitai took the nitpick too, in 0df12e4 — it sits on a real trust boundary, since the reported identity crosses the socket from a possibly remote client and lands in the spawn environment of every pane.

Went slightly wider than the proposed diff: rejecting all control characters rather than NUL alone, since no real TERM_PROGRAM contains one and the NUL case is only the subset that happens to be fatal at CString conversion. Extracted the predicate as normalize_reported_term_program with unit tests for the empty, NUL, newline, and escape-sequence cases.

Linux suite now 3119/3119, lint clean on Linux and macOS.

[coderabbitai]

Tip

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

@jvm Thanks—this is the right boundary to harden. Rejecting all control characters in normalize_reported_term_program prevents untrusted handshake data from affecting pane child environments, including the fatal NUL case and terminal-control injection cases. The focused regression tests cover the relevant inputs well.

🐇 ✅

---

✏️ Learnings added

Learnt from: jvm
URL: https://github.com/herdrdev/herdr/pull/2106

Timestamp: 2026-07-30T23:10:38.133Z
Learning: In the Rust terminal-identity handshake path, `TERM_PROGRAM` is client-reported data crossing a socket trust boundary before being applied to pane child-process environments. Normalize it with `normalize_reported_term_program` by treating empty values as absent and rejecting every Unicode control character (not only NUL); unit coverage should include empty, NUL, newline, and escape-sequence values.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

_{You are interacting with an AI system.}

[ogulcancelik]

thanks for the detailed work. i think the coding agent in your reproduction is pi: pi’s terminal ui chooses image support from TERM_PROGRAM and falls back to no images when it is absent.

we tested current herdr 0.7.5 over --remote against linux. yazi asks herdr whether kitty graphics are supported and renders the image correctly without TERM_PROGRAM. sending kitty graphics directly also works. the remote image path itself is not broken.

we do not want panes to claim they are running directly in the attached outer terminal. the outer client can change while a running process’s environment cannot, and forwarding the identity can make applications choose protocols herdr does not support. for example, yazi chooses the iterm image protocol when it sees wezterm, while herdr currently supports kitty graphics.

this requires fix on upstream pi.