Repository navigation
fix(pty): strip inherited Claude child-session stamps at spawn (silent transcript-persistence loss) - #9961
Conversation
📝 WalkthroughWalkthroughDaemon-host PTY spawn preparation now removes inherited Claude child-session stamp environment variables in both spawn paths while retaining explicitly supplied values. Tests cover both cases. Terminal foreground reattachment now replays 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches⚔️ Resolve merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/main/ipc/pty.ts (1)
2800-2805: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the separate controller spawn path.
The new tests use
daemonSpawnAndGetOptions, while this runtime-controller path has its ownenvToDeleteconstruction. Add inherited and explicit-stamp assertions throughruntime.setPtyController().spawn, or verify that the existing helper exercises both paths, so they cannot regress independently.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 96a189b2-b4d0-432e-a223-d494cbf3d501
📥 Commits
Reviewing files that changed from the base of the PR and between 6ad6241 and 9aff79cb760f7de46d25587eba511d423d31b6d4.
📒 Files selected for processing (4)
src/main/ipc/pty.test.tssrc/main/ipc/pty.tssrc/renderer/src/components/terminal-pane/pty-connection.test.tssrc/renderer/src/components/terminal-pane/pty-connection.ts
| function getInheritedClaudeSessionStampEnvKeysToDelete( | ||
| spawnEnv: Record<string, string> | undefined | ||
| ): string[] { | ||
| const env = spawnEnv ?? {} | ||
| // Why: strip only values inherited from the pty host; a caller that explicitly | ||
| // provides a stamp (deliberately spawning a nested Claude child) keeps it. | ||
| return CLAUDE_CHILD_SESSION_STAMP_ENV_KEYS.filter((key) => env[key] === undefined) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | rg '(^|/)pty\.ts$|(^|/)mobile/\.oxlintrc\.json$|package\.json$' || true
echo "== target excerpt =="
if [ -f src/main/ipc/pty.ts ]; then
wc -l src/main/ipc/pty.ts
sed -n '660,760p' src/main/ipc/pty.ts | cat -n
fi
echo "== env key usages =="
rg -n "CLAUDE_CHILD_SESSION_STAMP_ENV_KEYS|getInheritedClaudeSessionStampEnvKeysToDelete|Claude.*stamp|session.*stamp|spawnEnv|deleteInherited|env\[" src/main/ipc src || true
echo "== subprocess env deletion patterns =="
rg -n "delete |Object\.keys|env\[.*\] === undefined|spawnEnv|Claude" -S src/main 2>/dev/null | head -n 200Repository: stablyai/orca
Length of output: 50369
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== pty.ts relevant sections =="
sed -n '180,230p' src/main/ipc/pty.ts | cat -n
sed -n '3650,3715p' src/main/ipc/pty.ts | cat -n
sed -n '1690,1735p' src/main/ipc/pty.test.ts | cat -n
echo "== deleteRequestedEnvKeys and env deletion helpers =="
rg -n "function deleteRequestedEnvKeys|const deleteRequestedEnvKeys|deleteRequestedEnvKeys|function remove.*EnvKeys|remove.*EnvKeys|for \\(const key of|delete env\\[key\\]" src/main/ipc/pty.ts
sed -n '2760,2825p' src/main/ipc/pty.ts | cat -n
echo "== read-only behavioral probe for case-sensitive missing-keys detection and delete implementation shape =="
python3 - <<'PY'
import re
from pathlib import Path
p = Path("src/main/ipc/pty.ts")
s = p.read_text()
m = re.search(r'function deleteRequestedEnvKeys\([^=]*=\)\s*{\s*([^}\n]*(?:\n[^}\n]*)*)\n\s*}\s*\n', s)
print("deleteRequestedEnvKeys found:", bool(m))
if m:
block = m.group(0)
# capture simple delete statements and loop semantics without executing repo code
deletes = re.findall(r'(?:for \([^)]*\) \{[^}]*\s*)?\s*(?:const|let|var)?\s*\w+\s*=\s*([^;\n]*);?\s*(?:return [^;\n]*;)?\s*(?:\}|;|}\s*$)', block)
print("delete block:")
print(block.split('\n', 80)[:30], "...")
print("explicit delete env[key] string within block:", bool(re.search(r'\bdelete\s+env\s*\[\s*key\s*\]', block)))
print("case-insensitive lookup string within block:", bool(re.search(r'\benv\s*\[\s*\w*\.toLowerCase\s*\[\s*\w*\]|\benv\s*\[\s*key\s*\]\.toLowerCase\s*\[\s*\w*', block)))
m = re.search(r'function getInheritedClaudeSessionStampEnvKeysToDelete\([^=.*]\)\s*{\s*([^}]*)\}', s)
print("getter find:", bool(m))
if m:
print(m.group(0))
node = re.search(r'const CLAUDE_CHILD_SESSION_STAMP_ENV_KEYS\s*=\s*\[[\s\S]*?\];', s)
if node:
print("CLAUDE_CHILD_SESSION_STAMP_ENV_KEYS:", node.group(0))
# Model requested deletion behavior with exact-key lookup from plain object.
CLAUDE_CHILD_SESSION_STAMP_ENV_KEYS = ['CLAUDE_CHILD_SESSION_STAMP_PATH_READER_FILE']
def requested(env, keys):
for key in keys:
if key in env:
del env[key]
env = {'CLUDE_CHILD_SESSION_STAMP_PATH_READER_FILE': 'bad'}
keys = ['CLAUDE_CHILD_SESSION_STAMP_PATH_READER_FILE']
requested(env, keys)
print("exact-delete preserves lowercase variant:", env)
print("matches expected filter:", ('CLAUDE_CHILD_SESSION_STAMP_PATH_READER_FILE' in {'CLUDE_CHILD_SESSION_STAMP_PATH_READER_FILE': 'bad'}))
PYRepository: stablyai/orca
Length of output: 12857
Make inherited-stamp deletion Windows case-insensitive.
getInheritedClaudeSessionStampEnvKeysToDelete() compares explicit spawn keys with uppercase stamp names, and deleteRequestedEnvKeys() deletes exact keys. On Windows, an inherited stamp with the canonical casing — e.g. claude_code_child_session, clause_code_session_id, or CLAUDE_CODE_BRIDGE_session_id — would not be requested for deletion, so it reaches the PTY and can disable Claude transcript persistence. Match explicit overrides at the same casing and delete inherited stamp keys case-insensitively on Windows, then add a casing-variant test.
Source: Coding guidelines
| // Why: a hard-killed agent leaves mouse/focus/kitty modes armed, and the | ||
| // surviving shell then receives pointer moves as typed SGR reports; the | ||
| // replay guard keeps xterm's auto-replies from leaking to the shell. | ||
| replayIntoTerminal(pane, deps.replayingPanesRef, POST_REPLAY_REATTACH_RESET, { | ||
| shouldRefreshViewportSynchronously: shouldRefreshForegroundSynchronously | ||
| }) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Flush queued terminal output before applying the reset.
Unlike writeReplayData at Lines 4948-4955, this direct replay does not call flushTerminalOutput first. Older queued PTY bytes can therefore parse after POST_REPLAY_REATTACH_RESET and re-arm stale TUI modes, leaving the confirmed shell contaminated.
Proposed fix
clearStaleAgentTabTitleOnConfirmedShell()
+ flushTerminalOutput(pane.terminal)
// Why: a hard-killed agent leaves mouse/focus/kitty modes armed, and the📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Why: a hard-killed agent leaves mouse/focus/kitty modes armed, and the | |
| // surviving shell then receives pointer moves as typed SGR reports; the | |
| // replay guard keeps xterm's auto-replies from leaking to the shell. | |
| replayIntoTerminal(pane, deps.replayingPanesRef, POST_REPLAY_REATTACH_RESET, { | |
| shouldRefreshViewportSynchronously: shouldRefreshForegroundSynchronously | |
| }) | |
| clearStaleAgentTabTitleOnConfirmedShell() | |
| flushTerminalOutput(pane.terminal) | |
| // Why: a hard-killed agent leaves mouse/focus/kitty modes armed, and the | |
| // surviving shell then receives pointer moves as typed SGR reports; the | |
| // replay guard keeps xterm's auto-replies from leaking to the shell. | |
| replayIntoTerminal(pane, deps.replayingPanesRef, POST_REPLAY_REATTACH_RESET, { | |
| shouldRefreshViewportSynchronously: shouldRefreshForegroundSynchronously | |
| }) |
A pty host (daemon or relay) launched from inside a Claude Code session inherits CLAUDE_CODE_CHILD_SESSION / CLAUDE_CODE_SESSION_ID / CLAUDE_CODE_BRIDGE_SESSION_ID, and the wholesale process.env spread then marks every spawned terminal as a nested Claude child. Claude silently disables transcript persistence for such sessions, so real user sessions lose their on-disk history with no visible error. Because daemons for older protocol versions are deliberately preserved across upgrades (and the auto-updater relaunch inherits the previous app's env), one contaminated launch propagates through every subsequent update until a fully clean-environment relaunch. Orca never sets these variables itself. Mirror the existing AGENT_HOOK_RUNTIME_ENV_KEYS pattern: a deny constant plus an inherited-only filter merged into envToDelete at both spawn call sites, so a stamp explicitly provided in args.env (a caller deliberately spawning a nested Claude child) is preserved while host-inherited values are dropped. Observed in production: a GUI launched on 2026-07-20 from an agent shell stamped three daemon generations (protocols v23-v25) across two auto-updates; every pane spawned in that window ran with transcript persistence silently off.
The nested mergePtyEnvDeletions composition passed a `string[] | undefined` intermediate into a `readonly string[]` parameter, so the patch did not typecheck. Make the helper variadic instead: spawn sites now list deletion sources flat in precedence order, and the six-deep nest at the renderer spawn site collapses. Tests only covered the renderer pty:spawn handler under the daemon adapter. Add the runtime-controller path (`orca` CLI / automations, a separate envToDelete assembly) and a local-provider case at each site, so a strip dropped at either site — or gated on isDaemonHostSpawn, which the daemon-only tests would not have caught — fails.
The strip makes envToDelete always non-empty, so the SSH route's exact spawn-options assertion no longer matched and provider-dispatch.test.ts went red (green on main, red on this branch — PR CI never caught it because the branch was conflicting and the suites never ran). Assert the stamp list exactly rather than loosening the assertion: SSH relay hosts can be launched from a Claude session too, so this now pins the SSH parity the fix claims and fails if the strip is dropped or if an unrelated key leaks into envToDelete on that path.
envToDelete is consumed by membership only, so pinning it positionally made the test fail on a pure reordering of the merge sources with zero behavior change. Compare sorted copies instead; the exact-set property that catches an unrelated key leaking onto the remote-host deny-list is preserved.
9aff79c to
8ddc08d
Compare
brennanb2025
left a comment
brennanb2025
left a comment
There was a problem hiding this comment.
Good find, and a nasty failure mode — silent, and it survives auto-updates. Rebased onto current main (#9608 landed separately, so this branch is now just the stamp fix) and pushed three review commits.
Two things needed fixing first. Neither was caught locally because oxlint/oxfmt were run but not typecheck, and CI never ran on this PR while it was conflicting:
- It didn't compile.
TS2345at the runtime-controller call site — nestingmergePtyEnvDeletionspasses astring[] | undefinedintermediate into itsreadonly string[]second parameter. Made the helper variadic instead, so spawn sites list deletion sources flat in precedence order; that also collapsed a six-deep nest at the renderer call site and benchmarks slightly faster (one Set instead of five). - It broke a green test.
provider-dispatch.test.ts > routes to SSH providerpasses onmainand failed here. Root cause is worth knowing: the stamps are essentially never inargs.env, so the filter always returns all three keys andenvToDeleteis now neverundefinedat either call site. That broke an exact spawn-options assertion. Fixed by asserting the stamp list on the SSH path (as a set —envToDeleteis consumed by membership only, so a reordering of the merge sources shouldn't be able to fail it).
Test coverage went 2 → 7. The original pair only covered the renderer pty:spawn handler under the daemon adapter. Added the runtime-controller path (orca CLI / automations — a separate envToDelete assembly), a local-provider case asserting the actual node-pty env, and the SSH path. That matrix matters because the strip is deliberately ungated on isDaemonHostSpawn: a daemon-only test still passes if someone later adds that gate. Five negative controls each fail exactly the intended tests.
Verified live on a naturally-contaminated host. Unpatched main: all three stamps present in a terminal pane. Patched: zero — while CLAUDE_CODE_ENTRYPOINT/_PROXY/_EXECPATH are still inherited, confirming the deny-list doesn't over-delete.
Two notes on the description, neither blocking:
CLAUDE_CODE_BRIDGE_SESSION_IDisn't set in an ordinary Claude Code shell — only the other two are. Stripping it is still correct and free, just less universal than described.- "both spawn paths" is narrower than it reads:
rate-limits/claude-pty.ts,claude-accounts/service.tsandtext-generation/commit-message-text-generation.tsspawnclaudewith rawprocess.env, so the stamps survive there. Deliberately out of scope — those are utility spawns with no user transcript at stake — but worth a follow-up.
One bound worth recording: envToDelete only reached the daemon protocol in #2350 (2026-05-25), so daemons preserved from older builds ignore it. The generations in your report (2026-07-20) are well after that, so the propagation fix holds in practice.
…yai#9961) An Orca GUI or daemon launched from inside a Claude Code session inherits CLAUDE_CODE_CHILD_SESSION / CLAUDE_CODE_SESSION_ID / CLAUDE_CODE_BRIDGE_SESSION_ID. Every spawn path spreads the host's process.env, so each terminal Orca opens is marked a nested Claude child and Claude silently disables transcript persistence — real sessions stop writing on-disk history with no visible error. Older-protocol daemons are deliberately preserved across upgrades and the auto-updater relaunch inherits the previous app's env, so one contaminated launch propagates through subsequent updates. Orca never sets these variables, so an inherited value is always poison. Add a deny constant plus an inherited-only filter merged into envToDelete at both pty spawn call sites, keeping a stamp explicitly passed in args.env. Unlike the agent-hook keys this is not gated on isDaemonHostSpawn, because the local provider and the relay host spread their own process.env too. Review fixes: mergePtyEnvDeletions is now variadic (the nested form passed a `string[] | undefined` intermediate into a `readonly string[]` parameter and did not typecheck); coverage extends to the runtime-controller spawn path, the local provider, and the SSH route, whose exact spawn-options assertion had to be updated because envToDelete is no longer ever undefined.
…arkers from PTY env (#10188) An Orca launched from inside a Claude Code session inherits CLAUDECODE, CLAUDE_CODE_EXECPATH, and CLAUDE_CODE_ENTRYPOINT alongside the child-session stamps #9961 already strips. Every pane then looks nested to Claude Code and to agent-aware tools, and the inherited entrypoint overrides Claude's own detection. Add them to the existing inherited-stamp deletion list so every local, daemon, and relay spawn drops them while explicit pane values win. Closes #9155 Co-authored-by: Neil <neil@stably.ai>
Problem
If an Orca GUI (or its daemon) is ever launched from inside a Claude Code session — a terminal where
claudeis running, a shell spawned by it, or an auto-update relaunch chained from such a launch — it inherits three environment variables:Both spawn paths start from a wholesale spread of the host's env (the daemon fork env in
daemon-init.ts,buildSpawnEnvinsrc/relay/pty-handler.ts), so every terminal Orca spawns inherits the stamps. Claude Code treats a stamped process as a nested child session and silently disables transcript persistence — the user's real sessions stop writing their on-disk history, with no visible error and no way to notice until a resume fails.Two otherwise-good design choices amplify the blast radius: older-protocol daemons are deliberately preserved across upgrades and never respawn, and the auto-updater relaunch inherits the previous app's env. So one contaminated launch propagates through every subsequent auto-update until a fully clean-environment relaunch. Measured on a production machine: a GUI launched once from a Claude session on 07-20 stamped three daemon generations (protocol v23–v25) across two auto-updates; every pane spawned in that window ran with persistence silently off.
Fix
Orca never sets these variables itself, so on the host they are always inherited poison. This mirrors the existing
AGENT_HOOK_RUNTIME_ENV_KEYSpattern insrc/main/ipc/pty.ts:CLAUDE_CHILD_SESSION_STAMP_ENV_KEYSdeny constant,getInheritedClaudeSessionStampEnvKeysToDelete()— the same inherited-only filter as the agent-hook helper, so a stamp explicitly passed inargs.env(a caller deliberately spawning a nested Claude child) is preserved,envToDeleteat both spawn call sites. Unlike the agent-hook keys this is not gated onisDaemonHostSpawn, because the relay host's ownprocess.envcan be contaminated the same way.Tests
Two tests in
pty.test.tsusing the existingdaemonSpawnAndGetOptionshelper: a contaminated host env puts all three keys inenvToDelete; an explicitargs.envstamp is preserved. The strip test fails against unpatched source (verified); the file suite is 304/304 with the patch.oxlint/oxfmtclean.This patch was authored by fable, an AI agent working on @akapug's machine, where the bug was found and measured live; @akapug reviewed and submitted.
ELI5
If Orca was started from inside a Claude session, it inherited Claude child-session env vars and silently broke transcript persistence for new sessions. Spawn paths now strip those stamps so each session keeps its own history.