Keep local terminals responsive when the cloud terminal mirror stalls

[alexeyzimarev]

Closes #1022 — AI-2962

What & why

A slow server, or another agent's output, freezes kcap agent attach and the desktop app, because the PTY read loop awaits a cloud queue shared by every agent. Each agent registered with the server now gets its own non-blocking CloudTerminalSink with its own pump, fed under SinksLock beside the local sinks; the read loop never waits on it. A mirror that falls behind, meets a send that keeps failing, or crosses a reconnect is repaired in-band: a terminal reset (ESC c) followed by the daemon's 2 MB output ring, on the same ordered lane as live output. No server change.

Where to look

• Every re-registration replays up to 2 MB per registered agent. Sends stay one at a time: the SignalR client holds its connection lock across each write and flush.
• The sink stops before an agent finalizes, because the server deletes the terminal buffer on unregister. A completed sink gives up a tail it cannot send to a connection that is not ready.
• Ordered recovery rests on the server handling one connection's messages in arrival order, which holds by the shape of its hub method, not by guarantee — AI-3025.

Verification

• dotnet build Capacitor.slnx after merging main: 16 projects, 0 errors, 0 warnings.
• Capacitor.Cli.Daemon.Tests.Unit on the merged tree: 3442 total, 0 failed, 37 skipped — run with KCAP_DAEMON_ID and KCAP_DAEMON_EPOCH unset, since PiHostedLaunchTests asserts they are absent and a hosted-agent shell exports them.
• A server-launched agent with a local sink receives 200 chunks against a 64-byte cloud budget while every cloud send is blocked; a second agent is untouched; the mirror then recovers through a reset.
• The stop-before-unregister test fails when the stop is moved after finalization, and the disposal test fails when DisposeAsync stops no sinks.
• dotnet publish -c Release for the CLI and the daemon: no IL2xxx/IL3xxx output.
• Not run: the Windows and Linux legs, and a live daemon against a throttled server.

🤖 Generated with Claude Code

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-09-21T15:31:37.349098Z	4bfdf35	PR opened

ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[qodo-code-review]

PR Summary by Qodo

Keep local terminals responsive during cloud mirror stalls

🐞 Bug fix ✨ Enhancement 🧪 Tests 📝 Documentation 🕐 40+ Minutes

AI Description

• Give each registered agent an independent, non-blocking cloud terminal output lane.
• Recover stalled or reconnected mirrors with terminal reset and bounded ring replay.
• Bound shutdown waits and verify ordering, isolation, recovery, and desktop reset handling.

Diagram

sequenceDiagram
    participant PTY
    participant Loop as PTY Read Loop
    participant Ring as Output Ring
    participant Local as Local Sinks
    participant Client as Local Clients
    participant Cloud as Cloud Sink
    participant Hub as SignalR Hub
    participant Web as Cloud Terminal
    PTY->>Loop: Output chunk
    Loop->>Ring: Append under lock
    Loop->>Local: Non-blocking enqueue
    Local->>Client: Deliver output
    Loop->>Cloud: Non-blocking enqueue
    Cloud->>Hub: Ordered pump send
    Hub->>Web: Mirror output
    alt Overflow, failure, or reconnect
        Cloud->>Ring: Snapshot retained output
        Cloud->>Hub: Reset then replay
        Hub->>Web: Reset then replay
    end

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Shared dispatcher with per-agent queues

• ➕ Could centralize scheduling and provide explicit fairness across cloud mirrors.
• ➕ Would create fewer long-running pump tasks.
• ➖ Adds multiplexing and lifecycle complexity to a concurrency-sensitive path.
• ➖ A dispatcher failure could again affect every agent's cloud delivery.
• ➖ Still requires per-agent budgets, resynchronization state, and ordered replay lanes.

2. Server-side acknowledged reset protocol

• ➕ Could atomically replace server buffers instead of appending an in-band reset.
• ➕ Sequence acknowledgements could provide stronger delivery and ordering guarantees.
• ➖ Requires coordinated daemon, server, and client protocol changes.
• ➖ Substantially expands scope and deployment risk.
• ➖ Does not directly remove cloud back-pressure from the PTY read loop.

Recommendation: Use the PR's per-agent sink design for this fix. It isolates local responsiveness and backlog accounting with no server change, while reusing the established output ring for recovery. A server-side reset or sequencing protocol is a valuable follow-up for stronger guarantees, but should not block this scoped reliability improvement.

Files changed (19) +3957 / -192

Bug fix (2) +453 / -67

AgentOrchestrator.csIntegrate per-agent cloud sinks into PTY fan-out +92/-67

Integrate per-agent cloud sinks into PTY fan-out

