carverauto/serviceradar

Fork 0

fixing rule builder #2693

Merged

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

2026-01-18 09:13:49 +00:00

mfreeman451 commented

2026-01-18 09:12:46 +00:00

(Migrated from github.com)

Owner

Imported from GitHub pull request.

Original GitHub pull request: #2344
Original author: @mfreeman451
Original URL: https://github.com/carverauto/serviceradar/pull/2344
Original created: 2026-01-18T09:12:46Z
Original updated: 2026-01-18T09:14:18Z
Original head: carverauto/serviceradar:2343-bug-create-event-rule-from-log-is-broken
Original base: staging
Original merged: 2026-01-18T09:13:49Z by @mfreeman451

Label

bug fix

User description

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?

PR Type

Bug fix, Tests

Description

Fix rule builder form data structure from atom keys to string keys
- Prevents runtime errors when prefilling form from log entries
Add comprehensive test for log-to-event rule creation workflow
- Validates rule builder opens and saves with prefilled log data
Document the fix and affected requirements in openspec
- Clarifies the unified rule builder UI specification

Diagram Walkthrough

flowchart LR
  LogEntry["Log Entry"] -- "Create Event Rule" --> RuleBuilder["Rule Builder Component"]
  RuleBuilder -- "Prefill with string keys" --> FormData["Form Data Map"]
  FormData -- "Submit" --> SaveRule["Save Promotion Rule"]
  SaveRule -- "Success" --> RuleCreated["Rule Created"]

File Walkthrough

Relevant files

Bug fix

promotion_rule_builder.ex

Normalize form data keys to strings

web-ng/lib/serviceradar_web_ng_web/components/promotion_rule_builder.ex

Changed map keys in build_form/1 from atom syntax to string keys
Converts all 12 form field keys from :name to "name" format
Ensures compatibility with Phoenix form expectations for prefilled
data

+12/-12

Tests

show_test.exs

Add test for log-to-event rule creation

web-ng/test/serviceradar_web_ng_web/live/log_live/show_test.exs

Added new test "creates promotion rule from log entry" with full
workflow
Tests opening rule builder from log, prefilling fields, and saving
rule
Validates saved rule contains correct match conditions from log data
Added unwrap_page/1 helper function to extract results from Ash
queries

+47/-0

Documentation

proposal.md `Document rule builder fix proposal` openspec/changes/fix-log-event-rule-builder/proposal.md Documents the bug where rule builder crashes with incompatible prefill params Explains the fix normalizes log-derived data for form compatibility Lists impact on observability-rule-management specification	+13/-0
spec.md `Define unified rule builder requirements` openspec/changes/fix-log-event-rule-builder/specs/observability-rule-management/spec.md Adds modified requirement for "Unified Rule Builder UI" Defines three scenarios: create log normalization rule, response rule, and from log entry Specifies that rule builder must render with prefilled fields from log entry	+18/-0
tasks.md `Track implementation tasks` openspec/changes/fix-log-event-rule-builder/tasks.md Lists implementation tasks for the rule builder fix Marks tasks 1.1-1.3 as complete (tracing, normalization, testing) Task 1.4 (smoke testing) remains pending	+5/-0

