Repository navigation
Conversation
Removes the fork's agent-resume work: fork PRs #1, #5, #6, #9, #11 and #30. Every file only those PRs touched is restored to upstream d2dbe2c, the upstream commit this branch was last synced to, and the files they added are deleted. In the six locale files, which also carry other fork strings, only the AgentResumeChooser block is removed. Files are restored rather than each merge reverted because later upstream syncs rewrote the code around the resume changes, so plain reverts conflict. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
kiwidream
added
the
optional revert
|
Review complete. No blocking issues — approved ✅; 1 nitpick below. 🧹 Nitpicks (1) — 🟢 1 low
This PR reverts the fork's agent-resume-after-terminal-loss changes, cleanly removing the resume chooser, relay user-ended-session signaling, and related state plumbing across 134 files. Reviewed commit: e6fe271 |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e6fe271b4f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| existing && | ||
| existing.tabs.length > 0 && | ||
| options.force !== true && | ||
| options.onlyRuntimeOwnedTerminals !== true && | ||
| !completesRuntimeOnlySnapshot | ||
| options.onlyRuntimeOwnedTerminals !== true |
There was a problem hiding this comment.
When a fresh headless host first handles a list or subscribe request, the runtime-only hydrate can create a nonempty snapshot containing only serve/SSH-owned tabs. This condition then makes the subsequent full hydrate return early, so other persisted client-created panes never appear; activating one fails with tab_not_found and may cause the client to retire the pane. The removed runtimeOnlyHydratedWorktreeIds distinction is still required even without agent auto-resume so the first request's ordering does not determine which persisted tabs survive restoration.
Useful? React with 👍 / 👎.
| const previousIncarnationId = authoritativePtyIncarnationId | ||
| rebindRemoteTerminalHandle(update.terminalHandle, update.incarnationId) | ||
| announceProcessReplacement(previousIncarnationId, update.incarnationId) | ||
| rebindRemoteTerminalHandle(update.terminalHandle) |
There was a problem hiding this comment.
When a remote host replaces a lost PTY process, this path keeps the existing xterm instance but now discards the published incarnation ID and rebinds without notifying the output callbacks. Modes armed by the dead TUI—such as mouse reporting, focus reporting, and bracketed paste—therefore remain active in the replacement shell, causing ordinary input and mouse activity to arrive as escape sequences. Process replacement remains a generic transport recovery path after agent auto-resume is removed, so preserve the incarnation comparison and mode reset while still allowing a re-minted handle for the same process to retain its modes.
Useful? React with 👍 / 👎.
![tenki-reviewer[bot]](https://avatars.githubusercontent.com/in/2900641?s=60&v=4){kind=small}
ELI5
The fork added its own way of bringing agent sessions back after a restart or a lost terminal. It has been making terminals less stable. This PR takes all of that out, so on this point the fork behaves exactly like upstream Orca again.
What Changed
Before: after a host restart, the fork tried to start each pane's agent session again by itself. If it found more than one session that could belong to a pane, it showed a "Which session should this terminal resume?" prompt. A paired host also restarted a pane's agent when the pane's terminal was lost. Along the way, the fork changed how agent hook events, sleeping session records and session ownership are kept and routed.
After: none of that exists. Restoring agent sessions works exactly as it does upstream, and the session prompt is gone.
This undoes six fork PRs: #1 (keep a pane's resume record), #5 (connection-id host routing for those records), #6 (auto-resume after a host restart, with the session prompt), #9 (resume owner type and the prompt's strings), #11 (start resume only on keystrokes) and #30 (host-side resume after terminal loss).
How:
d2dbe2c385, the upstream commit this branch was last synced to: 80 files restored and 48 files the resume work added deleted.AgentResumeChooserstrings are removed. The updater, release-source and titlebar strings stay.LOCAL:updater/CI/titlebar commit, or from the unreachable-host (fix(repos): a project on an unreachable host can still leave Orca #3), remote-worker nesting (fix(sidebar): nest a delegated remote worker under its coordinator #4) and mobile test commits.Why
The resume work has been the fork's main source of terminal instability, and it is the biggest merge burden in the daily upstream syncs. The open sync (#32) is blocked on exactly this code: upstream stablyai#23947 changes
server-cleanup.tsand the relay'sagent-hook-server.tsin ways that clash with the fork's resume rules. This PR puts both files back to upstream.I restored files to upstream instead of running
git reverton each merge. Later upstream syncs rewrote the code around the resume changes, so plain reverts conflict (#11's revert already conflicts indirect-ssh-retry-status.ts). Restoring the upstream file gives exactly "no fork changes here". It is also easy to check: every changed file is either identical to upstreamd2dbe2c385or one of the six locale files.Mixed versions: an older fork relay may still send the "user ended this Claude session" notice. A host built from this branch rejects it when it validates the payload and never applies it, which is how that notice was designed to fail.
Linked Issue
None. This is a fork-only change with no upstream issue.
Visual Proof
N/A. No new UI. The only visible change is that the resume prompt stops appearing. It only showed after a host restart with more than one candidate session, so no before/after was captured.
Testing
Linux only (minidev).
pnpm tcprojects (node, web, cli): pass.tsconfig.e2e.jsonstill reports type errors, but only in files that are byte-identical to upstream. It is not part ofpnpm tc.Unit tests: every changed test file plus every test that imports a changed file, 603 files: 602 passed, 1 skipped (7379 tests passed, 11 skipped).
verify:localization-catalogs,verify:localization-extraction,verify:localization-coverage,verify:rpc-params-catalog,check:max-lines-ratchet,check:dead-classes: pass.oxlintandoxfmt --checkon every changed file: clean.I manually tested these changes locally
Automated tests added/updated, or explained why not below
No new tests. The removed tests covered removed code, and the restored files bring back upstream's own tests.
AI Disclosure
Prepared with Claude Code (Claude Opus 5.5).
Review
Agent skill upstream boundary
docs/reference/agent-skill-sharing-upstream-boundary.mdand copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.Notes
Ensure no issues in: Security, Cross-platoform support (Linux, Windows, Mac), Remote SSH, Mobile, general backwards compatibility, performance
Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.