Repository navigation
Conversation
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. 🧰 Additional context used📚 Code guidelines (1)No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📥 CommitsReviewing files that changed from the base of the PR and between a9ee5016de0275972cb614ba884909875a528d2c and fd0c6d8. 📒 Files selected for processing (2)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review. 📝 WalkthroughWalkthrough
Priority: ➖ Normal Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to The change settles a Codex dispatch as failed and escalates to the coordinator when the reconnect-failed banner appears, while keeping the PTY and worktree. The earlier review concerns appear fixed and covered by tests. No merge-blocking risk is evident. A live Codex restart was not tested, so a quick manual check is worthwhile. Security Architecture ReviewSecurity architecture risk: 🟡 Moderate · up to The eligibility checks contain the impact, but matching terminal text can become authoritative evidence that a task has failed without independently confirming the session failure. Failure reporting is also separate from the task-state change. These warrant review of output authenticity and recovery behavior. Retained concerns
Security review detailsSecurity Blast Radius
Security Findings and Attack Paths
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
c42c42b0-cf09-4e9c-86c8-728b0fd57d75
📥 Commits
Reviewing files that changed from the base of the PR and between 87a2c3c and aa2ef773cb040a67a4e47462f459b22dd8034e5f.
📒 Files selected for processing (4)
src/main/runtime/orca-runtime-on-pty-data.tssrc/main/runtime/orca-runtime-subscribe-to-terminal-resize.tssrc/main/runtime/orchestration/codex-reconnect-failed-dispatch.test.tssrc/main/runtime/orchestration/db/dispatch-context/dispatch-completion.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.
| @@ -115,6 +148,17 @@ export class OrcaRuntimeWithOnPtyData extends OrcaRuntimeWithPreparePtyExecution | |||
| pty.tailLinesTotal += nextTail.newCompleteLines | |||
| pty.preview = buildPreview(pty.tailBuffer, pty.tailPartialLine) | |||
| this.scheduleWaitBlockedCheck(ptyId, normalized.text, at) | |||
| if ( | |||
| !codexReconnectFailureHandledPtys.has(pty) && | |||
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
#!/bin/bash
set -e
file=src/main/runtime/orca-runtime-on-pty-data.ts
printf '%s\n' '--- PR diff ---'
git diff --no-ext-diff --unified=25 b5869eeaae72dbf2b7a4bd409f2dd66860bfcf08 aa2ef773cb040a67a4e47462f459b22dd8034e5f -- "$file"
printf '%s\n' '--- current handler symbols ---'
rg -n -C 4 'codexReconnectFailureHandledPtys|failActiveDispatchOnCodexSessionUnrecoverable|onPtyData|ptyRecords|ptyRecord|PtyRecord' src/mainRepository: stablyai/orca
Length of output: 45651
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- Handler ---'
nl -ba src/main/runtime/orca-runtime-on-pty-data.ts | sed -n '1,180p'
printf '%s\n' '--- Settlement method definitions and non-test callers ---'
rg -n -C 5 'failActiveDispatchOnCodexSessionUnrecoverable' src/main -g '*.ts' -g '!*.test.ts'
printf '%s\n' '--- PTY record declarations ---'
rg -n -C 3 'interface [A-Za-z]*PtyRecord|type [A-Za-z]*PtyRecord|PtyRecord\s*=' src/main/runtime -g '*.ts' -g '!*.test.ts'
printf '%s\n' '--- Dispatch identifiers in runtime source ---'
rg -n 'activeDispatchId|dispatchId' src/main/runtime -g '*.ts' -g '!*.test.ts' | head -n 120Repository: stablyai/orca
Length of output: 11335
🏁 Script executed:
#!/bin/bash
printf '%s\n' '--- Settlement implementation ---'
nl -ba src/main/runtime/orca-runtime-subscribe-to-terminal-resize.ts | sed -n '85,175p'
printf '%s\n' '--- PTY worktree record access and declarations ---'
rg -n -C 3 'getOrCreatePtyWorktreeRecord|ptyWorktreeRecords|PtyWorktree|worktreeRecord' src/main/runtime -g '*.ts' -g '!*.test.ts' || true
printf '%s\n' '--- Dispatch ownership references ---'
rg -n 'activeDispatchId|dispatchId|activeDispatch|currentDispatch' src/main/runtime -g '*.ts' -g '!*.test.ts' | head -n 160 || trueRepository: stablyai/orca
Length of output: 41397
🏁 Script executed:
#!/bin/bash
printf '%s\n' '--- Dispatch and session identity for PTY records ---'
nl -ba src/main/runtime/orca-runtime-get-pty-record-for-pane-key.ts | sed -n '220,325p'
printf '%s\n' '--- Structured worker terminal lifecycle ---'
nl -ba src/main/runtime/rpc/orchestration-structured-worker-lifecycle.ts | sed -n '40,110p'
printf '%s\n' '--- Worker start and terminal assignment symbols ---'
rg -n -C 3 'agent_terminal_handle|markWorkerDispatchReady|startWorker|workerStart|terminalHandle' src/main/runtime/rpc/methods/orchestration/worker src/main/runtime/rpc -g '*.ts' -g '!*.test.ts' | head -n 180Repository: stablyai/orca
Length of output: 25940
🏁 Script executed:
#!/bin/bash
printf '%s\n' '--- Worker-start handler ---'
nl -ba src/main/runtime/rpc/methods/orchestration/worker/workers.ts | sed -n '1,230p'
printf '%s\n' '--- Worker start readiness settlement ---'
nl -ba src/main/runtime/rpc/methods/orchestration/worker/worker-start-readiness-settlement.ts | sed -n '1,175p'
printf '%s\n' '--- Agent terminal creation by dispatch ---'
nl -ba src/main/runtime/rpc/methods/orchestration/worker/worker-worktree-creation.ts | sed -n '45,145p'Repository: stablyai/orca
Length of output: 15549
🏁 Script executed:
#!/bin/bash
printf '%s\n' '--- Local worker-start module outline and size ---'
wc -l src/main/runtime/rpc/methods/orchestration/worker/local-worker-start.ts src/main/runtime/rpc/methods/orchestration/worker/worker-start-schema.ts
ast-grep outline src/main/runtime/rpc/methods/orchestration/worker/local-worker-start.ts
printf '%s\n' '--- Local worker-start terminal selection ---'
rg -n -C 5 'terminal|createManagedWorktree|startupTerminal|createStructuredWorkerSession|terminalHandle|dispatchId' src/main/runtime/rpc/methods/orchestration/worker/local-worker-start.ts
printf '%s\n' '--- Worker-start terminal parameters ---'
nl -ba src/main/runtime/rpc/methods/orchestration/worker/worker-start-schema.ts | sed -n '1,180p'Repository: stablyai/orca
Length of output: 8644
🏁 Script executed:
#!/bin/bash
printf '%s\n' '--- Explicit worker terminal validation ---'
nl -ba src/main/runtime/rpc/methods/orchestration/worker/explicit-worker-terminal-validation.ts | sed -n '1,220p'
printf '%s\n' '--- placeWorkerAgent definition ---'
rg -n -C 12 'placeWorkerAgent' src/main/runtime/rpc/methods/orchestration/worker -g '*.ts' -g '!*.test.ts'
printf '%s\n' '--- Explicit terminal start flow ---'
nl -ba src/main/runtime/rpc/methods/orchestration/worker/local-worker-start.ts | sed -n '82,200p'Repository: stablyai/orca
Length of output: 17201
🏁 Script executed:
#!/bin/bash
printf '%s\n' '--- Authority preparation definitions ---'
rg -n -C 5 'prepareStartingWorkerAuthority' src/main -g '*.ts' -g '!*.test.ts'
printf '%s\n' '--- Terminal agent recognition definitions ---'
rg -n -C 4 'isTerminalRunningAgent' src/main -g '*.ts' -g '!*.test.ts'
printf '%s\n' '--- Explicit terminal placement ---'
nl -ba src/main/runtime/rpc/methods/orchestration/worker/worker-start-agent-placement.ts | sed -n '78,150p'Repository: stablyai/orca
Length of output: 41962
🏁 Script executed:
#!/bin/bash
nl -ba src/main/orchestration/db/worker-dispatch/worker-dispatch-authority.ts | sed -n '1,175p'Repository: stablyai/orca
Length of output: 253
🏁 Script executed:
#!/bin/bash
nl -ba src/main/runtime/orchestration/db/worker-dispatch/worker-dispatch-authority.ts | sed -n '1,175p'Repository: stablyai/orca
Length of output: 8006
🏁 Script executed:
#!/bin/bash
rg -n -C 9 'getOrCreatePtyWorktreeRecord|recordPtyWorktree' src/main/runtime/orca-runtime-record-pty-worktree.tsRepository: stablyai/orca
Length of output: 2386
🏁 Script executed:
#!/bin/bash
rg -n -C 8 'findActiveDispatchForAssignee\(' src/main/runtime/orchestration/db -g '*.ts' -g '!*.test.ts'Repository: stablyai/orca
Length of output: 13252
🏁 Script executed:
#!/bin/bash
nl -ba src/main/runtime/orchestration/db/dispatch-context/dispatch-lookup.ts | sed -n '1,24p;131,175p'Repository: stablyai/orca
Length of output: 3373
🏁 Script executed:
#!/bin/bash
rg -n -C 3 'findTransferableWorkerTerminalResource|createWorkerTerminalResourceStatement|transferWorkerTerminalResourceStatement' src/main/runtime/orchestration/db -g '*.ts' -g '!*.test.ts'Repository: stablyai/orca
Length of output: 15282
Key reconnect-banner handling to the dispatch.
workerStart --terminal can reuse a recognized Codex terminal in the same worktree for a later dispatch. If that terminal keeps the same PTY record, codexReconnectFailureHandledPtys suppresses its later reconnect banner, so the new dispatch can remain active. Track handling by dispatch ID and match only banner output produced after that dispatch starts. pty.preview retains earlier output, so a dispatch ID alone could also reprocess the previous banner.
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
One functional bug: the fallback handle lookup passes the PTY record object where a ptyId string is expected, so it can never match. Two further points are worth a deliberate decision before merge: the detector is pinned to the issue text instead of a captured transcript, and the retained live terminal becomes transferable by a later dispatch.
Reviewed changes
- Codex reconnect-failed detection —
src/main/runtime/orca-runtime-on-pty-data.tsscans the bounded preview after every chunk for the exact three-line Codex banner and, on a match, settles the active dispatch through the newfailActiveDispatchOnCodexSessionUnrecoverable. workerSessionUnrecoverablesettlement —dispatch-completion.tsfails areadyworker (stagesession_unrecoverable) and returns the task toreadywithout marking the process exited or touching the terminal resource.- Escalation refactor —
orca-runtime-subscribe-to-terminal-resize.tsfactors the coordinator escalation intonotifyWorkerDispatchFailureand reuses it for the new session-failure path. - Regression tests —
codex-reconnect-failed-dispatch.test.tscovers the full banner, a split PTY chunk, a partial banner, and non-Codex isolation.
ℹ️ The detector is pinned to a remembered banner, not a captured transcript
The three banner lines and the exact-match matcher come from the issue text, and the author notes no PTY transcript was captured. AGENTS.md and docs/reference/agent-pty-transcript-capture.md require a text rule over an agent screen to be written against a captured transcript, because the real TUI can repaint with cursor/erase controls that buildPreview (6 lines / 300 chars, trimmed, \n-joined) may not present as three consecutive clean lines. The new test feeds a hand-authored string through onPtyData, so it proves the matcher works on that string but not that the live Codex screen ever produces that shape.
Technical details
# Transcript evidence for the Codex reconnect-failed rule
## Affected sites
- src/main/runtime/orca-runtime-on-pty-data.ts:19-32 — the banner constant and `hasCodexReconnectFailureBanner`.
- src/main/runtime/orchestration/codex-reconnect-failed-dispatch.test.ts:140-145 — the test's hand-authored banner string.
## Required outcome
- A committed transcript fixture (or an explicit note that the screen is unreproducible) so the rule
is validated against bytes the terminal actually received, not a reconstructed string.
## Open questions for the human
- Can the Codex app-server restart be reproduced with `config/scripts/capture-agent-pty-transcript.mjs`?
If not, is the issue text acceptable as the authority?ℹ️ The retained terminal is silently reusable by a later dispatch
Keeping worker_terminal_resources at (owned, not_requested) with a live PTY matches the "keep for diagnosis" intent, but it also makes the row transferable: findTransferableWorkerTerminalResource accepts a failed owner whose resource is owned/not_requested, keyed only on pane + process incarnation. A later --terminal/retry dispatch on that pane can transfer ownership and reset release_state, and the original dispatch's release then reports ownership_transferred instead of closing the pane. Pre-PR the dispatch was still active, so neither was reachable. This is opt-in, but worth a deliberate decision rather than a side effect.
Technical details
# Retained-terminal transfer after a session-unrecoverable failure
## Affected sites
- src/main/runtime/orchestration/db/worker-terminal/worker-terminal-transfer.ts:44-53 — admits a
`failed` owner with `(ownership_state='owned', release_state='not_requested')`.
- src/main/runtime/orchestration/db/worker-dispatch/worker-dispatch-authority.ts:121-137 — an
explicit `--terminal` reuse transfers the resource.
- src/main/runtime/orchestration/db/worker-terminal/worker-terminal-release.ts:52-57 — the original
dispatch then reads `ownership_transferred` rather than owning the pane.
## Required outcome
- Decide whether a live-but-dead session's terminal should be transferable while retained for
diagnosis, and whether the failed dispatch should keep release authority over it.
## Open questions for the human
- Is reusing the exact pane of an explicitly-unrecoverable Codex session intended?deepseek-v4.1-flash (free via Pullfrog for OSS) ｜ 𝕏
aa2ef77 to
a9ee501
Compare
Sync update (
|
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
c6e0c9d4-a8ea-4ecc-b92e-e900e74f54f4
📥 Commits
Reviewing files that changed from the base of the PR and between aa2ef773cb040a67a4e47462f459b22dd8034e5f and a9ee5016de0275972cb614ba884909875a528d2c.
📒 Files selected for processing (4)
src/main/runtime/orca-runtime-on-pty-data.tssrc/main/runtime/orca-runtime-subscribe-to-terminal-resize.tssrc/main/runtime/orchestration/codex-reconnect-failed-dispatch.test.tssrc/main/runtime/orchestration/db/dispatch-context/dispatch-completion.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review.
| @@ -115,6 +182,12 @@ export class OrcaRuntimeWithOnPtyData extends OrcaRuntimeWithPreparePtyExecution | |||
| pty.tailLinesTotal += nextTail.newCompleteLines | |||
| pty.preview = buildPreview(pty.tailBuffer, pty.tailPartialLine) | |||
| this.scheduleWaitBlockedCheck(ptyId, normalized.text, at) | |||
| const leaf = this.getLeavesForPty(ptyId)[0] | |||
| const handle = this.handleByPtyId.get(ptyId) ?? this.findHandleForPtyRecord(pty) | |||
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
sed -n '165,205p' src/main/runtime/orca-runtime-on-pty-data.ts
sed -n '115,160p' src/main/runtime/orca-runtime-bind-pty-incarnation-handle.ts
rg -n 'findHandleForPtyRecord|handleByPtyId\.(delete|clear|set)' src/main/runtimeRepository: stablyai/orca
Length of output: 6022
🏁 Script executed:
git diff --unified=8 5c62cb5320c399662b569da22a1399393381979b a9ee5016de0275972cb614ba884909875a528d2c -- src/main/runtime/orca-runtime-on-pty-data.ts
rg -n 'observeCodexReconnectFailureOutput|interface .*Pty|type .*Pty|handleByPtyId|syntheticTerminalHandles|findHandleForPtyRecord' src/main/runtime/orca-runtime-on-pty-data.ts src/main/runtime/orca-runtime-bind-pty-incarnation-handle.ts src/main/runtime/orca-runtime-invalidate-all-handles-for-pty.ts src/main/runtime/orca-runtime-refresh-floating-workspace-pty-liveness.ts
sed -n '1,100p' src/main/runtime/orca-runtime-on-pty-data.ts
sed -n '70,155p' src/main/runtime/orca-runtime-bind-pty-incarnation-handle.ts
sed -n '1,120p' src/main/runtime/orca-runtime-invalidate-all-handles-for-pty.ts
sed -n '125,180p' src/main/runtime/orca-runtime-refresh-floating-workspace-pty-liveness.tsRepository: stablyai/orca
Length of output: 22475
🏁 Script executed:
rg -n 'observeCodexReconnectFailureOutput|handleByPtyId\.delete|handleByPtyId\.clear|handleByPtyId\.set' src/main/runtime
sed -n '140,195p' src/main/runtime/orca-runtime-has-terminals-for-worktree.ts
sed -n '1,115p' src/main/runtime/orca-runtime-refresh-floating-workspace-pty-liveness.ts
sed -n '1,90p' src/main/runtime/orca-runtime-bind-pty-incarnation-handle.tsRepository: stablyai/orca
Length of output: 12092
🏁 Script executed:
sed -n '95,180p' src/main/runtime/orca-runtime-subscribe-to-terminal-resize.ts
sed -n '145,190p' src/main/runtime/orca-runtime-has-terminals-for-worktree.ts
sed -n '145,230p' src/main/runtime/orca-runtime-refresh-floating-workspace-pty-liveness.ts
rg -n -C 8 'dispatchId|pendingDispatchId|handledDispatchId|settle|Codex|reconnect' src/main/runtime/orca-runtime-subscribe-to-terminal-resize.ts src/main/runtimeRepository: stablyai/orca
Length of output: 45665
🏁 Script executed:
sed -n '90,185p' src/main/runtime/orca-runtime-resolve-known-workspace-file-target.ts
sed -n '1,125p' src/main/runtime/orca-runtime-invalidate-all-handles-for-pty.ts
rg -n -C 12 'handleByPtyId\.(delete|clear)|handles\.(delete|clear)|handleByPtyId\.set' src/main/runtime --glob '!*.test.ts' --glob '!*.spec.ts'
sed -n '80,200p' src/main/runtime/orca-runtime-on-pty-data.tsRepository: stablyai/orca
Length of output: 42670
Pass ptyId to findHandleForPtyRecord.
findHandleForPtyRecord accepts a string and compares it with record.ptyId. Passing the pty object makes this fallback return null, so a remaining synthetic handle is not found and reconnect-failure settlement is skipped. The remint example is overstated because the inspected invalidation paths remove stale aliases and records together.
🐛 Suggested fix
- const handle = this.handleByPtyId.get(ptyId) ?? this.findHandleForPtyRecord(pty)
+ const handle = this.handleByPtyId.get(ptyId) ?? this.findHandleForPtyRecord(ptyId)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const handle = this.handleByPtyId.get(ptyId) ?? this.findHandleForPtyRecord(pty) | |
| const handle = this.handleByPtyId.get(ptyId) ?? this.findHandleForPtyRecord(ptyId) |
There was a problem hiding this comment.
Important
The reworked scan moves a dispatch_contexts lookup onto the per-chunk PTY hot path, and the new retry branch can latch without ever re-attempting settlement. Both are worth addressing before merge.
Reviewed changes
- Replaced the module-level handled-PTY
WeakSetand directpty.previewcheck with a per-PTY incremental scan (CodexReconnectOutputScan, 512-char tail) plus exportedfindCodexReconnectFailureBanner/stripTerminalControlSequenceshelpers. - Moved Codex-session settlement into a new DB method
failCodexSessionUnrecoverableDispatch, which now marks the taskfailedpending coordinator review instead of returning it toready. - Factored the worker-exit escalation into
notifyWorkerDispatchFailurewith ataskDispositionNoteoverride and added a second-dispatch regression scenario.
deepseek-v4.1-flash (free via Pullfrog for OSS) ｜ 𝕏
| ): void { | ||
| const db = this._orchestrationDb | ||
| const scan = codexReconnectOutputScans.get(pty) ?? createCodexReconnectOutputScan() | ||
| const dispatch = db?.getActiveDispatchForTerminal(handle, paneKey ?? undefined) |
There was a problem hiding this comment.
getActiveDispatchForTerminal now runs once per PTY output chunk for every terminal, because observeCodexReconnectFailureOutput is invoked unconditionally from onPtyData (orca-runtime-on-pty-data.ts:189) before any banner check. In the previous revision this same lookup only ran after hasCodexReconnectFailureBanner(pty.preview) matched, so the rework adds a DB query (up to three prepared dispatch_contexts statements) to the hot terminal path — and contradicts the PR's "database work runs only when the complete banner matches" note.
Technical details
# Per-chunk dispatch lookup on the PTY hot path
## Affected sites
- `src/main/runtime/orca-runtime-subscribe-to-terminal-resize.ts:128` — unconditional `db.getActiveDispatchForTerminal` per chunk.
- `src/main/runtime/orca-runtime-on-pty-data.ts:189` — call site; runs for every PTY that has a handle.
- `src/main/runtime/orchestration/db/dispatch-context/dispatch-lookup.ts:131-159` — up to three prepared `dispatch_contexts` queries per call.
## Required outcome
- No `dispatch_contexts` query when the accumulated output does not contain the banner.
- Preserve the existing per-dispatch scan state and settlement behavior.
## Suggested approach (optional)
- Compute `findCodexReconnectFailureBanner(combinedOutput)` first — it is dispatch-independent — and only resolve the active dispatch when a new match exists. `db.hasAnyDispatchContexts()` (a cached probe) is a cheaper pre-gate if a banner-independent guard is preferred.| if (dispatch && scan.pendingDispatchId === dispatchId) { | ||
| const result = this.failCodexSessionUnrecoverableDispatch(dispatch, handle) | ||
| if (result === 'settled' || result === 'ignored') { | ||
| scan.handledDispatchId = dispatchId | ||
| scan.pendingDispatchId = null | ||
| } | ||
| } |
There was a problem hiding this comment.
When failCodexSessionUnrecoverableDispatch returns 'retry' (worker state is starting/start_unknown), pendingDispatchId stays latched but settlement is only re-attempted on the next onPtyData chunk. If the banner is emitted before the worker is ready and the worker then reaches ready/input_accepted without emitting further PTY bytes, the dispatch is never settled — the exact stall this PR fixes.
Technical details
# Latched pending settlement without a readiness trigger
## Affected sites
- `src/main/runtime/orca-runtime-subscribe-to-terminal-resize.ts:147-153` — `'retry'` leaves `pendingDispatchId` set and does not mark handled; re-run happens only on the next chunk.
- `src/main/runtime/orchestration/db/dispatch-context/dispatch-completion.ts:261-263` — returns `'retry'` for `starting`/`start_unknown`.
## Required outcome
- A banner observed while the worker is not yet `ready` must still settle the dispatch once the worker reaches `ready`/`input_accepted`, even if no further PTY output arrives.
## Open questions for the human (optional)
- Is the pre-`ready` case reachable in practice (for example after a runtime restart re-adopts a live Codex PTY)? If not, `'retry'` may be dead code and could be simplified to `'ignored'`.
a9ee501 to
fd0c6d8
Compare
Sync update (
|
There was a problem hiding this comment.
ℹ️ No new issues in this delta — the fix is correct and covered by a regression test.
Reviewed changes
- Fixed the fallback handle resolution in
onPtyDatato pass the PTY id string tofindHandleForPtyRecord, so a synthetic handle resolves when the cached PTY-id index is absent and the reconnect-banner settlement is no longer silently skipped. - Added a regression test that clears the cached PTY-id index and re-tags the handle as synthetic, then still expects the unrecoverable-session dispatch to fail, escalate, and keep the PTY alive; a second dispatch on the same handle is exercised too.
deepseek-v4.1-flash (free via Pullfrog for OSS) ｜ 𝕏
This branch has not been deployed
1 participant
ELI5
When Codex says its session cannot be restored, Orca now releases the stuck task assignment and tells the coordinator while keeping the terminal and worktree available for diagnosis.
What Changed
Why
A Codex TUI can remain alive after its app-server session reports that it cannot be restored. Without an authoritative session-failure outcome, the dispatch remains active indefinitely. This path uses Codex's explicit terminal message and leaves missing or stale heartbeat evidence unchanged.
Linked Issue
Fixes #24791
Visual Proof
N/A
No renderer or layout changed; the regression covers the existing coordinator mailbox escalation and dispatch lifecycle.
Testing
onPtyData, including ANSI styling and a split PTY chunk. It checks dispatch failure, coordinator escalation, terminal retention, non-Codex isolation, and partial-banner handling.node node_modules/vitest/vitest.mjs run --config config/vitest.config.ts src/main/runtime/orchestration/codex-reconnect-failed-dispatch.test.ts src/main/runtime/orchestration/lightweight-run-worker-exit-escalation.test.ts src/main/runtime/orchestration/db-task-dispatch-lifecycle-guards.test.ts: 42/42 passed.ORCA_CODE_QUALITY_BASE=upstream/main node config/scripts/check-changed-code-quality.mjs: passed with 0 findings across 4 files.node node_modules/typescript/bin/tsc --noEmit -p config/tsconfig.node.json: passed.pnpm lint,pnpm typecheck,pnpm test, andpnpm buildare left to CI.AI Disclosure
GPT-6
Review
Self-reviewed the dispatch settlement, worker stage, mailbox delivery, and terminal-resource retention paths.
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
Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)