Imported from GitHub pull request. Original GitHub pull request: #2344 Original author: @mfreeman451 Original URL: https://github.com/carverauto/serviceradar/pull/2344 Original created: 2026-01-18T09:12:46Z Original updated: 2026-01-18T09:14:18Z Original head: carverauto/serviceradar:2343-bug-create-event-rule-from-log-is-broken Original base: staging Original merged: 2026-01-18T09:13:49Z by @mfreeman451 --- ### **Label** bug fix ___ ### **User description** ## 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? ___ ### **PR Type** Bug fix, Tests ___ ### **Description** - Fix rule builder form data structure from atom keys to string keys - Prevents runtime errors when prefilling form from log entries - Add comprehensive test for log-to-event rule creation workflow - Validates rule builder opens and saves with prefilled log data - Document the fix and affected requirements in openspec - Clarifies the unified rule builder UI specification ___ ### Diagram Walkthrough ```mermaid flowchart LR LogEntry["Log Entry"] -- "Create Event Rule" --> RuleBuilder["Rule Builder Component"] RuleBuilder -- "Prefill with string keys" --> FormData["Form Data Map"] FormData -- "Submit" --> SaveRule["Save Promotion Rule"] SaveRule -- "Success" --> RuleCreated["Rule Created"] ``` <details><summary><h3>File Walkthrough</h3></summary> <table><thead><tr><th></th><th align="left">Relevant files</th></tr></thead><tbody><tr><td><strong>Bug fix</strong></td><td><table> <tr> <td> <details> <summary><strong>promotion_rule_builder.ex</strong><dd><code>Normalize form data keys to strings</code>                                            </dd></summary> <hr> web-ng/lib/serviceradar_web_ng_web/components/promotion_rule_builder.ex <ul><li>Changed map keys in <code>build_form/1</code> from atom syntax to string keys<br> <li> Converts all 12 form field keys from <code>:name</code> to <code>"name"</code> format<br> <li> Ensures compatibility with Phoenix form expectations for prefilled <br>data</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2344/files#diff-0226580d3777904915943339ececa4e0e314a03a7c43a0e9afec64fe2a8f9354">+12/-12</a>  </td> </tr> </table></td></tr><tr><td><strong>Tests</strong></td><td><table> <tr> <td> <details> <summary><strong>show_test.exs</strong><dd><code>Add test for log-to-event rule creation</code>                                    </dd></summary> <hr> web-ng/test/serviceradar_web_ng_web/live/log_live/show_test.exs <ul><li>Added new test "creates promotion rule from log entry" with full <br>workflow<br> <li> Tests opening rule builder from log, prefilling fields, and saving <br>rule<br> <li> Validates saved rule contains correct match conditions from log data<br> <li> Added <code>unwrap_page/1</code> helper function to extract results from Ash <br>queries</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2344/files#diff-2d62472b3e212a4da643a2f66d2a07d358795602e680f31249a162b71fa0ea3b">+47/-0</a>    </td> </tr> </table></td></tr><tr><td><strong>Documentation</strong></td><td><table> <tr> <td> <details> <summary><strong>proposal.md</strong><dd><code>Document rule builder fix proposal</code>                                              </dd></summary> <hr> openspec/changes/fix-log-event-rule-builder/proposal.md <ul><li>Documents the bug where rule builder crashes with incompatible prefill <br>params<br> <li> Explains the fix normalizes log-derived data for form compatibility<br> <li> Lists impact on observability-rule-management specification</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2344/files#diff-74a820be5d9b425a3bfcde5e89d11ec8e8f6c14bb5e2bebf834f37bfd124f444">+13/-0</a>    </td> </tr> <tr> <td> <details> <summary><strong>spec.md</strong><dd><code>Define unified rule builder requirements</code>                                  </dd></summary> <hr> openspec/changes/fix-log-event-rule-builder/specs/observability-rule-management/spec.md <ul><li>Adds modified requirement for "Unified Rule Builder UI"<br> <li> Defines three scenarios: create log normalization rule, response rule, <br>and from log entry<br> <li> Specifies that rule builder must render with prefilled fields from log <br>entry</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2344/files#diff-be649af9f6e0709f9e7be63469761c5bc62ce4f83ab3ffe6a023118b0efefb6f">+18/-0</a>    </td> </tr> <tr> <td> <details> <summary><strong>tasks.md</strong><dd><code>Track implementation tasks</code>                                                              </dd></summary> <hr> openspec/changes/fix-log-event-rule-builder/tasks.md <ul><li>Lists implementation tasks for the rule builder fix<br> <li> Marks tasks 1.1-1.3 as complete (tracing, normalization, testing)<br> <li> Task 1.4 (smoke testing) remains pending</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2344/files#diff-01a81039e821ab987ad72641650e4200a79488680a4f275069b8c7afada26122">+5/-0</a>      </td> </tr> </table></td></tr></tbody></table> </details> ___

qodo-code-review[bot] commented

2026-01-18 09:13:21 +00:00

(Migrated from github.com)

Author

Owner

Imported GitHub PR comment.

Original author: @qodo-code-review[bot]
Original URL: https://github.com/carverauto/serviceradar/pull/2344#issuecomment-3765087871
Original created: 2026-01-18T09:13:21Z

ⓘ Your approaching your monthly quota for Qodo. Upgrade your plan

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance

🟢

No security concerns identified

No security vulnerabilities detected by AI analysis. Human verification advised for critical code.

Ticket Compliance

🟡

🎫 #2343

🟢	Ensure prefilled form parameters passed to Phoenix forms use string keys (not atom keys) to avoid Phoenix.HTML.FormData.Map warnings/errors.
🔴	Prevent runtime errors when prefilling the rule builder from log entries (as shown by the `:erlang.not(nil)` / ArgumentError crash in the report).
⚪	Fix "Create Event Rule from log" flow so the rule builder does not crash when opened from a log entry.

Codebase Duplication Compliance

⚪

Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance

🟢

Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

🔴

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status:
Leaky error message: The user-facing :preview_error includes inspect(reason) which may expose internal details
(e.g., stack/struct contents) to the UI instead of a generic message.

Referred Code

socket
|> assign(:preview_state, :error)
|> assign(:preview_error, "Query failed: #{inspect(reason)}")

Learn more about managing compliance generic rules or creating your own custom rules

⚪

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Swallowed query errors: The new unwrap_page/1 helper returns an empty list for any unexpected Ash read result
which can hide underlying query failures instead of surfacing actionable context.

Referred Code

