Conversation
|
|
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughMigrates the solution from .NET Framework to .NET 10 (SDK-style projects), adds a Resgrid V4 OIDC-enabled API client, implements a containerizable SMTP relay mode with telemetry and attachment upload, refactors the audio pipeline for Media Foundation MP3 encoding, and updates tests and documentation. Changes
Sequence DiagramssequenceDiagram
participant SMTPClient as SMTP Client
participant SMTPServer as Relay SMTP Server
participant Parser as SmtpDispatchAddressParser
participant Store as ProcessedMessageStore
participant Calls as CallsApi
participant Files as CallsApi (SaveCallFile)
participant Telemetry as SmtpTelemetry
SMTPClient->>SMTPServer: MAIL/RCPT/DATA (MIME)
SMTPServer->>Parser: Parse recipients
Parser-->>SMTPServer: [DispatchCode] or []
alt no targets
SMTPServer->>Telemetry: Record unroutable
SMTPServer-->>SMTPClient: 250 OK
else targets found
SMTPServer->>Store: Check stable sha256 id
alt duplicate
SMTPServer->>Telemetry: Record duplicate
SMTPServer-->>SMTPClient: 250 OK
else new
SMTPServer->>Telemetry: Record received
SMTPServer->>Calls: SaveCallAsync(NewCallInput)
Calls-->>SMTPServer: callId
SMTPServer->>Files: SaveCallFileAsync(base64 attachment)
Files-->>SMTPServer: fileId
SMTPServer->>Store: Register id
SMTPServer->>Telemetry: Record processed
SMTPServer-->>SMTPClient: 250 OK
end
end
sequenceDiagram
participant ClientApp as ResgridV4ApiClient
participant Cache as Token Cache (file)
participant OIDC as OIDC Discovery (.well-known)
participant TokenEP as Token Endpoint
participant API as Resgrid API
ClientApp->>ClientApp: SendRawAsync(request)
alt token missing/expired
ClientApp->>OIDC: GET /.well-known/openid-configuration
OIDC-->>ClientApp: token_endpoint
ClientApp->>Cache: Load cached refresh token/state
Cache-->>ClientApp: refresh_token (or options)
ClientApp->>TokenEP: POST refresh_token grant
TokenEP-->>ClientApp: access_token, expires_in, id/sub
ClientApp->>Cache: Save new token state
end
ClientApp->>API: HTTP request with Bearer token
API-->>ClientApp: 200/401/other
alt 401 on first try
ClientApp->>ClientApp: Clear token and retry once
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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: 4
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Resgrid.Audio.Core/AudioProcessor.cs (1)
58-93:⚠️ Potential issue | 🔴 CriticalMake the timer pass single-entry and snapshot
_startedWatchersunder the same lock.Lines 60-62 still use a non-atomic
_timerProcessingguard, and the dictionary walk on Lines 69-77 is still outsidelock (_lock). TwoElapsedcallbacks can enter together, or another callback can mutate_startedWatcherswhile this loop is enumerating it, which can double-finish watchers or throw at Line 85. Use an atomic gate (Interlocked) and take a locked snapshot before iterating.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Resgrid.Audio.Core/AudioProcessor.cs` around lines 58 - 93, _timer_Elapsed currently uses a non-atomic _timerProcessing flag and iterates _startedWatchers outside lock(_lock), risking concurrent Enter/modify and exceptions; change the gate to an atomic Interlocked compare-and-exchange on _timerProcessing to ensure single-entry, then inside the method take lock(_lock) once and create a snapshot (e.g., a List or array) of _startedWatchers values to iterate outside the lock; perform per-watcher finish work (calling _audioRecorder.SaveWatcherAudio, _watcherAudioStorage.FinishWatcher, TriggerProcessingFinished, _startedWatchers.Remove, and _audioEvaluator.RemoveActiveWatcher) while holding the lock or remove from the real dictionary under the lock using the snapshot to avoid double-finishes and enumeration exceptions.
🟠 Major comments (25)
Resgrid.Audio.Core/AudioProcessor.cs-83-93 (1)
83-93:⚠️ Potential issue | 🟠 MajorDon't hold
_lockacross audio save, storage finalization, and event dispatch.Lines 87-89 now run I/O and
TriggerProcessingFinishedwhile_lockis held. If any of those calls block or re-enter code that also needs_lock, the audio path can stall or deadlock. Remove the watcher from_startedWatchersinside the lock, then do the save/finalize/invoke work after releasing it.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Resgrid.Audio.Core/AudioProcessor.cs` around lines 83 - 93, The critical section currently holds _lock while performing I/O and event dispatch (calls to _audioRecorder.SaveWatcherAudio, _watcherAudioStorage.FinishWatcher and TriggerProcessingFinished) which can block or re-enter code; instead, inside the lock just retrieve and remove the watcher from _startedWatchers and call _audioEvaluator.RemoveActiveWatcher(id), then release the lock and perform the SaveWatcherAudio, FinishWatcher and TriggerProcessingFinished invocation outside the lock using the captured watcher variable; adjust null checks and error handling accordingly to avoid using the watcher after removal.Resgrid.Audio.Relay.Console/Configuration/RelayHostOptions.cs-44-44 (1)
44-44:⚠️ Potential issue | 🟠 MajorDon't persist raw SMTP messages by default.
Line 44 enables full raw-message retention out of the box. For a relay handling dispatch mail, that means bodies and attachments are written to disk unless operators explicitly opt out, which is a privacy/compliance footgun. Default this to
falseand make archival an explicit opt-in with retention controls.Suggested fix
- public bool SaveRawMessages { get; set; } = true; + public bool SaveRawMessages { get; set; } = false;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Resgrid.Audio.Relay.Console/Configuration/RelayHostOptions.cs` at line 44, Change the default for the RelayHostOptions.SaveRawMessages property to false so raw SMTP messages are not persisted by default; locate the SaveRawMessages auto-property in the RelayHostOptions class and set its initializer to false, and consider adding configuration/README notes or additional options for explicit opt-in and retention controls (e.g., retention period/enable flag) so operators must opt in to archival.Resgrid.Audio.Relay.Console/Configuration/RelayHostOptions.cs-40-40 (1)
40-40:⚠️ Potential issue | 🟠 MajorUse a cross-platform default for
DataDirectory.Line 40 sets
DataDirectoryto".\\data", which becomes a literal path name on Linux rather than a directory separator. That breaks the out-of-the-box storage path for the new containerized SMTP mode. Use"./data"here, or build the path withPath.Combineat the call site.Suggested fix
- public string DataDirectory { get; set; } = ".\\data"; + public string DataDirectory { get; set; } = "./data";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Resgrid.Audio.Relay.Console/Configuration/RelayHostOptions.cs` at line 40, The DataDirectory default in RelayHostOptions (`public string DataDirectory { get; set; }`) uses a Windows-style literal path ".\\data" which breaks on Linux; change the default to a cross-platform value (e.g. "./data") or remove the hardcoded separator and construct paths using Path.Combine at the call sites that use DataDirectory so consumers get a portable storage path.Resgrid.Audio.Core/Functions.cs-88-91 (1)
88-91:⚠️ Potential issue | 🟠 MajorDispose the WAV stream after
ReadExactly.Line 88 opens a
FileStream, and this method never closes it. Repeated calls will leak handles and can leave the file locked on Windows. Wrap the stream inusing/using var, or replace the whole block withFile.ReadAllBytes.Suggested fix
- System.IO.FileStream WaveFile = System.IO.File.OpenRead(wavePath); - wave = new byte[WaveFile.Length]; + using var waveFile = System.IO.File.OpenRead(wavePath); + wave = new byte[waveFile.Length]; data = new double[(wave.Length - 44) / 4];//shifting the headers out of the PCM data; - WaveFile.ReadExactly(wave, 0, Convert.ToInt32(WaveFile.Length));//read the wave file into the wave variable + waveFile.ReadExactly(wave, 0, Convert.ToInt32(waveFile.Length));//read the wave file into the wave variable🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Resgrid.Audio.Core/Functions.cs` around lines 88 - 91, The FileStream opened as WaveFile in the code (used to populate wave and data and passed to WaveFile.ReadExactly) is never disposed, leaking file handles; update the ReadWave/related method to either wrap the FileStream in a using / using var (ensuring WaveFile.ReadExactly is called inside the using scope) or replace the block with File.ReadAllBytes(wavePath) to load the bytes into wave and then compute data from wave, so the stream is properly closed after reading.Resgrid.Audio.Relay.Console/appsettings.json-18-18 (1)
18-18:⚠️ Potential issue | 🟠 MajorPII collection/retention is enabled by default.
Line 18 and Line 34 default to collecting/saving potentially sensitive message/user data. Safer default should be opt-in.
🔒 Suggested safer defaults
- "SendDefaultPii": true + "SendDefaultPii": false ... - "SaveRawMessages": true, + "SaveRawMessages": false,Also applies to: 34-34
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Resgrid.Audio.Relay.Console/appsettings.json` at line 18, The appsettings key SendDefaultPii is currently set to true which enables collection/retention of potentially sensitive PII by default; change SendDefaultPii to false in the appsettings.json entries (both occurrences of the "SendDefaultPii" key) so PII collection is opt-in, and update any related configuration documentation or code comments that reference SendDefaultPii to state that operators must explicitly enable it to collect PII.Dockerfile-8-18 (1)
8-18:⚠️ Potential issue | 🟠 MajorRun the runtime image as a non-root user.
The final image runs as root because no
USERis set. This weakens container isolation and increases blast radius on compromise.🔧 Suggested hardening patch
FROM mcr.microsoft.com/dotnet/runtime:10.0 AS final WORKDIR /app COPY --from=build /app/publish . + +RUN adduser --disabled-password --gecos "" relay \ + && mkdir -p /app/data \ + && chown -R relay:relay /app +USER relay ENV RELAY_Mode=smtp ENV RELAY_Smtp__Port=2525 EXPOSE 2525🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Dockerfile` around lines 8 - 18, The final image runs as root; create and switch to a non-root user in the final stage by adding a dedicated user/group, ensure /app ownership is changed to that user (chown) and set USER before the ENTRYPOINT so the container runs as non-root; reference the Dockerfile's final stage (WORKDIR /app, COPY --from=build /app/publish ., and ENTRYPOINT ["dotnet", "Resgrid.Audio.Relay.Console.dll"]) and perform the user creation, chown of /app, and USER change there.Resgrid.Audio.Core/ComService.cs-104-115 (1)
104-115:⚠️ Potential issue | 🟠 MajorAdd a final catch to prevent unhandled exception escape from event flow.
Current catches are specific; unexpected exception types (e.g., serialization/runtime) can still escape and destabilize processing. Add a final
catch (Exception ex)with logging.Suggested fix
catch (TaskCanceledException ex) { _logger.Error(ex.ToString()); } + catch (Exception ex) + { + _logger.Error(ex.ToString()); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Resgrid.Audio.Core/ComService.cs` around lines 104 - 115, Add a final broad exception handler after the existing specific catches in the ComService event flow: append a catch (Exception ex) block after the HttpRequestException / InvalidOperationException / TaskCanceledException handlers that logs the exception via _logger.Error(ex.ToString()) (or the preferred logging method) and prevents rethrow so unexpected runtime/serialization errors do not escape the event processing in ComService.cs.Resgrid.Audio.Core/Inputs/AudioRecorder.cs-187-193 (1)
187-193:⚠️ Potential issue | 🟠 MajorUse a writable temp location and collision-safe file names.
Lines 187-193 write under
AppContext.BaseDirectoryand use second-precision timestamps. This can fail in read-only install paths and collide on rapid/concurrent calls. PreferPath.GetTempPath()+ a GUID suffix.Suggested fix
- var directory = Path.Combine(AppContext.BaseDirectory, "DispatchAudio"); + var directory = Path.Combine(Path.GetTempPath(), "ResgridRelay", "DispatchAudio"); Directory.CreateDirectory(directory); - var fileRoot = Path.Combine(directory, $"RelayAudio_{DateTime.Now:s}".Replace(":", "_")); + var fileRoot = Path.Combine(directory, $"RelayAudio_{DateTime.UtcNow:yyyyMMdd_HHmmss}_{Guid.NewGuid():N}");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Resgrid.Audio.Core/Inputs/AudioRecorder.cs` around lines 187 - 193, The current code in AudioRecorder.cs creates files under AppContext.BaseDirectory and uses DateTime with second precision for file names (variables directory, fileRoot, waveFilePath, mp3FilePath), which can be read-only and prone to collisions; change it to use a writable temp folder (Path.GetTempPath()) and append a collision-safe unique suffix (e.g., Guid.NewGuid()) to the file root, building paths with Path.Combine so temporary WAV/MP3 files are created reliably and atomically in a writable location.Providers/Resgrid.Providers.ApiClient/V4/ResgridApiClientOptions.cs-17-18 (1)
17-18:⚠️ Potential issue | 🟠 MajorFail fast on malformed API base URLs.
Line 17 only checks whitespace; malformed URLs still pass and fail later in request flow. Validate URI format here to surface config errors early.
Suggested fix
if (String.IsNullOrWhiteSpace(BaseUrl)) throw new InvalidOperationException("A Resgrid API base URL is required."); + if (!Uri.TryCreate(BaseUrl, UriKind.Absolute, out _)) + throw new InvalidOperationException("The Resgrid API base URL must be a valid absolute URI.");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Providers/Resgrid.Providers.ApiClient/V4/ResgridApiClientOptions.cs` around lines 17 - 18, The constructor/initializer in ResgridApiClientOptions currently only checks String.IsNullOrWhiteSpace(BaseUrl); extend this to validate the URL format (e.g., use Uri.TryCreate(BaseUrl, UriKind.Absolute, out var uriResult) and/or Uri.IsWellFormedUriString) and throw a clear InvalidOperationException when the BaseUrl is not a well-formed absolute URI; update the check near where BaseUrl is validated so malformed values fail fast before any request flow.Resgrid.Audio.Tests/Resgrid.Audio.Tests.csproj-27-29 (1)
27-29:⚠️ Potential issue | 🟠 MajorReplace legacy
HintPathfor NAudio—file path does not exist in repository.Line 28 uses a legacy HintPath to
..\packages\NAudio.1.8.4\lib\net35\NAudio.dll, but this path does not exist. This will fail on clean CI agents. Multiple projects have the same issue (Resgrid.Audio.Core, Resgrid.Audio.Relay). Migrate toPackageReferencewith NuGet or use a consistent References folder path.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Resgrid.Audio.Tests/Resgrid.Audio.Tests.csproj` around lines 27 - 29, The project currently references NAudio via a stale Reference element using HintPath ("Reference Include=\"NAudio\"" with HintPath to ..\packages\NAudio.1.8.4\lib\net35\NAudio.dll) which doesn't exist on CI; replace that legacy Reference with a PackageReference for the NAudio NuGet package (e.g., add a PackageReference Include="NAudio" Version="1.8.4" or newer) in the .csproj, remove the HintPath/Reference block, and apply the same change to the other affected projects (Resgrid.Audio.Core, Resgrid.Audio.Relay) so builds use NuGet restore instead of a local packages folder.Resgrid.Audio.Core/Inputs/AudioRecorder.cs-201-205 (1)
201-205:⚠️ Potential issue | 🟠 MajorMove Media Foundation API initialization to app startup, not per save operation.
MediaFoundationApi.Startup()is called on line 201 each time this method executes, but there's no matchingShutdown()call anywhere in the codebase. Since this appears to be part of an audio conversion method that runs multiple times, this creates a resource leak. Either move the startup/shutdown pair to application initialization/shutdown hooks (one-time initialization), or addMediaFoundationApi.Shutdown()in the finally block to balance the lifecycle per operation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Resgrid.Audio.Core/Inputs/AudioRecorder.cs` around lines 201 - 205, MediaFoundationApi.Startup() is being invoked inside the per-save conversion in AudioRecorder (the method that opens WaveFileReader and calls MediaFoundationEncoder.EncodeToMp3), causing a resource leak because Shutdown() is never called; fix by removing the per-operation Startup() and instead call MediaFoundationApi.Startup() once at application initialization and MediaFoundationApi.Shutdown() at application shutdown, or if you must keep per-operation lifecycle, wrap the conversion in try/finally and call MediaFoundationApi.Shutdown() in the finally block to balance MediaFoundationApi.Startup(). Ensure changes reference MediaFoundationApi.Startup(), MediaFoundationApi.Shutdown(), and the method that currently performs EncodeToMp3 so reviewers can locate the code.Resgrid.Audio.Relay/MainWindow.xaml.cs-58-78 (1)
58-78:⚠️ Potential issue | 🟠 MajorGuard start/stop with exception handling to avoid UI crashes.
Lines 67 and 74 can throw (device state/driver/runtime errors). In a click handler this can terminate the UI flow. Wrap start/stop in
try/catch, updateStatusTextBlock, and log/append an event.Suggested fix
private void ToggleMonitoring_Click(object sender, RoutedEventArgs e) { - if (_audioRecorder.RecordingState == RecordingState.Stopped) - { - var selectedDevice = DeviceComboBox.SelectedItem as DeviceOption; - if (selectedDevice == null) - { - StatusTextBlock.Text = "Select an input device before starting."; - return; - } - - _audioRecorder.BeginMonitoring(selectedDevice.Id); - ToggleButton.Content = "Stop Monitoring"; - StatusTextBlock.Text = $"Monitoring {selectedDevice.Name}"; - AppendEvent($"Started monitoring device {selectedDevice.Name}"); - } - else - { - _audioRecorder.Stop(); - ToggleButton.Content = "Start Monitoring"; - StatusTextBlock.Text = "Stopped"; - AppendEvent("Stopped monitoring"); - } + try + { + if (_audioRecorder.RecordingState == RecordingState.Stopped) + { + var selectedDevice = DeviceComboBox.SelectedItem as DeviceOption; + if (selectedDevice == null) + { + StatusTextBlock.Text = "Select an input device before starting."; + return; + } + + _audioRecorder.BeginMonitoring(selectedDevice.Id); + ToggleButton.Content = "Stop Monitoring"; + StatusTextBlock.Text = $"Monitoring {selectedDevice.Name}"; + AppendEvent($"Started monitoring device {selectedDevice.Name}"); + } + else + { + _audioRecorder.Stop(); + ToggleButton.Content = "Start Monitoring"; + StatusTextBlock.Text = "Stopped"; + AppendEvent("Stopped monitoring"); + } + } + catch (Exception ex) + { + StatusTextBlock.Text = "Monitoring error"; + AppendEvent($"Monitoring error: {ex.Message}"); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Resgrid.Audio.Relay/MainWindow.xaml.cs` around lines 58 - 78, Wrap the start/stop operations in the click handler with try/catch to prevent UI crashes: when calling _audioRecorder.BeginMonitoring(selectedDevice.Id) and _audioRecorder.Stop(), catch Exception (or a more specific exception) and in the catch update StatusTextBlock with an error message, call AppendEvent with the exception details, and ensure ToggleButton.Content is set to a safe state; also guard the selectedDevice null check before BeginMonitoring and only call BeginMonitoring inside the try block so any device/driver/runtime errors are logged and surfaced without crashing the UI.Resgrid.Audio.Core/ComService.cs-84-99 (1)
84-99:⚠️ Potential issue | 🟠 MajorBlocking API calls in event handler stalls trigger processing pipeline.
Lines 84, 97, and 99 perform three sequential synchronous calls to external APIs inside an event handler. This blocks the audio processor's trigger pipeline. Instead of blocking, use a background task to handle the call creation and file upload asynchronously, while the handler returns immediately. Alternatively, restructure the event subscription to support async operations.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Resgrid.Audio.Core/ComService.cs` around lines 84 - 99, The event handler currently blocks by calling CallsApi.SaveCallAsync, CallsApi.SaveCallFileAsync and CallsApi.GetCallAsync synchronously with .GetAwaiter().GetResult(); instead, offload the entire save/upload/get sequence to a fire-and-forget background async task (e.g., Task.Run or queuing to a shared background worker) so the handler returns immediately; inside that background task call the three APIs with await (CallsApi.SaveCallAsync, then CallsApi.SaveCallFileAsync, then CallsApi.GetCallAsync), wrap it in try/catch and log any exceptions (do not rethrow to the handler) and preserve the existing inputs (savedCallId, userId, e.Mp3Audio, e.Watcher) used to build SaveCallFileInput.Resgrid.Audio.Relay.Console/Program.cs-224-228 (1)
224-228:⚠️ Potential issue | 🟠 MajorUse
TryParseloops for CLI numeric input to avoid hard crashesThese parse calls throw on invalid input and terminate setup/monitor flows instead of re-prompting or returning a clean error.
💡 Proposed direction
- config.InputDevice = Int32.Parse(Prompt("Audio device number", config.InputDevice.ToString())); + config.InputDevice = PromptInt("Audio device number", config.InputDevice); - config.AudioLength = Int32.Parse(Prompt("Recording length in seconds", config.AudioLength.ToString())); + config.AudioLength = PromptInt("Recording length in seconds", config.AudioLength); - var watcherCount = Int32.Parse(Prompt("How many groups or department watchers?", config.Watchers.Count == 0 ? "1" : config.Watchers.Count.ToString())); + var watcherCount = PromptInt("How many groups or department watchers?", config.Watchers.Count == 0 ? 1 : config.Watchers.Count); - Count = Int32.Parse(Prompt("How many tones for this watcher (1 or 2)", "2")), - Frequency1 = Double.Parse(Prompt("Tone 1 frequency", "524")), - Time1 = Int32.Parse(Prompt("Tone 1 length in milliseconds", "500")) + Count = PromptInt("How many tones for this watcher (1 or 2)", 2), + Frequency1 = PromptDouble("Tone 1 frequency", 524d), + Time1 = PromptInt("Tone 1 length in milliseconds", 500) - watcher.Triggers[0].Frequency2 = Double.Parse(Prompt("Tone 2 frequency", "794")); - watcher.Triggers[0].Time2 = Int32.Parse(Prompt("Tone 2 length in milliseconds", "500")); + watcher.Triggers[0].Frequency2 = PromptDouble("Tone 2 frequency", 794d); + watcher.Triggers[0].Time2 = PromptInt("Tone 2 length in milliseconds", 500); - var device = args.Length > 0 ? Int32.Parse(args[0]) : 0; + var device = args.Length > 0 && Int32.TryParse(args[0], out var parsedDevice) ? parsedDevice : 0;Also applies to: 263-265, 271-272, 291-291
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Resgrid.Audio.Relay.Console/Program.cs` around lines 224 - 228, Replace direct Int32.Parse calls with input-validation loops that use Int32.TryParse and re-prompt via Prompt() until the user supplies a valid integer: apply this to the assignments to config.InputDevice, config.AudioLength and the watcherCount variable (and the other numeric parses noted in the file). Specifically, wrap Prompt(...) in a loop that calls Int32.TryParse on the returned string, only assigning to config.InputDevice, config.AudioLength or watcherCount when TryParse succeeds and otherwise redisplays the Prompt with an error message; keep the current boolean assignment (config.Multiple) logic as-is. Ensure you update every similar Parse usage in this file (the later numeric parses mentioned) to follow the same TryParse + re-prompt pattern so invalid CLI input won’t crash the program.Providers/Resgrid.Providers.ApiClient/V4/DispatchListBuilder.cs-51-57 (1)
51-57:⚠️ Potential issue | 🟠 MajorFallback to default when normalized prefix becomes empty
A prefix like
":"passes Line 53, then normalizes to empty at Line 56, producing invalid dispatch tokens.💡 Proposed fix
private static string NormalizePrefix(string prefix) { if (String.IsNullOrWhiteSpace(prefix)) return "G"; - return prefix.Trim().TrimEnd(':').ToUpperInvariant(); + var normalized = prefix.Trim().TrimEnd(':').ToUpperInvariant(); + return String.IsNullOrWhiteSpace(normalized) ? "G" : normalized; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Providers/Resgrid.Providers.ApiClient/V4/DispatchListBuilder.cs` around lines 51 - 57, NormalizePrefix currently returns "G" only when input is null/whitespace but can produce an empty string (e.g., ":" -> ""), causing invalid tokens; update NormalizePrefix to trim, remove trailing colons and uppercase as it does, then check the resulting string and return "G" if the normalized result is empty (use String.IsNullOrEmpty on the computed value) so callers like the dispatch token builder never receive an empty prefix.Providers/Resgrid.Providers.ApiClient/V4/CallsApi.cs-27-30 (1)
27-30:⚠️ Potential issue | 🟠 MajorAvoid silent success when call-file upload returns no id
Returning
nullfrom Line 30 can hide an upload failure and make downstream behavior ambiguous.💡 Proposed fix
public static async Task<string> SaveCallFileAsync(SaveCallFileInput file, CancellationToken cancellationToken = default) { + if (file == null) + throw new ArgumentNullException(nameof(file)); + var result = await ResgridV4ApiClient.PostAsync<SaveOperationResult>("CallFiles/SaveCallFile", file, cancellationToken).ConfigureAwait(false); - return result?.Id; + if (String.IsNullOrWhiteSpace(result?.Id)) + throw new InvalidOperationException("The Resgrid API did not return a call file id for the saved call file."); + + return result.Id; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Providers/Resgrid.Providers.ApiClient/V4/CallsApi.cs` around lines 27 - 30, SaveCallFileAsync currently returns null when ResgridV4ApiClient.PostAsync<SaveOperationResult>("CallFiles/SaveCallFile", file, ...) yields no result.Id, which masks upload failures; update SaveCallFileAsync to validate the response from ResgridV4ApiClient.PostAsync and when result is null or result.Id is null/empty throw a clear exception (e.g., InvalidOperationException) with a descriptive message like "Call file upload failed: no id returned" so callers of SaveCallFileAsync (and any callers expecting a non-null id) can detect and handle the failure instead of silently receiving null.Resgrid.Audio.Relay.Console/Smtp/SmtpTelemetry.cs-882-886 (1)
882-886:⚠️ Potential issue | 🟠 MajorUse bounded queue for Countly telemetry events
The unbounded channel can grow without limit during network issues/high throughput, risking memory pressure.
💡 Proposed fix
- _eventChannel = Channel.CreateUnbounded<CountlyEventRequest>(new UnboundedChannelOptions - { - SingleReader = true, - SingleWriter = false - }); + _eventChannel = Channel.CreateBounded<CountlyEventRequest>(new BoundedChannelOptions(2000) + { + SingleReader = true, + SingleWriter = false, + FullMode = BoundedChannelFullMode.DropOldest + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Resgrid.Audio.Relay.Console/Smtp/SmtpTelemetry.cs` around lines 882 - 886, The unbounded Channel created for CountlyEventRequest (_eventChannel = Channel.CreateUnbounded<CountlyEventRequest>(...)) can grow indefinitely; change it to a bounded Channel (Channel.CreateBounded<CountlyEventRequest>(new BoundedChannelOptions(capacity) { SingleReader = true, SingleWriter = false, FullMode = ... })) with a sensible capacity and FullMode (DropOldest/DropWrite or Wait) to avoid memory pressure, and update producers to use WaitToWriteAsync/TryWrite and handle failed writes (e.g., drop, log, or backpressure) so enqueuing is robust during network issues/high throughput.Providers/Resgrid.Providers.ApiClient/V4/CallsApi.cs-12-14 (1)
12-14:⚠️ Potential issue | 🟠 MajorHandle null API response before reading
IdIf
PostAsyncreturns null, Line 13 throwsNullReferenceExceptionbefore your explicit error path.💡 Proposed fix
- if (String.IsNullOrWhiteSpace(result.Id)) + if (String.IsNullOrWhiteSpace(result?.Id)) throw new InvalidOperationException("The Resgrid API did not return a call id for the saved call.");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Providers/Resgrid.Providers.ApiClient/V4/CallsApi.cs` around lines 12 - 14, Check for a null response from ResgridV4ApiClient.PostAsync before accessing result.Id: after calling PostAsync<SaveOperationResult>("Calls/SaveCall", call, cancellationToken) assign to result, validate result is not null and throw a clear InvalidOperationException (or similar) if it is null; only then inspect String.IsNullOrWhiteSpace(result.Id) to throw the existing "did not return a call id" error. This change affects the call site using PostAsync<SaveOperationResult>, the local variable result, and the SaveOperationResult object validation in CallsApi.cs.Resgrid.Audio.Relay.Console/Smtp/SmtpTelemetry.cs-528-553 (1)
528-553:⚠️ Potential issue | 🟠 MajorSentry payload currently includes high-risk message/user metadata
Lines 528-553 attach sender/recipient addresses and subject-level content to Sentry extras. This is a compliance/privacy risk for production mail traffic.
💡 Proposed fix (minimize PII)
- scope.SetExtra("smtp.sender", state.Sender); - scope.SetExtra("smtp.accepted_recipients", state.AcceptedRecipients.ToArray()); - scope.SetExtra("smtp.rejected_recipients", state.RejectedRecipients.ToArray()); + scope.SetExtra("smtp.sender_domain", GetDomain(state.Sender)); + scope.SetExtra("smtp.accepted_recipient_count", state.AcceptedRecipientCount); + scope.SetExtra("smtp.rejected_recipient_count", state.RejectedRecipientCount); ... - scope.SetExtra("smtp.subject", message.Subject); - scope.SetExtra("smtp.from_addresses", message.FromAddresses); - scope.SetExtra("smtp.to_addresses", message.ToAddresses); - scope.SetExtra("smtp.cc_addresses", message.CcAddresses); + scope.SetExtra("smtp.subject_length", message.Subject?.Length ?? 0); + scope.SetExtra("smtp.from_domain", GetDomain(message.FromAddresses.FirstOrDefault())); + scope.SetExtra("smtp.to_recipient_count", message.ToAddresses?.Length ?? 0); + scope.SetExtra("smtp.cc_recipient_count", message.CcAddresses?.Length ?? 0);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Resgrid.Audio.Relay.Console/Smtp/SmtpTelemetry.cs` around lines 528 - 553, The Sentry telemetry attaches PII (sender/recipient addresses and subject) via scope.SetExtra calls for SMTP state and message; remove or redact these sensitive fields (e.g., stop calling scope.SetExtra with message.Subject, message.FromAddresses, message.ToAddresses, message.CcAddresses, state.Sender, AcceptedRecipients/RejectedRecipients contents) and instead log non-identifying metadata such as counts (MessageCount, number of recipients) or hashed/truncated identifiers (e.g., hash of StableMessageId or ReferenceMessageId) using the existing scope.SetExtra/SetTag calls; update SmtpTelemetry.cs around the scope.SetExtra invocations for state and message to replace direct PII with sanitized alternatives and keep other non-PII fields (RouteKind, StableMessageId hash, RawMessagePath, CallId) intact.Resgrid.Audio.Relay.Console/Smtp/SmtpDispatchAddressParser.cs-48-59 (1)
48-59:⚠️ Potential issue | 🟠 MajorGuard nullable domain lists before calling
Any()Line 48 and Line 58 dereference option lists directly. If either list is null from config binding, parsing fails with
NullReferenceException.💡 Proposed fix
- if (_options.DepartmentAddressDomains.Any(x => String.Equals(x, domain, StringComparison.OrdinalIgnoreCase))) + var departmentDomains = _options.DepartmentAddressDomains ?? Array.Empty<string>(); + if (departmentDomains.Any(x => String.Equals(x, domain, StringComparison.OrdinalIgnoreCase))) { dispatchCode = new DispatchCode { Code = code, Type = DispatchCodeType.Department }; return true; } - if (_options.GroupAddressDomains.Any(x => String.Equals(x, domain, StringComparison.OrdinalIgnoreCase))) + var groupDomains = _options.GroupAddressDomains ?? Array.Empty<string>(); + if (groupDomains.Any(x => String.Equals(x, domain, StringComparison.OrdinalIgnoreCase))) { dispatchCode = new DispatchCode { Code = code, Type = DispatchCodeType.Group }; return true; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Resgrid.Audio.Relay.Console/Smtp/SmtpDispatchAddressParser.cs` around lines 48 - 59, The code in SmtpDispatchAddressParser (inside the method performing address parsing) calls _options.DepartmentAddressDomains.Any(...) and _options.GroupAddressDomains.Any(...) without null checks; guard these lists before calling Any() by checking for null (e.g., use _options.DepartmentAddressDomains != null && _options.DepartmentAddressDomains.Any(...) or the null-safe pattern _options.DepartmentAddressDomains?.Any(...) == true) and apply the same change for _options.GroupAddressDomains to avoid NullReferenceException during config binding failures.Resgrid.Audio.Relay.Console/Smtp/SmtpDispatchAddressParser.cs-24-30 (1)
24-30:⚠️ Potential issue | 🟠 MajorDeduplicate by dispatch type + code, not code alone
Line 27 currently treats
Department:ABC123andGroup:ABC123as duplicates and drops one route. That can misroute mixed-recipient emails.💡 Proposed fix
public List<DispatchCode> ParseRecipients(IEnumerable<string> addresses) { var dispatchCodes = new List<DispatchCode>(); + var seen = new HashSet<string>(StringComparer.OrdinalIgnoreCase); if (addresses == null) return dispatchCodes; foreach (var address in addresses) { - if (TryParse(address, out var dispatchCode) && - dispatchCodes.All(x => !String.Equals(x.Code, dispatchCode.Code, StringComparison.OrdinalIgnoreCase))) + if (TryParse(address, out var dispatchCode)) { - dispatchCodes.Add(dispatchCode); + var key = $"{dispatchCode.Type}:{dispatchCode.Code}"; + if (seen.Add(key)) + dispatchCodes.Add(dispatchCode); } } return dispatchCodes; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Resgrid.Audio.Relay.Console/Smtp/SmtpDispatchAddressParser.cs` around lines 24 - 30, The current duplicate-check in SmtpDispatchAddressParser's foreach loop only compares dispatchCode.Code, causing different dispatch types with the same code (e.g., "Department:ABC123" vs "Group:ABC123") to be treated as duplicates; update the duplicate test used when adding to dispatchCodes (the block that calls TryParse and then dispatchCodes.All(...)) to compare both dispatchCode.Type and dispatchCode.Code (case-insensitively) — e.g., replace the single-field equality check with a comparison of the pair (Type, Code) so only entries with the same type and code are considered duplicates.Providers/Resgrid.Providers.ApiClient/V4/ResgridV4ApiClient.cs-262-274 (1)
262-274:⚠️ Potential issue | 🟠 MajorAvoid persisting bearer tokens in plaintext cache.
This writes access/refresh tokens directly to disk. That weakens secret-at-rest posture if filesystem access is compromised.
🔒 Suggested hardening
- var payload = JsonSerializer.Serialize(tokenState, JsonOptions); + // Persist only what is required to recover; force fresh access token retrieval on startup. + var persistedState = new ResgridApiTokenState + { + RefreshToken = tokenState.RefreshToken, + UserId = tokenState.UserId, + ExpiresAtUtc = DateTimeOffset.MinValue + }; + var payload = JsonSerializer.Serialize(persistedState, JsonOptions); await File.WriteAllTextAsync(cachePath, payload, cancellationToken).ConfigureAwait(false);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Providers/Resgrid.Providers.ApiClient/V4/ResgridV4ApiClient.cs` around lines 262 - 274, PersistTokenStateAsync is writing the ResgridApiTokenState (containing access/refresh tokens) to disk in plaintext at _options.TokenCachePath; instead, encrypt the serialized payload before writing (and decrypt when reading) using a platform-appropriate data protection API (e.g., Windows DPAPI/ProtectedData, macOS Keychain/Keychain APIs via a library, or ASP.NET IDataProtector) so tokens are not stored in cleartext, update the corresponding read method to unprotect the data into the ResgridApiTokenState, and add error handling and tests for key management and decryption failures; refer to PersistTokenStateAsync, _options.TokenCachePath and the tokenState serialization to locate where to apply encryption/decryption.Resgrid.Audio.Relay.Console/Smtp/SmtpRelayInfrastructure.cs-502-504 (1)
502-504:⚠️ Potential issue | 🟠 MajorHarden processed-message store against corrupt JSON and normalize comparer.
A malformed
processed-messages.jsonwill throw and break dedupe flow. Also, deserialized dictionaries won’t retainOrdinalIgnoreCase, causing inconsistent key matching across runs.🔧 Proposed fix
- var items = JsonSerializer.Deserialize<Dictionary<string, DateTimeOffset>>(payload); - return items ?? new Dictionary<string, DateTimeOffset>(StringComparer.OrdinalIgnoreCase); + try + { + var items = JsonSerializer.Deserialize<Dictionary<string, DateTimeOffset>>(payload); + return items == null + ? new Dictionary<string, DateTimeOffset>(StringComparer.OrdinalIgnoreCase) + : new Dictionary<string, DateTimeOffset>(items, StringComparer.OrdinalIgnoreCase); + } + catch (JsonException) + { + return new Dictionary<string, DateTimeOffset>(StringComparer.OrdinalIgnoreCase); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Resgrid.Audio.Relay.Console/Smtp/SmtpRelayInfrastructure.cs` around lines 502 - 504, The deserialization in SmtpRelayInfrastructure (processed-message store reader) can throw on malformed JSON and yields a Dictionary without OrdinalIgnoreCase; wrap the JsonSerializer.Deserialize call in a try/catch for JsonException (and fallback) and on success return a new Dictionary constructed with the deserialized items and StringComparer.OrdinalIgnoreCase (e.g., new Dictionary<string, DateTimeOffset>(items, StringComparer.OrdinalIgnoreCase)), and on failure return an empty new Dictionary<string, DateTimeOffset>(StringComparer.OrdinalIgnoreCase) so corrupt JSON doesn't break the dedupe flow and keys are always compared case-insensitively.Providers/Resgrid.Providers.ApiClient/V4/ResgridV4ApiClient.cs-252-253 (1)
252-253:⚠️ Potential issue | 🟠 MajorHandle malformed token-cache JSON without breaking startup.
A corrupted token cache will throw during
JsonSerializer.Deserialize(...)and fail initialization, even though the client could recover via refresh token flow.🔧 Proposed fix
- var cachedState = JsonSerializer.Deserialize<ResgridApiTokenState>(payload, JsonOptions); + ResgridApiTokenState cachedState; + try + { + cachedState = JsonSerializer.Deserialize<ResgridApiTokenState>(payload, JsonOptions); + } + catch (JsonException) + { + return state; + } if (cachedState == null) return state;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Providers/Resgrid.Providers.ApiClient/V4/ResgridV4ApiClient.cs` around lines 252 - 253, Wrap the JsonSerializer.Deserialize<ResgridApiTokenState>(payload, JsonOptions) call in a try/catch that catches JsonException (and optionally Exception) so a malformed token-cache JSON does not throw during initialization; on catch, log a warning (using the class logger), treat cachedState as null (ignore/delete the cached payload) and continue so the client can recover via the refresh token flow instead of failing startup.Resgrid.Audio.Relay.Console/Smtp/SmtpRelayInfrastructure.cs-205-214 (1)
205-214:⚠️ Potential issue | 🟠 MajorValidate attachment upload results before reporting success.
SaveCallFileAsynccan returnnullon failure (Providers/Resgrid.Providers.ApiClient/V4/CallsApi.cs, Lines 27-31). Right now failures can be silently ignored and the message still ends as processed.🔧 Proposed fix
foreach (var attachment in uploadableAttachments) { - await _callsClient.SaveCallFileAsync(new SaveCallFileInput + var callFileId = await _callsClient.SaveCallFileAsync(new SaveCallFileInput { CallId = callId, UserId = userId, Type = (int)attachment.Type, Name = attachment.Name, Data = Convert.ToBase64String(attachment.Data), Note = $"SMTP attachment imported from message {stableMessageId}" - }, cancellationToken).ConfigureAwait(false); + }, cancellationToken).ConfigureAwait(false); + + if (String.IsNullOrWhiteSpace(callFileId)) + throw new InvalidOperationException($"Failed to upload attachment '{attachment.Name}' for call '{callId}'."); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Resgrid.Audio.Relay.Console/Smtp/SmtpRelayInfrastructure.cs` around lines 205 - 214, The code awaits _callsClient.SaveCallFileAsync with a SaveCallFileInput and currently ignores its return value; modify the attachment upload handling in SmtpRelayInfrastructure.cs to capture the result of SaveCallFileAsync, validate it is not null (and/or indicates success), and if it is null or indicates failure log an error including stableMessageId and attachment.Name via the existing logger and mark the message processing as failed (or throw/return accordingly) so failures aren't silently ignored; reference the SaveCallFileAsync call, SaveCallFileInput creation, _callsClient, attachment.Name and stableMessageId when implementing the check and error handling.
🟡 Minor comments (2)
README.md-98-98 (1)
98-98:⚠️ Potential issue | 🟡 MinorConfiguration key name appears inconsistent with runtime option
Line 98 documents
DuplicateWindowHours, but the SMTP infrastructure usesDuplicateMessageWindowHours. This can cause misconfiguration if copied directly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` at line 98, The README documents the config key as DuplicateWindowHours which doesn't match the runtime option DuplicateMessageWindowHours used by the SMTP infra; update the README entry to use DuplicateMessageWindowHours (or explicitly list both names and note the accepted alias) so docs and runtime are consistent, and ensure any example JSON/config blocks and references use DuplicateMessageWindowHours to prevent misconfiguration.README.md-218-225 (1)
218-225:⚠️ Potential issue | 🟡 MinorClarify command working directory vs
--projectpathsThe heading says “From
Resgrid.Audio.Relay.Console”, but the commands use repo-root style.\Resgrid.Audio.Relay.Console\...paths. One of these should be adjusted to avoid confusion.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` around lines 218 - 225, The README heading "From `Resgrid.Audio.Relay.Console`" is inconsistent with the example commands that use repo-root paths; either change the heading to "From repo root" or alter the commands to run from inside the project directory (e.g., remove the `--project .\Resgrid.Audio.Relay.Console\Resgrid.Audio.Relay.Console.csproj` portion and run `dotnet run -- <command>`), so update the README section containing the heading and the listed commands to make the working directory and `--project` usage consistent.
🧹 Nitpick comments (3)
Resgrid.Audio.Relay.Console/Resgrid.Audio.Relay.Console.csproj (1)
13-18: Consider removing excluded legacy sources once migration is stable.Lines 13–18 exclude several folders/files from compilation; deleting truly obsolete files later will reduce drift and confusion.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Resgrid.Audio.Relay.Console/Resgrid.Audio.Relay.Console.csproj` around lines 13 - 18, The project file currently excludes legacy source folders/files via Compile Remove entries (Args\**, Commands\**, Data\**, Models\**, ConsoleTable.cs); once the migration is verified stable, delete the obsolete files/folders (Args, Commands, Data, Models and ConsoleTable.cs) from the repository and remove the matching <Compile Remove="..."> entries from Resgrid.Audio.Relay.Console.csproj so the csproj no longer references stale exclusions; commit the cleanup in a single follow-up change with a brief commit message.Resgrid.Audio.Tests/SmtpDispatchAddressParserTests.cs (1)
37-39: Make recipient assertions order-independent.Lines 38-39 depend on list ordering. Consider asserting set membership (
Contain/ContainEquivalentOf) so behavior stays stable even if parser iteration order changes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Resgrid.Audio.Tests/SmtpDispatchAddressParserTests.cs` around lines 37 - 39, The test assertions on the parsed recipients are order-dependent; update SmtpDispatchAddressParserTests to assert membership instead of index-based checks by using order-independent FluentAssertions (e.g. assert result contains an element with Type == DispatchCodeType.Department and contains an element with Type == DispatchCodeType.Group), referencing the result variable so the test passes regardless of parser iteration order.Resgrid.Audio.Relay/MainWindow.xaml.cs (1)
83-96: Use non-blocking UI marshaling for high-frequency audio callbacks.Lines 83 and 93 use synchronous
Dispatcher.Invoke(); this blocks the audio callback thread unnecessarily while marshaling to the UI thread. Both handlers perform only simple TextBlock assignments with no return values or subsequent blocking logic. Replace withDispatcher.BeginInvoke()to avoid stalling audio sample processing.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Resgrid.Audio.Relay/MainWindow.xaml.cs` around lines 83 - 96, Both audio callbacks are using synchronous Dispatcher.Invoke which can block the audio thread; in the handlers that update UI (the earlier waveform/min/max/db updater and SampleAggregator_WaveformCalculated) replace Dispatcher.Invoke(...) with Dispatcher.BeginInvoke(...) so the TextBlock assignments (MinTextBlock, MaxTextBlock, DbTextBlock, FftTextBlock) are marshaled asynchronously to the UI thread and will not stall audio processing.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7f96998b-cd61-413c-bd3b-e3448740130d
📒 Files selected for processing (53)
.dockerignoreDockerfileProviders/Resgrid.Providers.ApiClient/Resgrid.Providers.ApiClient.csprojProviders/Resgrid.Providers.ApiClient/V4/CallsApi.csProviders/Resgrid.Providers.ApiClient/V4/DispatchListBuilder.csProviders/Resgrid.Providers.ApiClient/V4/HealthApi.csProviders/Resgrid.Providers.ApiClient/V4/Models/AuthModels.csProviders/Resgrid.Providers.ApiClient/V4/Models/CallModels.csProviders/Resgrid.Providers.ApiClient/V4/ResgridApiClientOptions.csProviders/Resgrid.Providers.ApiClient/V4/ResgridV4ApiClient.csProviders/Resgrid.Providers.ApiClient/app.configProviders/Resgrid.Providers.ApiClient/packages.configREADME.mdResgrid.Audio.Core/AudioEvaluator.csResgrid.Audio.Core/AudioProcessor.csResgrid.Audio.Core/ComService.csResgrid.Audio.Core/Events/CallCreatedEventArgs.csResgrid.Audio.Core/Functions.csResgrid.Audio.Core/Inputs/AudioRecorder.csResgrid.Audio.Core/Model/Config.csResgrid.Audio.Core/Model/Trigger.csResgrid.Audio.Core/Model/Watcher.csResgrid.Audio.Core/Resgrid.Audio.Core.csprojResgrid.Audio.Core/SampleAggregator.csResgrid.Audio.Core/app.configResgrid.Audio.Core/packages.configResgrid.Audio.Relay.Console/App.configResgrid.Audio.Relay.Console/Configuration/RelayHostOptions.csResgrid.Audio.Relay.Console/Program.csResgrid.Audio.Relay.Console/Properties/AssemblyInfo.csResgrid.Audio.Relay.Console/Resgrid.Audio.Relay.Console.csprojResgrid.Audio.Relay.Console/Smtp/SmtpDispatchAddressParser.csResgrid.Audio.Relay.Console/Smtp/SmtpRelayInfrastructure.csResgrid.Audio.Relay.Console/Smtp/SmtpTelemetry.csResgrid.Audio.Relay.Console/appsettings.jsonResgrid.Audio.Relay.Console/packages.configResgrid.Audio.Relay.Console/settings.jsonResgrid.Audio.Relay/App.configResgrid.Audio.Relay/App.xamlResgrid.Audio.Relay/App.xaml.csResgrid.Audio.Relay/MainWindow.xamlResgrid.Audio.Relay/MainWindow.xaml.csResgrid.Audio.Relay/Resgrid.Audio.Relay.csprojResgrid.Audio.Relay/packages.configResgrid.Audio.Tests/AudioEvaluatorWithFileTests.csResgrid.Audio.Tests/DispatchListBuilderTests.csResgrid.Audio.Tests/DtmfTests.csResgrid.Audio.Tests/Resgrid.Audio.Tests.csprojResgrid.Audio.Tests/SmtpDispatchAddressParserTests.csResgrid.Audio.Tests/SmtpRelayTelemetryTests.csResgrid.Audio.Tests/app.configResgrid.Audio.Tests/packages.configglobal.json
💤 Files with no reviewable changes (11)
- Resgrid.Audio.Tests/packages.config
- Providers/Resgrid.Providers.ApiClient/packages.config
- Providers/Resgrid.Providers.ApiClient/app.config
- Resgrid.Audio.Core/packages.config
- Resgrid.Audio.Relay/packages.config
- Resgrid.Audio.Relay/App.config
- Resgrid.Audio.Core/app.config
- Resgrid.Audio.Relay.Console/packages.config
- Resgrid.Audio.Tests/app.config
- Resgrid.Audio.Relay.Console/App.config
- Resgrid.Audio.Core/SampleAggregator.cs
| <Compile Include="V3\Models\HealthResult.cs" /> | ||
| <Compile Include="V3\Models\ValidateResult.cs" /> | ||
| <Compile Include="V3\ResgridV3ApiClient.cs" /> | ||
| <Compile Remove="V3\**\*.cs" /> |
There was a problem hiding this comment.
V3 source removal is premature and can break consumers.
Line 12 removes all V3 compilation, but Resgrid.Audio.Relay.Console/Commands/RunCommand.cs (Lines 14, 117-120 in provided context) still references Resgrid.Providers.ApiClient.V3 and ResgridV3ApiClient.Init(...). This needs to be migrated in the same PR or deferred.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Providers/Resgrid.Providers.ApiClient/Resgrid.Providers.ApiClient.csproj` at
line 12, The project removed all V3 sources via the Compile Remove "V3\**\*.cs"
entry, but RunCommand.cs in Resgrid.Audio.Relay.Console still references the V3
namespace and initializer (Resgrid.Providers.ApiClient.V3 and
ResgridV3ApiClient.Init(...)); either restore the V3 compile entries or migrate
the console command to the new API: revert or remove the Compile Remove line, or
update RunCommand.cs to use the new provider namespace/type and call the new
initialization method (replace references to Resgrid.Providers.ApiClient.V3 and
ResgridV3ApiClient.Init with the corresponding new class/method), and ensure the
project includes the appropriate source files or package so the symbols resolve.
Comment on lines
+25
to
+27
| <Reference Include="NAudio"> | ||
| <HintPath>..\packages\NAudio.1.8.4\lib\net35\NAudio.dll</HintPath> | ||
| <Private>true</Private> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether the repository still restores packages into a repo-local ./packages folder.
fd -HI '^packages$'
rg -n 'NAudio\.1\.8\.4|RestorePackagesPath|repositoryPath' -g '*.csproj' -g 'NuGet.Config' -g 'packages.config'Repository: Resgrid/Relay
Length of output: 424
🏁 Script executed:
# Check if packages folder is gitignored
cat .gitignore | grep -i packages || echo "No packages pattern found in .gitignore"
# Verify project file is SDK-style and check target framework
head -5 Resgrid.Audio.Core/Resgrid.Audio.Core.csproj
# Check if there's a NuGet.Config at repo root
ls -la NuGet.Config 2>/dev/null || echo "No NuGet.Config at repo root"
# Check if the actual packages DLL path exists
test -f '..\packages\NAudio.1.8.4\lib\net35\NAudio.dll' && echo "DLL exists" || echo "DLL path does not exist (expected)"Repository: Resgrid/Relay
Length of output: 394
🌐 Web query:
NAudio 1.8.4 NuGet package net10.0 net35 compatibility
💡 Result:
The NAudio 1.8.4 NuGet package targets .NET Framework 3.5 (net35) and includes a Universal Windows Platform build. It is compatible with .NET Framework 3.5 projects. Since it is a .NET Framework 3.5 assembly, it can run on later .NET Framework versions (e.g., 4.x) due to backward compatibility, as confirmed by NAudio maintainer statements that net35 builds benefit from later framework improvements when run on newer runtimes. .NET 10.0 (net10.0) refers to the modern .NET platform (successor to .NET 8/9). NAudio 1.8.4 does not target .NET Standard or net10.0/netcoreapp, so it is not directly compatible with pure .NET 10.0 projects without .NET Framework compatibility features (e.g., via hosting .NET Framework apps or multi-targeting). Newer NAudio versions (v2.x) support .NET Standard and modern .NET. For .NET 10.0 projects, upgrade to NAudio 2.2+ or later.
Citations:
- 1: https://www.nuget.org/packages/NAudio/1.8.4
- 2: https://stackoverflow.com/questions/51924726/vs2008-net-3-5-c-sharp-cant-include-naudio-csproj-meaning-of-and
- 3: Add support .NET Portable or .NET Standard. naudio/NAudio#173
- 4: https://github.com/naudio/NAudio/releases
- 5: How to rebuild NAudio to .Net Framework 4.8? naudio/NAudio#722
Upgrade NAudio to 2.2+ and remove the broken HintPath reference.
Line 26 hardcodes ..\packages\NAudio.1.8.4\lib\net35\NAudio.dll, but the /packages/ folder is excluded from version control (see .gitignore), so this path is broken and the project cannot build. More critically, NAudio 1.8.4 targets .NET Framework 3.5 and is incompatible with the project's net10.0-windows target. Modern NAudio (2.2+) supports .NET Standard and .NET 10.0.
Replace the Reference with a PackageReference to NAudio 2.2 or later:
Suggested project-file change
<ItemGroup>
<PackageReference Include="Accord.Math" Version="3.8.0" />
+ <PackageReference Include="NAudio" Version="2.2.1" />
<PackageReference Include="Serilog" Version="4.3.0" />
</ItemGroup>
...
- <Reference Include="NAudio">
- <HintPath>..\packages\NAudio.1.8.4\lib\net35\NAudio.dll</HintPath>
- <Private>true</Private>
- </Reference>This issue also exists in Resgrid.Audio.Tests/Resgrid.Audio.Tests.csproj and Resgrid.Audio.Relay/Resgrid.Audio.Relay.csproj.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Resgrid.Audio.Core/Resgrid.Audio.Core.csproj` around lines 25 - 27, The
project contains a hardcoded Reference Include="NAudio" with a broken HintPath
to ..\packages\NAudio.1.8.4 which targets .NET 3.5; replace that Reference
element with a PackageReference for NAudio (version 2.2.0 or later) so the
SDK/nuget-managed package is used and compatible with net10.0-windows, removing
the HintPath and Private attributes; apply the same replacement in the other
projects that have the same Reference Include="NAudio" entry
(Resgrid.Audio.Tests and Resgrid.Audio.Relay).
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (1)
Resgrid.Audio.Relay.Console/Smtp/SmtpRelayInfrastructure.cs (1)
510-514: Consider atomic file writes for crash resilience.
File.WriteAllTextAsyncis not atomic—if the process crashes mid-write, the file could be corrupted or truncated. For improved reliability, consider writing to a temporary file and then renaming:♻️ Optional: Atomic write pattern
private async Task SaveAsync(Dictionary<string, DateTimeOffset> entries, CancellationToken cancellationToken) { var payload = JsonSerializer.Serialize(entries, SerializerOptions); - await File.WriteAllTextAsync(_path, payload, cancellationToken).ConfigureAwait(false); + var tempPath = _path + ".tmp"; + await File.WriteAllTextAsync(tempPath, payload, cancellationToken).ConfigureAwait(false); + File.Move(tempPath, _path, overwrite: true); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Resgrid.Audio.Relay.Console/Smtp/SmtpRelayInfrastructure.cs` around lines 510 - 514, SaveAsync currently writes directly to _path which can corrupt the file if the process crashes mid-write; change it to write the serialized payload to a temporary file (e.g., _path + ".tmp" or Path.GetTempFileName()) using the same CancellationToken, flush/close it, then atomically replace the target file by renaming/moving the temp file into place (use File.Replace when available to swap safely and optionally keep a backup, or File.Move with overwrite semantics) and surface any exceptions—update the SaveAsync method to perform the temp-write + atomic-rename pattern to ensure crash-resistant writes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/auto-approve.yml:
- Line 16: Update the workflow "if" condition to verify the comment is on a pull
request and use the comment author's login; replace the existing condition (the
inline if expression) with a compound check that github.event.issue.pull_request
is present and github.event.comment.user.login == 'ucswift' and
contains(github.event.comment.body, 'Approve') so the job only runs for PR
comments by that user containing "Approve".
- Line 14: Update the GitHub Actions step that references the
actions/github-script action by replacing the pinned version
"actions/github-script@v6" with the latest stable tag
"actions/[email protected]"; locate the workflow step using the string "uses:
actions/github-script@v6" and update it to "uses: actions/[email protected]",
then run a quick workflow lint or dry-run to ensure no breaking changes in the
job that calls the github-script action.
- Line 23: Remove the invalid review_id parameter from the payload passed to
github.rest.pulls.createReview; the API accepts commit_id, body, event, and
comments only, so delete the line setting review_id (e.g. the review_id: 1 entry
in the auto-approve workflow) to ensure the call uses only supported fields when
invoking github.rest.pulls.createReview.
- Around line 8-26: The createReview call in the GitHub Actions script (inside
the actions/github-script@v6 step's script) is using an invalid parameter
`review_id`; remove the `review_id: 1,` entry from the
github.rest.pulls.createReview({...}) payload so the call only sends supported
fields (e.g., commit_id, body, event, comments) and will succeed; update the
object passed to github.rest.pulls.createReview in that script accordingly.
In @.github/workflows/dotnet.yml:
- Around line 12-17: The workflow mixes 'main' in the push trigger but still
uses 'master' in later branch checks, so align branch handling consistently:
update the push.branches list and any hardcoded branch checks (e.g., occurrences
checking for 'master' in publish/release job conditions or if-statements) to
either remove 'main' from triggers or replace every 'master' reference with
'main' (and add 'master' as needed) so the same branch names are used across
push, pull_request handling, and release/publish job conditions.
- Around line 217-243: Replace the API lookup and greedy regex in the
actions/github-script block: read the PR body from
context.payload.pull_request.body (instead of calling
github.rest.repos.listPullRequestsAssociatedWithCommit and finding pr) and
compute body = (context.payload.pull_request.body || '').replace(/##\s*Summary
by CodeRabbit[\s\S]*?(?=^\s*##\s|\z)/im, '').trim() || 'No release notes
provided.' so the removal of the "Summary by CodeRabbit" section stops at the
next markdown heading or EOF; then continue to build notes and write
release_notes.md as before (references: prs/pr variables,
context.payload.pull_request.body, the regex replacement, fs.writeFileSync,
release_notes.md).
In `@Resgrid.Audio.Core/Resgrid.Audio.Core.csproj`:
- Around line 13-24: DtmfDetection.NAudio.dll is binding to NAudio v1.8.4.0
while the project references NAudio v1.10.0, causing runtime assembly load
failures; fix by either rebuilding the DtmfDetection.NAudio project against
NAudio 1.10.0 (replace the References/DtmfDetection.NAudio.dll with the rebuilt
binary) or by pinning the project's NuGet NAudio package back to Version="1.8.4"
in Resgrid.Audio.Core.csproj (and the other two projects) so the runtime
assembly versions match; ensure DtmfDetection.dll is left untouched since it has
no NAudio dependency.
In `@Resgrid.Audio.Relay.Console/Smtp/SmtpRelayInfrastructure.cs`:
- Around line 207-218: The loop uploading attachments via
_callsClient.SaveCallFileAsync over uploadableAttachments can leave the newly
created call (callId) orphaned if one upload fails; change this to collect
per-attachment tasks (e.g., create a List<Task> of SaveCallFileAsync calls and
await Task.WhenAll) and wrap in try/catch to aggregate errors; on failure invoke
a compensating action using existing calls API (for example call a
DeleteCallAsync(callId) or UpdateCallAsync/AddCallNoteAsync to mark attachments
incomplete) so the system either rolls back the created call or records a clear
partial-upload note, and ensure duplicate-message registration
(RegisterDuplicateMessageAsync) remains consistent.
- Around line 493-508: The LoadAsync method currently deserializes into a
case-sensitive Dictionary causing duplicate detection issues and will throw on
malformed JSON; update LoadAsync to (1) attempt deserialization inside a
try/catch that handles JsonException/IOException and returns new
Dictionary<string, DateTimeOffset>(StringComparer.OrdinalIgnoreCase) on error,
and (2) ensure the returned dictionary uses StringComparer.OrdinalIgnoreCase —
e.g., create a new case-insensitive dictionary and copy entries from the result
of JsonSerializer.Deserialize<Dictionary<string, DateTimeOffset>>(...) into it
(or use JsonSerializer to populate a Dictionary then construct a new
Dictionary<string, DateTimeOffset>(deserialized,
StringComparer.OrdinalIgnoreCase)); preserve use of _path and cancellationToken
and keep ConfigureAwait(false).
---
Nitpick comments:
In `@Resgrid.Audio.Relay.Console/Smtp/SmtpRelayInfrastructure.cs`:
- Around line 510-514: SaveAsync currently writes directly to _path which can
corrupt the file if the process crashes mid-write; change it to write the
serialized payload to a temporary file (e.g., _path + ".tmp" or
Path.GetTempFileName()) using the same CancellationToken, flush/close it, then
atomically replace the target file by renaming/moving the temp file into place
(use File.Replace when available to swap safely and optionally keep a backup, or
File.Move with overwrite semantics) and surface any exceptions—update the
SaveAsync method to perform the temp-write + atomic-rename pattern to ensure
crash-resistant writes.
🪄 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: 48a93f6b-f16b-4226-bbe9-92a60b0f5add
📒 Files selected for processing (8)
.github/copilot-instructions.md.github/workflows/auto-approve.yml.github/workflows/dotnet.ymlResgrid.Audio.Core/Resgrid.Audio.Core.csprojResgrid.Audio.Relay.Console/Configuration/RelayHostOptions.csResgrid.Audio.Relay.Console/Smtp/SmtpRelayInfrastructure.csResgrid.Audio.Relay/Resgrid.Audio.Relay.csprojResgrid.Audio.Tests/Resgrid.Audio.Tests.csproj
✅ Files skipped from review due to trivial changes (2)
- .github/copilot-instructions.md
- Resgrid.Audio.Relay.Console/Configuration/RelayHostOptions.cs
Comment on lines
+12
to
+17
| push: | ||
| branches: | ||
| - master | ||
| - main | ||
| pull_request: | ||
| types: [opened, synchronize, reopened, closed] |
There was a problem hiding this comment.
Align main handling across the trigger and release gates.
This workflow subscribes to main, but the merged-PR path is still hardcoded to master. A PR merged into main will build on the separate push run, but this workflow’s publish/release jobs will never start. Either remove main from the trigger or widen the branch checks consistently.
Also applies to: 28-31, 59-59, 124-124, 197-197
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/dotnet.yml around lines 12 - 17, The workflow mixes 'main'
in the push trigger but still uses 'master' in later branch checks, so align
branch handling consistently: update the push.branches list and any hardcoded
branch checks (e.g., occurrences checking for 'master' in publish/release job
conditions or if-statements) to either remove 'main' from triggers or replace
every 'master' reference with 'main' (and add 'master' as needed) so the same
branch names are used across push, pull_request handling, and release/publish
job conditions.
Comment on lines
+217
to
+243
| - name: Get merged PR info | ||
| id: pr-info | ||
| uses: actions/github-script@v7 | ||
| with: | ||
| script: | | ||
| const prs = await github.rest.repos.listPullRequestsAssociatedWithCommit({ | ||
| owner: context.repo.owner, | ||
| repo: context.repo.repo, | ||
| commit_sha: context.sha, | ||
| }); | ||
| const pr = prs.data.find(p => p.merged_at); | ||
| const fs = require('fs'); | ||
| if (pr) { | ||
| const body = (pr.body || '') | ||
| .replace(/##\s*Summary by CodeRabbit[\s\S]*/i, '') | ||
| .trim() || 'No release notes provided.'; | ||
| const notes = [ | ||
| body, | ||
| '', | ||
| '## Docker image', | ||
| `- \`${{ env.DOCKER_IMAGE }}:${{ env.RELEASE_VERSION }}\``, | ||
| `- \`${{ env.DOCKER_IMAGE }}:latest\`` | ||
| ].join('\n'); | ||
| fs.writeFileSync('release_notes.md', notes); | ||
| } else { | ||
| fs.writeFileSync('release_notes.md', 'No release notes provided.'); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the full workflow file to understand the event trigger and context
cat -n .github/workflows/dotnet.yml | head -50Repository: Resgrid/Relay
Length of output: 1717
🏁 Script executed:
# Find the event trigger for this workflow
rg "^on:" .github/workflows/dotnet.yml -A 10Repository: Resgrid/Relay
Length of output: 261
🏁 Script executed:
# Look at the specific lines mentioned in the review (217-243)
sed -n '210,250p' .github/workflows/dotnet.ymlRepository: Resgrid/Relay
Length of output: 1550
🏁 Script executed:
# Check if there are any other mentions of pull_request or github-script in this file
rg "pull_request|github-script|context\.payload" .github/workflows/dotnet.yml -nRepository: Resgrid/Relay
Length of output: 1012
🏁 Script executed:
# Test the regex patterns with a sample PR body
node <<'EOF'
// Test case 1: PR body with CodeRabbit section and content after
const body1 = `Some intro
## Summary by CodeRabbit
Auto-generated stuff
## Changelog
- Feature X`;
// Current regex (greedy)
const current = body1.replace(/##\s*Summary by CodeRabbit[\s\S]*/i, '').trim();
console.log("Current regex result:");
console.log(JSON.stringify(current));
console.log();
// Proposed regex (with lookahead)
const proposed = body1.replace(/(^|\n)##\s*Summary by CodeRabbit\b[\s\S]*?(?=\n##\s|\s*$)/i, '$1').trim();
console.log("Proposed regex result:");
console.log(JSON.stringify(proposed));
console.log();
// Test case 2: No CodeRabbit section
const body2 = `Some content
## Changelog
- Feature Y`;
const proposed2 = body2.replace(/(^|\n)##\s*Summary by CodeRabbit\b[\s\S]*?(?=\n##\s|\s*$)/i, '$1').trim();
console.log("Proposed regex on body without CodeRabbit:");
console.log(JSON.stringify(proposed2));
EOFRepository: Resgrid/Relay
Length of output: 244
Fix regex that truncates release notes after CodeRabbit section.
The regex at line 231 uses greedy matching ([\s\S]*) to remove the CodeRabbit summary, but this removes all content through EOF, not just that section. Any manual sections (like ## Changelog) added after the CodeRabbit heading disappear from the published release notes.
Since the github-release job only runs on merged PRs (condition at line 197 requires github.event.action == 'closed' and pull_request.merged == true), use context.payload.pull_request.body directly instead of the redundant API lookup. Replace the greedy regex with one that stops at the next markdown heading or end of string.
Proposed fix
- name: Get merged PR info
id: pr-info
uses: actions/github-script@v7
with:
script: |
- const prs = await github.rest.repos.listPullRequestsAssociatedWithCommit({
- owner: context.repo.owner,
- repo: context.repo.repo,
- commit_sha: context.sha,
- });
- const pr = prs.data.find(p => p.merged_at);
const fs = require('fs');
- if (pr) {
- const body = (pr.body || '')
- .replace(/##\s*Summary by CodeRabbit[\s\S]*/i, '')
- .trim() || 'No release notes provided.';
- const notes = [
- body,
- '',
- '## Docker image',
- `- \`${{ env.DOCKER_IMAGE }}:${{ env.RELEASE_VERSION }}\``,
- `- \`${{ env.DOCKER_IMAGE }}:latest\``
- ].join('\n');
- fs.writeFileSync('release_notes.md', notes);
- } else {
- fs.writeFileSync('release_notes.md', 'No release notes provided.');
- }
+ const body = context.payload.pull_request?.body ?? '';
+ const cleanedBody = body
+ .replace(/(^|\n)##\s*Summary by CodeRabbit\b[\s\S]*?(?=\n##\s|\s*$)/i, '$1')
+ .trim();
+ const notes = [
+ cleanedBody || 'No release notes provided.',
+ '',
+ '## Docker image',
+ `- \`${{ env.DOCKER_IMAGE }}:${{ env.RELEASE_VERSION }}\``,
+ `- \`${{ env.DOCKER_IMAGE }}:latest\``
+ ].join('\n');
+ fs.writeFileSync('release_notes.md', notes);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - name: Get merged PR info | |
| id: pr-info | |
| uses: actions/github-script@v7 | |
| with: | |
| script: | | |
| const prs = await github.rest.repos.listPullRequestsAssociatedWithCommit({ | |
| owner: context.repo.owner, | |
| repo: context.repo.repo, | |
| commit_sha: context.sha, | |
| }); | |
| const pr = prs.data.find(p => p.merged_at); | |
| const fs = require('fs'); | |
| if (pr) { | |
| const body = (pr.body || '') | |
| .replace(/##\s*Summary by CodeRabbit[\s\S]*/i, '') | |
| .trim() || 'No release notes provided.'; | |
| const notes = [ | |
| body, | |
| '', | |
| '## Docker image', | |
| `- \`${{ env.DOCKER_IMAGE }}:${{ env.RELEASE_VERSION }}\``, | |
| `- \`${{ env.DOCKER_IMAGE }}:latest\`` | |
| ].join('\n'); | |
| fs.writeFileSync('release_notes.md', notes); | |
| } else { | |
| fs.writeFileSync('release_notes.md', 'No release notes provided.'); | |
| } | |
| - name: Get merged PR info | |
| id: pr-info | |
| uses: actions/github-script@v7 | |
| with: | |
| script: | | |
| const fs = require('fs'); | |
| const body = context.payload.pull_request?.body ?? ''; | |
| const cleanedBody = body | |
| .replace(/(^|\n)##\s*Summary by CodeRabbit\b[\s\S]*?(?=\n##\s|\s*$)/i, '$1') | |
| .trim(); | |
| const notes = [ | |
| cleanedBody || 'No release notes provided.', | |
| '', | |
| '## Docker image', | |
| `- \`${{ env.DOCKER_IMAGE }}:${{ env.RELEASE_VERSION }}\``, | |
| `- \`${{ env.DOCKER_IMAGE }}:latest\`` | |
| ].join('\n'); | |
| fs.writeFileSync('release_notes.md', notes); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/dotnet.yml around lines 217 - 243, Replace the API lookup
and greedy regex in the actions/github-script block: read the PR body from
context.payload.pull_request.body (instead of calling
github.rest.repos.listPullRequestsAssociatedWithCommit and finding pr) and
compute body = (context.payload.pull_request.body || '').replace(/##\s*Summary
by CodeRabbit[\s\S]*?(?=^\s*##\s|\z)/im, '').trim() || 'No release notes
provided.' so the removal of the "Summary by CodeRabbit" section stops at the
next markdown heading or EOF; then continue to build notes and write
release_notes.md as before (references: prs/pr variables,
context.payload.pull_request.body, the regex replacement, fs.writeFileSync,
release_notes.md).
Comment on lines
+13
to
+24
| <PackageReference Include="NAudio" Version="1.10.0" /> | ||
| <PackageReference Include="Serilog" Version="4.3.0" /> | ||
| </ItemGroup> | ||
|
|
||
| <ItemGroup> | ||
| <Reference Include="Accord, Version=3.8.0.0, Culture=neutral, PublicKeyToken=fa1a88e29555ccf7, processorArchitecture=MSIL"> | ||
| <HintPath>..\packages\Accord.3.8.0\lib\net462\Accord.dll</HintPath> | ||
| </Reference> | ||
| <Reference Include="Accord.Math, Version=3.8.0.0, Culture=neutral, PublicKeyToken=fa1a88e29555ccf7, processorArchitecture=MSIL"> | ||
| <HintPath>..\packages\Accord.Math.3.8.0\lib\net462\Accord.Math.dll</HintPath> | ||
| </Reference> | ||
| <Reference Include="Accord.Math.Core, Version=3.8.0.0, Culture=neutral, PublicKeyToken=fa1a88e29555ccf7, processorArchitecture=MSIL"> | ||
| <HintPath>..\packages\Accord.Math.3.8.0\lib\net462\Accord.Math.Core.dll</HintPath> | ||
| </Reference> | ||
| <Reference Include="DtmfDetection"> | ||
| <HintPath>..\References\DtmfDetection.dll</HintPath> | ||
| <Private>true</Private> | ||
| </Reference> | ||
| <Reference Include="DtmfDetection.NAudio"> | ||
| <HintPath>..\References\DtmfDetection.NAudio.dll</HintPath> | ||
| <Private>true</Private> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Read-only verification:
# 1) Confirm the NAudio package version now pinned in the SDK-style projects.
# 2) Inspect managed AssemblyRef metadata in the retained local DLLs.
# Expected result:
# - The retained DLLs either do not reference NAudio, or they reference the same assembly version you now ship.
python -m pip install --quiet dnfile
echo "=== NAudio PackageReference versions ==="
rg -n --type xml '<PackageReference Include="NAudio" Version="' \
Resgrid.Audio.Core/Resgrid.Audio.Core.csproj \
Resgrid.Audio.Relay/Resgrid.Audio.Relay.csproj \
Resgrid.Audio.Tests/Resgrid.Audio.Tests.csproj
python - <<'PY'
import os
import dnfile
for path in ("References/DtmfDetection.dll", "References/DtmfDetection.NAudio.dll"):
print(f"\n=== {path} ===")
if not os.path.exists(path):
print("missing")
continue
pe = dnfile.dnPE(path)
net = getattr(pe, "net", None)
tables = getattr(net, "mdtables", None)
refs = getattr(tables, "AssemblyRef", None)
if refs is None:
print("no AssemblyRef metadata found")
continue
for row in refs:
name = getattr(row, "Name", None)
version = f"{row.MajorVersion}.{row.MinorVersion}.{row.BuildNumber}.{row.RevisionNumber}"
print(f"{name}: {version}")
PYRepository: Resgrid/Relay
Length of output: 639
Assembly binding incompatibility: DtmfDetection.NAudio.dll expects NAudio 1.8.4.0 but project ships 1.10.0.
The retained DtmfDetection.NAudio.dll (at References/DtmfDetection.NAudio.dll) declares an AssemblyRef to NAudio 1.8.4.0, but all three projects now reference NAudio 1.10.0 via NuGet. The DtmfDetection.dll binary itself has no NAudio dependency and is unaffected. However, DtmfDetection.NAudio.dll will fail at runtime when attempting to load a mismatched NAudio assembly version. Either rebuild the DtmfDetection binaries against NAudio 1.10.0, or pin the NAudio package version in the project files back to 1.8.4.0.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Resgrid.Audio.Core/Resgrid.Audio.Core.csproj` around lines 13 - 24,
DtmfDetection.NAudio.dll is binding to NAudio v1.8.4.0 while the project
references NAudio v1.10.0, causing runtime assembly load failures; fix by either
rebuilding the DtmfDetection.NAudio project against NAudio 1.10.0 (replace the
References/DtmfDetection.NAudio.dll with the rebuilt binary) or by pinning the
project's NuGet NAudio package back to Version="1.8.4" in
Resgrid.Audio.Core.csproj (and the other two projects) so the runtime assembly
versions match; ensure DtmfDetection.dll is left untouched since it has no
NAudio dependency.
Comment on lines
+207
to
+218
| foreach (var attachment in uploadableAttachments) | ||
| { | ||
| await _callsClient.SaveCallFileAsync(new SaveCallFileInput | ||
| { | ||
| CallId = callId, | ||
| UserId = userId, | ||
| Type = (int)attachment.Type, | ||
| Name = attachment.Name, | ||
| Data = Convert.ToBase64String(attachment.Data), | ||
| Note = $"SMTP attachment imported from message {stableMessageId}" | ||
| }, cancellationToken).ConfigureAwait(false); | ||
| } |
There was a problem hiding this comment.
Partial attachment upload leaves orphaned call on failure.
If SaveCallFileAsync fails for one of the attachments (e.g., network error on the second attachment), the call is already created at line 185 but left with incomplete attachments. There's no rollback mechanism for the call itself, only for the duplicate-message registration.
Consider either:
- Collecting all attachment upload tasks and using
Task.WhenAllwith error aggregation - Adding a note to the call on partial failure
- Implementing a compensating action (e.g., updating call status or adding a note indicating incomplete attachments)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Resgrid.Audio.Relay.Console/Smtp/SmtpRelayInfrastructure.cs` around lines 207
- 218, The loop uploading attachments via _callsClient.SaveCallFileAsync over
uploadableAttachments can leave the newly created call (callId) orphaned if one
upload fails; change this to collect per-attachment tasks (e.g., create a
List<Task> of SaveCallFileAsync calls and await Task.WhenAll) and wrap in
try/catch to aggregate errors; on failure invoke a compensating action using
existing calls API (for example call a DeleteCallAsync(callId) or
UpdateCallAsync/AddCallNoteAsync to mark attachments incomplete) so the system
either rolls back the created call or records a clear partial-upload note, and
ensure duplicate-message registration (RegisterDuplicateMessageAsync) remains
consistent.
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Tests