• Creates and tracks a cloud sink for each registered agent, enqueues it beside local sinks without awaiting, and stops it before finalization. Re-registration requests a reset-and-ring resync, while daemon disposal closes admission and stops all remaining pumps.

src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.cs

CloudTerminalSink.csImplement non-blocking cloud terminal delivery and recovery +361/-0

Implement non-blocking cloud terminal delivery and recovery

• Adds a per-agent ordered pump with byte-budgeted admission, readiness gating, bounded retries, desync detection, and reset-plus-ring replay. Stop and disposal logic enforce the earliest drain deadline and safely abandon sends that ignore cancellation.

src/Capacitor.Cli.Daemon/Services/CloudTerminalSink.cs

Refactor (2) +5 / -70

AgentOrchestrator.LocalIpc.csRemove launch-origin terminal routing state +0/-1

Remove launch-origin terminal routing state

• Stops marking locally spawned agents specially because every registered agent now uses the same non-blocking cloud sink path.

src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.LocalIpc.cs

ServerConnection.csExpose terminal output as a raw SignalR send +5/-69

Expose terminal output as a raw SignalR send

• Removes ownership of the shared terminal sender and its connection and disposal lifecycle. Terminal sends now go directly to the hub while callers own ordering, retry, and cancellation.

src/Capacitor.Cli.Daemon/Services/ServerConnection.cs

Tests (9) +1073 / -48

TerminalTranscriptTests.csVerify terminal reset clears stale desktop content +19/-0

Verify terminal reset clears stale desktop content

• Adds coverage proving XTerm.NET honors 'ESC c', allowing reset-and-replay recovery to replace stale terminal contents.

test/Capacitor.App.Tests.Unit/TerminalTranscriptTests.cs

ScriptedPtyProcess.csAdd an on-demand PTY process test double +33/-0

Add an on-demand PTY process test double

• Introduces a channel-backed PTY fake that lets tests emit deterministic output and explicitly end the stream.

test/Capacitor.Cli.Daemon.Tests.Unit/Pty/ScriptedPtyProcess.cs

AgentOrchestratorCloudSinkTests.csCover orchestrator cloud sink isolation and lifecycle +307/-0

Cover orchestrator cloud sink isolation and lifecycle

• Tests that blocked cloud sends do not stall local output or other agents, private agents remain local, and sinks stop before unregistering. It also covers disposal admission and reconnect-triggered recovery.

test/Capacitor.Cli.Daemon.Tests.Unit/Services/AgentOrchestratorCloudSinkTests.cs

AgentOrchestratorVendorTests.csUpdate agent stop coverage for cloud pumps +22/-10

Update agent stop coverage for cloud pumps

• Reworks the blocked-terminal test to verify the sink pump is cancelled and finishes before the agent unregisters.

test/Capacitor.Cli.Daemon.Tests.Unit/Services/AgentOrchestratorVendorTests.cs

CaptureServerConnection.csExtend the server fake for cloud sink scenarios +34/-13

Extend the server fake for cloud sink scenarios

• Adds settable readiness, terminal-send capture and gating, send counters, and status failure injection. These seams support reconnect, cancellation, isolation, and ordering tests.

test/Capacitor.Cli.Daemon.Tests.Unit/Services/CaptureServerConnection.cs

CloudTerminalSinkMirror.csAdd a reconstructing cloud mirror test double +41/-0

Add a reconstructing cloud mirror test double

• Captures decoded terminal chunks, identifies reset sequences, and reconstructs the mirror state after the latest reset. Send hooks allow tests to block or fail transport operations.

test/Capacitor.Cli.Daemon.Tests.Unit/Services/CloudTerminalSinkMirror.cs

CloudTerminalSinkTests.csExercise sink ordering, recovery, and bounded shutdown +590/-0

Exercise sink ordering, recovery, and bounded shutdown

• Adds extensive coverage for ordering, readiness, retries, queue overflow, concurrent resyncs, sustained overload, warning throttling, and reconnect replay. It also validates concurrent stop deadlines, cancellation, draining, and abandoned pumps.

test/Capacitor.Cli.Daemon.Tests.Unit/Services/CloudTerminalSinkTests.cs

RecordingTerminalSink.csAdd a recording local terminal sink +13/-0

Add a recording local terminal sink

• Introduces a thread-safe local sink fake for verifying that PTY output continues while cloud delivery is blocked.

test/Capacitor.Cli.Daemon.Tests.Unit/Services/RecordingTerminalSink.cs

ServerConnectionDisposeTests.csAdapt disposal tests after removing the shared sender +14/-25

Adapt disposal tests after removing the shared sender

• Replaces assertions against the removed terminal-sender token source with checks that the SignalR hub is disposed. Existing idempotency and fault-containment guarantees remain covered.

