Conversation
This comment has been minimized.
This comment has been minimized.
|
Warning Review limit reached
More reviews will be available in 32 minutes and 40 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughAdds LiveKit-mode retry/backoff and telemetry plumbing to relay services, hard-disconnect handling in the voice modes, new resilience tests, and Linux systemd deployment updates with SIGTERM-driven console shutdown. ChangesLiveKit resilience and shutdown
Sequence Diagram(s)sequenceDiagram
participant RelayServiceBase
participant IRelayModeTelemetry
participant DispatchVoiceMode
participant session
RelayServiceBase->>IRelayModeTelemetry: ModeStarting(mode)
RelayServiceBase->>DispatchVoiceMode: ExecuteAsync(cancellationToken)
session-->>DispatchVoiceMode: ConnectionChanged(reconnecting)
DispatchVoiceMode->>DispatchVoiceMode: status.LiveKit = Degraded
session-->>DispatchVoiceMode: ConnectionChanged(hard disconnect)
DispatchVoiceMode-->>RelayServiceBase: InvalidOperationException
RelayServiceBase->>IRelayModeTelemetry: ModeRetrying(mode, ex, attempt, nextDelay)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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: 5
🧹 Nitpick comments (1)
Resgrid.Relay.Engine/Services/RelayServiceBase.cs (1)
28-31: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueVirtual member
IsLiveKitModeis invoked from the base constructor.Reading
IsLiveKitMode(Line 30) during base construction runs the derived override before the derived constructor body executes. It's safe today since all overrides return a constant, but it's a fragile pattern (CA2214) if an override later depends on instance state.🤖 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 `@Resgrid.Relay.Engine/Services/RelayServiceBase.cs` around lines 28 - 31, The base constructor in RelayServiceBase is calling the virtual IsLiveKitMode property, which can invoke derived overrides before the derived constructor has finished. Remove the virtual dispatch from the constructor by moving the decision into a non-virtual initializer, a constructor parameter, or a static/helper method used by RelayServiceBase, so the telemetry selection no longer depends on override behavior during construction.
🤖 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.
Inline comments:
In `@Resgrid.Relay.Engine/Telemetry/RelayModeTelemetry.cs`:
- Around line 13-16: The Create method in RelayModeTelemetry currently returns a
new SentryRelayModeTelemetry per service, which causes each instance to own
SentrySdk.Init and disposal. Refactor SentryRelayModeTelemetry so Sentry SDK
initialization, flush, and disposal are managed by a single shared process-wide
instance instead of per-service ownership, and update Create to reuse that
shared telemetry instance for all LiveKit services.
In `@Resgrid.Relay.Engine/Voice/DispatchVoiceMode.cs`:
- Around line 56-72: `DispatchVoiceMode` leaves `status.LiveKit` stuck at
Degraded after a transient reconnect because the `onConnectionChanged` handler
returns immediately when `change.Connected` is true. Update the
`ConnectionChanged` logic so the same handler that sets Degraded on the
"reconnecting" reason also restores `status.LiveKit` to Connected once the
session reports a healthy connected state, using the existing `disconnect`,
`status`, and `onConnectionChanged` flow.
In `@Resgrid.Relay.Engine/Voice/RadioMode.cs`:
- Around line 86-99: The connection-state handler in RadioMode’s
onConnectionChanged leaves status.LiveKit stuck at Degraded after an
auto-reconnect because the change.Connected path returns early. Update the
reconnect flow so that when the SDK reports a successful reconnect, the status
is restored to Connected (or equivalent recovery state) before returning, and
keep the degraded transition only for the reconnecting case; use the
onConnectionChanged handler and the status.LiveKit updates as the key
touchpoints.
In `@Resgrid.Relay.Engine/Voice/RecordMode.cs`:
- Around line 55-68: `RecordMode` leaves `status.LiveKit` stuck in `Degraded`
after an SDK reconnect because the `VoiceConnectionStateChange` handler returns
immediately on `change.Connected == true` and never restores the state. Update
the reconnect recovery path in the `handler` inside `RecordMode` so that when
the connection becomes connected again it sets `status.LiveKit` back to
`ConnectionState.Connected` before returning, while still preserving the
existing `reconnecting` degraded handling and hard-disconnect restart behavior.
- Around line 84-86: The disconnect path in RecordMode should not throw during a
concurrent shutdown; mirror the RadioMode cancellation guard. In the logic that
awaits Task.WhenAny for VoiceModeRuntime.WaitForCancellationAsync and
disconnect.Task, only throw the InvalidOperationException when
cancellationToken.IsCancellationRequested is false, so a Ctrl+C/SIGTERM shutdown
can complete cleanly without a spurious fault.
---
Nitpick comments:
In `@Resgrid.Relay.Engine/Services/RelayServiceBase.cs`:
- Around line 28-31: The base constructor in RelayServiceBase is calling the
virtual IsLiveKitMode property, which can invoke derived overrides before the
derived constructor has finished. Remove the virtual dispatch from the
constructor by moving the decision into a non-virtual initializer, a constructor
parameter, or a static/helper method used by RelayServiceBase, so the telemetry
selection no longer depends on override behavior during construction.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 802dd7a2-6b94-4576-a289-3fe7aede0078
📒 Files selected for processing (19)
Resgrid.Audio.Relay.Console/Program.csResgrid.Audio.Voice.Tests/RelayResilienceTests.csResgrid.Audio.Voice.Tests/Resgrid.Audio.Voice.Tests.csprojResgrid.Relay.Engine/Configuration/RelayHostOptions.csResgrid.Relay.Engine/Properties/AssemblyInfo.csResgrid.Relay.Engine/Services/DispatchRelayService.csResgrid.Relay.Engine/Services/RadioRelayService.csResgrid.Relay.Engine/Services/RecordRelayService.csResgrid.Relay.Engine/Services/RelayServiceBase.csResgrid.Relay.Engine/Telemetry/IRelayModeTelemetry.csResgrid.Relay.Engine/Telemetry/NullRelayModeTelemetry.csResgrid.Relay.Engine/Telemetry/RelayModeTelemetry.csResgrid.Relay.Engine/Telemetry/SentryRelayModeTelemetry.csResgrid.Relay.Engine/Voice/DispatchVoiceMode.csResgrid.Relay.Engine/Voice/RadioMode.csResgrid.Relay.Engine/Voice/RecordMode.csdeploy/systemd/README.mddeploy/systemd/relay.env.exampledeploy/systemd/resgrid-relay.service
| using var sigTermRegistration = PosixSignalRegistration.Create(PosixSignal.SIGTERM, context => | ||
| { | ||
| context.Cancel = true; | ||
| cancellationTokenSource.Cancel(); | ||
| }); |
There was a problem hiding this comment.
PlatformNotSupportedException thrown at startup on Windows because PosixSignalRegistration.Create(PosixSignal.SIGTERM) runs without a platform guard and sits outside the try block in Main, causing an unhandled exception since the Engine targets net10.0-windows. Guard the registration with OperatingSystem.IsLinux() or OperatingSystem.IsMacOS() so the SIGTERM handler only registers on supported platforms.
PosixSignalRegistration sigTermRegistration = null;
if (OperatingSystem.IsLinux() || OperatingSystem.IsMacOS())
{
sigTermRegistration = PosixSignalRegistration.Create(PosixSignal.SIGTERM, context =>
{
context.Cancel = true;
cancellationTokenSource.Cancel();
});
}Prompt for LLM
File Resgrid.Audio.Relay.Console/Program.cs:
Line 93 to 97:
WHAT: PosixSignalRegistration.Create(PosixSignal.SIGTERM) is called without a platform guard and sits outside any try/catch, but SIGTERM is not a supported PosixSignal on Windows — .NET throws PlatformNotSupportedException. WHY: The Engine targets net10.0-windows, so running the CLI on Windows now crashes at startup (the throw is before the try block, so Main receives an unhandled exception). HOW: Guard the registration with OperatingSystem.IsLinux() (or wrap in try/catch PlatformNotSupportedException) so the SIGTERM handler only registers on platforms that support it.
Suggested Code:
PosixSignalRegistration sigTermRegistration = null;
if (OperatingSystem.IsLinux() || OperatingSystem.IsMacOS())
{
sigTermRegistration = PosixSignalRegistration.Create(PosixSignal.SIGTERM, context =>
{
context.Cancel = true;
cancellationTokenSource.Cancel();
});
}
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
Kody Review CompleteGreat news! 🎉 Keep up the excellent work! 🚀 Kody Guide: Usage and ConfigurationInteracting with Kody
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
2 participants
Summary
This PR hardens the relay's long-lived LiveKit voice modes (radio, record, dispatch) and adds proper systemd support for the CLI.
Graceful shutdown under systemd / Docker
SIGTERMhandling in the console host, sosystemctl stopanddocker stoptrigger the same clean teardown path as Ctrl+C instead of killing the process mid-work.Resilience layer for LiveKit voice modes
RelayServiceBasethat wraps the run for LiveKit modes. Faulted runs are automatically restarted after a jittered, capped delay; after a configurable number of consecutive quick failures the circuit opens and the service faults.ResilienceOptions.Hard disconnect detection and reconnection
Sentry telemetry for mode lifecycle
IRelayModeTelemetry) with Sentry-backed and no-op implementations. Mode start, retry, fault, and stop events are reported to Sentry when a DSN is configured.Tests