Skip to content

fix(ssh): omit the PTY marker without a remote terminal - #858

Open
Aschenbath wants to merge 1 commit into
Kuddev:mainfrom
Aschenbath:feat/no-tty-marker-856
Open

Aschenbath wants to merge 1 commit into
Kuddev:mainfrom
Aschenbath:feat/no-tty-marker-856

Conversation

@Aschenbath

Copy link
Copy Markdown
Contributor

Result / 用户结果

Closes #856

An SSH pane's startup wrapper no longer prints PEBREL_PTY_<token>=not a tty:<pid> when tty cannot name a terminal, which is what the report shows for a JumpServer (KoKo) direct connection. With a terminal, the marker is sent and hidden as before.

Design / 设计边界

  • Responsibility and affected modules: ssh_session::resume::startup builds the POSIX startup wrapper. It now runs if pebrel_tty=$(tty 2>/dev/null); then printf '<marker>' "$pebrel_tty" "$$"; fi. When stdin is not a terminal, tty exits non-zero and prints not a tty, and that text used to become the marker value.
  • Why this belongs here; unchanged interfaces: IdentityInput still keeps foreign, malformed, partial and oversized markers byte-exact, as recorded in 2026-10-25-pane-session-recovery.md. Hiding malformed markers in the filter would drop bytes that contract preserves, so the wrapper simply stops producing one. A pane without a terminal still gets no PTY identity, because the old marker already failed PtyIdentity::parse. Its recovery behaviour is therefore unchanged.
  • Dependency, data-format, threading, or lifetime changes: none. pebrel_tty is not exported, so the exec'd shell's environment is unchanged. The script uses only the quoting and expansion forms that the startup probe already checks.
  • Compatibility and fallback: hosts with a terminal emit identical marker bytes. Not changed here: the Android adapter (TmuxSessionControl.kt, PtyIdentityInput.kt) has the same unguarded $(tty) and also keeps malformed markers visible. I can't build or test Android in this environment, so that is left for a follow-up.

Evidence / 验证依据

  • New regression test ssh_session::resume::tests::startup_reports_a_named_terminal_and_sends_no_marker_without_one runs the real startup script through sh twice. With an openpty terminal as stdin, the marker is parsed and hidden, and the identity equals ttyname. With null stdin, only the shell's own output is visible and no identity is recorded.
  • Before the fix (the new test against main's resume.rs): it fails with "\u{1e}PEBREL_PTY_0123…=not a tty:11056\u{1f}DEFAULT\n" instead of "DEFAULT\n", which is the reported symptom. The other six resume tests pass.
  • After the fix: cargo test --config .github/ci-profile.toml --profile ci --locked --workspace --features nebula/gpui-test-support --bin pebrel -- ssh_session::resume: 7 passed, 1 ignored on the final commit e0a152cb. The ignored test is the opt-in OpenSSH acceptance, which needs PEBREL_SSH_RESUME_FIXTURE and was not run.
  • Manual shell check of the final guard: under a real PTY (script), dash (/bin/sh here) and bash emit /dev/pts/N:<pid>. With stdin /dev/null or closed, or with no tty on PATH, the guard prints nothing to stdout or stderr.
  • cargo clippy -j 1 --locked -p nebula --no-deps --all-targets --features gpui-test-support: no diagnostics in resume.rs or resume/tests.rs. It ran on 3fe4efc8; the final commit differs only by adding 2>/dev/null inside the startup string literal. The command still fails on the existing deny-level backlog: 1,797 errors in other files of the pebrel test target. No lint configuration changed.
  • cargo fmt --all -- --check, git diff --check, python3 scripts/check_architecture.py --base 42ffd11ff95db61eacaf026319c0bfb3bed3bea1 and the three architecture unittest modules (55 passed): all passed.
  • Not completed: the full --bin pebrel suite. Both attempts were SIGKILLed while compiling the test binary, before any test ran: this shared 7.7 GiB VM ran out of memory during a concurrent build. Also not run: the i18n contract and nebula-settings tests (nothing changed there), and a real JumpServer/KoKo server or Windows client.

Required Review / 必须确认

  • I followed CONTRIBUTING.md, docs/architecture.md, and docs/project-constraints.md.
  • The fix stays in the existing startup authority; no duplicate behaviour authority was added.
  • python3 scripts/check_architecture.py --base 42ffd11ff95db61eacaf026319c0bfb3bed3bea1 passes; no budget changed.
  • Tests cover the terminal and no-terminal paths; validation limits are stated above.
  • No new UI messages; i18n is not applicable.
  • No governance changes.

🤖 Generated with Claude Code

A JumpServer (KoKo) direct login runs the startup wrapper without a
remote terminal. tty then prints "not a tty", the identity filter keeps
that malformed marker verbatim by design, and the pane shows
PEBREL_PTY_<token>=not a tty:<pid>.

Emit the marker only when tty succeeds. The filter contract is
unchanged, and such a pane gets no PTY identity, as before.

Closes Kuddev#856

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Aschenbath
Aschenbath requested a review from Kuddev as a code owner November 1, 2026 15:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant