chore: Simplify bridge heartbeat and disconnect handling

[hatayama]

Summary

• Keep project IPC heartbeat and client-disconnect monitoring behavior unchanged while moving those loops out of the bridge server body.
• Preserve the existing shutdown, request cancellation, and per-client write-lock ownership.

User Impact

• No runtime behavior change is intended.
• This reduces the bridge server surface area so follow-up server refactors can review concurrency ownership more clearly.

Changes

• Add stateless infrastructure services for heartbeat sending/stopping and client-disconnect monitoring.
• Inject those services from the bridge server instance factory.
• Retarget the existing heartbeat tests without changing their timing structure.

Verification

• dist/darwin-arm64/uloop compile --project-path "$(git rev-parse --show-toplevel)" -> 0 errors / 0 warnings
• dist/darwin-arm64/uloop run-tests --project-path "$(git rev-parse --show-toplevel)" --test-mode EditMode --filter-type regex --filter-value "(JsonRpcHeartbeatTests|UnityCliLoopBridgeServerShutdownTests|UnityCliLoopServerControllerRecoveryTests|JsonRpcProcessorCliVersionGateTests|OnionAssemblyDependencyTests)" -> 122/122 passed

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR extracts heartbeat sending and client-disconnect monitoring logic from UnityCliLoopBridgeServer into two new internal service classes (UnityCliLoopBridgeHeartbeatService and UnityCliLoopBridgeClientDisconnectMonitor), wires them via constructor injection, removes the previous static helpers, and updates existing tests accordingly.

Changes

Heartbeat and Disconnect Monitor Extraction

Layer / File(s)	Summary
New heartbeat service Packages/src/Editor/Infrastructure/UnityCliLoopBridgeHeartbeatService.cs	New internal sealed class implements SendHeartbeatsAsync (loop writing heartbeat frames, exits silently on cancellation/IO errors) and StopHeartbeatsAsync (cancels and awaits the task).
New client disconnect monitor Packages/src/Editor/Infrastructure/UnityCliLoopBridgeClientDisconnectMonitor.cs	New internal sealed class implements MonitorClientDisconnectAsync (polls connection state, cancels token source on disconnect) and StopClientDisconnectMonitorAsync.
Server construction and wiring Packages/src/Editor/Infrastructure/UnityCliLoopBridgeServer.cs	Factory Create() instantiates both services and passes them to the server; the server stores them as fields and the constructor validates them via null checks/asserts, replacing the prior single-dependency constructor.
Request processing and cleanup path Packages/src/Editor/Infrastructure/UnityCliLoopBridgeServer.cs	ProcessRequestFrameAsync now calls the injected service instances for heartbeat send/stop and disconnect monitoring; old static helper implementations are removed.
Heartbeat test updates Assets/Tests/Editor/JsonRpcHeartbeatTests.cs	Existing heartbeat tests instantiate UnityCliLoopBridgeHeartbeatService and call its SendHeartbeatsAsync instead of the removed static server method.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant UnityCliLoopBridgeServer
  participant UnityCliLoopBridgeHeartbeatService
  participant UnityCliLoopBridgeClientDisconnectMonitor
  participant BridgeClientConnection

  Client->>UnityCliLoopBridgeServer: request frame
  UnityCliLoopBridgeServer->>UnityCliLoopBridgeHeartbeatService: SendHeartbeatsAsync()
  UnityCliLoopBridgeServer->>UnityCliLoopBridgeClientDisconnectMonitor: MonitorClientDisconnectAsync(BridgeClientConnection)
  loop poll interval
    UnityCliLoopBridgeHeartbeatService->>BridgeClientConnection: write heartbeat frame
    UnityCliLoopBridgeClientDisconnectMonitor->>BridgeClientConnection: check connected
  end
  BridgeClientConnection-->>UnityCliLoopBridgeClientDisconnectMonitor: disconnected
  UnityCliLoopBridgeClientDisconnectMonitor->>UnityCliLoopBridgeServer: cancel CancellationTokenSource
  UnityCliLoopBridgeServer->>UnityCliLoopBridgeHeartbeatService: StopHeartbeatsAsync()
  UnityCliLoopBridgeServer->>UnityCliLoopBridgeClientDisconnectMonitor: StopClientDisconnectMonitorAsync()
  UnityCliLoopBridgeServer-->>Client: final response

Loading

Possibly related PRs

