Repository navigation
feat(terminal): report execution host and listing scope in terminal list - #14973
Merged
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughTerminal listings now include execution-host identifiers and host-scope metadata. Shared utilities encode and parse remote PTY identifiers and resolve PTY ownership. Runtime services discover known hosts, attach host identity to terminal summaries, and report covered or omitted hosts. RPC forwarding and CLI output preserve and display this metadata. Tests cover local, SSH, paired, remote, empty, filtered, JSON, and human-readable listings. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
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: 3161401d-d937-4d25-b25f-af39a587f809
📒 Files selected for processing (11)
src/cli/index-terminal-list-host-scope.test.tssrc/cli/terminal-format.tssrc/cli/terminal-list-host-scope-format.test.tssrc/main/runtime/orca-runtime.tssrc/main/runtime/rpc/terminal-list-host-scope-transport.test.tssrc/main/runtime/terminal-list-execution-host-scope.test.tssrc/main/ssh/ssh-remote-cli-terminal-host-scope.test.tssrc/renderer/src/runtime/runtime-terminal-stream.tssrc/shared/remote-runtime-pty-id.tssrc/shared/runtime-types.tssrc/shared/terminal-execution-host.ts
Included review availability: Your plan includes up to 10 reviews per rolling hour; 4 remain after this review.
Comment on lines
+11
to
+16
| export function toRemoteRuntimePtyId(handle: string, environmentId?: string | null): string { | ||
| const owner = environmentId?.trim() | ||
| if (!owner) { | ||
| return `${REMOTE_PTY_ID_PREFIX}${handle}` | ||
| } | ||
| return `${REMOTE_PTY_ID_PREFIX}${encodeURIComponent(owner)}${REMOTE_PTY_OWNER_SEPARATOR}${encodeURIComponent(handle)}` |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Preserve legacy handles that contain @@.
Line 14 emits ownerless handles without escaping. Lines 24-31 then interpret any @@ in a legacy handle as an owner separator. For example, toRemoteRuntimePtyId('a@@b') cannot round-trip.
Use an unambiguous, validated discriminator for owner-bearing IDs. Preserve all existing ownerless remote:${handle} values as ownerless IDs. Add a regression test for an ownerless handle containing @@.
`orca terminal list` returned rows with no host identity and no statement of what the listing covered, so a scoped listing that saw nothing read as "nothing exists anywhere" — an agent reported a live remote worker dead. Each row now carries an optional `executionHostId` derived from the PTY id (SSH and paired-runtime ids embed their owner), and the result carries an optional `hostScope` naming the hosts covered and the known hosts skipped. Both are surfaced in `--json` and in the human-readable CLI output, where an absent field renders as `unknown` rather than `local`. Both row builders route through one resolver, so the rule lives in one place.
Contributor
Author
Electron QA evidence — terminal list host + listing scopeQA only. Did not review code, change product code, or merge. What I ran
1. Non-local row with host populated — observedPR CLI unscoped listing against the live app (two connected terminals):
The SSH PTY id was 2. Empty / scoped case — the defect under test — observed
This is not a bare 3.
|
brennanb2025
force-pushed
the
brennanb2025/terminal-list-host-scope-2
branch
from
August 17, 2026 02:49
b4bf53c to
ec3d5ea
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/main/runtime/terminal-list-execution-host-scope.test.ts (1)
8-10: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueShorten or remove this narrative comment.
Replace it with one concise rationale. The suite name and test cases already define the behavior.
Proposed change
-// An agent once declared a task exited because `terminal list` came back without -// its worker: the worker was live on an SSH host, and nothing in the response -// said the listing was scoped or which host each row ran on. +// Preserve execution-host identity and listing scope for remote terminals.As per coding guidelines, “Comments must be concise, non-obvious, and brief—prefer one line.”
Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 8e2b28e4-7ceb-43b2-9bc6-58100141d1d4
📒 Files selected for processing (8)
src/cli/index-terminal-list-host-scope.test.tssrc/cli/terminal-format.tssrc/cli/terminal-list-host-scope-format.test.tssrc/main/ipc/pty.tssrc/main/runtime/orca-runtime.tssrc/main/runtime/terminal-list-execution-host-scope.test.tssrc/shared/runtime-types.tssrc/shared/terminal-execution-host.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- src/shared/terminal-execution-host.ts
- src/cli/terminal-format.ts
- src/cli/terminal-list-host-scope-format.test.ts
- src/cli/index-terminal-list-host-scope.test.ts
- src/shared/runtime-types.ts
Included review availability: Your plan includes up to 10 reviews per rolling hour; 5 remain after this review.
Contributor
Author
Electron QA — PR #14973 @ ec3d5ea — UNVERIFIEDQA only. Did not review code, change product code, or merge. Pin
Binary usedI did not use Built from
How I confirmed the running app was this SHA's build: {"name":"Orca: qa2-14973-ec3d5ea28a","isDev":true,"devLabel":"qa2-grok-14973 @ ec3d5ea28a","devBranch":"brennanb2025/terminal-list-host-scope-2","devWorktreeName":"qa2-grok-14973","devRepoRoot":"/Users/brennanbenson/orca/workspaces/orca/qa2-grok-14973","dockBadgeLabel":null}CDP port Compiled formatter on this SHA (not live output):
What I saw live
One Electron instance only. Did not relaunch after the hang (previous QA GPU-crashed on relaunch; load was still high). Legs
Not claimedNo pass. No screenshot of the new format. No PR image upload, because there was no captured listing to attach. Hung instance left running: Electron PID |
Contributor
Author
Electron QA retry — PR #14973 @ ec3d5ea — UNVERIFIED (partial)QA only. Did not review code, change product code, or merge. Pin
Binary usedNot Built and launched from
Confirmed running app identity via Playwright CDP before the hang: {"name":"Orca: qa2-14973-ec3d5ea28a","isDev":true,"devLabel":"qa2-grok-14973 @ ec3d5ea28a","devBranch":"brennanb2025/terminal-list-host-scope-2","devWorktreeName":"qa2-grok-14973","devRepoRoot":"/Users/brennanbenson/orca/workspaces/orca/qa2-grok-14973"}Electron PID Compiled formatter on this SHA (source of the built CLI, not a live mixed-version host):
What I observed liveCLI listing against this instance, after adding a local repo and creating one terminal:
That local row was local. I did not see The in-app pane showed Terminal 1 on SSH target Legs
No screenshot upload: Playwright screenshot timed out after CDP died. Do not treat the CLI text as an in-app visual of the new format. Hung instance left running: Electron PID |
Contributor
Author
Electron QA — PR #14973 @ ec3d5eaQA only. Did not review code, change product code, or merge. Pin
Binary usedNot In the app’s local terminal I invoked:
Confirmed in-pane:
Same CLI used from the host: 1. Local + SSH hosts — observedIn-app
2. Empty / scoped — observed
Not a bare
3. In-app path — observedSee screenshots. Command was the profile wrapper → 4.
|
brennanb2025
added a commit
that referenced
this pull request
Aug 17, 2026
The Known gaps section was accurate when written and becomes actively misleading as its fixes land: it told a reader to go fix restoreRequired, the missing unverifiable verdict, and the absent terminal-list host field, three things now addressed by #14974, #14977 and #14973. Mark the section as dated, require verification against current code before acting on any entry, name the PR per entry, and move landed items out. Also correct the two body claims that the landed fixes invalidated. The rules above are durable; only this section rots.
brennanb2025
added a commit
that referenced
this pull request
Aug 20, 2026
…bulary (#14971) * docs(ssh): state the SSH execution boundary and pin the liveness vocabulary Nothing under docs/ described how work splits between the client and an SSH host, so agents and humans inferred it from error strings and got it wrong: loss of contact was repeatedly reported as process death, which orphaned live remote agents and cold-started duplicates over the same worktree. Pins the vocabulary to the incumbent live/unverifiable/exited verdict from unstopped-pty-verification so no synonym is introduced, records the one real discriminator (all of a host's terminals drop together on link loss; one alone means process exit), and lists the outstanding gaps with citations. Tracked via the docs allow-list and linked from AGENTS.md, per the convention in .gitignore. * docs(ssh): cite the live restoreRequired site after it moved The throw now lives in reattachSshPtySessionForSpawn; ssh-pty-provider.ts no longer contains it. Caught by the worker fixing it, against a newer main than the audit ran on. * docs(ssh): require host evidence for liveness verdicts * docs(ssh): keep boundary references stable * docs(ssh): fence liveness evidence to its host identity * docs(ssh): state replay and environment boundaries precisely * docs(ssh): correct replay and platform boundary claims * docs(ssh): describe headless runtime continuity accurately * docs(ssh): distinguish authority from client metadata * docs(ssh): describe pending fixes accurately * docs(ssh): date the gap list and name the PR that closes each entry The Known gaps section was accurate when written and becomes actively misleading as its fixes land: it told a reader to go fix restoreRequired, the missing unverifiable verdict, and the absent terminal-list host field, three things now addressed by #14974, #14977 and #14973. Mark the section as dated, require verification against current code before acting on any entry, name the PR per entry, and move landed items out. Also correct the two body claims that the landed fixes invalidated. The rules above are durable; only this section rots. * docs(ssh): make the boundary doc a durable ruleset, not an incident record The Known gaps section was 18 of 93 lines enumerating specific defects from one investigation, several already fixed by sibling PRs in the same batch. A reference doc that needs a 'this section rots' warning is telling you the section belongs somewhere else; those entries belong in issues. Replace the six-row table of currently-lying signals with the method that outlands any particular bug: ask whether the owning host produced the signal, whether every PTY on the target went quiet together, whether the termination event matches the current incarnation and generation, and whether a returned status is actually a claim. Same for artifacts - state what ls-remote and a PR head each do and do not prove, rather than listing which command is currently wrong. Nothing here goes stale when the open fixes land.
paidaxingyo666
pushed a commit
to paidaxingyo666/Manta
that referenced
this pull request
Aug 21, 2026
…ist (stablyai#14973) * feat(terminal): report execution host and listing scope in terminal list `orca terminal list` returned rows with no host identity and no statement of what the listing covered, so a scoped listing that saw nothing read as "nothing exists anywhere" — an agent reported a live remote worker dead. Each row now carries an optional `executionHostId` derived from the PTY id (SSH and paired-runtime ids embed their owner), and the result carries an optional `hostScope` naming the hosts covered and the known hosts skipped. Both are surfaced in `--json` and in the human-readable CLI output, where an absent field renders as `unknown` rather than `local`. Both row builders route through one resolver, so the rule lives in one place. * fix(terminal): preserve unverifiable host scope * fix(terminal): fail closed on unverifiable hosts * test(terminal): name unverifiable scope explicitly * perf(terminal): keep graph hydration host scans narrow * fix(terminal): reject blank foreign host owners * fix(terminal): validate inferred inventory hosts * fix(terminal): preserve paired folder host scope * fix(terminal): keep inventory host inference typed * fix(terminal): disclose paired folder hosts
paidaxingyo666
pushed a commit
to paidaxingyo666/Manta
that referenced
this pull request
Aug 21, 2026
…bulary (stablyai#14971) * docs(ssh): state the SSH execution boundary and pin the liveness vocabulary Nothing under docs/ described how work splits between the client and an SSH host, so agents and humans inferred it from error strings and got it wrong: loss of contact was repeatedly reported as process death, which orphaned live remote agents and cold-started duplicates over the same worktree. Pins the vocabulary to the incumbent live/unverifiable/exited verdict from unstopped-pty-verification so no synonym is introduced, records the one real discriminator (all of a host's terminals drop together on link loss; one alone means process exit), and lists the outstanding gaps with citations. Tracked via the docs allow-list and linked from AGENTS.md, per the convention in .gitignore. * docs(ssh): cite the live restoreRequired site after it moved The throw now lives in reattachSshPtySessionForSpawn; ssh-pty-provider.ts no longer contains it. Caught by the worker fixing it, against a newer main than the audit ran on. * docs(ssh): require host evidence for liveness verdicts * docs(ssh): keep boundary references stable * docs(ssh): fence liveness evidence to its host identity * docs(ssh): state replay and environment boundaries precisely * docs(ssh): correct replay and platform boundary claims * docs(ssh): describe headless runtime continuity accurately * docs(ssh): distinguish authority from client metadata * docs(ssh): describe pending fixes accurately * docs(ssh): date the gap list and name the PR that closes each entry The Known gaps section was accurate when written and becomes actively misleading as its fixes land: it told a reader to go fix restoreRequired, the missing unverifiable verdict, and the absent terminal-list host field, three things now addressed by stablyai#14974, stablyai#14977 and stablyai#14973. Mark the section as dated, require verification against current code before acting on any entry, name the PR per entry, and move landed items out. Also correct the two body claims that the landed fixes invalidated. The rules above are durable; only this section rots. * docs(ssh): make the boundary doc a durable ruleset, not an incident record The Known gaps section was 18 of 93 lines enumerating specific defects from one investigation, several already fixed by sibling PRs in the same batch. A reference doc that needs a 'this section rots' warning is telling you the section belongs somewhere else; those entries belong in issues. Replace the six-row table of currently-lying signals with the method that outlands any particular bug: ask whether the owning host produced the signal, whether every PTY on the target went quiet together, whether the termination event matches the current incarnation and generation, and whether a returned status is actually a claim. Same for artifacts - state what ls-remote and a PR head each do and do not prove, rather than listing which command is currently wrong. Nothing here goes stale when the open fixes land.
dallascrilley
pushed a commit
to dallascrilley/orca
that referenced
this pull request
Aug 27, 2026
…bulary (stablyai#14971) * docs(ssh): state the SSH execution boundary and pin the liveness vocabulary Nothing under docs/ described how work splits between the client and an SSH host, so agents and humans inferred it from error strings and got it wrong: loss of contact was repeatedly reported as process death, which orphaned live remote agents and cold-started duplicates over the same worktree. Pins the vocabulary to the incumbent live/unverifiable/exited verdict from unstopped-pty-verification so no synonym is introduced, records the one real discriminator (all of a host's terminals drop together on link loss; one alone means process exit), and lists the outstanding gaps with citations. Tracked via the docs allow-list and linked from AGENTS.md, per the convention in .gitignore. * docs(ssh): cite the live restoreRequired site after it moved The throw now lives in reattachSshPtySessionForSpawn; ssh-pty-provider.ts no longer contains it. Caught by the worker fixing it, against a newer main than the audit ran on. * docs(ssh): require host evidence for liveness verdicts * docs(ssh): keep boundary references stable * docs(ssh): fence liveness evidence to its host identity * docs(ssh): state replay and environment boundaries precisely * docs(ssh): correct replay and platform boundary claims * docs(ssh): describe headless runtime continuity accurately * docs(ssh): distinguish authority from client metadata * docs(ssh): describe pending fixes accurately * docs(ssh): date the gap list and name the PR that closes each entry The Known gaps section was accurate when written and becomes actively misleading as its fixes land: it told a reader to go fix restoreRequired, the missing unverifiable verdict, and the absent terminal-list host field, three things now addressed by stablyai#14974, stablyai#14977 and stablyai#14973. Mark the section as dated, require verification against current code before acting on any entry, name the PR per entry, and move landed items out. Also correct the two body claims that the landed fixes invalidated. The rules above are durable; only this section rots. * docs(ssh): make the boundary doc a durable ruleset, not an incident record The Known gaps section was 18 of 93 lines enumerating specific defects from one investigation, several already fixed by sibling PRs in the same batch. A reference doc that needs a 'this section rots' warning is telling you the section belongs somewhere else; those entries belong in issues. Replace the six-row table of currently-lying signals with the method that outlands any particular bug: ask whether the owning host produced the signal, whether every PTY on the target went quiet together, whether the termination event matches the current incarnation and generation, and whether a returned status is actually a claim. Same for artifacts - state what ls-remote and a PR head each do and do not prove, rather than listing which command is currently wrong. Nothing here goes stale when the open fixes land.
Jinwoo-H
added a commit
that referenced
this pull request
Aug 27, 2026
Pre-hostScope hosts (before #14973) cannot prove emptiness, so parking them forever would strand agent auto-resume on every truly-empty legacy host. Keep main's behavior for them: a scope-less empty probe result settles the mirror. New hosts always publish hostScope, so the authoritative-inventory fix for STA-5377 is unaffected; outdated hosts keep main's existing narrow duplicate-writer window until they update.
enrick-dev
pushed a commit
to enrick-dev/orca
that referenced
this pull request
Oct 15, 2026
…ist (stablyai#14973) * feat(terminal): report execution host and listing scope in terminal list `orca terminal list` returned rows with no host identity and no statement of what the listing covered, so a scoped listing that saw nothing read as "nothing exists anywhere" — an agent reported a live remote worker dead. Each row now carries an optional `executionHostId` derived from the PTY id (SSH and paired-runtime ids embed their owner), and the result carries an optional `hostScope` naming the hosts covered and the known hosts skipped. Both are surfaced in `--json` and in the human-readable CLI output, where an absent field renders as `unknown` rather than `local`. Both row builders route through one resolver, so the rule lives in one place. * fix(terminal): preserve unverifiable host scope * fix(terminal): fail closed on unverifiable hosts * test(terminal): name unverifiable scope explicitly * perf(terminal): keep graph hydration host scans narrow * fix(terminal): reject blank foreign host owners * fix(terminal): validate inferred inventory hosts * fix(terminal): preserve paired folder host scope * fix(terminal): keep inventory host inference typed * fix(terminal): disclose paired folder hosts
enrick-dev
pushed a commit
to enrick-dev/orca
that referenced
this pull request
Oct 15, 2026
…bulary (stablyai#14971) * docs(ssh): state the SSH execution boundary and pin the liveness vocabulary Nothing under docs/ described how work splits between the client and an SSH host, so agents and humans inferred it from error strings and got it wrong: loss of contact was repeatedly reported as process death, which orphaned live remote agents and cold-started duplicates over the same worktree. Pins the vocabulary to the incumbent live/unverifiable/exited verdict from unstopped-pty-verification so no synonym is introduced, records the one real discriminator (all of a host's terminals drop together on link loss; one alone means process exit), and lists the outstanding gaps with citations. Tracked via the docs allow-list and linked from AGENTS.md, per the convention in .gitignore. * docs(ssh): cite the live restoreRequired site after it moved The throw now lives in reattachSshPtySessionForSpawn; ssh-pty-provider.ts no longer contains it. Caught by the worker fixing it, against a newer main than the audit ran on. * docs(ssh): require host evidence for liveness verdicts * docs(ssh): keep boundary references stable * docs(ssh): fence liveness evidence to its host identity * docs(ssh): state replay and environment boundaries precisely * docs(ssh): correct replay and platform boundary claims * docs(ssh): describe headless runtime continuity accurately * docs(ssh): distinguish authority from client metadata * docs(ssh): describe pending fixes accurately * docs(ssh): date the gap list and name the PR that closes each entry The Known gaps section was accurate when written and becomes actively misleading as its fixes land: it told a reader to go fix restoreRequired, the missing unverifiable verdict, and the absent terminal-list host field, three things now addressed by stablyai#14974, stablyai#14977 and stablyai#14973. Mark the section as dated, require verification against current code before acting on any entry, name the PR per entry, and move landed items out. Also correct the two body claims that the landed fixes invalidated. The rules above are durable; only this section rots. * docs(ssh): make the boundary doc a durable ruleset, not an incident record The Known gaps section was 18 of 93 lines enumerating specific defects from one investigation, several already fixed by sibling PRs in the same batch. A reference doc that needs a 'this section rots' warning is telling you the section belongs somewhere else; those entries belong in issues. Replace the six-row table of currently-lying signals with the method that outlands any particular bug: ask whether the owning host produced the signal, whether every PTY on the target went quiet together, whether the termination event matches the current incarnation and generation, and whether a returned status is actually a claim. Same for artifacts - state what ls-remote and a PR head each do and do not prove, rather than listing which command is currently wrong. Nothing here goes stale when the open fixes land.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
orca terminal listdid not identify where each terminal executes or state which execution hosts the result covered. A partial listing could therefore be mistaken for proof that a remote worker had exited.The fixed liveness vocabulary is
live/unverifiable/exited. Failure to query an execution host remainsunverifiable; it is never evidence of either neighboring state.Invariant
terminal-list.execution-host-scope: every row names its execution host when that identity is verifiable, and every listing distinguishes hosts that answered from known hosts that were not covered. An absent row is evidence ofexitedonly within a positively covered host inventory.executionHostId, result-levelhostScope, and focused tests covering local, SSH, paired-runtime, folder-workspace, malformed-owner, unavailable-provider, and scoped-list behavior.What changed
RuntimeTerminalSummary.executionHostIdandRuntimeTerminalListResult.hostScopefields.host=unverifiable,scope: unverifiable, andNo terminals listed.Wire compatibility
Both fields are optional additions to an existing RPC response. Older hosts omit them and new clients render the scope as
unverifiable; older clients ignore the extra fields. No existing field, stream opcode, or persisted mobile state changed.Testing
omittedHostIds, never as exited; the remote repo and worktree remained clean.Visual proof
Exact-head Electron QA is unverified. The Grok worker pinned to
ec3d5ea28anever accepted its prompt after three delivery methods, so no Electron action or screenshot was produced for the reviewed head.Earlier Electron evidence is explicitly pre-fix and is not evidence for
ec3d5ea28a. It observed live local/SSH CLI rows and a scoped empty listing, but the in-app command resolved to the installed old client and the app GPU-crashed before a second capture. The in-app new-format path and a live old-hostunverifiable-scope case remain unverified.Compatibility checklist
Review
Same-model adversarial review repeated until clean, followed by a fresh end readiness review. Final result: no P0/P1/P2 findings at
ec3d5ea28a.No linked issue. No merge performed. AI-assisted review and implementation. Author: @BrennanKB5.