Fix OpenCode and Codex session resilience#338
Conversation
![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: cb78417627
ℹ️ 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".
| this.sendError(ws, { | ||
| code: 'INVALID_TERMINAL_ID', | ||
| message: 'Terminal not running', | ||
| requestId: m.attachRequestId, | ||
| terminalId: m.terminalId, |
There was a problem hiding this comment.
terminal.attach now includes requestId: m.attachRequestId in INVALID_TERMINAL_ID errors, but the client’s recovery path in TerminalView only runs when msg.requestId is absent (msg.code === 'INVALID_TERMINAL_ID' && !msg.requestId). In attach-failure cases (for example, stale terminal IDs after reconnect), this change prevents the auto-recreate logic from running and leaves the pane stuck instead of recovering. Either keep attach errors requestless (as before) or update client-side handling to treat attach request IDs as recoverable invalid-terminal signals.
Useful? React with 👍 / 👎.
| forgetTerminal(terminalId: string): void { | ||
| pendingByTerminalId.delete(terminalId) | ||
| }, |
There was a problem hiding this comment.
The new association publisher keeps publishedPairs and activeTerminalBySession entries forever, while forgetTerminal only removes pendingByTerminalId. On long-lived servers that cycle through many sessions/terminals, these sets/maps grow unbounded and retain stale dedupe state, creating a memory/perf regression introduced by this change. forgetTerminal should also evict keys tied to the terminal from the other caches.
Useful? React with 👍 / 👎.
1 participant
Summary
sessionRefacross terminal create, websocket inventory, sidebar state, and agent-chat restore flows.Verification
npm run typecheckpassed.npm run test:vitest -- test/unit/client/components/App.ws-bootstrap.test.tsx --runnpm run test:vitest -- test/unit/server/coding-cli/opencode-activity-tracker.test.ts --runnpm run test:vitest -- test/server/ws-handshake-snapshot.test.ts test/server/session-association-broadcast.test.ts --runnpm run test:vitest -- test/e2e/terminal-restart-recovery.test.tsx test/unit/client/components/TerminalView.resumeSession.test.tsx test/e2e/agent-chat-resume-history-flow.test.tsx --rungit diff --checkpassed.npm run verifystill fails in existing broad-suite clusters outside the final review patch: agent tab write tests, legacy/noncanonical agent-chat restore expectations, and Codex registry/app-server recovery tests.