test/Capacitor.Cli.Daemon.Tests.Unit/Services/ServerConnectionDisposeTests.cs

Documentation (5) +2415 / -7

CLAUDE.mdDocument the non-blocking PTY fan-out invariant +5/-0

Document the non-blocking PTY fan-out invariant

• Adds an invariant requiring the PTY read loop to use non-blocking sinks. It records how local and cloud consumers recover instead of back-pressuring the agent.

CLAUDE.md

CHANGES.mdExplain per-agent cloud mirror lanes and recovery +28/-0

Explain per-agent cloud mirror lanes and recovery

• Documents the original shared-queue freeze, the new per-agent pumps, reset-and-ring recovery, reconnect behavior, and lifecycle constraints.

docs/CHANGES.md

2026-09-21-cloud-terminal-sink.mdAdd the cloud terminal sink implementation plan +1958/-0

Add the cloud terminal sink implementation plan

• Provides the staged implementation and verification plan covering sink behavior, orchestration, reconnect recovery, tests, documentation, and NativeAOT checks.

docs/superpowers/plans/2026-09-21-cloud-terminal-sink.md

2026-09-20-cloud-terminal-sink-design.mdSpecify per-agent terminal mirroring semantics +421/-0

Specify per-agent terminal mirroring semantics

• Defines queue budgeting, synchronization, resync ordering, transport assumptions, stop behavior, lifecycle ownership, and required regression coverage.

docs/superpowers/specs/2026-09-20-cloud-terminal-sink-design.md

LocalSocketSink.csClarify local sink overflow recovery behavior +3/-7

Clarify local sink overflow recovery behavior

• Updates documentation to state that local producers never block and slow clients detach before reattaching from the output replay.

src/Capacitor.Cli.Daemon/Services/LocalSocketSink.cs

Other (1) +11 / -0

CloudTerminalSinkOptions.csDefine cloud sink reliability and lifecycle limits +11/-0

Define cloud sink reliability and lifecycle limits

• Adds defaults for the 2 MB backlog budget, retry cadence, failing-send threshold, drain bound, cancellation grace, and warning rate limit.

src/Capacitor.Cli.Daemon/Services/CloudTerminalSinkOptions.cs

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Repeated shutdown calls retain timer resources ✓ Resolved 🐞 Bug ☼ Reliability

Description

DisposeSources() disposes _deadlineTimer without _sinksLock, while StopAsync() replaces and
assigns that field under the lock. A concurrent later stop can install a shorter-deadline timer
after disposal has read the old value, leaving the new timer and its registration alive until it
fires against an already-disposed deadline source.

Code

src/Capacitor.Cli.Daemon/Services/CloudTerminalSink.cs[R225-229]

+    void DisposeSources() {
+        _pumpCts.Dispose();
+        _deadlineCts.Dispose();
+        _deadlineTimer?.Dispose();
+    }

Relevance

●●● Strong

Recent history accepts shutdown race fixes and resource-lifetime cleanup; this lock-protected timer
replacement can leak registrations.

PR-#508
PR-#734
PR-#430

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The timer field is assigned only while holding _sinksLock, but final cleanup accesses the same
mutable field without that lock. Consequently, cleanup can dispose the prior timer while a
concurrent stop assigns a replacement that no later cleanup observes.

src/Capacitor.Cli.Daemon/Services/CloudTerminalSink.cs[154-182]
src/Capacitor.Cli.Daemon/Services/CloudTerminalSink.cs[194-229]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`DisposeSources()` races with concurrent `StopAsync()` calls when disposing `_deadlineTimer`. A later stop can replace the timer after cleanup has read the field, so the replacement timer is never disposed and later invokes its callback against the disposed deadline cancellation source.

## Fix Focus Areas
- src/Capacitor.Cli.Daemon/Services/CloudTerminalSink.cs[154-182]
- src/Capacitor.Cli.Daemon/Services/CloudTerminalSink.cs[225-229]

## Recommended Fix
Synchronize timer replacement and final cleanup with `_sinksLock`. During final cleanup, atomically detach `_deadlineTimer` under that lock and dispose the detached instance; prevent a completed termination from installing a new deadline timer, or immediately dispose any timer created by a concurrent later `StopAsync()` call.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Context sources

✅ Compliance rules (platform): 64 rules

✅ Cross-repo context — repo relationships

  Explored: repo: kurrent-io/kcap-server (sha: b56e8a37)

Review mode: 🧠 Deep: This is a concurrency- and lifecycle-sensitive redesign spanning multiple production paths, with a substantial new asynchronous sink, reconnect/replay behavior, and many independent logic sites where redundant review can catch subtle defects.

Tip of the day

💡 Did you know, you can route each action level your way: inline, summary, both, or drop

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