Repository navigation
Add local shell command identity markers - #16915
brennanb2025 wants to merge 16 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis change adds OSC 777 shell command markers for Bash, Zsh, Fish, and PowerShell. It adds nonce-based injection policy, command decoding, shell identity v2 markers, and WSL propagation. PTY providers and daemon sessions scan markers and emit Merge Risk: 🔵 Low · up to The change adds local shell command identity markers and keeps them display-only, but a few bounded correctness issues remain around fish command lifecycle reporting, unusual command text, snapshot boundaries, and identity behavior during recovery. The PR is mergeable with explicit owner awareness and follow-up on these cases. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 28.85% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 52 functions across 57 files. (2 skipped: 2 unsupported.) Full details: Description checkExplanation The description provides detailed scope, rationale, safety notes, and testing results, but it omits several required template sections, including a Linked Issue, Visual Proof or an explicit N/A statement, AI Disclosure, Review, Agent skill upstream boundary, Notes, and the Checklist. Resolution Add the missing required sections. Provide a linked issue after "Fixes #"; add visual proof or state N/A with a reason; complete AI Disclosure, Review, Agent skill upstream boundary, Notes, and Checklist. Preserve the existing ELI5, scope, safety, and testing details.
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
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 |
7fe8475 to
b6b7144
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/providers/windows-shell-args.ts (1)
32-42: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPreserve the Codex preflight when the Git Bash wrapper is unavailable. If
ensureShellReadyWrappersAt()fails, the fallback starts Bash without the rcfile that containsgetPosixCodexShellLaunchPreflight().codexLaunchPreflightCommandis ignored on this path, so Codex can skipagent hooks prepare-codex.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: da25bfe4-9ab0-4754-aff3-e1f99483bb6d
📒 Files selected for processing (86)
.github/workflows/pr.ymlconfig/scripts/pr-code-change-scope.mjssrc/main/__fixtures__/shell-wrapper-snapshots/daemon-bash-rcfile.txtsrc/main/__fixtures__/shell-wrapper-snapshots/daemon-fish-init.txtsrc/main/__fixtures__/shell-wrapper-snapshots/daemon-zsh-zshenv.txtsrc/main/__fixtures__/shell-wrapper-snapshots/local-bash-rcfile.txtsrc/main/__fixtures__/shell-wrapper-snapshots/local-fish-init.txtsrc/main/__fixtures__/shell-wrapper-snapshots/local-zsh-zshenv.txtsrc/main/daemon/daemon-bash-shell-ready-rcfile.tssrc/main/daemon/daemon-server.tssrc/main/daemon/daemon-stream-events.tssrc/main/daemon/daemon-zsh-shell-ready-wrapper-spec.tssrc/main/daemon/pty-subprocess-env-inheritance.test.tssrc/main/daemon/pty-subprocess.tssrc/main/daemon/pty-subprocess/native-pty-spawn.tssrc/main/daemon/pty-subprocess/shell-launch-plan.tssrc/main/daemon/pty-subprocess/subprocess-handle.tssrc/main/daemon/session-command-marker-intake.tssrc/main/daemon/session-ingress-pipeline.tssrc/main/daemon/session-options.tssrc/main/daemon/session-snapshot-access.tssrc/main/daemon/session-subprocess-handle.tssrc/main/daemon/session-terminal-control.test.tssrc/main/daemon/session.tssrc/main/daemon/shell-ready-bash-wrapper.test.tssrc/main/daemon/shell-ready.test.tssrc/main/daemon/shell-ready.tssrc/main/daemon/terminal-host-options.tssrc/main/daemon/terminal-host-session-create.tssrc/main/daemon/terminal-host.tssrc/main/fish-command-marker-emission.live-shell.test.tssrc/main/ipc/pty/provider/local-configure.tssrc/main/powershell-osc133-bootstrap.test.tssrc/main/powershell-osc133-bootstrap.tssrc/main/providers/__tests__/shell-ready-framework/shell-script-test.tssrc/main/providers/local-pty-provider-io-events.test.tssrc/main/providers/local-pty-provider.tssrc/main/providers/local-pty-shell-ready-bash-rcfile.tssrc/main/providers/local-pty-shell-ready-test-harness.tssrc/main/providers/local-pty-shell-ready-wrapper-fileset.tssrc/main/providers/local-pty-shell-ready-wrapper-generation.test.tssrc/main/providers/local-pty-shell-ready.tssrc/main/providers/local-pty-utils-windows-fallback.test.tssrc/main/providers/local-pty-utils.tssrc/main/providers/pty-provider-events.tssrc/main/providers/windows-shell-args.test.tssrc/main/providers/windows-shell-args.tssrc/main/pty/wsl-orca-env.test.tssrc/main/pty/wsl-orca-env.tssrc/main/runtime/orca-runtime.tssrc/main/shell-command-marker-launch-policy.test.tssrc/main/shell-command-marker-scanner.test.tssrc/main/shell-command-marker-scanner.tssrc/main/shell-command-marker-template.tssrc/main/shell-integration-injection-policy.test.tssrc/main/shell-integration-injection-policy.tssrc/main/shell-startup-feature-channel.test.tssrc/main/shell-startup-features.tssrc/main/shell-startup-identity-scanner.test.tssrc/main/shell-startup-identity-scanner.tssrc/main/shell-startup-launch-intent-fixtures.tssrc/main/shell-templates.tssrc/main/shell-wrapper-generated-file-snapshot.test.tssrc/main/zsh-scoped-histfile.live-shell.test.tssrc/main/zsh-startup-hook-pty-harness.tssrc/main/zsh-startup-hook-user-config-equivalence.live-shell.test.tssrc/main/zsh-startup-wrapper-builder.tssrc/renderer/src/components/terminal-pane/parked-terminal-byte-watcher.test.tssrc/renderer/src/components/terminal-pane/parked-terminal-byte-watcher.tssrc/renderer/src/components/terminal-pane/pty-connection-test-store-fixtures.tssrc/renderer/src/components/terminal-pane/pty-connection-test-store-state.tssrc/renderer/src/components/terminal-pane/pty-connection/terminal-keydown-fit.tssrc/renderer/src/components/terminal-pane/terminal-side-effect-facts-handler.test.tssrc/renderer/src/components/terminal-pane/terminal-side-effect-facts-handler.tssrc/renderer/src/lib/tab-agent-signal-identity.tssrc/renderer/src/lib/use-tab-agent-command-identity.test.tssrc/renderer/src/lib/use-tab-agent.tssrc/renderer/src/store/index.tssrc/renderer/src/store/slices/pane-command-identity.test.tssrc/renderer/src/store/slices/pane-command-identity.tssrc/renderer/src/store/slices/store-test-helpers.tssrc/renderer/src/store/types.tssrc/shared/pane-agent-identity-inventory.test.tssrc/shared/terminal-side-effect-facts.tssrc/shared/wsl-login-shell-command.tstests/e2e/paired-runtime-rejected-input-remount.unit.test.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 3 remain after this review.
Review pass — CI diagnosed and fixed, plus one dead code pathCI was red at CI root causes
Fixed each at the cause. The store doubles now mirror the real slice's stale-epoch and A dead code path the tests could not seeEvery fish pane silently emitted no marker at all. Confirmed on a real fish 4.8.1 PTY before and after — Added PowerShell: emission was not gated on the same signal as stripping
This one is reasoned from the code and pinned by a generated-text test; I could not execute PowerShell on this host, so it is unverified on a real Windows box. Worth a second look from someone who can run it. RebaseRebased onto current That rebase also pushed Verified
Not fixed, worth your call
Not verifiedWindows and WSL were not exercised at runtime — no Windows host in this session. Dev-mode CDP is structurally blind to packaged-build differences, so the live check above covers the dev build only. The remote/SSH path was reasoned about from the code (no nonce injection on any relay provider) but not executed. |
Correction, and the real fish bugI got the headline of my last comment wrong. Correcting it before anything else. I claimed every fish pane emitted no marker because The The actual fish bug is on that same linefish single quotes do not interpret escapes, so GNU coreutils Fixed by mirroring bash/zsh ( Three more, all with executable reproszsh had no The scanner republished the nonce. This replaces the assertion at An over-cap unterminated candidate discarded the rest of the chunk. Both branches
Still open — your call, not mine
Not verifiedWindows and WSL were not exercised at runtime. The last CI red was |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/shell-command-marker-scanner.ts (1)
28-28: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winCount Unicode code points before rejecting the command.
TextDecoderreturns a UTF-16 string, socommand.lengthcounts each astral code point twice. A command withSHELL_COMMAND_MAX_CHARSemoji code points can fitMAX_ENCODED_CHARSbut is rejected here. The scanner then drops the marker instead of emittingcommand-started.Use
Array.from(command).lengthand add a regression test with'\u{1F642}'.repeat(SHELL_COMMAND_MAX_CHARS).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 3ae46597-3e65-4681-9e91-0012df65e89d
📒 Files selected for processing (12)
src/main/__fixtures__/shell-wrapper-snapshots/daemon-bash-rcfile.txtsrc/main/__fixtures__/shell-wrapper-snapshots/daemon-fish-init.txtsrc/main/__fixtures__/shell-wrapper-snapshots/daemon-zsh-zshenv.txtsrc/main/__fixtures__/shell-wrapper-snapshots/local-bash-rcfile.txtsrc/main/__fixtures__/shell-wrapper-snapshots/local-fish-init.txtsrc/main/__fixtures__/shell-wrapper-snapshots/local-zsh-zshenv.txtsrc/main/fish-command-marker-emission.live-shell.test.tssrc/main/shell-command-marker-scanner.test.tssrc/main/shell-command-marker-scanner.tssrc/main/shell-command-marker-template.tssrc/main/shell-templates.tssrc/main/zsh-startup-wrapper-builder.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/fixtures/shell-wrapper-snapshots/local-bash-rcfile.txt
- src/main/fixtures/shell-wrapper-snapshots/daemon-bash-rcfile.txt
Included review availability: Your plan provides up to 10 included reviews per hour; 2 remain after this review.
…marker nonce Three review findings on #16915: - fish emitted 133;C from fish_preexec with no matching 133;D, so a wrapped fish pane never reported command-finished: no clear-on-finish, no exit code, and every command fell back to an identity RPC instead of the D fast path. Adds a fish_postexec handler that captures $status first (any later command overwrites it) and is gated by the same markers-allowed predicate as C, so the pair stays balanced under tmux/screen. - selectShellStartupFeatures restricted command markers to bash/zsh/fish, so a plain PowerShell pane never received a nonce and the nonce-gated orca-cmd emitter this PR added to the PowerShell bootstrap could never fire. PowerShell is wrapped unconditionally, so selecting markers changes no startup-file chain. - the parked-terminal byte-watcher mock accepted stale writes and unconditional clears that createPaneCommandIdentitySlice rejects. The guards move to a shared test double so the mock cannot drift from the store again. Both behaviour fixes are pinned by tests that fail without them: real fish 4.8.1 via the inline `fish -C` launch form, and a plain-PowerShell selector case.
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 73e51540-40fc-4234-b6f5-32c6ff4c54a9
📒 Files selected for processing (8)
src/main/__fixtures__/shell-wrapper-snapshots/daemon-fish-init.txtsrc/main/__fixtures__/shell-wrapper-snapshots/local-fish-init.txtsrc/main/fish-command-marker-emission.live-shell.test.tssrc/main/shell-command-marker-template.tssrc/main/shell-startup-feature-channel.test.tssrc/main/shell-startup-features.tssrc/renderer/src/components/terminal-pane/pane-command-identity-test-double.tssrc/renderer/src/components/terminal-pane/parked-terminal-byte-watcher.test.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 1 remains after this review.
Round 2 — CI green, all four threads settledHead
|
| path:line | verdict | fix | reply |
|---|---|---|---|
__fixtures__/.../daemon-fish-init.txt:22 — fish OSC 133 lifecycle |
Real | 42cfe29a6e |
r3880443864 |
daemon/session-snapshot-access.ts:32 — drain intake before final snapshot |
Not real | — | r3880444001 |
shell-startup-features.ts:62 — markers for PowerShell |
Real | 42cfe29a6e |
r3880444144 |
parked-terminal-byte-watcher.test.ts:103 — mock vs store guards |
Real | 42cfe29a6e |
r3880444272 |
Two are worth calling out because the labels were misleading in both directions.
fish (Major, correct). The PR made fish emit 133;C from fish_preexec with nothing emitting 133;D. main had no fish wrapper at all, so this PR introduced the unbalanced half — fish was the only wrapped shell opening the lifecycle without closing it. onCommandFinished (pane-foreground-agent-tracker.ts:284) is the sole path that clears command identity and fast-paths back to shellForeground: true, so every fish command instead fell through to an identity RPC and nothing downstream ever fired. Fixed with a fish_postexec handler that captures $status first (the allow-check is itself a command and would clobber it) and is gated by the same predicate as C, so the pair stays balanced under tmux.
Drain (Minor, wrong). commandMarkerIntake.drain() already runs at session.ts:362 — after drainAndClose(), before flushPending(), exactly the requested order. The suggested second drain site would also republish the withheld orca-cmd nonce prefix into a persisted snapshot, which is the leak fixed earlier in this PR.
Both behaviour fixes are pinned by tests that fail without them (verified by ablation): real fish 4.8.1 driven through the inline fish -C form the launcher actually uses, and a plain-PowerShell selector case.
Still yours to decide — unchanged from round 1
markersis an overloaded feature name, so the kill switch disables all OSC 133, not just command markers.- Every bash / Git Bash pane stops being a login shell.
I deliberately did not let any thread move these. Thread 3 touches the same selector, so for the record: adding PowerShell changes no startup-file chain — that branch already returns mode: 'wrapped' unconditionally, so it only supplies the nonce its emitter needs. It has no bearing on either open question.
Not verified
No Windows or WSL runtime in this session — the PowerShell fix is reasoned from the code plus the unit selector test, and the orca-cmd emission it unblocks was never executed. pnpm tc clean, oxlint clean on changed files, 806 tests green across the provider/daemon/shell suites locally. pnpm run check:code-quality:changed could not run here (its oxlint JSON parser crashes under Node 26); oxlint was run directly on the changed files instead.
Greptile SummaryThe PR adds authenticated shell command-start markers for local, daemon, and WSL terminals and uses the resulting display-only facts when resolving tab agent identity.
Confidence Score: 3/5The PR does not appear safe to merge until split-marker sequence accounting and lossy malformed-marker handling are corrected. Split markers still inflate local PTY raw positions by charging retained bytes against a later emission, and malformed terminated marker-shaped output is still silently removed from the terminal stream. Files Needing Attention: src/main/providers/local-pty-session-activation.ts; src/main/shell-command-marker-scanner.ts
|
| Filename | Overview |
|---|---|
| src/main/shell-command-marker-scanner.ts | Implements bounded marker parsing and stripping, but still discards terminated malformed private-prefix output. |
| src/main/providers/local-pty-session-activation.ts | Integrates marker scanning with local PTY sequence accounting, where split markers still inflate the reported raw cursor. |
| src/main/daemon/session-command-marker-intake.ts | Integrates marker scanning into daemon ingress using persistent raw-sequence accounting. |
| src/main/pty/wsl-orca-env.ts | Forwards shell marker feature and nonce inputs into WSL for guest-side wrapper initialization. |
| src/renderer/src/lib/tab-agent-signal-identity.ts | Places trusted command identity below focused hooks and process evidence but above title evidence. |
Sequence Diagram
sequenceDiagram
participant S as Shell integration
participant P as Local/daemon PTY authority
participant R as Renderer state
participant T as Terminal tab
S->>P: Private nonce-bearing command marker
P->>P: Scan, validate, and strip marker bytes
P-->>R: Normalized agent, trust, and command epoch
P-->>T: Ordinary terminal output only
R->>T: Resolve agent identity by evidence precedence
Reviews (7): Last reviewed commit: "fix(shell): emit the alias-resolved comm..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| let rawCursor = emission.rawStartSeq | ||
| for (const item of commandMarkerScanner.accept(emission.data)) { | ||
| if (item.kind === 'data') { | ||
| rawCursor += item.data.length | ||
| emitProviderData(item.data, item.data.length, false) | ||
| continue | ||
| } | ||
| rawCursor += item.rawLength | ||
| emitProviderData('', item.rawLength, true, rawCursor) |
There was a problem hiding this comment.
When a command marker spans two ingress emissions, rawCursor starts at the second emission while item.rawLength includes bytes retained from the first. This double-counts the retained prefix, reports a high-water position beyond the real stream, and can trigger incorrect gap recovery or mishandle subsequent output.
Knowledge Base Used: Terminal runtime and interaction
| const fields = candidate.slice(COMMAND_MARKER_PREFIX.length, terminatorStart).split(';') | ||
| const command = | ||
| fields.length === 2 && fields[0]!.length <= MAX_NONCE_CHARS | ||
| ? decodeCommand(fields[1] as string) | ||
| : null | ||
| if (command === null) { | ||
| // Why dropped and not forwarded: nothing else may emit this private prefix, and | ||
| // the row carries the nonce — re-emitting it would publish the nonce downstream. | ||
| } else { |
There was a problem hiding this comment.
A terminated OSC sequence that shares the private prefix but fails field or base64 validation is consumed without producing either terminal data or a fact. This makes the metadata parser lossy for colliding application output and can remove all text through the next BEL or ST terminator.
Knowledge Base Used: Terminal runtime and interaction
`native-smoke (windows-latest)` fails ~4.5% of runs on `preserves a multiline argument through the compiled remote launcher` with "Test timed out in 15000ms" — on unrelated PRs, for reasons that have nothing to do with them. Across 176 sampled attempts it is the only red that job produced, and it hit seven different PRs in two days: #16900, #16904, #16915, #16955 (twice), #16979, #17014, #17085. The test is six process creations: powershell.exe forks csc.exe, then the freshly compiled orca.exe forks node.exe, twice. Hosted Windows runners periodically slow process creation down, and this test amplifies that far harder than anything else in the job. Comparing the 80 attempts where it ran under 3s against the 12 where it ran over 12s, its own median goes 2198ms -> 15917ms (7.2x) while the same file's powershell-only test moves 556 -> 686ms (1.2x), the cmd.exe and Git Bash process tests in the neighbouring file move 1.4x, and the other 35 files put together move 1.5x. Measured across those 176 attempts: 1881ms to 35438ms, p50 4264ms, correlation +0.881 with the job's total Vitest duration. 8 of 176 (4.5%) exceeded the 15s cap; 2 of 176 (1.1%) also exceeded the shared 30s testTimeout, so deleting the override and inheriting the config is not enough on its own. 60s clears all 176 with 1.7x headroom on the worst. This is slow, not hung. Every body here is synchronous spawnSync, so Vitest cannot interrupt one — the timer fires only after the body returns and the reported duration is real elapsed time. That is why a failure reads `× ... 22464ms` under `Test timed out in 15000ms`. The work finished; the stopwatch was short. Seven reruns at one identical head measured 2053 / 4680 / 5551 / 8732 / 13506 / 14868 / 21937ms — the last of those would have been red on code that had not changed. The 15s came from #8897, which raised this test off Vitest's built-in 5s default because the job then ran bare `pnpm vitest run`. #8909 landed 3h27m later and pointed the job at config/vitest.config.ts, which is the real fix for that. The constant stayed behind and has been the binding budget ever since.
`set -l` scoped the fish marker nonce to the sourced init file, which is already out of scope when `fish_preexec` fires, so every fish pane failed `__orca_command_markers_allowed` and emitted no marker at all. The generated script looked correct, so only a live shell shows it. Also repairs the four hand-rolled renderer store doubles that omit the new pane-command-identity slice, the agent-identity inventory path that moved to tab-agent-signal-identity.ts, and the env-inheritance expectation now that `markers` rides along on every startup-command pane. Adds a real-fish contract test (red before this change, green after) and puts the marker sources in the shell-contracts path scope so the lane actually runs when they change.
…b-agent Rebasing onto current main put use-tab-agent.ts at 303 counted lines against the renderer's 300 ceiling: main grew the file and this branch's command-identity rung tipped it over. `titleShowsNoAgent` and `resolveLaunchedAgentExitEvidence` are a self-contained "did the launched agent exit" question, so they move to tab-agent-exit-evidence.ts rather than the file carrying a suppression. No behavior change; the exit-evidence test now imports from the new module.
CommandMarkersAllowed accepted the multiplexer heuristic on its own, so it never required ORCA_SHELL_INTEGRATION_CONTEXT to be present at all. The PTY authority only builds its marker scanner when it set that variable, and the OSC 133 bootstrap is installed unconditionally — so a pane whose markers were turned off by the per-host-class kill switch would still emit orca-cmd rows with nothing to strip them, painting them into the terminal. Require the context in both arms. The nonce cannot be the gate here: Windows 10 deliberately emits untrusted rows with no nonce. Reasoned from the code and pinned by a generated-text assertion; not executed against a real PowerShell host.
…ounds Corrects an earlier claim of mine on this branch: fish markers were NOT dead. The launcher passes the init text straight to `fish -C`, whose top-level scope the event handler reaches, so `set -l` worked. That earlier `set -g` is kept — it states the requirement rather than depending on that — but it fixed nothing, and the test that "proved" it sourced the init from a file, which is not how the launcher runs it. The test now drives the real `-C` form. The real fish bug is on the same line: `string replace -a '\n'` never matches a newline, because fish single quotes do not interpret escapes. GNU coreutils base64 wraps at 76 columns (macOS does not), so on every Linux/WSL/SSH fish host any command over 57 bytes produced a space-bearing payload that the scanner's base64 test rejected — markers silently dropped. Mirrors bash/zsh's `tr` instead. zsh: __orca_osc133_preexec had no `emulate -L zsh`, so under KSH_ARRAYS the unbraced commands[base64] test parsed as arithmetic and printed "bad output format specification" before every command, while the capture lost its first character. Its sibling __orca_deferred_init already guards for exactly this. Scanner: the shells truncate in characters but the cap was computed in bytes, so a multibyte command over ~1366 chars failed to decode and the raw row — nonce included — was republished into the pane's own stream. An undecodable private row is now dropped rather than forwarded, and an over-cap unterminated candidate keeps scanning instead of discarding the rest of the chunk (9016 chars lost in the repro). Adds a split-fuzz test over every chunk boundary. awk in the startup identity block is now guarded like every sibling around it.
…marker nonce Three review findings on #16915: - fish emitted 133;C from fish_preexec with no matching 133;D, so a wrapped fish pane never reported command-finished: no clear-on-finish, no exit code, and every command fell back to an identity RPC instead of the D fast path. Adds a fish_postexec handler that captures $status first (any later command overwrites it) and is gated by the same markers-allowed predicate as C, so the pair stays balanced under tmux/screen. - selectShellStartupFeatures restricted command markers to bash/zsh/fish, so a plain PowerShell pane never received a nonce and the nonce-gated orca-cmd emitter this PR added to the PowerShell bootstrap could never fire. PowerShell is wrapped unconditionally, so selecting markers changes no startup-file chain. - the parked-terminal byte-watcher mock accepted stale writes and unconditional clears that createPaneCommandIdentitySlice rejects. The guards move to a shared test double so the mock cannot drift from the store again. Both behaviour fixes are pinned by tests that fail without them: real fish 4.8.1 via the inline `fish -C` launch form, and a plain-PowerShell selector case.
…capability fish_preexec returns before emitting OSC 133;C when base64 is unavailable, but fish_postexec still emitted OSC 133;D. The scanner turns any D into a command-finished fact without requiring a preceding C, and onCommandStarted is what cancels the deferred identity drop a D schedules -- so on a fish host with no base64 every command finish retired the pane's live agent identity with nothing left to invalidate it. base64 is the only condition that can stop preexec emitting C, so the same guard makes the two markers a pair.
33a494c added the `type -q base64; or return` guard to the fish __orca_command_finished body in shell-command-marker-template.ts but did not regenerate the two generated-file fixtures, so shell-wrapper-generated-file-snapshot.test.ts has been failing shard 8/8 on the recorded-vs-generated comparison ever since. Fixture-only: both files gain exactly the four lines the template now emits. The snapshot still pins the whole generated file, so the assertion is unchanged in strength -- it now records the reviewed generator output instead of the pre-gate one.
b403848 to
17dd552
Compare
`native-smoke (windows-latest)` fails ~4.5% of runs on `preserves a multiline argument through the compiled remote launcher` with "Test timed out in 15000ms" — on unrelated PRs, for reasons that have nothing to do with them. Across 176 sampled attempts it is the only red that job produced, and it hit seven different PRs in two days: #16900, #16904, #16915, #16955 (twice), #16979, #17014, #17085. The test is six process creations: powershell.exe forks csc.exe, then the freshly compiled orca.exe forks node.exe, twice. Hosted Windows runners periodically slow process creation down, and this test amplifies that far harder than anything else in the job. Comparing the 80 attempts where it ran under 3s against the 12 where it ran over 12s, its own median goes 2198ms -> 15917ms (7.2x) while the same file's powershell-only test moves 556 -> 686ms (1.2x), the cmd.exe and Git Bash process tests in the neighbouring file move 1.4x, and the other 35 files put together move 1.5x. Measured across those 176 attempts: 1881ms to 35438ms, p50 4264ms, correlation +0.881 with the job's total Vitest duration. 8 of 176 (4.5%) exceeded the 15s cap; 2 of 176 (1.1%) also exceeded the shared 30s testTimeout, so deleting the override and inheriting the config is not enough on its own. 60s clears all 176 with 1.7x headroom on the worst. This is slow, not hung. Every body here is synchronous spawnSync, so Vitest cannot interrupt one — the timer fires only after the body returns and the reported duration is real elapsed time. That is why a failure reads `× ... 22464ms` under `Test timed out in 15000ms`. The work finished; the stopwatch was short. Seven reruns at one identical head measured 2053 / 4680 / 5551 / 8732 / 13506 / 14868 / 21937ms — the last of those would have been red on code that had not changed. The 15s came from #8897, which raised this test off Vitest's built-in 5s default because the job then ran bare `pnpm vitest run`. #8909 landed 3h27m later and pointed the job at config/vitest.config.ts, which is the real fix for that. The constant stayed behind and has been the binding budget ever since.
…-slice1a # Conflicts: # src/main/providers/local-pty-provider.ts
`native-smoke (windows-latest)` fails ~4.5% of runs on `preserves a multiline argument through the compiled remote launcher` with "Test timed out in 15000ms" — on unrelated PRs, for reasons that have nothing to do with them. Across 176 sampled attempts it is the only red that job produced, and it hit seven different PRs in two days: #16900, #16904, #16915, #16955 (twice), #16979, #17014, #17085. The test is six process creations: powershell.exe forks csc.exe, then the freshly compiled orca.exe forks node.exe, twice. Hosted Windows runners periodically slow process creation down, and this test amplifies that far harder than anything else in the job. Comparing the 80 attempts where it ran under 3s against the 12 where it ran over 12s, its own median goes 2198ms -> 15917ms (7.2x) while the same file's powershell-only test moves 556 -> 686ms (1.2x), the cmd.exe and Git Bash process tests in the neighbouring file move 1.4x, and the other 35 files put together move 1.5x. Measured across those 176 attempts: 1881ms to 35438ms, p50 4264ms, correlation +0.881 with the job's total Vitest duration. 8 of 176 (4.5%) exceeded the 15s cap; 2 of 176 (1.1%) also exceeded the shared 30s testTimeout, so deleting the override and inheriting the config is not enough on its own. 60s clears all 176 with 1.7x headroom on the worst. This is slow, not hung. Every body here is synchronous spawnSync, so Vitest cannot interrupt one — the timer fires only after the body returns and the reported duration is real elapsed time. That is why a failure reads `× ... 22464ms` under `Test timed out in 15000ms`. The work finished; the stopwatch was short. Seven reruns at one identical head measured 2053 / 4680 / 5551 / 8732 / 13506 / 14868 / 21937ms — the last of those would have been red on code that had not changed. The 15s came from #8897, which raised this test off Vitest's built-in 5s default because the job then ran bare `pnpm vitest run`. #8909 landed 3h27m later and pointed the job at config/vitest.config.ts, which is the real fix for that. The constant stayed behind and has been the binding budget ever since.
* fix(ssh): fence stale kills and retired pane replay * fix(ssh): support cancellable interactive authentication * fix(ssh): await remote catalog before snapshot adoption * fix(pty): contain Windows ConPTY input failures * fix(power): avoid redundant macOS display blocking * perf(editor): narrow markdown override subscriptions * fix(quick-open): close directory handles after reads * refactor(linux): remove unused proc socket scanner * fix(usage): apply flat Sonnet 4.6 pricing * ci: prime Node next native test cache * docs(skills): resolve snapshot cleanup data path * fix(ssh): recover install locks after host reboot * test(ssh): recognize boot-aware install locks * test(ssh): prove previous-boot lock recovery live * test(wire): pin pre-metadata release coverage * fix(terminal): preserve remote tab ownership through recovery races * test(runtime): fence replaced terminal handles in agent guard * fix(ssh): preserve remote snapshot authority across polls * fix(pty): contain late ConPTY output EPIPE * test(pty): register Windows exit watcher before kill * fix: close SSH and tab readiness race gaps * fix(tabs): retain headless order and placeholder titles * fix(build): avoid parallel electron-vite config race * test(windows): avoid MSYS temp path rewriting * test(windows): avoid killing exited PTY * fix(pty): avoid late ConPTY input teardown race * fix(terminal): sync reconnect error ownership after commit * fix(runtime): use canonical worktree identity comparison * test(ssh): assert complete cold-hydration baseline * test(windows): invoke quoted retention fixture via PowerShell * test(windows): read ConPTY grid through mode con * fix(terminal): publish PTY replacements atomically * fix(terminal): infer stale identity on reattach * fix(terminal): fence stale pane PTY callbacks * fix(terminal): fence stale pane binds after rebind * fix(terminal): reject stale pane transport callbacks * fix(terminal): fence mirrored reattach spawn callbacks * fix(terminal): replace stale pane PTYs on remount * fix(ci): size the Windows launcher-compile test budget from measurement `native-smoke (windows-latest)` fails ~4.5% of runs on `preserves a multiline argument through the compiled remote launcher` with "Test timed out in 15000ms" — on unrelated PRs, for reasons that have nothing to do with them. Across 176 sampled attempts it is the only red that job produced, and it hit seven different PRs in two days: #16900, #16904, #16915, #16955 (twice), #16979, #17014, #17085. The test is six process creations: powershell.exe forks csc.exe, then the freshly compiled orca.exe forks node.exe, twice. Hosted Windows runners periodically slow process creation down, and this test amplifies that far harder than anything else in the job. Comparing the 80 attempts where it ran under 3s against the 12 where it ran over 12s, its own median goes 2198ms -> 15917ms (7.2x) while the same file's powershell-only test moves 556 -> 686ms (1.2x), the cmd.exe and Git Bash process tests in the neighbouring file move 1.4x, and the other 35 files put together move 1.5x. Measured across those 176 attempts: 1881ms to 35438ms, p50 4264ms, correlation +0.881 with the job's total Vitest duration. 8 of 176 (4.5%) exceeded the 15s cap; 2 of 176 (1.1%) also exceeded the shared 30s testTimeout, so deleting the override and inheriting the config is not enough on its own. 60s clears all 176 with 1.7x headroom on the worst. This is slow, not hung. Every body here is synchronous spawnSync, so Vitest cannot interrupt one — the timer fires only after the body returns and the reported duration is real elapsed time. That is why a failure reads `× ... 22464ms` under `Test timed out in 15000ms`. The work finished; the stopwatch was short. Seven reruns at one identical head measured 2053 / 4680 / 5551 / 8732 / 13506 / 14868 / 21937ms — the last of those would have been red on code that had not changed. The 15s came from #8897, which raised this test off Vitest's built-in 5s default because the job then ran bare `pnpm vitest run`. #8909 landed 3h27m later and pointed the job at config/vitest.config.ts, which is the real fix for that. The constant stayed behind and has been the binding budget ever since. * fix(terminal): fence stale remount reattach ownership * fix(terminal): reconcile mounted pane identity after replacement * fix(terminal): fence stale reattach fallback ownership * fix(terminal): fence deferred SSH reattach ownership * fix(terminal): fence stale split pane ownership callbacks * fix(terminal): keep stale spawns from consuming startup --------- Co-authored-by: Brennan Benson <79079362+brennanb2025@users.noreply.github.com>
|
SSH validation on real Linux host (Orca SSH provider) Host: Results:
Verdict: DO NOT LAND for the stated SSH safety claim. Normal Orca-injected SSH commands do not emit markers and identity is unchanged, but the explicit boundary/kill requirement is disproved: remote private marker bytes (including a truncated nonce-bearing fragment) cross into the client raw stream, and a complete marker is retained by the main buffer snapshot. The relay/SSH path is unchanged as designed, so this cannot be called a pass for “private marker bytes may not cross the SSH boundary.” |
Correction to the verdict aboveThe This PR touches zero relay and zero SSH files. Against That means the adversarial result — a remote The evidence gathered actually supports this: the bytes arrived unstripped. If this PR had modified the SSH path, they would have been stripped, because stripping is what it adds. Finding them unstripped is confirmation the SSH path is untouched. What the adversarial case does and doesn't showA PTY relay forwards bytes; a remote program can print any escape sequence. The consequences here were inert — not painted, and the identity/foreground store maps stayed empty, so a forged marker created no identity. There was also no real nonce to leak: nothing on an SSH host emits one, so the nonce in the probe was invented by the test. What the validation got right, and is being fixed1. The wording in this PR was too strong. "Neither the private marker bytes nor the pane command identity may cross the SSH boundary" overstates the guarantee. What actually holds, and what the normal-path evidence confirms: Orca does not emit markers on SSH panes, and no pane command identity is derived from remote output. Typed 2. A real forward risk for the next slice. When #17737 lands and the SSH path does scan markers, a hostile or careless remote program could forge one. That is precisely what the nonce is for — a forgery fails the check, is marked 3. Coverage limit, stated plainly. Revised verdict: land, with the wording above corrected. |
Readiness review (2026-09-01)Verdict: LAND (no release-blocking defect found in this PR). The PR ref was verified at Platform matrix
Confirmed coverage limits (documented here for users)
No code changes were needed. Cleanup removed the temporary SSH/Windows worktree sessions after evidence collection; the PR worktree remains clean. |
* fix(ssh): fence stale kills and retired pane replay * fix(ssh): support cancellable interactive authentication * fix(ssh): await remote catalog before snapshot adoption * fix(pty): contain Windows ConPTY input failures * fix(power): avoid redundant macOS display blocking * perf(editor): narrow markdown override subscriptions * fix(quick-open): close directory handles after reads * refactor(linux): remove unused proc socket scanner * fix(usage): apply flat Sonnet 4.6 pricing * ci: prime Node next native test cache * docs(skills): resolve snapshot cleanup data path * fix(ssh): recover install locks after host reboot * test(ssh): recognize boot-aware install locks * test(ssh): prove previous-boot lock recovery live * test(wire): pin pre-metadata release coverage * fix(terminal): preserve remote tab ownership through recovery races * test(runtime): fence replaced terminal handles in agent guard * fix(ssh): preserve remote snapshot authority across polls * fix(pty): contain late ConPTY output EPIPE * test(pty): register Windows exit watcher before kill * fix: close SSH and tab readiness race gaps * fix(tabs): retain headless order and placeholder titles * fix(build): avoid parallel electron-vite config race * test(windows): avoid MSYS temp path rewriting * test(windows): avoid killing exited PTY * fix(pty): avoid late ConPTY input teardown race * fix(terminal): sync reconnect error ownership after commit * fix(runtime): use canonical worktree identity comparison * test(ssh): assert complete cold-hydration baseline * test(windows): invoke quoted retention fixture via PowerShell * test(windows): read ConPTY grid through mode con * fix(terminal): publish PTY replacements atomically * fix(terminal): infer stale identity on reattach * fix(terminal): fence stale pane PTY callbacks * fix(terminal): fence stale pane binds after rebind * fix(terminal): reject stale pane transport callbacks * fix(terminal): fence mirrored reattach spawn callbacks * fix(terminal): replace stale pane PTYs on remount * fix(ci): size the Windows launcher-compile test budget from measurement `native-smoke (windows-latest)` fails ~4.5% of runs on `preserves a multiline argument through the compiled remote launcher` with "Test timed out in 15000ms" — on unrelated PRs, for reasons that have nothing to do with them. Across 176 sampled attempts it is the only red that job produced, and it hit seven different PRs in two days: stablyai#16900, stablyai#16904, stablyai#16915, stablyai#16955 (twice), stablyai#16979, stablyai#17014, stablyai#17085. The test is six process creations: powershell.exe forks csc.exe, then the freshly compiled orca.exe forks node.exe, twice. Hosted Windows runners periodically slow process creation down, and this test amplifies that far harder than anything else in the job. Comparing the 80 attempts where it ran under 3s against the 12 where it ran over 12s, its own median goes 2198ms -> 15917ms (7.2x) while the same file's powershell-only test moves 556 -> 686ms (1.2x), the cmd.exe and Git Bash process tests in the neighbouring file move 1.4x, and the other 35 files put together move 1.5x. Measured across those 176 attempts: 1881ms to 35438ms, p50 4264ms, correlation +0.881 with the job's total Vitest duration. 8 of 176 (4.5%) exceeded the 15s cap; 2 of 176 (1.1%) also exceeded the shared 30s testTimeout, so deleting the override and inheriting the config is not enough on its own. 60s clears all 176 with 1.7x headroom on the worst. This is slow, not hung. Every body here is synchronous spawnSync, so Vitest cannot interrupt one — the timer fires only after the body returns and the reported duration is real elapsed time. That is why a failure reads `× ... 22464ms` under `Test timed out in 15000ms`. The work finished; the stopwatch was short. Seven reruns at one identical head measured 2053 / 4680 / 5551 / 8732 / 13506 / 14868 / 21937ms — the last of those would have been red on code that had not changed. The 15s came from stablyai#8897, which raised this test off Vitest's built-in 5s default because the job then ran bare `pnpm vitest run`. stablyai#8909 landed 3h27m later and pointed the job at config/vitest.config.ts, which is the real fix for that. The constant stayed behind and has been the binding budget ever since. * fix(terminal): fence stale remount reattach ownership * fix(terminal): reconcile mounted pane identity after replacement * fix(terminal): fence stale reattach fallback ownership * fix(terminal): fence deferred SSH reattach ownership * fix(terminal): fence stale split pane ownership callbacks * fix(terminal): keep stale spawns from consuming startup --------- Co-authored-by: Brennan Benson <79079362+brennanb2025@users.noreply.github.com>
ELI5
Orca has to work out which agent — Codex, Claude, Gemini, Pi — is running in each terminal tab, so it
can show the right icon and status. Until now its best clue for a terminal you drove yourself was the
window title, and titles are whatever a program feels like setting. If you typed
codexat a plainprompt and Orca's process scan couldn't identify the foreground program, the tab showed a bare
"Terminal 1" and no agent at all.
This adds a better clue. Orca's own shell setup now quietly tells Orca which command the shell just
started, stamped with a private tag so a program can't fake it. Orca strips those stamps out before
anything reaches your screen, and only uses them to label the tab — never to decide whether
something is alive, where your typing goes, or whether a terminal is safe to close.
User-facing before / after
codexyourself at a plain prompt and Orca's process scan hasn't identified itWhen it bites
People who start agents by typing them into a terminal instead of using Orca's launcher, on local and
WSL panes running bash, zsh, fish or PowerShell. The symptom was a tab that never picked up the
agent's identity.
Summary
Author: @BrennanKB5
Safety
Tests
pnpm tcpassed after a temporary TS2322/TS6133 violation proved the gate was liveoxfmt --check, max-lines ratchet,pnpm build:relay, fullpnpm build, andgit diff --checkpassedorca-cmdbytes on the SSH pane and derived no pane command identity; arbitrary marker-shaped bytes printed remotely pass through unchanged, identically to main, because this PR changes zero SSH/relay filesbrennanb2025/identity-slice1aand the expected repo root; the rendered local terminal visibly printedIDENTITY_SLICE1A_LOCAL_OKdemo-project: the rendered terminal visibly printedTRUNCATED_MARKER_EXIT_VISIBLE_OK, the captured realpty:datastream contained zero nonce-bearing chunks, and exit delivered an empty transformed frame charging all 39 stripped raw charactersRed proofs
Each implementation change below was narrowly reverted, its focused test was run red, then the implementation was restored and rerun green:
The typecheck gate was also proven live with a temporary number-to-string assignment; pnpm tc failed with TS2322 and TS6133 before the probe was removed and the clean gate reran successfully.
Identity coverage limits
Fish and PowerShell marker payloads carry the typed command line; aliases and functions are not resolved in those hooks. Typing cc where cc aliases codex therefore does not produce codex identity. zsh is the only shell path in this PR that uses the alias-expanded preexec argument.
Identity recognition is intentionally first-token and known-entrypoint based. A wrapper script such as ~/bin/myagent that execs codex remains unrecognizable even when it ultimately resolves to codex; invoke the canonical codex command or an already-supported interpreter entrypoint for identity.
SSH marker pass-through is unchanged and expected: this PR changes zero SSH and zero relay files, so arbitrary remote marker-shaped bytes are not scrubbed by this slice.