defp unwrap_page({:ok, %Ash.Page.Keyset{results: results}}), do: results
defp unwrap_page({:ok, results}) when is_list(results), do: results
defp unwrap_page(_), do: []

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status:
Unvalidated map passthrough: build_form/1 passes params["parsed_attributes"] directly into form data without
visible validation/sanitization, which may be unsafe depending on how these attributes are
later processed or persisted.

Referred Code

defp build_form(params) do
  data = %{
    "name" => params["name"] || "",
    "body_contains" => params["body_contains"] || "",
    "body_contains_enabled" => to_boolean(params["body_contains_enabled"], false),
    "severity_text" => params["severity_text"] || "",
    "severity_enabled" => to_boolean(params["severity_enabled"], false),
    "service_name" => params["service_name"] || "",
    "service_name_enabled" => to_boolean(params["service_name_enabled"], false),
    "attribute_key" => params["attribute_key"] || "",
    "attribute_value" => params["attribute_value"] || "",
    "attribute_enabled" => to_boolean(params["attribute_enabled"], false),
    "auto_alert" => to_boolean(params["auto_alert"], false),
    "parsed_attributes" => params["parsed_attributes"] || %{}
  }

Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/2344#issuecomment-3765087871 Original created: 2026-01-18T09:13:21Z --- <pre>ⓘ Your approaching your monthly quota for Qodo. <a href="https://www.qodo.ai/pricing">Upgrade your plan</a></pre> ## PR Compliance Guide 🔍  Below is a summary of compliance checks for this PR:<br> <table><tbody><tr><td colspan='2'><strong>Security Compliance</strong></td></tr> <tr><td>🟢</td><td><details><summary><strong>No security concerns identified</strong></summary> No security vulnerabilities detected by AI analysis. Human verification advised for critical code. </details></td></tr> <tr><td colspan='2'><strong>Ticket Compliance</strong></td></tr> <tr><td>🟡</td> <td> <details> <summary>🎫 <a href=https://github.com/carverauto/serviceradar/issues/2343>#2343</a></summary> <table width='100%'><tbody> <tr><td rowspan=1>🟢</td> <td>Ensure prefilled form parameters passed to Phoenix forms use string keys (not atom keys) <br>to avoid Phoenix.HTML.FormData.Map warnings/errors.<br></td></tr> <tr><td rowspan=1>🔴</td> <td>Prevent runtime errors when prefilling the rule builder from log entries (as shown by the <br><code>:erlang.not(nil)</code> / ArgumentError crash in the report).<br></td></tr> <tr><td rowspan=1>⚪</td> <td>Fix "Create Event Rule from log" flow so the rule builder does not crash when opened from <br>a log entry.<br></td></tr> </tbody></table> </details> </td></tr> <tr><td colspan='2'><strong>Codebase Duplication Compliance</strong></td></tr> <tr><td>⚪</td><td><details><summary><strong>Codebase context is not defined </strong></summary> Follow the <a href='https://qodo-merge-docs.qodo.ai/core-abilities/rag_context_enrichment/'>guide</a> to enable codebase context checks. </details></td></tr> <tr><td colspan='2'><strong>Custom Compliance</strong></td></tr> <tr><td rowspan=3>🟢</td><td> <details><summary><strong>Generic: Comprehensive Audit Trails</strong></summary><br> **Objective:** To create a detailed and reliable record of critical system actions for security analysis <br>and compliance.<br> **Status:** Passed<br> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td> <details><summary><strong>Generic: Meaningful Naming and Self-Documenting Code</strong></summary><br> **Objective:** Ensure all identifiers clearly express their purpose and intent, making code <br>self-documenting<br> **Status:** Passed<br> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td> <details><summary><strong>Generic: Secure Logging Practices</strong></summary><br> **Objective:** To ensure logs are useful for debugging and auditing without exposing sensitive <br>information like PII, PHI, or cardholder data.<br> **Status:** Passed<br> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td rowspan=1>🔴</td> <td><details> <summary><strong>Generic: Secure Error Handling</strong></summary><br> **Objective:** To prevent the leakage of sensitive system information through error messages while <br>providing sufficient detail for internal debugging.<br> **Status:** <br><a href='https://github.com/carverauto/serviceradar/pull/2344/files#diff-0226580d3777904915943339ececa4e0e314a03a7c43a0e9afec64fe2a8f9354R536-R538'><strong>Leaky error message</strong></a>: The user-facing <code>:preview_error</code> includes <code>inspect(reason)</code> which may expose internal details <br>(e.g., stack/struct contents) to the UI instead of a generic message.<br> <details open><summary>Referred Code</summary> ```elixir socket |> assign(:preview_state, :error) |> assign(:preview_error, "Query failed: #{inspect(reason)}") ``` </details> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td rowspan=2>⚪</td> <td><details> <summary><strong>Generic: Robust Error Handling and Edge Case Management</strong></summary><br> **Objective:** Ensure comprehensive error handling that provides meaningful context and graceful <br>degradation<br> **Status:** <br><a href='https://github.com/carverauto/serviceradar/pull/2344/files#diff-2d62472b3e212a4da643a2f66d2a07d358795602e680f31249a162b71fa0ea3bR228-R230'><strong>Swallowed query errors</strong></a>: The new <code>unwrap_page/1</code> helper returns an empty list for any unexpected Ash read result <br>which can hide underlying query failures instead of surfacing actionable context.<br> <details open><summary>Referred Code</summary> ```elixir defp unwrap_page({:ok, %Ash.Page.Keyset{results: results}}), do: results defp unwrap_page({:ok, results}) when is_list(results), do: results defp unwrap_page(_), do: [] ``` </details> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td><details> <summary><strong>Generic: Security-First Input Validation and Data Handling</strong></summary><br> **Objective:** Ensure all data inputs are validated, sanitized, and handled securely to prevent <br>vulnerabilities<br> **Status:** <br><a href='https://github.com/carverauto/serviceradar/pull/2344/files#diff-0226580d3777904915943339ececa4e0e314a03a7c43a0e9afec64fe2a8f9354R542-R556'><strong>Unvalidated map passthrough</strong></a>: <code>build_form/1</code> passes <code>params["parsed_attributes"]</code> directly into form data without <br>visible validation/sanitization, which may be unsafe depending on how these attributes are <br>later processed or persisted.<br> <details open><summary>Referred Code</summary> ```elixir defp build_form(params) do data = %{ "name" => params["name"] || "", "body_contains" => params["body_contains"] || "", "body_contains_enabled" => to_boolean(params["body_contains_enabled"], false), "severity_text" => params["severity_text"] || "", "severity_enabled" => to_boolean(params["severity_enabled"], false), "service_name" => params["service_name"] || "", "service_name_enabled" => to_boolean(params["service_name_enabled"], false), "attribute_key" => params["attribute_key"] || "", "attribute_value" => params["attribute_value"] || "", "attribute_enabled" => to_boolean(params["attribute_enabled"], false), "auto_alert" => to_boolean(params["auto_alert"], false), "parsed_attributes" => params["parsed_attributes"] || %{} } ``` </details> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td align="center" colspan="2">   </td></tr></tbody></table> <details><summary>Compliance status legend</summary> 🟢 - Fully Compliant<br> 🟡 - Partial Compliant<br> 🔴 - Not Compliant<br> ⚪ - Requires Further Human Verification<br> 🏷️ - Compliance label<br> </details>