• hatayama/unity-cli-loop#1240: Both PRs modify the client-disconnect cancellation path in ProcessRequestFrameAsync/ProcessRequestWithEarlyResponseAsync.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: simplifying heartbeat and disconnect handling in the bridge.
Description check	✅ Passed	The description matches the changeset and accurately describes the refactor, tests, and intended behavior.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch codex/c4-7-bridge-heartbeat-monitor

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[cubic-dev-ai]

No issues found across 6 files

_{Re-trigger cubic}

[coderabbitai]

🧹 Nitpick comments (1)

Packages/src/Editor/Infrastructure/UnityCliLoopBridgeHeartbeatService.cs (1)

45-56: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Null-conditional cancel can hang the shutdown wait.

heartbeatCancellationSource?.Cancel() silently no-ops if the source is null, but await heartbeatTask afterward still waits for the loop to end — which, per SendHeartbeatsAsync, only happens on cancellation or a write failure. If callers ever pass a non-null heartbeatTask with a null heartbeatCancellationSource, this will hang indefinitely. The sibling UnityCliLoopBridgeClientDisconnectMonitor.StopClientDisconnectMonitorAsync calls .Cancel() directly (no ?.), which is the fail-fast convention used elsewhere in this codebase — this method should match that pattern (or assert the invariant) instead of silently swallowing a null source.

🔧 Suggested fix

         internal async Task StopHeartbeatsAsync(
             Task heartbeatTask,
             CancellationTokenSource heartbeatCancellationSource)
         {
             if (heartbeatTask == null)
             {
                 return;
             }

-            heartbeatCancellationSource?.Cancel();
+            Debug.Assert(heartbeatCancellationSource != null, "heartbeatCancellationSource must be provided when heartbeatTask is non-null.");
+            heartbeatCancellationSource.Cancel();
             await heartbeatTask;
         }

Based on learnings, the codebase follows a fail-fast/Debug.Assert convention for internal preconditions rather than silently no-op'ing invalid state, and the sibling disconnect-monitor class already follows this pattern without ?..

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Packages/src/Editor/Infrastructure/UnityCliLoopBridgeHeartbeatService.cs`
around lines 45 - 56, The shutdown path in StopHeartbeatsAsync should not
silently ignore a missing CancellationTokenSource while still awaiting the
heartbeat loop. Update UnityCliLoopBridgeHeartbeatService.StopHeartbeatsAsync to
follow the same fail-fast precondition style as
UnityCliLoopBridgeClientDisconnectMonitor.StopClientDisconnectMonitorAsync:
assert or require that heartbeatCancellationSource is non-null whenever
heartbeatTask is non-null, then call Cancel() directly before awaiting the task.
This keeps the heartbeat loop from hanging and aligns the method with the
codebase’s internal invariant handling.

Source: Learnings

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@Packages/src/Editor/Infrastructure/UnityCliLoopBridgeHeartbeatService.cs`:
- Around line 45-56: The shutdown path in StopHeartbeatsAsync should not
silently ignore a missing CancellationTokenSource while still awaiting the
heartbeat loop. Update UnityCliLoopBridgeHeartbeatService.StopHeartbeatsAsync to
follow the same fail-fast precondition style as
UnityCliLoopBridgeClientDisconnectMonitor.StopClientDisconnectMonitorAsync:
assert or require that heartbeatCancellationSource is non-null whenever
heartbeatTask is non-null, then call Cancel() directly before awaiting the task.
This keeps the heartbeat loop from hanging and aligns the method with the
codebase’s internal invariant handling.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: d06d4ba6-bdc6-4b40-b140-4c497760b2e0

📥 Commits

Reviewing files that changed from the base of the PR and between d5e5d55 and 4d499a0.

⛔ Files ignored due to path filters (2)

• Packages/src/Editor/Infrastructure/UnityCliLoopBridgeClientDisconnectMonitor.cs.meta is excluded by none and included by none
• Packages/src/Editor/Infrastructure/UnityCliLoopBridgeHeartbeatService.cs.meta is excluded by none and included by none

📒 Files selected for processing (4)

• Assets/Tests/Editor/JsonRpcHeartbeatTests.cs
• Packages/src/Editor/Infrastructure/UnityCliLoopBridgeClientDisconnectMonitor.cs
• Packages/src/Editor/Infrastructure/UnityCliLoopBridgeHeartbeatService.cs
• Packages/src/Editor/Infrastructure/UnityCliLoopBridgeServer.cs