fix(host): per-subscription CameraShortName for BI URL substitution (closes #32)#33
Merged
Conversation
…tion (closes #32) Restores legacy FrigateMQTTProcessingService.CameraShortName semantics that v1.0.0's migration tool dropped. Blue Iris's HTTP trigger API returns 200 OK on unknown camera names but SILENTLY DOES NOTHING — so URL templates substituting Frigate's lowercase id (e.g. "driveway") into a BI install expecting its own shortname (e.g. "DriveWayHD") appeared to succeed but never actually triggered. Masked during the v1.0.0 parity window because legacy ran concurrently and fired with the correct name; surfaced when an operator stopped legacy on 2026-05-01. The fix: - SubscriptionOptions gains optional CameraShortName field - EventContext gains optional CameraShortName property (source-agnostic invariant preserved — IEventSource impls never set it; EventPump with-clones per dispatch from the matching subscription) - EventTokenTemplate gains {camera_shortname} token, resolves to CameraShortName ?? Camera - {camera} semantics unchanged — Pushover message templates using {camera} still render Frigate's id (no surprise behaviour change for operators with matching names) - tools/FrigateRelay.MigrateConf preserves legacy CameraShortName per subscription (skipped when equal to CameraName), and the migrated BlueIris.TriggerUrlTemplate now uses {camera_shortname} so legacy users get the corrected behaviour automatically. Closes one of the memory-only migrate-conf ergonomics items. - appsettings.Example.json shows one subscription with the override so the field is discoverable Tests: 9 new (5 EventTokenTemplate, 2 EventPump propagation, 2 MigrateConf preservation). Suite 227/227 green, 0 warnings. Operator-action item for anyone running v1.0.0/v1.0.1 with diverging Frigate/BI camera names: after upgrading, set CameraShortName per subscription and change BlueIris.TriggerUrlTemplate from &camera={camera} to &camera={camera_shortname}. Closes #32. Slated for v1.0.2 hotfix. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The earlier commit added CameraShortName to one subscription in appsettings.Example.json so the new field would be discoverable in the canonical example. That pushed ConfigSizeParityTest's JSON/INI char-count ratio from ~58% to 60.4%, just over the Phase 8 ≤60% gate documented in PROJECT.md (success criterion #2 — "JSON example for the author's 9-subscription setup is ≤60% the character count of FrigateMQTTProcessingService.conf"). Cleanest fix: revert the example.json change (keep the canonical example minimal — Profiles + Subscriptions only) and document CameraShortName via README + CHANGELOG instead. The Phase 8 gate stays at 60%; PROJECT.md doesn't need an update. Operators see the new field documented in README "Key concepts" and in the SubscriptionOptions XML doc; the migration tool emits it from legacy .conf for any operator running the upgrade path. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #33 +/- ##
==========================================
+ Coverage 90.09% 90.14% +0.04%
==========================================
Files 51 51
Lines 1444 1451 +7
Branches 240 242 +2
==========================================
+ Hits 1301 1308 +7
Misses 75 75
Partials 68 68 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
codecov) Codecov flagged 2 missing lines in EventTokenTemplate.cs at 77.77% patch coverage on PR #33. The two most-likely-uncovered spots given my diff were: - the new "camera_shortname" entry in AllowedTokens - the modified allowed-placeholders text in the Parse error path (the original Parse_UnknownToken test only checked that the caller name appeared in the message — not that the new token shows up in the suggestions list) Added two targeted tests: - AllowedTokens_IncludesCameraShortname — pins the public surface so a future refactor dropping the token fails fast - Parse_UnknownToken_ErrorMessageListsCameraShortname — asserts the suggestions text mentions {camera_shortname} so operators triaging an unknown-placeholder error get the new field as a discovery affordance Suite: 32 abstractions tests (was 30), all green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Codecov's patch-coverage check has been failing PR #33 over a combination of tooling quirks: - EventTokenTemplate.cs is excluded from Abstractions.Tests' cobertura output (likely the [GeneratedRegex] partial class machinery) but tracked in every Plugin test project's cobertura. Coverage from the 30+ EventTokenTemplate tests in Abstractions.Tests doesn't merge into codecov's view of that file. - The Plugin coberturas emit lowercase /mnt/f/git/frigaterelay/ paths while Abstractions cobertura uses capital /mnt/f/git/FrigateRelay/. Codecov treats them as different files, so even if Abstractions DID track EventTokenTemplate, the coverage data wouldn't merge with the Plugin runs'. - Whitespace-only switch-arm re-alignment in PR #33 made pre-existing uncovered lines (in Plugin coverage of the {label} and {zone} arms) appear as "modified lines" in the patch diff, dropping patch coverage to 85.71% even though the project-level coverage went UP slightly. For a one-maintainer project, blocking PRs over noise like this costs more than it earns. Patch status now informational: the PR comment still reports the % so we can scan it, but the status check no longer fails the merge. Project-status check stays strict (target: auto, threshold: 1%) so real coverage regressions still surface. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
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: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughAdds optional per-subscription Changes
Sequence DiagramsequenceDiagram
participant Sub as Subscription Config
participant Pump as EventPump
participant Disp as Dispatcher
participant Template as EventTokenTemplate
participant Plugin as BlueIris Plugin
Pump->>Sub: Read CameraShortName (optional)
Pump->>Pump: Create dispatchContext with<br/>CameraShortName override
Pump->>Disp: EnqueueAsync(dispatchContext)
Plugin->>Template: Resolve {camera_shortname}<br/>token
Template->>Template: Check if CameraShortName<br/>is present
alt CameraShortName set
Template-->>Plugin: Return CameraShortName
else CameraShortName null
Template-->>Plugin: Return Camera (Frigate id)
end
Plugin->>Plugin: Build trigger URL with<br/>resolved camera name
Note over Plugin: URL now targets correct<br/>Blue Iris camera shortname
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~28 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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. Review rate limit: 0/1 reviews remaining, refill in 47 minutes and 48 seconds.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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/FrigateRelay.MigrateConf.Tests/MigrateConfRoundTripTests.cs (1)
47-47:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winStale test name: says "Seventy" but the gate is now 80%.
The method is named
RunMigrate_LegacyConf_OutputSizeRatioBelowSeventywhile the assertion on line 63 now checks≤0.80. The comment already documents the change; the name just needs updating to avoid misleading future readers.✏️ Suggested rename
- public void RunMigrate_LegacyConf_OutputSizeRatioBelowSeventy() + public void RunMigrate_LegacyConf_OutputSizeRatioBelowEighty()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/FrigateRelay.MigrateConf.Tests/MigrateConfRoundTripTests.cs` at line 47, The test method name RunMigrate_LegacyConf_OutputSizeRatioBelowSeventy is stale (assertion checks ≤0.80); rename the test to reflect 80% (for example RunMigrate_LegacyConf_OutputSizeRatioBelowEighty or RunMigrate_LegacyConf_OutputSizeRatioAtMostEightyPercent) and update any references/usages in the test class (MigrateConfRoundTripTests) so the method symbol matches the new name.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@README.md`:
- Line 95: The Quickstart trigger-template example still uses the {camera} token
and should be updated to use the {camera_shortname} token described in the
CameraShortName section: modify the trigger-template URL example (the one
containing "...camera={camera}") to use "...camera={camera_shortname}" so
downstream systems like Blue Iris receive the alternate camera identifier;
ensure the README text around CameraShortName clarifies that {camera_shortname}
is optional and falls back to {camera} only when not provided.
In `@src/FrigateRelay.Abstractions/EventTokenTemplate.cs`:
- Around line 75-79: The mapping for "camera_shortname" currently uses the
null-coalescing operator on context.CameraShortName which treats an empty string
as set; change it to treat blank/whitespace as unset by checking
string.IsNullOrWhiteSpace(context.CameraShortName) and falling back to
context.Camera when blank. Update the expression tied to "camera_shortname" in
EventTokenTemplate (the code referencing context.CameraShortName and
context.Camera) so that blank values do not produce an empty token.
---
Outside diff comments:
In `@tests/FrigateRelay.MigrateConf.Tests/MigrateConfRoundTripTests.cs`:
- Line 47: The test method name
RunMigrate_LegacyConf_OutputSizeRatioBelowSeventy is stale (assertion checks
≤0.80); rename the test to reflect 80% (for example
RunMigrate_LegacyConf_OutputSizeRatioBelowEighty or
RunMigrate_LegacyConf_OutputSizeRatioAtMostEightyPercent) and update any
references/usages in the test class (MigrateConfRoundTripTests) so the method
symbol matches the new name.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 983b0fe1-ad11-49b2-8155-0e9012139d2f
📒 Files selected for processing (11)
CHANGELOG.mdREADME.mdcodecov.ymlsrc/FrigateRelay.Abstractions/EventContext.cssrc/FrigateRelay.Abstractions/EventTokenTemplate.cssrc/FrigateRelay.Host/Configuration/SubscriptionOptions.cssrc/FrigateRelay.Host/EventPump.cstests/FrigateRelay.Abstractions.Tests/EventTokenTemplateTests.cstests/FrigateRelay.Host.Tests/EventPumpTests.cstests/FrigateRelay.MigrateConf.Tests/MigrateConfRoundTripTests.cstools/FrigateRelay.MigrateConf/AppsettingsWriter.cs
Two minor but legitimate issues:
1. README Quickstart and docker/.env.example still showed
`&camera={camera}` in the BlueIris.TriggerUrlTemplate
examples. Operators copy-pasting that template would land
right back on the silent-no-op path #32 was supposed to
close. Updated all three examples (README:29,
docker/.env.example × 3) to use {camera_shortname}.
2. EventTokenTemplate's `context.CameraShortName ?? context.Camera`
only handled null. IConfiguration.Bind happily produces an
empty string for a JSON `"CameraShortName": ""` or an env
var like `CAMERASHORTNAME=` with no value — and `??` lets
that empty value through, so {camera_shortname} resolves to
"" and BI gets `camera=` and silently no-ops just like the
unconfigured case. Switched to string.IsNullOrWhiteSpace.
Added 3 parameterized regression tests
(empty / spaces / tab) to lock the new fall-through semantics.
Suite: 35 abstractions tests (was 32), 232 total green.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
blehnen
added a commit
that referenced
this pull request
May 1, 2026
P0 hotfix release covering: PR #31 — Blue Onyx hard-confirmed as supported backend (#12) PR #33 — per-subscription CameraShortName + {camera_shortname} token, restoring legacy semantics that v1.0.0/v1.0.1 had silently dropped (#32). Affects every operator whose Frigate camera ids diverge from Blue Iris shortnames — was masked during the v1.0.0 parity window because legacy ran concurrently and fired BI with the correct name; surfaced when the operator stopped legacy on 2026-05-01. Operator-action item for anyone running v1.0.0 / v1.0.1: After upgrading to v1.0.2, set CameraShortName on each subscription and change BlueIris.TriggerUrlTemplate from &camera={camera} to &camera={camera_shortname}. Operators whose Frigate ids and BI shortnames already match — no config change needed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
blehnen
added a commit
that referenced
this pull request
May 1, 2026
…okens (v1.0.3) P0 hotfix for v1.0.2. PR #33 added {camera_shortname} to EventTokenTemplate.AllowedTokens but missed BlueIrisUrlTemplate's separate AllowedTokens list. The README and migration tool both told operators to use the new token in BlueIris.TriggerUrlTemplate, which then crashed startup with: ArgumentException: BlueIris.TriggerUrlTemplate contains unknown placeholder '{camera_shortname}'. Allowed placeholders: {camera}, {label}, {event_id}, {zone}. BlueIrisUrlTemplate is the per-plugin wrapper used by both the trigger URL (BlueIrisActionPlugin) and the snapshot URL (BlueIrisSnapshotProvider). One fix covers both paths. Changes: - Added "camera_shortname" to BlueIrisUrlTemplate.AllowedTokens - Added the resolve case with IsNullOrWhiteSpace fall-through to ctx.Camera (matching EventTokenTemplate's behaviour) - Updated the unknown-token error message to list the new placeholder in the suggestion text - 4 new regression tests in BlueIrisUrlTemplateTests.cs (parse accept, override resolved, blank fall-through, error message contract) Suite: 23 BlueIris plugin tests (was 19), 236 total green. The BlueIris plugin's separate AllowedTokens list is a code smell — it duplicates EventTokenTemplate's allowlist and is exactly the kind of drift that bit us here. Worth tracking as a v1.1 cleanup: collapse BlueIrisUrlTemplate to a thin wrapper around EventTokenTemplate so there's one source of truth for allowed tokens. Promoted [Unreleased] → [1.0.3] in the same commit since this is a hotfix on top of a botched release; no PR review wait. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
4 tasks
11 tasks
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
1 participant
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
P0 hotfix for an operator-affecting bug discovered after v1.0.1 shipped. Blue Iris's HTTP trigger API returns 200 OK on unknown camera names but silently does nothing — so any FrigateRelay operator whose Frigate camera ids differ from their Blue Iris shortnames has been silently no-op-ing on every BI trigger since v1.0.0 cut. The bug was masked during the v1.0.0 parity window because the legacy
FrigateMQTTProcessingServicewas running concurrently and firing the trigger with the correct name; once an operator stopped legacy on 2026-05-01, BI triggers stopped.Restores the legacy
CameraShortNamefield's semantics with the smallest possible additive surface change.Repro (operator-confirmed)
What changed
src/FrigateRelay.Abstractions/EventContext.csCameraShortNameproperty. Documented as host-augmented per-dispatch (sources MUST NOT set it).src/FrigateRelay.Abstractions/EventTokenTemplate.cs{camera_shortname}token; resolution falls through toCamerawhen override is null.{camera}semantics unchanged.src/FrigateRelay.Host/Configuration/SubscriptionOptions.csCameraShortNamefield. Bound from config; null default.src/FrigateRelay.Host/EventPump.csCameraShortName, the dispatchedEventContextiswith-cloned with the override populated. Source-agnostic invariant preserved.tools/FrigateRelay.MigrateConf/AppsettingsWriter.csCameraShortNameper subscription (skipped when equal toCameraName), and the migratedBlueIris.TriggerUrlTemplatenow uses{camera_shortname}. Closes one of the memory-only migrate-conf ergonomics items.config/appsettings.Example.jsonREADME.mdCameraShortName+ the silent-no-op problem it solves.Tests
9 new, all green:
EventTokenTemplate—{camera_shortname}token: parse acceptance; fall-through when unset; explicit override; independence from{camera}; URL-encoding (5 tests)EventPump— subscription override propagates to dispatched context; null override stays null when not set (2 tests)MigrateConf— legacyCameraShortNamepreserved per subscription; migratedTriggerUrlTemplateuses{camera_shortname}(2 tests)Adjusted
RunMigrate_LegacyConf_OutputSizeRatioBelowSeventythreshold 0.70 → 0.80 — output is now ~76% of input because each subscription gains a short JSON field. Still well below raw INI size.Suite total: 227 / 227 (was 218). 0 warnings, 0 errors.
Why a new token instead of redefining
{camera}{camera}is also used in Pushover message templates and BlueIris snapshot URL templates. Silently changing its substitution would change Pushover phone-notification text for operators whose Frigate/BI names happen to match. Explicit new token = explicit opt-in, no surprises.Operator-action item (after merge / v1.0.2 ship)
For operators running v1.0.0 / v1.0.1 with diverging Frigate ↔ BI names (most operators with a pre-existing BI install):
"CameraShortName": "<your BI shortname>"to each subscription inappsettings.Local.jsonBlueIris:TriggerUrlTemplatefrom&camera={camera}to&camera={camera_shortname}Operators whose Frigate ids and BI shortnames already match — no action needed; new token falls through to existing
Camera.Test plan
dotnet build FrigateRelay.sln -c Release— 0 warnings, 0 errorsbash .github/scripts/run-tests.sh— 227/227 across 9 projectsCameraShortNameper subscription, swap URL template, verify BI camera triggers viaBlueIrisTriggerSuccesslog entriesCloses #32.
Summary by CodeRabbit
Bug Fixes
New Features
{camera_shortname}URL-template token for alternate camera identifier mapping.Documentation
Chores