Skip to content

fix: recover tmux terminal streams after server restart - #597

Open
TiantianFlow wants to merge 1 commit into
awslabs:mainfrom
TiantianFlow:fix/recover-tmux-streams-after-restart
Open

fix: recover tmux terminal streams after server restart#597
TiantianFlow wants to merge 1 commit into
awslabs:mainfrom
TiantianFlow:fix/recover-tmux-streams-after-restart

Conversation

@TiantianFlow

@TiantianFlow TiantianFlow commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Problem

cao-server can restart while tmux-backed terminals intentionally remain alive. The old process's FIFO reader threads disappear, but tmux continues forwarding pane output to the old FIFO target. The restarted server therefore receives no output for those persisted terminals, leaves their status as unknown, and cannot deliver pending worker callbacks because InboxService only delivers to ready terminals.

This was reproduced with a live Claude Tech Lead: two worker reports remained pending even though the Tech Lead was visibly idle.

Fix

For tmux-backed terminals at API startup:

  • enumerate persisted terminals whose tmux session/window still exists;
  • recreate the FIFO reader and explicitly stop/re-arm that pane's pipe-pane target;
  • seed StatusMonitor from a read-only rendered pane snapshot, restoring an idle/completed state without sending terminal input;
  • run the existing pending-inbox reconciliation once after recovery, so callbacks stranded before restart are retried immediately.

Event-inbox (Herdr) backends are explicitly skipped because they do not use tmux/FIFO output capture.

Validation

Automated

  • Added targeted recovery wiring tests: 2 passed.
    • tmux recovery creates a reader, re-arms the pipe, and seeds status;
    • event-inbox backends are skipped.

These are deliberately narrow unit tests; they do not yet exercise end-to-end inbox delivery or failure/multi-pane cases.

Live end-to-end validation

  • Restarted only cao-server; the existing Claude Tech Lead and OpenCode Builder tmux panes remained running.
  • The Tech Lead recovered from unknown to completed.
  • Two previously pending Builder/Verifier callbacks were delivered.
  • The Tech Lead consumed the reports, authorised the next Builder increment, and delivered the follow-up to the Builder.

Scope / follow-up

This is a tmux restart-recovery fix. CAO's intended topology is one terminal per window/pane. Before marking this ready, follow up with a guard for manually split panes plus end-to-end/failure-path coverage.

@TiantianFlow
TiantianFlow marked this pull request as ready for review August 15, 2026 00:53
@haofeif
haofeif requested a balanced review from Copilot August 15, 2026 14:16

Copilot AI 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.

Pull request overview

Restores tmux terminal output and inbox delivery after server restarts.

Changes:

  • Reattaches FIFO readers and tmux pipe targets.
  • Seeds terminal status from pane snapshots.
  • Reconciles pending inbox messages and adds recovery tests.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

File Description
terminal_service.py Implements persisted terminal recovery.
status_monitor.py Adds snapshot-based status seeding.
api/main.py Runs recovery and reconciliation during startup.
test_terminal_recovery.py Tests recovery wiring and backend skipping.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +239 to +240
_rearm_pipe()
status_monitor.seed_from_snapshot(terminal_id, snapshot)
Comment on lines +213 to +215
snapshot = backend.get_history(
session_name, window_name, tail_lines=PIPE_LIVENESS_TAIL_LINES
)
# Recovery publishes initial status before the consumer tasks get their first
# scheduling turn. Reconcile once here so a callback already pending at
# restart is not left waiting for the periodic sweep.
await asyncio.to_thread(inbox_service.reconcile_orphaned_messages, registry)
list_terminals.return_value = [
{"id": "abc12345", "tmux_window": "tech_lead-a1b2"}
]
status_monitor.seed_from_snapshot.return_value = TerminalStatus.COMPLETED
Comment on lines +506 to +510
self._buffers[terminal_id] = output[-state_buffer_max:]
# A persisted terminal is quiescent at recovery time. Do not let a
# prior process's debounce state suppress its first real status.
self._bursting[terminal_id] = False
detected = self._detect_status(terminal_id, output[-state_buffer_max:])
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 62.96296% with 20 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (main@0903561). Learn more about missing BASE report.

Files with missing lines Patch % Lines
...li_agent_orchestrator/services/terminal_service.py 69.76% 13 Missing ⚠️
.../cli_agent_orchestrator/services/status_monitor.py 12.50% 7 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main     #597   +/-   ##
=======================================
  Coverage        ?   91.26%           
=======================================
  Files           ?      182           
  Lines           ?    24463           
  Branches        ?        0           
=======================================
  Hits            ?    22326           
  Misses          ?     2137           
  Partials        ?        0           
Flag Coverage Δ
unittests 91.26% <62.96%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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.

3 participants