fix: adding defensive checks #3090

Merged

mfreeman451 merged 1 commit from refs/pull/3090/head into staging

2026-03-27 20:15:45 +00:00

mfreeman451 commented

2026-03-27 20:15:09 +00:00

(Migrated from github.com)

Owner

Imported from GitHub pull request.

Original GitHub pull request: #3092
Original author: @mfreeman451
Original URL: https://github.com/carverauto/serviceradar/pull/3092
Original created: 2026-03-27T20:15:09Z
Original updated: 2026-03-27T20:19:38Z
Original head: carverauto/serviceradar:3085-bugagent-gateway-mtr----too-many-concurrent-mtr-traces
Original base: staging
Original merged: 2026-03-27T20:15:45Z by @mfreeman451

IMPORTANT: Please sign the Developer Certificate of Origin

Thank you for your contribution to ServiceRadar. Please note, when contributing, the developer must include
a DCO sign-off statement indicating the DCO acceptance in one commit message. Here
is an example DCO Signed-off-by line in a commit message:

Signed-off-by: J. Doe <j.doe@domain.com>

Describe your changes

Issue ticket number and link

Code checklist before requesting a review

I have signed the DCO?
The build completes without errors?
All tests are passing when running make test?

Imported from GitHub pull request. Original GitHub pull request: #3092 Original author: @mfreeman451 Original URL: https://github.com/carverauto/serviceradar/pull/3092 Original created: 2026-03-27T20:15:09Z Original updated: 2026-03-27T20:19:38Z Original head: carverauto/serviceradar:3085-bugagent-gateway-mtr----too-many-concurrent-mtr-traces Original base: staging Original merged: 2026-03-27T20:15:45Z by @mfreeman451 --- ## IMPORTANT: Please sign the Developer Certificate of Origin Thank you for your contribution to ServiceRadar. Please note, when contributing, the developer must include a [DCO sign-off statement]( https://developercertificate.org/) indicating the DCO acceptance in one commit message. Here is an example DCO Signed-off-by line in a commit message: ``` Signed-off-by: J. Doe <j.doe@domain.com> ``` ## Describe your changes ## Issue ticket number and link ## Code checklist before requesting a review - [ ] I have signed the DCO? - [ ] The build completes without errors? - [ ] All tests are passing when running make test?

Copilot commented

2026-03-27 20:19:37 +00:00

(Migrated from github.com)

Author

Owner

Imported GitHub PR review comment.

Original author: @Copilot
Original URL: https://github.com/carverauto/serviceradar/pull/3092#discussion_r3003019723
Original created: 2026-03-27T20:19:37Z
Original path: elixir/serviceradar_core/lib/serviceradar/edge/agent_command_bus.ex
Original line: 270

ensure_dispatch_capacity/3 runs a separate read before create_command/2, so concurrent dispatches can race (both see <2 active commands) and still create more than @max_concurrent_on_demand_mtr. If the goal is to enforce the limit reliably, this needs an atomic guard (e.g., DB transaction + per-agent lock/advisory lock, or a dedicated counter row updated with SELECT ... FOR UPDATE, or another serialization mechanism) around the check+create.

Imported GitHub PR review comment. Original author: @Copilot Original URL: https://github.com/carverauto/serviceradar/pull/3092#discussion_r3003019723 Original created: 2026-03-27T20:19:37Z Original path: elixir/serviceradar_core/lib/serviceradar/edge/agent_command_bus.ex Original line: 270 --- `ensure_dispatch_capacity/3` runs a separate read before `create_command/2`, so concurrent dispatches can race (both see <2 active commands) and still create more than `@max_concurrent_on_demand_mtr`. If the goal is to enforce the limit reliably, this needs an atomic guard (e.g., DB transaction + per-agent lock/advisory lock, or a dedicated counter row updated with `SELECT ... FOR UPDATE`, or another serialization mechanism) around the check+create.

Copilot commented

2026-03-27 20:19:37 +00:00

(Migrated from github.com)

Author

Owner

Imported GitHub PR review comment.

Original author: @Copilot
Original URL: https://github.com/carverauto/serviceradar/pull/3092#discussion_r3003019744
Original created: 2026-03-27T20:19:37Z
Original path: elixir/web-ng/lib/serviceradar_web_ng_web/live/diagnostics_live/mtr.ex
Original line: 217

handle_info/2 now calls schedule_refresh/1 for every {:command_result, %{command_type: "mtr.run"}} received on the global "agent:commands" topic. Since these events are broadcast to all subscribers, this can trigger periodic DB refreshes for all connected MTR diagnostics sessions even when the result is unrelated to the current view’s active command; there’s already a separate :mtr_trace_ingested subscription that refreshes when a trace is written. Consider keeping the refresh conditional (e.g., only when it matches :mtr_command_id) or scoping the PubSub topic/events so unrelated MTR results don’t cause refresh work.

    if active_mtr_command?(socket, command_id) do
      socket =
        socket
        |> assign(:mtr_running, false)
        |> assign(:mtr_command_id, nil)

      {:noreply, schedule_refresh(socket)}
    else
      {:noreply, socket}
    end

Imported GitHub PR review comment. Original author: @Copilot Original URL: https://github.com/carverauto/serviceradar/pull/3092#discussion_r3003019744 Original created: 2026-03-27T20:19:37Z Original path: elixir/web-ng/lib/serviceradar_web_ng_web/live/diagnostics_live/mtr.ex Original line: 217 --- `handle_info/2` now calls `schedule_refresh/1` for *every* `{:command_result, %{command_type: "mtr.run"}}` received on the global "agent:commands" topic. Since these events are broadcast to all subscribers, this can trigger periodic DB refreshes for all connected MTR diagnostics sessions even when the result is unrelated to the current view’s active command; there’s already a separate `:mtr_trace_ingested` subscription that refreshes when a trace is written. Consider keeping the refresh conditional (e.g., only when it matches `:mtr_command_id`) or scoping the PubSub topic/events so unrelated MTR results don’t cause refresh work. ```suggestion if active_mtr_command?(socket, command_id) do socket = socket |> assign(:mtr_running, false) |> assign(:mtr_command_id, nil) {:noreply, schedule_refresh(socket)} else {:noreply, socket} end ```