qodo-code-review[bot] commented

2026-01-18 09:13:38 +00:00

(Migrated from github.com)

Author

Owner

Imported GitHub PR comment.

Original author: @qodo-code-review[bot]
Original URL: https://github.com/carverauto/serviceradar/pull/2344#issuecomment-3765088067
Original created: 2026-01-18T09:13:38Z

ⓘ Your approaching your monthly quota for Qodo. Upgrade your plan

CI Feedback 🧐

A test triggered by this PR failed. Here is an AI-generated analysis of the failure:

Action: build

Failed stage: Configure SRQL fixture database for tests [❌]

Failed test name: ""

Failure summary:

The action failed because a required secret/environment variable for TLS verification was missing:
-
The workflow explicitly checks for SRQL_TEST_DATABASE_CA_CERT and aborted with:
SRQL_TEST_DATABASE_CA_CERT secret must be configured to verify SRQL fixture TLS. (log line 636).
-
SRQL_TEST_DATABASE_CA_CERT is shown as empty in the environment (log lines 321, 477, 540, 626),
causing the step to exit with code 1 (log line 637).

Relevant error logs:

1:  Runner name: 'arc-runner-set-hk6mk-runner-k7kjz'
2:  Runner group name: 'Default'
...

139:  ^[[36;1mif command -v apt-get >/dev/null 2>&1; then^[[0m
140:  ^[[36;1m  sudo apt-get update^[[0m
141:  ^[[36;1m  sudo apt-get install -y build-essential pkg-config libssl-dev protobuf-compiler cmake flex bison^[[0m
142:  ^[[36;1melif command -v dnf >/dev/null 2>&1; then^[[0m
143:  ^[[36;1m  sudo dnf install -y gcc gcc-c++ make openssl-devel protobuf-compiler cmake flex bison^[[0m
144:  ^[[36;1melif command -v yum >/dev/null 2>&1; then^[[0m
145:  ^[[36;1m  sudo yum install -y gcc gcc-c++ make openssl-devel protobuf-compiler cmake flex bison^[[0m
146:  ^[[36;1melif command -v microdnf >/dev/null 2>&1; then^[[0m
147:  ^[[36;1m  sudo microdnf install -y gcc gcc-c++ make openssl-devel protobuf-compiler cmake flex bison^[[0m
148:  ^[[36;1melse^[[0m
149:  ^[[36;1m  echo "Unsupported package manager; please install gcc, g++ (or clang), make, OpenSSL headers, pkg-config, and protoc manually." >&2^[[0m
150:  ^[[36;1m  exit 1^[[0m
151:  ^[[36;1mfi^[[0m
152:  ^[[36;1m^[[0m
153:  ^[[36;1mensure_pkg_config^[[0m
154:  ^[[36;1mprotoc --version || (echo "protoc installation failed" && exit 1)^[[0m
155:  shell: /usr/bin/bash -e {0}
...

316:  shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}
317:  env:
318:  BUILDBUDDY_ORG_API_KEY: ***
319:  SRQL_TEST_DATABASE_URL: ***
320:  SRQL_TEST_ADMIN_URL: ***
321:  SRQL_TEST_DATABASE_CA_CERT: 
322:  DOCKERHUB_USERNAME: ***
323:  DOCKERHUB_TOKEN: ***
324:  TEST_CNPG_DATABASE: serviceradar_web_ng_test
325:  INSTALL_DIR_FOR_OTP: /home/runner/_work/_temp/.setup-beam/otp
326:  INSTALL_DIR_FOR_ELIXIR: /home/runner/_work/_temp/.setup-beam/elixir
327:  ##[endgroup]
328:  ##[group]Run : install rustup if needed
329:  ^[[36;1m: install rustup if needed^[[0m
330:  ^[[36;1mif ! command -v rustup &>/dev/null; then^[[0m
331:  ^[[36;1m  curl --proto '=https' --tlsv1.2 --retry 10 --retry-connrefused --location --silent --show-error --fail https://sh.rustup.rs | sh -s -- --default-toolchain none -y^[[0m
332:  ^[[36;1m  echo "$CARGO_HOME/bin" >> $GITHUB_PATH^[[0m
...

472:  shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}
473:  env:
474:  BUILDBUDDY_ORG_API_KEY: ***
475:  SRQL_TEST_DATABASE_URL: ***
476:  SRQL_TEST_ADMIN_URL: ***
477:  SRQL_TEST_DATABASE_CA_CERT: 
478:  DOCKERHUB_USERNAME: ***
479:  DOCKERHUB_TOKEN: ***
480:  TEST_CNPG_DATABASE: serviceradar_web_ng_test
481:  INSTALL_DIR_FOR_OTP: /home/runner/_work/_temp/.setup-beam/otp
482:  INSTALL_DIR_FOR_ELIXIR: /home/runner/_work/_temp/.setup-beam/elixir
483:  CARGO_HOME: /home/runner/.cargo
484:  CARGO_INCREMENTAL: 0
485:  CARGO_TERM_COLOR: always
486:  ##[endgroup]
487:  ##[group]Run : work around spurious network errors in curl 8.0
488:  ^[[36;1m: work around spurious network errors in curl 8.0^[[0m
489:  ^[[36;1m# https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/timeout.20investigation^[[0m
...

540:  SRQL_TEST_DATABASE_CA_CERT: 
541:  DOCKERHUB_USERNAME: ***
542:  DOCKERHUB_TOKEN: ***
543:  TEST_CNPG_DATABASE: serviceradar_web_ng_test
544:  INSTALL_DIR_FOR_OTP: /home/runner/_work/_temp/.setup-beam/otp
545:  INSTALL_DIR_FOR_ELIXIR: /home/runner/_work/_temp/.setup-beam/elixir
546:  CARGO_HOME: /home/runner/.cargo
547:  CARGO_INCREMENTAL: 0
548:  CARGO_TERM_COLOR: always
549:  ##[endgroup]
550:  Attempting to download 1.x...
551:  Acquiring v1.28.0 from https://github.com/bazelbuild/bazelisk/releases/download/v1.28.0/bazelisk-linux-amd64
552:  Adding to the cache ...
553:  Successfully cached bazelisk to /home/runner/_work/_tool/bazelisk/1.28.0/x64
554:  Added bazelisk to the path
555:  ##[warning]Failed to restore: Cache service responded with 400
556:  Restored bazelisk cache dir @ /home/runner/.cache/bazelisk
...

622:  env:
623:  BUILDBUDDY_ORG_API_KEY: ***
624:  SRQL_TEST_DATABASE_URL: ***
625:  SRQL_TEST_ADMIN_URL: ***
626:  SRQL_TEST_DATABASE_CA_CERT: 
627:  DOCKERHUB_USERNAME: ***
628:  DOCKERHUB_TOKEN: ***
629:  TEST_CNPG_DATABASE: serviceradar_web_ng_test
630:  INSTALL_DIR_FOR_OTP: /home/runner/_work/_temp/.setup-beam/otp
631:  INSTALL_DIR_FOR_ELIXIR: /home/runner/_work/_temp/.setup-beam/elixir
632:  CARGO_HOME: /home/runner/.cargo
633:  CARGO_INCREMENTAL: 0
634:  CARGO_TERM_COLOR: always
635:  ##[endgroup]
636:  SRQL_TEST_DATABASE_CA_CERT secret must be configured to verify SRQL fixture TLS.
637:  ##[error]Process completed with exit code 1.
638:  Post job cleanup.

Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/2344#issuecomment-3765088067 Original created: 2026-01-18T09:13:38Z --- <pre>ⓘ Your approaching your monthly quota for Qodo. <a href="https://www.qodo.ai/pricing">Upgrade your plan</a></pre> ## CI Feedback 🧐 A test triggered by this PR failed. Here is an AI-generated analysis of the failure: <table><tr><td> **Action:** build</td></tr> <tr><td> **Failed stage:** [Configure SRQL fixture database for tests](https://github.com/carverauto/serviceradar/actions/runs/21109302356/job/60705463213) [❌] </td></tr> <tr><td> **Failed test name:** "" </td></tr> <tr><td> **Failure summary:** The action failed because a required secret/environment variable for TLS verification was missing:<br> - <br>The workflow explicitly checks for <code>SRQL_TEST_DATABASE_CA_CERT</code> and aborted with: <br><code>SRQL_TEST_DATABASE_CA_CERT secret must be configured to verify SRQL fixture TLS.</code> (log line 636).<br> - <br><code>SRQL_TEST_DATABASE_CA_CERT</code> is shown as empty in the environment (log lines 321, 477, 540, 626), <br>causing the step to exit with code 1 (log line 637).<br> </td></tr> <tr><td> <details><summary>Relevant error logs:</summary> ```yaml 1: Runner name: 'arc-runner-set-hk6mk-runner-k7kjz' 2: Runner group name: 'Default' ... 139: ^[[36;1mif command -v apt-get >/dev/null 2>&1; then^[[0m 140: ^[[36;1m sudo apt-get update^[[0m 141: ^[[36;1m sudo apt-get install -y build-essential pkg-config libssl-dev protobuf-compiler cmake flex bison^[[0m 142: ^[[36;1melif command -v dnf >/dev/null 2>&1; then^[[0m 143: ^[[36;1m sudo dnf install -y gcc gcc-c++ make openssl-devel protobuf-compiler cmake flex bison^[[0m 144: ^[[36;1melif command -v yum >/dev/null 2>&1; then^[[0m 145: ^[[36;1m sudo yum install -y gcc gcc-c++ make openssl-devel protobuf-compiler cmake flex bison^[[0m 146: ^[[36;1melif command -v microdnf >/dev/null 2>&1; then^[[0m 147: ^[[36;1m sudo microdnf install -y gcc gcc-c++ make openssl-devel protobuf-compiler cmake flex bison^[[0m 148: ^[[36;1melse^[[0m 149: ^[[36;1m echo "Unsupported package manager; please install gcc, g++ (or clang), make, OpenSSL headers, pkg-config, and protoc manually." >&2^[[0m 150: ^[[36;1m exit 1^[[0m 151: ^[[36;1mfi^[[0m 152: ^[[36;1m^[[0m 153: ^[[36;1mensure_pkg_config^[[0m 154: ^[[36;1mprotoc --version || (echo "protoc installation failed" && exit 1)^[[0m 155: shell: /usr/bin/bash -e {0} ... 316: shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0} 317: env: 318: BUILDBUDDY_ORG_API_KEY: *** 319: SRQL_TEST_DATABASE_URL: *** 320: SRQL_TEST_ADMIN_URL: *** 321: SRQL_TEST_DATABASE_CA_CERT: 322: DOCKERHUB_USERNAME: *** 323: DOCKERHUB_TOKEN: *** 324: TEST_CNPG_DATABASE: serviceradar_web_ng_test 325: INSTALL_DIR_FOR_OTP: /home/runner/_work/_temp/.setup-beam/otp 326: INSTALL_DIR_FOR_ELIXIR: /home/runner/_work/_temp/.setup-beam/elixir 327: ##[endgroup] 328: ##[group]Run : install rustup if needed 329: ^[[36;1m: install rustup if needed^[[0m 330: ^[[36;1mif ! command -v rustup &>/dev/null; then^[[0m 331: ^[[36;1m curl --proto '=https' --tlsv1.2 --retry 10 --retry-connrefused --location --silent --show-error --fail https://sh.rustup.rs | sh -s -- --default-toolchain none -y^[[0m 332: ^[[36;1m echo "$CARGO_HOME/bin" >> $GITHUB_PATH^[[0m ... 472: shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0} 473: env: 474: BUILDBUDDY_ORG_API_KEY: *** 475: SRQL_TEST_DATABASE_URL: *** 476: SRQL_TEST_ADMIN_URL: *** 477: SRQL_TEST_DATABASE_CA_CERT: 478: DOCKERHUB_USERNAME: *** 479: DOCKERHUB_TOKEN: *** 480: TEST_CNPG_DATABASE: serviceradar_web_ng_test 481: INSTALL_DIR_FOR_OTP: /home/runner/_work/_temp/.setup-beam/otp 482: INSTALL_DIR_FOR_ELIXIR: /home/runner/_work/_temp/.setup-beam/elixir 483: CARGO_HOME: /home/runner/.cargo 484: CARGO_INCREMENTAL: 0 485: CARGO_TERM_COLOR: always 486: ##[endgroup] 487: ##[group]Run : work around spurious network errors in curl 8.0 488: ^[[36;1m: work around spurious network errors in curl 8.0^[[0m 489: ^[[36;1m# https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/timeout.20investigation^[[0m ... 540: SRQL_TEST_DATABASE_CA_CERT: 541: DOCKERHUB_USERNAME: *** 542: DOCKERHUB_TOKEN: *** 543: TEST_CNPG_DATABASE: serviceradar_web_ng_test 544: INSTALL_DIR_FOR_OTP: /home/runner/_work/_temp/.setup-beam/otp 545: INSTALL_DIR_FOR_ELIXIR: /home/runner/_work/_temp/.setup-beam/elixir 546: CARGO_HOME: /home/runner/.cargo 547: CARGO_INCREMENTAL: 0 548: CARGO_TERM_COLOR: always 549: ##[endgroup] 550: Attempting to download 1.x... 551: Acquiring v1.28.0 from https://github.com/bazelbuild/bazelisk/releases/download/v1.28.0/bazelisk-linux-amd64 552: Adding to the cache ... 553: Successfully cached bazelisk to /home/runner/_work/_tool/bazelisk/1.28.0/x64 554: Added bazelisk to the path 555: ##[warning]Failed to restore: Cache service responded with 400 556: Restored bazelisk cache dir @ /home/runner/.cache/bazelisk ... 622: env: 623: BUILDBUDDY_ORG_API_KEY: *** 624: SRQL_TEST_DATABASE_URL: *** 625: SRQL_TEST_ADMIN_URL: *** 626: SRQL_TEST_DATABASE_CA_CERT: 627: DOCKERHUB_USERNAME: *** 628: DOCKERHUB_TOKEN: *** 629: TEST_CNPG_DATABASE: serviceradar_web_ng_test 630: INSTALL_DIR_FOR_OTP: /home/runner/_work/_temp/.setup-beam/otp 631: INSTALL_DIR_FOR_ELIXIR: /home/runner/_work/_temp/.setup-beam/elixir 632: CARGO_HOME: /home/runner/.cargo 633: CARGO_INCREMENTAL: 0 634: CARGO_TERM_COLOR: always 635: ##[endgroup] 636: SRQL_TEST_DATABASE_CA_CERT secret must be configured to verify SRQL fixture TLS. 637: ##[error]Process completed with exit code 1. 638: Post job cleanup. ``` </details></td></tr></table>

qodo-code-review[bot] commented

2026-01-18 09:14:18 +00:00

(Migrated from github.com)

Author

Owner

Imported GitHub PR comment.

Original author: @qodo-code-review[bot]
Original URL: https://github.com/carverauto/serviceradar/pull/2344#issuecomment-3765088591
Original created: 2026-01-18T09:14:18Z

ⓘ Your approaching your monthly quota for Qodo. Upgrade your plan

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
General	Stringify parsed_attributes keys In `build_form/1`, ensure all keys within the `parsed_attributes` map are converted to strings. This prevents potential issues with form data binding if the original map contains atom keys. web-ng/lib/serviceradar_web_ng_web/components/promotion_rule_builder.ex [555] `-"parsed_attributes" => params["parsed_attributes"] \|\| %{} +"parsed_attributes" => + (params["parsed_attributes"] \|\| %{}) + \|> Enum.into(%{}, fn {k, v} -> {to_string(k), v} end)` Apply / Chat Suggestion importance[1-10]: 7 __ Why: This suggestion correctly identifies a potential bug where nested atom keys in `parsed_attributes` would not be stringified, contrary to the PR's goal. Applying this change proactively prevents a likely runtime error and aligns the code with the PR's intent to normalize all keys to strings for form binding.	Medium
General	Query for specific test resource Refactor the test to query for the specific rule by its unique name instead of fetching all rules and using `Enum.find`. This makes the test more efficient and its assertions more precise. web-ng/test/serviceradar_web_ng_web/live/log_live/show_test.exs [160-162] `-rules = unwrap_page(Ash.read(ServiceRadar.Observability.LogPromotionRule, scope: scope)) -rule = Enum.find(rules, &(&1.name == rule_name)) +{:ok, [rule]} = Ash.read(ServiceRadar.Observability.LogPromotionRule, filter: [name: ^rule_name], scope: scope) assert rule` Apply / Chat Suggestion importance[1-10]: 5 __ Why: The suggestion correctly points out a more efficient and precise way to fetch the created resource in the test. Using a direct query with a filter is better than fetching all resources and filtering in memory, improving test performance and robustness.	Low
Possible issue	Avoid silently swallowing test errors Modify the `unwrap_page/1` test helper to fail explicitly on error cases from `Ash.read/2` instead of silently returning an empty list. Remove the catch-all clause to allow pattern matching errors on failure, improving test debuggability. web-ng/test/serviceradar_web_ng_web/live/log_live/show_test.exs [228-230] `defp unwrap_page({:ok, %Ash.Page.Keyset{results: results}}), do: results defp unwrap_page({:ok, results}) when is_list(results), do: results -defp unwrap_page(_), do: []` Apply / Chat Suggestion importance[1-10]: 6 __ Why: This is a valid suggestion that improves test robustness by preventing silent failures. Removing the catch-all clause in `unwrap_page/1` will make test failures more explicit and easier to debug, which is a good practice for test helpers.	Low
More

Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/2344#issuecomment-3765088591 Original created: 2026-01-18T09:14:18Z --- <pre>ⓘ Your approaching your monthly quota for Qodo. <a href="https://www.qodo.ai/pricing">Upgrade your plan</a></pre> ## PR Code Suggestions ✨  Explore these optional code suggestions: <table><thead><tr><td><strong>Category</strong></td><td align=left><strong>Suggestion                                                                                                                                    </strong></td><td align=center><strong>Impact</strong></td></tr><tbody><tr><td rowspan=2>General</td> <td> <details><summary>Stringify parsed_attributes keys</summary> ___ **In <code>build_form/1</code>, ensure all keys within the <code>parsed_attributes</code> map are converted <br>to strings. This prevents potential issues with form data binding if the <br>original map contains atom keys.** [web-ng/lib/serviceradar_web_ng_web/components/promotion_rule_builder.ex [555]](https://github.com/carverauto/serviceradar/pull/2344/files#diff-0226580d3777904915943339ececa4e0e314a03a7c43a0e9afec64fe2a8f9354R555-R555) ```diff -"parsed_attributes" => params["parsed_attributes"] || %{} +"parsed_attributes" => + (params["parsed_attributes"] || %{}) + |> Enum.into(%{}, fn {k, v} -> {to_string(k), v} end) ``` - [ ] **Apply / Chat**  <details><summary>Suggestion importance[1-10]: 7</summary> __ Why: This suggestion correctly identifies a potential bug where nested atom keys in `parsed_attributes` would not be stringified, contrary to the PR's goal. Applying this change proactively prevents a likely runtime error and aligns the code with the PR's intent to normalize all keys to strings for form binding. </details></details></td><td align=center>Medium </td></tr><tr><td> <details><summary>Query for specific test resource</summary> ___ **Refactor the test to query for the specific rule by its unique name instead of <br>fetching all rules and using <code>Enum.find</code>. This makes the test more efficient and <br>its assertions more precise.** [web-ng/test/serviceradar_web_ng_web/live/log_live/show_test.exs [160-162]](https://github.com/carverauto/serviceradar/pull/2344/files#diff-2d62472b3e212a4da643a2f66d2a07d358795602e680f31249a162b71fa0ea3bR160-R162) ```diff -rules = unwrap_page(Ash.read(ServiceRadar.Observability.LogPromotionRule, scope: scope)) -rule = Enum.find(rules, &(&1.name == rule_name)) +{:ok, [rule]} = Ash.read(ServiceRadar.Observability.LogPromotionRule, filter: [name: ^rule_name], scope: scope) assert rule ``` - [ ] **Apply / Chat**  <details><summary>Suggestion importance[1-10]: 5</summary> __ Why: The suggestion correctly points out a more efficient and precise way to fetch the created resource in the test. Using a direct query with a filter is better than fetching all resources and filtering in memory, improving test performance and robustness. </details></details></td><td align=center>Low </td></tr><tr><td rowspan=1>Possible issue</td> <td> <details><summary>Avoid silently swallowing test errors</summary> ___ **Modify the <code>unwrap_page/1</code> test helper to fail explicitly on error cases from <br><code>Ash.read/2</code> instead of silently returning an empty list. Remove the catch-all <br>clause to allow pattern matching errors on failure, improving test <br>debuggability.** [web-ng/test/serviceradar_web_ng_web/live/log_live/show_test.exs [228-230]](https://github.com/carverauto/serviceradar/pull/2344/files#diff-2d62472b3e212a4da643a2f66d2a07d358795602e680f31249a162b71fa0ea3bR228-R230) ```diff defp unwrap_page({:ok, %Ash.Page.Keyset{results: results}}), do: results defp unwrap_page({:ok, results}) when is_list(results), do: results -defp unwrap_page(_), do: [] ``` - [ ] **Apply / Chat**  <details><summary>Suggestion importance[1-10]: 6</summary> __ Why: This is a valid suggestion that improves test robustness by preventing silent failures. Removing the catch-all clause in `unwrap_page/1` will make test failures more explicit and easier to debug, which is a good practice for test helpers. </details></details></td><td align=center>Low </td></tr> <tr><td align="center" colspan="2"> - [ ] More  </td><td></td></tr></tbody></table>