Skip to content

refactor(native-chat): remove the unused terminal handoff - #22783

Merged
brennanb2025 merged 18 commits into
mainfrom
brennanb2025/remove-tui-handoff
Sep 25, 2026
Merged

brennanb2025 merged 18 commits into
mainfrom
brennanb2025/remove-tui-handoff

Conversation

@brennanb2025

@brennanb2025 brennanb2025 commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 115 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​1390 $\color{#cf222e}{\Huge{\mathbf{−}}}$​9847 $\color{#cf222e}{\Huge{\mathbf{−}}}$​8457
Prod 154 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​533 $\color{#cf222e}{\Huge{\mathbf{−}}}$​7738 $\color{#cf222e}{\Huge{\mathbf{−}}}$​7205

Scope: which chat this affects

  • Structured chat only. Everything removed or converted here belongs to the experimental structured native chat, the chat that runs the agent through its SDK. It is behind the experimentalStructuredNativeChat setting, which is off by default (src/shared/default-global-settings.ts:137).
  • Default chat view: no change. The default chat view, which is backed by a terminal and reads the agent's transcript, does not use any of the removed code, and nothing about it changes.
  • Terminals: no behaviour change. The one piece that touched every terminal is the agent-session PTY write gate, which was checked on each terminal write path. Only the removed handoff could ever switch it on, so it always let every write through. Removing it changes nothing about typing, pasting or terminal.send in plain terminals or in the terminal-backed chat.
  • Saved data. Only structured chat records are converted when they are read. Terminal sessions and terminal-backed chats are not touched.

ELI5

Orca had a hidden feature that could move a chat into a terminal running the agent, then back into the chat. No button, command or app ever turned it on. This PR deletes it, along with a terminal safety check that only that feature could ever switch on. Chats saved by any older version still open, and you can still send to them.

What Changed

What the user sees. Almost nothing, because the feature was never reachable. The only visible difference: if a command-line, mobile or remote client sends to a chat that is still starting, the refusal now says "The chat is still starting." instead of "The session is mid-handoff (new-owner-proving)." The desktop app never shows either message, because it holds a message until the chat is ready.

Removed. Every desktop and headless host ran this code, even though no client could start it:

  • the host method that handed a chat to a terminal (agentSession.requestHandoff), the coordinator that moved the chat between the two and resumed that move after a restart, and the host code that launched a terminal agent and proved it owned the chat
  • the agent-session PTY write gate and everything built on it: the checks on each terminal write path (renderer input, runtime controller, terminal writer, agent-prompt writer, preview input), the refusal fields on terminal.send and worker start, the CLI and plugin refusal text ("Switch it to Terminal, then retry"), and orchestration's rerouting of mail to a terminal taken over by a chat. Only the handoff ever bound a terminal to a chat, so the gate let every write through.
  • two UI pieces that were never mounted ("Open agent TUI" and "Return to chat") and their strings in every locale
  • helpers that only the handoff called, with their test coverage moved onto the code paths that still run

Kept, and why.

  • agentSession.handoffStatus. Desktop clients that have already shipped call it whenever they open a workspace with a chat tab, and a missing method would make them refuse to activate that workspace. It now reports only which runtime owns the chat, worked out from the saved record. It still waits behind a chat that is mid-start, as before, so it reports the chat as the owner rather than "no owner".
  • The persisted field names handoffStage, handoffOperationId and runtimeKind. Despite the names, they are the ownership state machine for every chat: each normal chat start writes new-owner-proving, and failure and restart paths write recovering and manual-recovery. Renaming persisted fields would need a data migration, which belongs in its own change.
  • The claimed-agent PTY owner and the SSH claim validation. terminal.createAgentSession and terminal.ensureAgentSession use them; they are not part of the handoff.

Records an older build left behind. Only the handoff wrote three values: the stages preparing and old-owner-stopped, and a terminal owner (runtimeKind: 'tui'). They are now converted once, when a record is read from disk, and the in-memory types no longer allow them, so the compiler flags any code that still branches on them:

  • preparing and old-owner-stopped become recovering, which the normal restart check then settles.
  • A terminal owner becomes a chat owner. If a terminal process was recorded, the claim is also marked conflicted. That is the existing state that every build, including older ones, checks but never stops, because that terminal is the user's own agent. Once the process is gone, the chat takes the session back. Until then, opening the chat says "Another Orca window or terminal still owns this session. Close it, then retry."
  • Reading a record does not write it; the converted record reaches disk with the next normal save, usually the restart check at launch. Every value written is one an older build already accepts, and an older build reading it never stops the recorded process.

One adjacent fix. Quit's resume offer (from #22560) read the sidebar state before the provider's already-delivered events had been applied. It only worked because the removed "drain handoffs" quit step added a delay first. Quit now waits for delivered events before taking that snapshot, for at most 1 second, so a slow journal write can never stop the chat's agent process from being stopped. Without the wait, offers only unfinished work when an accepted completed event is queued at quit fails.

Why

The feature was never reachable. git log -S on origin/main:

So in practice a leftover handoff record can only exist on a machine where someone called the RPC by hand. The load path handles those records anyway rather than trusting that.

Alternatives considered:

  • Leave the legacy values in the types and let the normal restart check overwrite them. That works for the stages, but not for runtimeKind: restart and eviction copy it forward, so a legacy chat that is never reopened would keep 'tui' forever, and eight readers would keep branches for a value nothing produces. Converting once at load removes those branches and lets the compiler catch any that remain.
  • Keep the PTY write gate "for safety". Nothing can switch it on any more, so it is dead code on every terminal write path, not a check.
  • Rename handoffStage / handoffOperationId. Both are persisted on every record, so that is a migration and belongs in its own change.

Mixed versions (clients and hosts update independently):

  • New client, old host: the client no longer reads the handoff status that snapshot and batch frames carry; the reducer ignores it. handoffStatus keeps its name and shape, and the client still only treats owner: 'native' as a chat owner. An old host never sent the removed write refusal in practice (nothing bound a terminal); if one did, the CLI prints the plain "Input refused by …" error.
  • Old client, new host: handoffStatus answers as before for chat-owned sessions. requestHandoff now returns method_not_found, and no shipped client ever sends it. Snapshot frames no longer include the handoff status, which only the never-mounted handoff UI read.
  • Downgrade: every value this build writes is valid to the previous build's validator, and the previous build treats a converted terminal owner the same way: it checks it but never stops it.
  • The cross-version manifest drops requestHandoff; the cross-version wire suites pass.

Linked Issue

No issue; follow-up to an internal decision to remove unused features.

Visual Proof

N/A for the change itself: the removed UI was never mounted, and no reachable desktop screen changes. Screenshots from the regression QA are under Testing.

Testing

  • I manually tested these changes locally
  • Automated tests added/updated, or explained why not below

Live Electron QA (macOS, background launch, isolated profile, PR build):

  • Workspace activates; terminal typing and a two-line paste work: terminal, paste ran
  • A Claude chat runs to completion: chat
  • A message sent while the chat is starting is held and then delivered, as before: held, delivered
  • Upgrade and downgrade on one profile, main → this PR → main → this PR: the chat restores with its history and a follow-up remembers earlier turns each time: after upgrade, after downgrade, after re-upgrade
  • A record hand-edited to the old handoff values loads, is converted once, and the chat still works: legacy record. No record-store errors, and no rewrite loop.
  • Quit with a Claude chat open takes about 15 s on this PR and on main alike, so this PR does not change it. It is an existing, separate issue in the Claude session shutdown fallback.

Automated:

  • New tests: the legacy-record test (four shapes an older build could persist, each loads and becomes usable, and a live terminal owner is never stopped), the load-time conversion (every legacy shape, and no change for every current shape), a store test (a legacy record loads, is not marked unreadable, opening writes nothing, the next save rewrites it), a downgrade test (the previous build's recovery rules never stop a converted terminal owner), the bounded quit drain, handoffStatus waiting behind a start, and the new refusal text.
  • Each of those was checked by ablation: removing the fix turns its test red for the expected reason.
  • Typecheck (tc:node, tc:web, tc:cli), full-file oxlint, check:code-quality:changed, check:react-doctor:changed, the max-lines, ts-nocheck and runtime-electron ratchets, verify:rpc-params-catalog, and the localization checks pass. The agent-session, orchestration, PTY IPC, terminal RPC and cross-version wire suites pass; the only failures seen were timeouts under parallel load that pass alone.
  • Not verified: the mobile suite (mobile dependencies aren't installed here; the mobile change is one call-site argument and one test fixture), Codex chats in live QA, SSH and Windows in live QA (the removed code was host-local and no Windows- or SSH-specific path changed).

AI Disclosure

Written with Claude (Opus 5.5).

Review

Agent skill upstream boundary

  • Not applicable, or this change follows docs/reference/agent-skill-sharing-upstream-boundary.md and copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Notes

  • Security: removes a PTY proof-write path and a launch option that bound a terminal to a chat. No new surface.
  • Cross-platform / SSH: the removed launch path was host-local terminal code. Ownership stays on the host that runs the chat, and recovery never signals a terminal process on any host.
  • Mobile: mobile never called either handoff method; handoffStatus stays on the mobile allowlist.
  • Performance: less work on every terminal write; the load-time conversion is a pure pass per record; the only added wait is the 1-second cap at quit.
  • Overlap: fix(workspace): a restored workspace paints before its terminals reconnect #22293 edits structured-agent-session-restart-restore.ts and nearby host files that this PR also changes; whichever lands second needs a small rebase.

Checklist

  • This PR is small and focused
  • I explained what changed and why (ELI5, the user-facing before/after, the mechanism, and why over the alternatives)
  • Before/after screenshots or videos attached for UI changes, or N/A with reason
  • Self-reviewed for correctness, security, and performance
  • Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
  • pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred)

No client ever called agentSession.requestHandoff or mounted the handoff
chrome. Delete the handoff coordinator, the terminal-owner runtime, the
proof write path and the unmounted UI. Keep agentSession.handoffStatus,
which released desktop clients read for worktree activation, and let
records an older build left mid handoff reconcile through the ordinary
restart and recovery paths.
@coderabbitai

coderabbitai Bot commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

This change removes structured-session terminal handoff and TUI-owner recovery flows, along with their RPC, wire, and renderer interfaces. Lease reconciliation and session restoration now handle legacy handoff records without restoring terminal ownership. The change also adds a Claude transcript-entry UUID reader and Codex pinned rollout proof, and removes PTY write-gate enforcement.

Priority: ⬇️ Low

Merge Risk: 🟡 Moderate · up to 2bd44

After an upgrade, sessions left with a dead terminal owner may be unable to use hold/resume. The PR also adds a file outside the shrink-only type-checking baseline without an established approved exception; resolve these risks before merging.

Security Architecture Review

Security architecture risk: 🟡 Moderate · up to 2bd44

Legacy chats are designed to remain accessible after the terminal handoff is removed, but the change also removes a write control and spans restart behavior. The remaining uncertainty warrants architecture review; no exploitable path was established.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The removed lease check affected bytes entering a host PTY, not merely renderer presentation. The former gate admitted ordinary unbound PTYs; the examined production bindings belonged to terminal-owner flows. A surviving path that binds a protected PTY was not established.

Trust Boundaries and Controls

  • observed — The examined PTY IPC route still verifies the main-window sender and payload shape. Its writer still rejects mobile-driver input and requires an existing provider or PTY ownership before writing. These checks do not themselves replace lease fencing for a bound agent-session PTY.

Resilience and Maintainability Implications

  • observed — For a legacy owner proven dead, the examined adjudication advances the fence and clears ownership; an indeterminate probe does not evict it. The changed fixture directly tests the matching-live-owner case, not every legacy terminal outcome.

Hardening Proposals

  • proposed — Before relying on the removed write gate, verify that no supported or upgrade-time path can still bind an agent-session lease to a writable PTY, including writes that yield between chunks.
🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 32.69% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 104 functions across 116 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ⚠️ Warning The description covers the required scope, rationale, testing, compatibility, visual proof, AI disclosure, and checklist sections. However, the required Linked Issue is missing because it states “No i… Add the issue reference under Linked Issue, using a valid entry such as Fixes #12345`` or another project tracking issue. If the work has no existing issue, create one before merging and link it here.
✅ Passed checks (3 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly summarizes the main change: removing the unused terminal handoff from native chat.
Full details: Description check

Explanation

The description covers the required scope, rationale, testing, compatibility, visual proof, AI disclosure, and checklist sections. However, the required Linked Issue is missing because it states “No issue” instead of providing an issue link.

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR

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.

❤️ Share

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: 2a417c85-54f2-47e2-a7e2-0c76a9770037

📥 Commits

Reviewing files that changed from the base of the PR and between fec8fe4 and c867bd2.

⛔ Files ignored due to path filters (1)
  • src/shared/rpc-contract/rpc-params-catalog.generated.ts is excluded by !**/*.generated.*
📒 Files selected for processing (174)
  • config/ts-nocheck-baseline.txt
  • mobile/src/session/mobile-structured-send-delivery.ts
  • mobile/src/session/use-mobile-structured-agent-session-prompt-cancel.test.tsx
  • src/main/claude/claude-structured-session-acquisition.ts
  • src/main/claude/claude-transcript-entry-uuid.test.ts
  • src/main/claude/claude-transcript-entry-uuid.ts
  • src/main/claude/claude-tui-exit.test.ts
  • src/main/claude/claude-tui-exit.ts
  • src/main/claude/claude-tui-resume-launch.test.ts
  • src/main/claude/claude-tui-resume-launch.ts
  • src/main/claude/claude-tui-resume-proof.test.ts
  • src/main/claude/claude-tui-resume-proof.ts
  • src/main/claude/claude-tui-resume-real-binary.integration.test.ts
  • src/main/codex/codex-resume-process-proof.test.ts
  • src/main/codex/codex-resume-process-proof.ts
  • src/main/codex/codex-tui-resume-real-binary.integration.test.ts
  • src/main/codex/codex-tui-rollout-proof.test.ts
  • src/main/codex/codex-tui-rollout-proof.ts
  • src/main/ipc/pty/runtime/controller.ts
  • src/main/ipc/pty/runtime/operations.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-attach.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-background-task-channel.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-claude-option-queue.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-command-publication.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-dead-tui-recovery.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-eviction.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-handoff-admission.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-handoff-flow-context.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-handoff-flow-runner.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-handoff-flow-runner.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-handoff-forward.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-handoff-operation-guard.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-handoff-operation-guard.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-handoff-options.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-handoff-options.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-handoff-owner-close.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-handoff-owner-close.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-handoff-queue-start.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-handoff-queue.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-handoff-recover.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-handoff-restart-tui.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-handoff-restart.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-handoff-result.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-handoff-revalidation.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-handoff-reverse.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-handoff-reverse.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-handoff-state.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-handoff-status.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-handoff-test-coordinator.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-handoff-test-identities.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-handoff-test-requests.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-handoff-types.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-handoff.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-handoff.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-host-handoff-stop.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-host-handoff.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-host-handoff.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-host-runtime-state.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-host-runtime-state.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-host-teardown.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-host-teardown.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-host-types.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-host.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-host.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-lease-renewer.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-lease-renewer.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-legacy-handoff-record.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-live-tui-restart-survival.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-manual-recovery.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-native-handoff-acquisition.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-option-settlement.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-owner-status.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-proven-dead-retry.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-read-restore.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-readable-restorer.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-readable-restorer.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-recovery-resolution.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-recovery-resolution.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-refusal-retry.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-restart-restore.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-restart-restore.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-resume-eligibility.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-reveal.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-reveal.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-send-preparation.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-settlement-retry.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-subscribers.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-subscribers.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-surface-lifetime.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-teardown-handoff-drain.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-wire-admission.test.ts
  • src/main/native-chat/agent-session-wire/structured-tui-transcript-boundary.ts
  • src/main/native-chat/agent-session-wire/structured-tui-transcript-catchup.test.ts
  • src/main/native-chat/agent-session-wire/structured-tui-transcript-catchup.ts
  • src/main/native-chat/agent-session-wire/structured-tui-transcript-close.test.ts
  • src/main/native-chat/agent-session-wire/structured-tui-transcript-ownership.test.ts
  • src/main/native-chat/agent-session-wire/structured-tui-transcript-teardown-test-fixture.ts
  • src/main/native-chat/agent-session-wire/structured-tui-transcript-teardown.test.ts
  • src/main/native-chat/session-file-resolver.test.ts
  • src/main/native-chat/session-file-resolver.ts
  • src/main/runtime/agent-session-handoff-lease-transitions.test.ts
  • src/main/runtime/agent-session-handoff-lease-transitions.ts
  • src/main/runtime/agent-session-handoff-record-transitions.ts
  • src/main/runtime/agent-session-lease-transitions.ts
  • src/main/runtime/agent-session-pty-write-gate.test.ts
  • src/main/runtime/agent-session-pty-write-gate.ts
  • src/main/runtime/agent-session-record-store.test.ts
  • src/main/runtime/agent-session-restart-handoff-adjudication.test.ts
  • src/main/runtime/agent-session-restart-handoff-adjudication.ts
  • src/main/runtime/agent-session-restart-lease-transitions.ts
  • src/main/runtime/agent-session-resume-args.test.ts
  • src/main/runtime/agent-session-resume-args.ts
  • src/main/runtime/claude-structured-session-integration.test.ts
  • src/main/runtime/orca-runtime-create-terminal-dependencies.ts
  • src/main/runtime/orca-runtime-create-terminal.ts
  • src/main/runtime/orca-runtime-get-agent-session-execution-namespace.ts
  • src/main/runtime/orca-runtime-get-structured-agent-session-create-support.ts
  • src/main/runtime/orca-runtime-get-worktree-ps.ts
  • src/main/runtime/orca-runtime-prove-recovered-structured-tui-pty-process.ts
  • src/main/runtime/orca-runtime-restore-structured-agent-session-tabs-once.ts
  • src/main/runtime/orca-runtime-stop-structured-session-process.ts
  • src/main/runtime/orca-runtime-structured-agent-session-launch-tui.ts
  • src/main/runtime/orca-runtime-structured-agent-session-recover-tui-owner.ts
  • src/main/runtime/orca-runtime-structured-agent-session-reprove-tui-owner.ts
  • src/main/runtime/orca-runtime-structured-tui-tab-binding.test.ts
  • src/main/runtime/rpc/methods/orchestration/worker/agent-status-producer-census.test.ts
  • src/main/runtime/rpc/methods/structured-agent-session-gate-classification.test-fixture.ts
  • src/main/runtime/rpc/methods/structured-agent-session-gate.ts
  • src/main/runtime/rpc/methods/structured-agent-session-rpc.test-fixture.ts
  • src/main/runtime/rpc/methods/structured-agent-session-schemas.ts
  • src/main/runtime/rpc/methods/structured-agent-session.test.ts
  • src/main/runtime/rpc/methods/structured-agent-session.ts
  • src/main/runtime/runtime-extraction-regressions.test.ts
  • src/main/runtime/runtime-pty-controller-contract.ts
  • src/main/runtime/runtime-terminal-contracts.ts
  • src/main/runtime/structured-agent-session-create-adoption.ts
  • src/main/runtime/structured-agent-session-pty-binding.test.ts
  • src/main/runtime/structured-agent-session-runtime.ts
  • src/main/runtime/structured-tui-exit-proof.test.ts
  • src/main/runtime/structured-tui-exit-proof.ts
  • src/main/runtime/structured-tui-idle-evidence.test.ts
  • src/main/runtime/structured-tui-idle-evidence.ts
  • src/main/runtime/structured-tui-process-identity.test.ts
  • src/main/runtime/structured-tui-process-identity.ts
  • src/main/runtime/structured-tui-recovery-claim-match.test.ts
  • src/main/runtime/structured-tui-recovery-claim-match.ts
  • src/renderer/src/components/native-chat/StructuredAgentSessionHandoffChrome.test.tsx
  • src/renderer/src/components/native-chat/StructuredAgentSessionHandoffChrome.tsx
  • src/renderer/src/components/native-chat/use-native-chat-provisional-launch.test.tsx
  • src/renderer/src/components/native-chat/use-structured-agent-session-mutate.ts
  • src/renderer/src/components/native-chat/use-structured-agent-session-provisional.test.tsx
  • src/renderer/src/components/terminal-pane/StructuredAgentSessionTerminalReturnButton.tsx
  • src/renderer/src/i18n/locales/en.json
  • src/renderer/src/i18n/locales/fr.json
  • src/renderer/src/i18n/locales/ja.json
  • src/renderer/src/i18n/locales/ko.json
  • src/renderer/src/i18n/locales/zh.json
  • src/renderer/src/lib/worktree-agent-activation-gate.test.ts
  • src/renderer/src/lib/worktree-agent-activation-gate.ts
  • src/renderer/src/lib/worktree-agent-structured-inventory.ts
  • src/renderer/src/runtime/structured-agent-session-handoff-store.ts
  • src/shared/agent-session-definitive-refusal.test.ts
  • src/shared/agent-session-lease-adjudication.test.ts
  • src/shared/agent-session-lease-adjudication.ts
  • src/shared/agent-session-mutation-envelope.ts
  • src/shared/agent-session-record.ts
  • src/shared/agent-session-refusal-retry.ts
  • src/shared/agent-session-wire.ts
  • src/shared/rpc-contract/structured-agent-session-params.ts
  • src/shared/structured-agent-session-coalescer.ts
  • src/shared/structured-agent-session-outbox.ts
  • src/shared/structured-agent-session-reducer.ts
  • tests/e2e/cross-version-wire/structured-agent-session-host-fixture.ts
  • tests/e2e/cross-version-wire/structured-agent-session-surface-manifest.ts
💤 Files with no reviewable changes (113)
  • src/shared/structured-agent-session-coalescer.ts
  • src/main/runtime/rpc/methods/structured-agent-session-rpc.test-fixture.ts
  • src/main/native-chat/agent-session-wire/structured-tui-transcript-ownership.test.ts
  • src/main/runtime/structured-tui-exit-proof.test.ts
  • src/main/codex/codex-resume-process-proof.test.ts
  • src/main/native-chat/agent-session-wire/structured-tui-transcript-catchup.test.ts
  • src/renderer/src/i18n/locales/fr.json
  • src/renderer/src/components/native-chat/StructuredAgentSessionHandoffChrome.test.tsx
  • src/main/ipc/pty/runtime/controller.ts
  • src/main/runtime/runtime-pty-controller-contract.ts
  • src/main/runtime/orca-runtime-structured-tui-tab-binding.test.ts
  • src/main/runtime/structured-tui-idle-evidence.ts
  • src/main/claude/claude-tui-exit.test.ts
  • src/main/runtime/agent-session-restart-handoff-adjudication.test.ts
  • src/main/native-chat/agent-session-wire/structured-tui-transcript-teardown-test-fixture.ts
  • src/main/codex/codex-tui-resume-real-binary.integration.test.ts
  • src/renderer/src/components/terminal-pane/StructuredAgentSessionTerminalReturnButton.tsx
  • src/main/native-chat/agent-session-wire/structured-agent-session-handoff-types.ts
  • src/renderer/src/components/native-chat/use-native-chat-provisional-launch.test.tsx
  • src/main/runtime/structured-tui-recovery-claim-match.test.ts
  • src/renderer/src/i18n/locales/en.json
  • src/main/native-chat/agent-session-wire/structured-agent-session-live-tui-restart-survival.test.ts
  • src/main/runtime/rpc/methods/orchestration/worker/agent-status-producer-census.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-dead-tui-recovery.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-handoff-owner-close.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-host-handoff-stop.test.ts
  • src/main/native-chat/agent-session-wire/structured-tui-transcript-close.test.ts
  • src/main/ipc/pty/runtime/operations.ts
  • src/renderer/src/components/native-chat/use-structured-agent-session-provisional.test.tsx
  • src/renderer/src/i18n/locales/ja.json
  • src/main/runtime/structured-tui-idle-evidence.test.ts
  • src/main/native-chat/agent-session-wire/structured-tui-transcript-boundary.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-handoff.test.ts
  • src/shared/agent-session-refusal-retry.ts
  • src/renderer/src/components/native-chat/StructuredAgentSessionHandoffChrome.tsx
  • src/main/native-chat/agent-session-wire/structured-agent-session-handoff-flow-runner.test.ts
  • src/renderer/src/i18n/locales/ko.json
  • src/main/runtime/orca-runtime-create-terminal.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-handoff-owner-close.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-handoff-options.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-host-handoff.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-handoff-recover.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-handoff-reverse.test.ts
  • src/main/native-chat/session-file-resolver.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-handoff-result.ts
  • src/main/runtime/agent-session-handoff-lease-transitions.test.ts
  • src/main/runtime/agent-session-resume-args.test.ts
  • src/main/runtime/agent-session-resume-args.ts
  • src/main/claude/claude-tui-resume-launch.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-readable-restorer.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-handoff-restart.ts
  • src/main/claude/claude-tui-resume-proof.test.ts
  • src/main/runtime/structured-tui-process-identity.test.ts
  • src/main/runtime/agent-session-pty-write-gate.test.ts
  • src/main/claude/claude-tui-resume-real-binary.integration.test.ts
  • src/main/runtime/runtime-terminal-contracts.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-handoff-operation-guard.test.ts
  • src/main/runtime/orca-runtime-structured-agent-session-launch-tui.ts
  • src/renderer/src/runtime/structured-agent-session-handoff-store.ts
  • tests/e2e/cross-version-wire/structured-agent-session-host-fixture.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-handoff-test-requests.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-handoff-queue-start.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-handoff-reverse.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-handoff-flow-runner.ts
  • src/main/runtime/orca-runtime-prove-recovered-structured-tui-pty-process.ts
  • src/main/runtime/structured-tui-exit-proof.ts
  • src/main/runtime/structured-agent-session-pty-binding.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-teardown-handoff-drain.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-handoff.ts
  • src/shared/agent-session-mutation-envelope.ts
  • src/main/runtime/agent-session-record-store.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-handoff-forward.ts
  • src/main/native-chat/agent-session-wire/structured-tui-transcript-teardown.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-proven-dead-retry.test.ts
  • src/main/runtime/agent-session-restart-handoff-adjudication.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-handoff-admission.ts
  • src/main/runtime/agent-session-handoff-record-transitions.ts
  • src/renderer/src/i18n/locales/zh.json
  • src/main/native-chat/agent-session-wire/structured-agent-session-handoff-test-coordinator.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-handoff-state.ts
  • tests/e2e/cross-version-wire/structured-agent-session-surface-manifest.ts
  • src/main/claude/claude-tui-resume-proof.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-handoff-revalidation.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-host-handoff.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-manual-recovery.ts
  • src/main/runtime/agent-session-pty-write-gate.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-handoff-operation-guard.ts
  • src/main/runtime/orca-runtime-structured-agent-session-recover-tui-owner.ts
  • src/main/runtime/rpc/methods/structured-agent-session-schemas.ts
  • src/main/runtime/structured-tui-process-identity.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-background-task-channel.ts
  • src/main/claude/claude-tui-exit.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-handoff-options.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-handoff-flow-context.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-native-handoff-acquisition.ts
  • src/main/runtime/agent-session-lease-transitions.ts
  • src/main/codex/codex-resume-process-proof.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-handoff-status.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-handoff-test-identities.ts
  • src/main/runtime/structured-agent-session-runtime.ts
  • src/main/runtime/orca-runtime-stop-structured-session-process.ts
  • src/main/runtime/orca-runtime-structured-agent-session-reprove-tui-owner.ts
  • src/main/runtime/agent-session-handoff-lease-transitions.ts
  • src/main/runtime/structured-tui-recovery-claim-match.ts
  • src/main/runtime/rpc/methods/structured-agent-session.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-handoff-queue.ts
  • src/main/claude/claude-tui-resume-launch.ts
  • src/main/native-chat/agent-session-wire/structured-tui-transcript-catchup.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-host-runtime-state.ts
  • src/main/runtime/orca-runtime-create-terminal-dependencies.ts
  • src/main/runtime/rpc/methods/structured-agent-session-gate-classification.test-fixture.ts
  • src/main/codex/codex-tui-rollout-proof.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-handoff-restart-tui.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 6 remain after this review.

src/main/runtime/orca-runtime-get-orchestration-dispatch-authority.ts
src/main/runtime/orca-runtime-get-pty-record-for-pane-key.ts
src/main/runtime/orca-runtime-get-runtime-id.ts
src/main/runtime/orca-runtime-get-structured-agent-session-create-support.ts

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

🔎 Supported by static analysis

🏁 Script executed:

#!/bin/bash
# Is the header new in this PR, and does the ratchet check compare against a base?
head -5 src/main/runtime/orca-runtime-get-structured-agent-session-create-support.ts
git log --oneline -3 -- src/main/runtime/orca-runtime-get-structured-agent-session-create-support.ts
git show HEAD~1:config/ts-nocheck-baseline.txt 2>/dev/null | rg -n 'get-structured-agent-session-create-support' || echo "not in previous baseline"
fd -i 'ts-nocheck' scripts tools . -t f -E node_modules | head
rg -n "ts-nocheck-ratchet" package.json

Repository: stablyai/orca

Length of output: 1725


🏁 Script executed:

set -e
printf '%s\n' '--- checker ---'
cat -n config/scripts/check-ts-nocheck-ratchet.mjs
printf '%s\n' '--- checker tests (relevant) ---'
rg -n -C 4 'SHRINK|add|baseline|base|ratchet|ts-nocheck' config/scripts/check-ts-nocheck-ratchet.test.mjs
printf '%s\n' '--- baseline diff ---'
git diff --unified=12 fec8fe48225e47b883a90aadf35d6369523f72a7 c867bd2c30d0c21be8365df2a56200d8794d0e57 -- config/ts-nocheck-baseline.txt
printf '%s\n' '--- source diff ---'
git diff --unified=12 fec8fe48225e47b883a90aadf35d6369523f72a7 c867bd2c30d0c21be8365df2a56200d8794d0e57 -- src/main/runtime/orca-runtime-get-structured-agent-session-create-support.ts

Repository: stablyai/orca

Length of output: 31373


Do not add a new entry to the ts-nocheck ratchet without an approved exception.

config/ts-nocheck-baseline.txt states that the list may only shrink and that entries must not be added to get CI green. This PR adds src/main/runtime/orca-runtime-get-structured-agent-session-create-support.ts and its @ts-nocheck directive.

The ratchet script checks the current files against the checked-in baseline. It does not compare the baseline with the merge base, so CI will not reject this addition automatically. Remove the directive, fix the forward-reference types, and delete the baseline entry. If the suppression is unavoidable, document the reason and obtain reviewer sign-off.

brennanb2025 and others added 6 commits September 24, 2026 23:54
Eviction now drains delivered events before quit's resume-offer snapshot. An
unbounded wait there sits ahead of the provider stop, so a sink whose journal
write stalls kept the child running until the step deadline aborted the
eviction. The offer is advisory: bound the drain and stop the child regardless.

Co-Authored-By: Claude <noreply@anthropic.com>
`claudeAuthEnvCarriedForward`, `isPathWithinDirectory` and
`queryWindowsProcessRowsFresh` lost their last caller with the handoff. The
fresh-scan tests now go through `queryWindowsProcessDescendants({ fresh: true })`,
the teardown path that still depends on that contract.

Co-Authored-By: Claude <noreply@anthropic.com>
Six comments still named the handoff coordinator, a handoff suspend, or a
terminal-owned session as live participants in the flows they describe.

Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
…ement

The removed restart handoff test pinned this branch; nothing else did.

Co-Authored-By: Claude <noreply@anthropic.com>
The handoff removal dropped the per-session queue from `handoffStatus`, so a
read landing mid-start reported the reservation (no owner) instead of the
settled chat owner, and shipped desktop clients blocked worktree activation on
it. The read is queued again, as it was before the removal.

Co-Authored-By: Claude <noreply@anthropic.com>

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ No critical issues — minor suggestions inline.

Reviewed changes

  • Removed the unreachable terminal-handoff feature end to end: the host RPC (agentSession.requestHandoff), the admission/coordinator files (structured-agent-session-handoff-admission, -forward, -revalidation, -result), the StructuredTuiOwner/StructuredTuiHandoff* types, the renderer handoff store + StructuredAgentSessionHandoffChrome/TerminalReturnButton, and the structured-agent-session-handoff* / structured-tui-transcript-catchup test suites and fixtures.
  • Kept the survivors intact: agentSession.handoffStatus and attach/send, the ownership state machine, the PTY write gate, and the restart/settlement recovery paths.
  • Legacy-record handling: 'tui' and 'preparing' stay in the persisted stage union so older records load; restart reconciliation now routes terminal-owned records to recovering (never stopped as before) and settlement retries no longer special-case terminal owners.
  • Wire narrowing: AttachParams.runtimeKind is now 'native' only; runtimeKind: 'tui' is rejected at the schema, and attach always adopts the chat owner.
  • Client cleanup: worktree-agent-activation-gate no longer offers a terminal-owner skip, dropped the structuredOwner argument on resume, and reduced snapshot/batch frames to a single target; i18n and e2e strings for the terminal flow are deleted.

I traced the removed surface for lingering callers (renderer store, i18n keys, status feed, activate/detach paths), checked the narrowed runtimeKind schema and its mobile call site, verified the legacy tui/preparing stage records still load and are retired by restart reconciliation, and re-checked the new structured-agent-session-legacy-handoff-record test against the code it exercises. One spawn-token test comment is now misleading, and four renderer test mocks keep a vestigial handoff: null field.

Pullfrog  | Fix it ➔ | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

The gate only refused a write when a PTY had been bound to a chat session, and the
only code that ever bound one was the terminal handoff this branch removes. With it
gone, every admit/readmit returned "admitted" unconditionally, so the checks on the
renderer write path, the runtime controller backstop, terminal.send, agent prompts,
preview input and orchestration pointers, the refusal fields on terminal.send and
worker-start receipts, the plugin and CLI refusal copy, and the adopted-pane
orchestration routing could no longer run. Ordinary writes take the same path in
the same order as before.

Co-Authored-By: Claude <noreply@anthropic.com>

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes — delta since the prior pullfrog review (c867bd2c30 → 12efdefd1f, 6 commits, 15 files):

  • handoffStatus reads behind an in-flight attach: the owner-status record read is wrapped in this.serialize(sessionId, …), so a chat that is mid-start answers with its settled owner rather than a transient record. A new structured-agent-session-host.test.ts case pins the starting-chat behavior.
  • Pre-stop drain bounded: snapshot-before-stop now awaits withTimeout(context.eventSink.drained(), 1_000, null), so a stalled journal write cannot hold the child's stop behind the advisory resume snapshot. The new eviction test covers the stalled-sink path.
  • Dead helpers removed: claudeAuthEnvCarriedForward, queryWindowsProcessRowsFresh, and isPathWithinDirectory had no callers outside the removed handoff code — verified zero remaining references across src/, mobile/, tests/, and config/.
  • Comment cleanups: surviving comments no longer cite the removed handoff coordinator (agent-session-empty-batch, attach, host-lifetime, status-feed, unexpected-exit, host-types).
  • Test-only: pins that a start which dies before proving its handle owes no settlement, and drops a cast in the stalled-drain test.

The behavioral changes are correct: withTimeout absorbs rejections, so the dropped drain cannot surface as an unhandled rejection, and the failed barrier still aborts eviction later at drain-published. The serialize wrapper restores the pattern the base already used for this read.

Pullfrog  | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

brennanb2025 and others added 3 commits September 25, 2026 00:27
…alled

appendLegacyTranscriptMessages fed the terminal transcript catch-up and
proveClaudeTranscriptBranch backed the terminal owner's exit proof. Both lost
their last caller with the handoff. Their tests now go through the live entry
points instead: the roster bounds through the legacy import, the pinned-read and
growth tests through the ancestry replay the history window uses, and the marker
rules through the string proof in their own file rather than the session-file
resolver's.

Co-Authored-By: Claude <noreply@anthropic.com>
A send refused because the chat's owner is not settled showed "The session is
mid-handoff (<stage>)." in the composer. With the handoff gone, the stages that
reach it are a chat that is still starting, or one whose previous agent process
has not yet been confirmed stopped. The message now says which of the two it is.
The refusal code is unchanged.

Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes — delta since the prior pullfrog review (12efdefd1f → faececed48, 4 commits, 52 files):

  • Removed the agent-session PTY write gate: deleted the gate, the shared admission/refusal types, the refusal-copy helper, and the renderer refusal reporter, and stripped the lease check from every byte-entry point (pty/ipc/write-input, the PTY runtime controller, runtime-terminal-writer, the agent-prompt write, orchestration pointer delivery, plugin sendText, terminal.send, and the preload bridge). RuntimeTerminalSend.agentSessionRefusal and the write_gate_denied settlement reason are gone with it.
  • Collapsed the adopted-pane redirect: resolveAdoptedStructuredMailboxTarget, decideStructuredPointerDelivery, isSettledNativeOwner, and the owner-not-settled-native retain reason are deleted; the structured pointer lane now decides solely from the session's gate facts, and every retain reason parks for the next journal edge.
  • Dropped transcript helpers only the handoff called: proveClaudeTranscriptBranch and appendLegacyTranscriptMessages removed; the marker-branch tests moved into a dedicated claude-transcript-marker-branch-proof.test.ts, and the streaming suite now drives replayClaudeTranscriptBranchAncestry.
  • Stopped calling a starting chat "mid-handoff": refuseUnlessWriterAdmitted now answers "The chat is still starting." for new-owner-proving and a recovery sentence for the other recorded stages.

I verified the gate was already inert before this commit: git grep bindPty at 12efdef finds agentSessionPtyWriteGate.bindPty(...) only in test files, so enforcing was always false and no production write path could be refused. The replacements are behavior-preserving. The four directly changed suites plus the gate-adjacent suites pass, and pnpm tc is clean.

ℹ️ The PR description still describes the PTY write gate as surviving

The description's "What Changed → After" section states that "The PTY write gate ... remain[s]" and that "every write passes through it unchecked"; commit 42d10eee15 deletes the gate, agentSessionRefusal, and structuredChatPtyWriteRefusalCopy outright. A reviewer reading the description would misjudge the blast radius of the final commit. The claimed-agent PTY owner and SSH claim validation do still remain, so only that portion needs rewording — and the title may want to mention the gate, since the change is no longer just the handoff.

Technical details
# Stale PR description

## Affected sites
- PR body, "What Changed → After" bullet "The terminal-agent pieces stay." — claims the PTY write gate remains and that "every write passes through it unchecked", and that the typed refusal copy is still produced.

## Required outcome
- The description should match `faececed48`: the gate, `AgentSessionPtyWriteRefusal`/`agentSessionRefusal`, and `structuredChatPtyWriteRefusalCopy` are removed. `agentSession.handoffStatus`, the ownership state machine (`handoffStage`/`handoffOperationId`), the claimed-agent PTY owner, and the SSH claim validation still remain.

## Open questions for the human
- Should the title name the PTY write-gate removal, or is it being split into a follow-up?

Pullfrog  | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

With the terminal handoff gone, the module named codex-tui-rollout-proof holds
only the pinned rollout lookup that structured Codex launches use to resume a
thread, so the name described code that no longer exists. Rename the module and
its options type. Also drop a mobile allowlist assertion that pinned the
removed agentSession.requestHandoff method, which no longer exists to allow.
The handoffStatus reply type still listed the terminal handoff's fields and
states (terminal placement, host label, proof retry, queued and waiting phases,
the to-terminal direction). No host writes them any more and the only client
reader parses the reply as unknown, so they described nothing. The reply on the
wire is unchanged.
…t decode

Nothing in this build writes a terminal owner (`runtimeKind: 'tui'`) or the
handoff's `preparing` / `old-owner-stopped` stages, but the in-memory types
still admitted them, so readers across the host kept branches for values no
path produces and the compiler could not point at them.

The store now validates the on-disk shape, which still accepts those values so
an older record is not quarantined, and maps them once while parsing:

- `preparing` and `old-owner-stopped` become `recovering`
- a `tui` lease becomes `native`; when it records a process it also becomes
  `conflicted`, the claim every build probes but never stops. A plain native
  owner would be stopped by restart recovery, here and in older builds.

Revisions are taken over the normalized state on both sides of every compare,
and the mapped record reaches disk with the store's first transaction, the
same way the tab-id backfill does.

The in-memory types narrow to what this build writes, and the branches that
existed only for the removed values go. Structured-worker identity keeps its
verdict for a former terminal owner by refusing a conflicted claim rather
than a non-native kind.
…ation

A reservation only ever names a native owner now, so the request no longer
carries a kind and the reserved lease records `native` directly. The attach
params keep `runtimeKind`: agentSession.ensure and create accept it, and the
operation fingerprint stored in the ledger covers it.
…at changes nothing else

Hiding a tab also committed the visibility index, so the no-op transaction
wrote the file even when its open-time revision was wrong. Committing the index
first leaves the pending rewrite as the only reason to write.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes — delta since the prior pullfrog review (b58818524d → 713b126feb, 3 commits, 49 files):

  • Normalized legacy terminal-handoff leases at decode: new agent-session-legacy-handoff-lease.ts maps a persisted runtimeKind: 'tui' to 'native' (a lease naming a process becomes a conflicted claim; one naming nobody keeps its claim) and handoffStage: 'preparing' | 'old-owner-stopped' to 'recovering'. parseState normalizes every record as it parses and flags legacyHandoffLeasesNormalized, so the rewrite lands with the store's first transaction rather than at open.
  • Narrowed the in-memory lease types: AgentSessionOwnerRuntimeKind is now 'native' and AgentSessionHandoffStage only the three live stages; the persisted superset moves behind the new PersistedAgentSession* types, and isAgentSessionRecord becomes isPersistedAgentSessionRecord.
  • Stopped threading the owner kind through a reservation: AgentSessionReserveRequest and AgentSessionReservation drop runtimeKind; every reservation and created record writes 'native'.
  • Dropped the now-impossible runtime-kind guards: recoveryMayStopOwner, structuredWorkerRecordIsCurrent, observeStructuredWorker, isSurfaceReleasableAgentSessionRecord, settleFailedAgentSessionPostAcquisitionAttachment, and the acquisition stage guard decide on claimStatus/stage alone.
  • Tests: new agent-session-legacy-handoff-lease and agent-session-record-store-legacy-handoff-lease suites pin the mapping and the rewrite/revision behavior, and a shared writeOlderBuildLease fixture re-seeds the existing suites with legacy leases.

I traced every persisted shape through the decode gate and its consumers: a recorded terminal owner always normalizes to conflicted, so it is probed but never signalled; live with no owner is rejected before normalization; and no normalized shape becomes a live writer. The focused suites (13 + 27 + 2 + 9 + 4 + 162 tests) pass.

Pullfrog  | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

…ackfill cannot supply its rewrite

The seeded record had no surface tab id, so the next open backfilled one and
that rewrite alone made the no-op transaction write. The test passed with the
legacy-lease rewrite signal removed.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes — delta since the prior pullfrog review (713b126feb → 2bd44f5f7d, 1 commit, 1 file):

  • Hardened the legacy-lease store test: persistLiveOwner in agent-session-record-store-legacy-handoff-lease.test.ts now passes surfaceTabId: 'tab-alpha' to reserveOwner, so the record on disk no longer needs the open-time surface-tab-id backfill. Without it, backfillAgentSessionSurfaceTabIds would itself set needsRewrite, and the first transaction would persist the normalized lease even if legacyHandoffLeasesNormalized were never wired — the flag this commit's predecessor added. The focused suite passes (2/2), and the assertion at line 96 can still fail if normalizeLegacyHandoffRecord stops mapping runtimeKind/handoffStage.

Pullfrog  | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

The handoff removal deleted it alongside the terminal-owner tests, but it
covers the surfaced-PTY block that still guards resume, including an agent
whose ownership is unknown.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

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

⚠️ Outside diff range comments (1)

🟡 Minor · Restore dead-TUI recovery before renewal. · structured-agent-session-lease-renewer.ts:69-86

src/main/native-chat/agent-session-wire/structured-agent-session-lease-renewer.ts:69-86
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Restore dead-TUI recovery before renewal.

A proven-dead TUI record now enters ordinary renewal. renewAgentSessionLease rejects that probe, so the record remains live with its old owner. Restart reconciliation skips already reconciled records. The reachable hold/resume path then returns agent_session_ownership_unknown because it accepts only released records. Restore the former dead-TUI recovery or eviction branch and skip ordinary renewal for that probe. Direct attach can replace the dead owner, but hold/resume cannot.


ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: e824eb07-a779-4bbf-9bf5-48bc4f8ffecf

📥 Commits

Reviewing files that changed from the base of the PR and between 713b126 and 2bd44f5.

📒 Files selected for processing (1)
  • src/main/runtime/agent-session-record-store-legacy-handoff-lease.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 4 remain after this review.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes — delta since the prior pullfrog review (2bd44f5f7d → f79eb9b88f, 1 commit, 1 file):

  • Restored an unrelated test the refactor deleted by accident. test(worktree-activation) puts back does not resume OMP when a surfaced %s agent lacks conversation ownership in worktree-agent-activation-gate.test.ts. The PR's first commit removed it as collateral while deleting two adjacent TUI-owner tests, even though it has nothing to do with the handoff. It guards the hasUnclaimedRecovery block (worktree-agent-activation-gate.ts:234-241), asserts an exact 'blocked' outcome, and is byte-identical to the base version. The suite passes 32/32 and oxlint reports no warnings.

Pullfrog  | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

@brennanb2025

Copy link
Copy Markdown
Contributor Author

Review status: ready for review

Head f79eb9b88f. CI: 29 checks pass, 14 skipped. Mergeable, no conflicts with main.

Review loops. Six independent review passes, plus a separate architecture review after the third. Each pass that found something fixed it here with a test that goes red without the fix. The last pass found no production issues; its one commit restores, unchanged from main, a test the original removal deleted by mistake.

What review changed, on top of the original removal:

  • The new wait at quit (for already-delivered chat events before the resume snapshot) is capped at 1 second, so a slow journal write can no longer stop the chat's agent process from being stopped.
  • agentSession.handoffStatus waits behind a chat that is mid-start again, as it did on main, so shipped clients see the chat as the owner rather than "no owner".
  • The agent-session PTY write gate is removed. Only the handoff ever bound a terminal to a chat, so after the removal it admitted every write and was dead code on every terminal write path.
  • Helpers only the handoff called are removed, with their test coverage moved onto live code paths; a Codex module named after the terminal handoff is renamed for what it still does.
  • The refusal "The session is mid-handoff (new-owner-proving)." now reads "The chat is still starting." (only command-line, mobile and remote clients can see it).
  • Architecture: values only the handoff ever saved (runtimeKind: 'tui', stages preparing / old-owner-stopped) are converted once when a record is read, and the in-memory types no longer allow them. Before this, a legacy terminal owner was never overwritten and eight readers kept branches for values nothing produces, which is why each review pass kept finding leftovers. A recorded terminal process is marked conflicted: checked, never stopped, by this build and by older builds reading the converted record.

Readiness checklist. Start of session: pass, with two minor findings (the uncapped quit wait and the handoffStatus ordering), both fixed. End of session: pass, no findings.

Live Electron QA (macOS, background launch, isolated profile). Pass: workspace activation, terminal typing and paste, a Claude chat to completion, restart restore, a message held while a chat starts, upgrade and downgrade on one profile (main → PR → main → PR, history and follow-ups intact each time), and a record hand-edited to the old handoff values. Screenshots are linked in the PR description.

Not covered: the mobile test suite (mobile dependencies aren't installed locally; the mobile change is one call-site argument and one fixture), Codex chats in live QA, and live SSH / Windows runs (no SSH- or Windows-specific path changed).

Separate, existing issue found during QA (not caused by this PR): quitting with a Claude chat open takes about 15 s on main and on this PR alike. The Claude session shutdown fallback runs about 10 s and then fails with "claude structured session shutdown could not prove every child stopped". It should be its own fix.

Follow-ups worth considering (all already on main): reword "manual recovery is required" in the owner-status reply, which names a step that no longer exists; handoffStage / handoffOperationId would read better under ownership names, but they are persisted, so renaming them is a migration.

@brennanb2025

Copy link
Copy Markdown
Contributor Author

Ready to merge. This is my view as coordinator of the native-chat redesign stack.

Why:

  • Scope matches the plan: the removed feature was never reachable by any client, and everything shared with ordinary chats is kept.
  • Its review converged: six passes plus an architecture review. The last pass found no production issue.
  • Readiness checklist: passed at start and end.
  • CI: all green (29 pass, 14 skipped by design).
  • Mergeable against main.
  • Live QA covered the risky part: upgrade and downgrade on one profile, and a record hand-edited to the old handoff values.
  • Converting legacy values once, when a record is read, is the right fix for the leftover branches earlier passes kept finding.

Merge order: this is the base of the stacked redesign PRs #22808, #22820, #22811 and #22812, which are all based on this branch. Merge this first. Each of those then needs its base switched to main and a rebase or merge onto main before its own merge. #22820 and #22808 already build on this head. #22811 and #22812 were built on an earlier head of this branch, so re-check them after they're retargeted.

Not blocking:

  • The mobile suite wasn't run locally. The mobile change is one call-site argument and one fixture, so mobile CI covers it.
  • The ~15 s quit with a Claude chat open is on main too. The open fix(claude): let the lease release close the adapter's session so an evicted chat resumes #22641 (bounded shutdown of the Claude process tree) targets that failure.
  • The follow-ups listed above (the "manual recovery" wording, and ownership-style names for the persisted handoff fields) can wait. Renaming the persisted fields would be a migration.

@brennanb2025
brennanb2025 merged commit d443320 into main Sep 25, 2026
43 checks passed
brennanb2025 added a commit that referenced this pull request Sep 25, 2026
Main removed the terminal handoff (#22783), so its root-exit stop reading and the
handoff integration test are dropped with it.
brennanb2025 added a commit that referenced this pull request Sep 25, 2026
Resolve conflicts with the terminal-handoff removal (#22783): take main's runtime wiring and integration test, and re-apply this branch's Claude child-work evidence wiring and the status-sink child-work test.
brennanb2025 added a commit that referenced this pull request Sep 25, 2026
Brings in the removal of the structured-chat terminal handoff (#22783): an agent session is now
owned natively only, and the agent-session PTY write gate is gone. The next commit removes this
branch's remnants of that handoff.
lurunzi added a commit to lurunzi/orca that referenced this pull request Sep 26, 2026
…tream removal

Upstream stablyai#22783 deletes the structured-session terminal handoff these fork
features were built on. Follow upstream: remove the Chat <-> agent TUI toggle,
the handoff status row, Release session, and the stale-handoff-status fix.
The top-right cluster keeps Continue in New Session and split.

Reverts 4a9be55, 164c08b, 386d474, f52e9dd, 55e1d4a.

Co-authored-by: Codex <noreply@openai.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
lurunzi added a commit to lurunzi/orca that referenced this pull request Sep 26, 2026
Follows upstream stablyai#22783 (terminal handoff removed; fork handoff features were
reverted in the previous commit). Upstream's merged Cursor usage replaces the
fork's earlier Cursor usage remnants. Keeps fork prompt suggestions, async
question card, Antigravity local quota probe, coordinator identity, and the
Windows argv reservation proof (Claude only; upstream dropped TUI leases).
Moves the Cursor transcript lookup out of session-file-resolver.ts to stay
within max-lines.

Co-authored-by: Codex <noreply@openai.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Pr1p Pr1p mentioned this pull request Sep 27, 2026
6 of 9 tasks
sj900211 added a commit to sj900211/orcinus that referenced this pull request Oct 12, 2026
The stage the staged plan existed for. Measured from the post-stage-2 main this
cost 78 commits and 14 conflicts (13 in the PTY delivery subsystem) instead of
881 commits and 32 conflicts from the original base.

**The PTY wall was not what the 월드맵 feared.** It recorded
`pty-project-window-stream-routing` as a ~1,500-line feature-scale
re-implantation and deferred it twice. In fact all 14 conflicts come from three
upstream commits:

- `9700030689` refactor(pty): make renderer delivery optional for headless
  runtimes (stablyai#23117) — `session.mainWindow` becomes optional and every
  `isDestroyed()` guard gains a null check.
- `82412dab8b` Persist profile state in SQLite with background writes (stablyai#22612).
- `d443320af2` refactor(native-chat): remove the unused terminal handoff (stablyai#22783).

On the same lines the fork had already replaced the main-window guard with
owner-window routing (`resolvePtyOwnerWindow`, `sendToPtyOwner`,
`ownerFlowStateForPty`). The fork's guard is strictly finer: in headless mode no
window is registered with the router, the owner resolves to null, and the fork
takes the same early-return upstream's null check adds — while upstream's coarser
guard would drop or misroute project-window delivery when the main window is
gone. So most of the cluster resolves to ours, and nothing had to be reimplanted.

Conflicts resolved (14)
- `pty/delivery/{accept,accounting,exit,flush}.ts`, `pty/provider/bind-listeners.ts`:
  ours. Verified mechanically — resolving every hunk to ours produced a file
  byte-identical to main, so upstream has no other change in them.
- `pty/delivery/payload.ts`, `pty/runtime/operations.ts`: per hunk, never
  whole-file. A whole-file ours on operations.ts would have restored imports of
  two modules `d443320af2` deleted and a 3-arg `writePtyFromRuntimeController`
  while the merged `runtime/controller.ts` calls the 2-arg form.
- `pty/ipc/{resize-visibility,serialize-buffer,spawn-commit-persist,write-input,write}.ts`,
  `pty/register-handlers.ts`: combined, preserving every per-window router
  injection. A previous attempt at this merge failed its gates by dropping them.
- `persistence/loading-store/store.ts`: combined for `82412dab8b`'s SQLite
  authority.

Edits outside the 14 conflicted files (6)
Each one is a defect the conflict list could not show, because the file merged
cleanly while the meaning did not.
1. `pty/session.ts` + `pty/delivery/window-flow-state.ts`: `session.ts` merged
   cleanly into a crash. `mainWindow` is now optional, upstream's
   `register-headless-runtime.ts` passes `undefined` where it used to pass a fake
   destroyed window, and the fork's `createWindowFlowState(session,
   args.mainWindow.webContents)` therefore threw on every `orca serve` and orcad
   registration. Fixed by narrowing a flow state's `webContents` to
   `Pick<WebContents, 'id' | 'isDestroyed' | 'send'>` and passing a destroyed
   stand-in, with no cast. `window-flow-state.ts` stays at 297 of its 300
   permitted code lines.
2. `persistence/loading-store/store-domain-composition.ts`: added
   `SatelliteWindowSessionPersistence &` to `StoreDomainOperations`, without
   which four satellite-session methods disappear from the `Store` type.
3. `pty/register-without-renderer.test.ts`: two `registerPtyHandlers(renderer as
   never, …)` casts, matching the shape 274 existing calls in this repo use.
   This is an upstream-owned file, so the next sync touching it will conflict here.
4. Three usage test fixtures (`usage-custom-range-filters.test.ts`,
   `UsageNumberFormatToggle.test.tsx`, `usage-pane-test-state.ts`): added the
   fields upstream `297cfe0cf3` (stablyai#22073) made required. **These were already
   broken on main**: a fresh worktree at `e0686816e8` reports exactly these 4
   type errors. Stage 2's gate missed them because a warm `composite`
   `.tsbuildinfo` cache under-reported. Typecheck cold before trusting a pass.
5. `pty/register-handlers.ts` + the window-affinity router:
   `setMainWindowForRouting` widened to `BrowserWindow | null` and called
   unconditionally as `setMainWindowForRouting(mainWindow ?? null)`. Skipping it
   when `mainWindow` is undefined left a previously routed live window routed, so
   a desktop → headless re-registration would send `pty:exit`, `pty:spawned`,
   `pty:writeUnavailable`, `pty:clearBuffer:request`,
   `pty:serializeBuffer:request` and `pty:modelRestoreNeeded` to a stale window.
   Current call order prevents it in production; that is not a guarantee worth
   depending on. Restores parity with main.

Gates (each captured from the command's own exit code)
- typecheck, run cold after deleting `config/*.tsbuildinfo`: 0 errors. The same
  command on a main worktree reports its 4 pre-existing errors.
- oxlint: exit 0 on the full tree and on the touched files.
- verify-orcinus-identity: OK, upstreamRefs=0.
- electron-builder smoke: unsigned, update signature checks disabled.
- fork contract tests: 3 files / 24 tests.
- `register-without-renderer.test.ts` (upstream's new 231-line headless test): 4/4.
- router-referencing tests: 3 files / 70 tests.
- Broad conflict-area suite: 122 failures, none attributable. 121 fail
  identically on a main worktree; the remaining one, upstream's own
  `profile-state-crash-recovery.test.ts`, times out identically on a
  `da6d483ab9` worktree. Per-test JSON diff: 0 failures unique to main.

Reviewed by an independent critic: APPROVE, no blocking defects, every gate
re-measured, every fork-only and upstream-only file confirmed equal to its parent
apart from the six edits above, no IPC channel dropped. Non-blocking and
deliberately not addressed: `hasUnpricedModels: false` does not match its own
fixture's data (behaviour-neutral); the upstream-owned test file will re-conflict;
plan-B's satellite-session durability question is still open.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant