Close network conduit for cluster epoch-specific engines

[tim-barry]

When a conduit is created by an engine, the network manager keeps a reference to the engine to send it messages, preventing it from being garbage collected even when stopped, until the conduit is closed.
All engines should close their network conduits on shutdown, not just these, but limiting to these cases for now as only the cluster sync and message hub engines use distinct channel IDs for each epoch.

Summary by CodeRabbit

• Bug Fixes

  • Improved shutdown sequencing to ensure network conduits are closed after workers finish, preventing lingering connections and resource leaks.

• Tests

  • Strengthened lifecycle tests to assert conduits are closed during shutdown, improving coverage and reliability of teardown behavior.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[github-actions]

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds coordinated shutdown sequencing: message hub workers use a sync.WaitGroup and a shutdown worker that waits for all dispatch workers before closing the network conduit; Engine.Done now closes the conduit after request-handler shutdown. Tests updated to expect a single Close() call.

Changes

Cohort / File(s)	Summary	Attention points
Message Hub core engine/collection/message_hub/message_hub.go	Adds sync.WaitGroup usage: each worker increments/decrements the wait group; introduces a shutdown goroutine that waits for all dispatch workers to finish, then closes the network conduit and reports errors via context.	Ensure correct wait-group usage and that conduit close occurs after all worker I/O; watch for potential races on conduit access.
Message Hub tests engine/collection/message_hub/message_hub_test.go	Test setup now expects con.Close() to be called once (.On("Close").Return(nil).Once()) to reflect new lifecycle.	Verify tests cover error/early-exit paths and avoid double-close flakiness.
Synchronization Engine core engine/collection/synchronization/engine.go	Engine.Done now closes the network conduit after request handler shutdown and logs on failure.	Confirm Close() idempotency and ordering relative to message hub shutdown.
Synchronization Engine tests engine/collection/synchronization/engine_test.go	Test now uses mocknetwork.NewConduit(...) and asserts Close() is called once on stop.	Align mock expectations with both engine and hub shutdown sequences to prevent test races.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant DW as DispatchWorkers
  participant SW as ShutdownWorker
  participant C as NetworkConduit
  participant L as Logger

  DW->>DW: spawn workers (wg.Add)
  DW->>DW: workers process messages
  DW->>SW: workers done (wg.Done until all)
  SW->>SW: wg.Wait()
  SW->>C: Close()
  alt Close returns error
    C-->>L: log error
  else Close ok
    C-->>SW: nil
  end

Loading

sequenceDiagram
  autonumber
  participant RH as RequestHandler
  participant E as Engine.Done
  participant C as NetworkConduit
  participant L as Logger

  RH->>E: shutdown complete
  E->>C: Close()
  alt Close returns error
    C-->>L: log error
  else Close ok
    C-->>E: nil
  end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

I twitch my whiskers, listen close,
Workers finish—no more prose.
One gentle nudge, the conduit sleeps,
The warren calm, the network keeps.
A hop, a hum, the shutdown sleeps. 🐇✨

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: closing network conduits for cluster epoch-specific engines in the message hub and synchronization engines.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

---

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 and usage tips.}

[codecov-commenter]

Codecov Report

❌ Patch coverage is 66.66667% with 4 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
engine/collection/message_hub/message_hub.go	77.77%	1 Missing and 1 partial ⚠️
engine/collection/synchronization/engine.go	33.33%	1 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

[coderabbitai]

Actionable comments posted: 2

🤖 Fix all issues with AI agents

In `@engine/collection/message_hub/message_hub.go`:
- Around line 180-186: The shutdown goroutine currently ignores the error
returned by hub.con.Close(); modify it to handle the error by calling the
component's logger (e.g., hub.Logger or hub.logger) and logging a descriptive
message with the error when Close() returns non-nil; locate the goroutine that
waits on hub.ComponentManager.Done() and replace "_ = hub.con.Close()" with code
that captures the error from hub.con.Close(), checks if err != nil, and logs it
(include context like "failed to close network conduit" and the error).

In `@engine/collection/synchronization/engine.go`:
- Around line 185-186: Replace the discarded call to e.con.Close() with explicit
error handling: call e.con.Close(), capture the returned error into a variable,
and if it is non-nil log it using the engine logger
(e.log.Error().Err(err).Msg("failed to close network conduit")) so shutdown
failures are recorded; apply the same fix to the analogous discard in
message_hub.go (replace the "_ = ...Close()" pattern with the
captured-error-and-log pattern).