Skip to content

feat: add functions to collect and apply restart fields for various configurations#140

Merged
yacosta738 merged 1 commit into
mainfrom
cqa
Mar 4, 2026
Merged

feat: add functions to collect and apply restart fields for various configurations#140
yacosta738 merged 1 commit into
mainfrom
cqa

Conversation

@yacosta738
Copy link
Copy Markdown
Contributor

This pull request refactors and modularizes the logic for determining and applying configuration changes that require a restart in the agent-runtime gateway admin code. The main improvements are the extraction of logic into themed helper functions, which enhances maintainability and readability. Additionally, there are minor consistency updates to shell scripts and workflow files.

Refactoring and modularization of configuration update logic:

  • The restart_required_updates function in clients/agent-runtime/src/gateway/admin.rs is refactored to delegate to new helper functions, each responsible for a specific configuration theme (core, runtime/identity, scheduler/gateway, webhook, secrets). This makes the code more organized and easier to extend. [1] [2] [3] [4] [5] [6] [7] [8]
  • The apply_patch function is similarly split into themed helper functions for applying patches to each configuration area, improving clarity and separation of concerns. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11]

Shell script consistency improvements:

  • All if [ ... ] conditional checks in .agents/skills/android-expert/scripts/analyze-apk-size.sh are updated to use the more modern and consistent [[ ... ]] syntax. This improves script robustness and readability. [1] [2] [3] [4] [5] [6] [7] [8] [9]

  • In .agents/skills/web-quality-audit/scripts/analyze.sh, the usage function now returns instead of exiting, and an explicit exit 1 is added after its call, improving script flow and error handling.

Workflow pinning for reproducibility:

  • The GitHub Actions workflow .github/workflows/sonarqube-analysis.yml now pins the Rust toolchain and cargo-llvm-cov install actions to specific commit SHAs, ensuring reproducible builds and improved security.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 4, 2026

📝 Walkthrough

Summary by CodeRabbit

  • Refactor

    • Improved code organization and maintainability of internal configuration management logic through better separation of concerns.
  • Chores

    • Pinned GitHub Actions tool versions to specific commits for improved build stability and reproducibility.
    • Updated internal shell script syntax for enhanced compatibility.

Walkthrough

The PR upgrades bash conditional syntax across shell scripts, pins GitHub Actions tool versions to specific commits, adjusts script control flow in web-quality-audit, and refactors gateway admin logic by extracting field collection and patch application into modular helper functions.

Changes

Cohort / File(s) Summary
Shell Script Syntax Upgrades
.agents/skills/android-expert/scripts/analyze-apk-size.sh, .agents/skills/web-quality-audit/scripts/analyze.sh
Converted POSIX test expressions [ ... ] to bash extended test syntax [[ ... ]] for enhanced conditional capabilities. Also adjusted analyze.sh control flow so usage returns a status instead of exiting directly, with explicit exit in caller.
CI Workflow Pinning
.github/workflows/sonarqube-analysis.yml
Pinned Rust toolchain and cargo-llvm-cov installation to specific commits (as of 2026-03-04) instead of generic HEAD references.
Gateway Admin Refactor
clients/agent-runtime/src/gateway/admin.rs
Decomposed restart_required_updates logic into five new helper functions for field collection (collect_core/runtime_identity/scheduler_gateway/webhook/secret_restart_fields) and refactored apply_patch into per-area handlers (apply_core/runtime_identity/scheduler_gateway/channels/integrations/memory_patch). Observable behavior preserved; logic now modular.
Gateway Module Formatting
clients/agent-runtime/src/gateway/mod.rs
Reformatted chained method call from single-line to multi-line. No functional changes.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Suggested labels

area:rust, area:ci, risk:security, risk:high

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Title check ⚠️ Warning Title exceeds the 72-character limit at 82 characters, violating the Conventional Commit style requirement specified in title_check_requirements. Shorten the title to 72 characters or fewer, e.g.: 'refactor: modularize restart field collection and patch application logic'.
Docstring Coverage ⚠️ Warning Docstring coverage is 13.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (1 passed)
Check name Status Explanation
Description check ✅ Passed Description covers all required sections (Summary, Tested Information implied via checklist, Breaking Changes, and Checklist), providing clear context for the refactoring and supporting changes across multiple files.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch cqa

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 4, 2026

✅ Contributor Report

User: @yacosta738
Status: Passed (12/13 metrics passed)

Metric Description Value Threshold Status
PR Merge Rate PRs merged vs closed 88% >= 30%
Repo Quality Repos with ≥100 stars 0 >= 0
Positive Reactions Positive reactions received 9 >= 1
Negative Reactions Negative reactions received 0 <= 5
Account Age GitHub account age 3050 days >= 30 days
Activity Consistency Regular activity over time 108% >= 0%
Issue Engagement Issues with community engagement 0 >= 0
Code Reviews Code reviews given to others 383 >= 0
Merger Diversity Unique maintainers who merged PRs 2 >= 0
Repo History Merge Rate Merge rate in this repo 91% >= 0%
Repo History Min PRs Previous PRs in this repo 108 >= 0
Profile Completeness Profile richness (bio, followers) 90 >= 0
Suspicious Patterns Spam-like activity detection 1 N/A

Contributor Report evaluates based on public GitHub activity. Analysis period: 2025-03-04 to 2026-03-04

@sentry
Copy link
Copy Markdown

sentry Bot commented Mar 4, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented Mar 4, 2026

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
clients/agent-runtime/src/gateway/admin.rs (1)

668-677: ⚠️ Potential issue | 🟠 Major

Webhook alias handling is inconsistent between restart detection and application.

collect_webhook_restart_fields inspects only one source (channels.webhook OR top-level webhook), but apply_channels_patch applies both. A request containing both can hide restart-required changes or apply conflicting operations.

🔧 Suggested fix
 fn collect_webhook_restart_fields(...) {
-    let channel_webhook = patch
-        .channels
-        .as_ref()
-        .and_then(|channels| channels.webhook.as_ref())
-        .or(patch.webhook.as_ref());
-    if let Some(webhook) = channel_webhook {
+    for webhook in [
+        patch.channels.as_ref().and_then(|channels| channels.webhook.as_ref()),
+        patch.webhook.as_ref(),
+    ]
+    .into_iter()
+    .flatten()
+    {
         ...
     }
 }
 
 fn apply_channels_patch(...) -> Result<(), AdminResponse> {
+    if patch.channels.as_ref().and_then(|c| c.webhook.as_ref()).is_some() && patch.webhook.is_some() {
+        return Err(bad_request("Specify either channels.webhook or webhook, not both"));
+    }
     ...
 }

Based on learnings: Applies to clients/agent-runtime/src/{security,gateway,tools}/**/*.rs : Treat src/security/, src/gateway/, src/tools/ as high-risk surfaces and never broaden filesystem/network execution scope without explicit policy checks.

Also applies to: 971-987

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@clients/agent-runtime/src/gateway/admin.rs` around lines 668 - 677,
collect_webhook_restart_fields currently only examines channels.webhook OR
top-level webhook, but apply_channels_patch uses both; update
collect_webhook_restart_fields to evaluate both sources (channels.webhook and
patch.webhook) individually and mark restart-required if either source changes
or if both are present and differ (treating that as a conflict), matching the
logic in apply_channels_patch; locate and modify the function
collect_webhook_restart_fields and mirror apply_channels_patch's resolution
rules so restart detection is consistent with application, and add explicit
conflict detection between channels.webhook and top-level webhook to ensure
unexpected combined values trigger a restart or error.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.agents/skills/web-quality-audit/scripts/analyze.sh:
- Around line 10-15: The usage function currently does "return 1" and later the
script calls usage followed by "exit 1", but with "set -e" the "return 1" causes
the script to exit immediately making "exit 1" unreachable; pick one approach
and make it consistent: either remove the "return 1" from the usage function and
keep the explicit "exit 1" after the usage call, or remove the trailing "exit 1"
and let "set -e" propagate the non-zero return from usage—update the usage
function and the conditional that calls it accordingly (referencing the usage
function and the if [[ -z "$1" ]] check).

In @.github/workflows/sonarqube-analysis.yml:
- Around line 78-80: The workflow currently uses the
taiki-e/install-action@2834d6555cea49e0759c35c2a877ea0228e73e06 invocation to
install cargo-llvm-cov but does not pin the tool version; update the step that
uses taiki-e/install-action (the "📦 Install cargo-llvm-cov" step) to add a
with: block specifying the desired cargo-llvm-cov version (e.g., version:
'0.6.14' or '0.6') so the installed tool is deterministic across runs.

In `@clients/agent-runtime/src/gateway/admin.rs`:
- Around line 537-541: The patch application is allowing updates to
runtime-bound fields without marking a restart required; ensure parity between
apply_patch and restart detection by adding the same fields to
restart_required_updates that apply_patch mutates: include observability.* and
autonomy.* keys, gateway.trust_forwarded_headers, gateway.rate_limit_max_keys,
all gateway.idempotency_* keys, and memory.backend; update the logic that builds
fields (see collect_core_restart_fields,
collect_runtime_identity_restart_fields,
collect_scheduler_gateway_restart_fields, collect_webhook_restart_fields,
collect_secret_restart_fields) or the restart_required_updates predicate so
these mutations trigger a restart and preserve secure-by-default behavior.

---

Outside diff comments:
In `@clients/agent-runtime/src/gateway/admin.rs`:
- Around line 668-677: collect_webhook_restart_fields currently only examines
channels.webhook OR top-level webhook, but apply_channels_patch uses both;
update collect_webhook_restart_fields to evaluate both sources (channels.webhook
and patch.webhook) individually and mark restart-required if either source
changes or if both are present and differ (treating that as a conflict),
matching the logic in apply_channels_patch; locate and modify the function
collect_webhook_restart_fields and mirror apply_channels_patch's resolution
rules so restart detection is consistent with application, and add explicit
conflict detection between channels.webhook and top-level webhook to ensure
unexpected combined values trigger a restart or error.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: e46db93a-211b-4981-b512-bf2eaa13f549

📥 Commits

Reviewing files that changed from the base of the PR and between d052e25 and 57b2087.

📒 Files selected for processing (5)
  • .agents/skills/android-expert/scripts/analyze-apk-size.sh
  • .agents/skills/web-quality-audit/scripts/analyze.sh
  • .github/workflows/sonarqube-analysis.yml
  • clients/agent-runtime/src/gateway/admin.rs
  • clients/agent-runtime/src/gateway/mod.rs
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: sonar
  • GitHub Check: pr-checks
  • GitHub Check: pr-checks
  • GitHub Check: Cloudflare Pages
🧰 Additional context used
📓 Path-based instructions (6)
clients/agent-runtime/src/{security,gateway,tools}/**/*.rs

📄 CodeRabbit inference engine (clients/agent-runtime/AGENTS.md)

Treat src/security/, src/gateway/, src/tools/ as high-risk surfaces and never broaden filesystem/network execution scope without explicit policy checks

Files:

  • clients/agent-runtime/src/gateway/mod.rs
  • clients/agent-runtime/src/gateway/admin.rs
clients/agent-runtime/src/**/*.rs

📄 CodeRabbit inference engine (clients/agent-runtime/AGENTS.md)

clients/agent-runtime/src/**/*.rs: Never log secrets, tokens, raw credentials, or sensitive payloads in any logging statements
Avoid unnecessary allocations, clones, and blocking operations to maintain performance and efficiency

Files:

  • clients/agent-runtime/src/gateway/mod.rs
  • clients/agent-runtime/src/gateway/admin.rs
clients/agent-runtime/**/*.rs

📄 CodeRabbit inference engine (clients/agent-runtime/AGENTS.md)

Run cargo fmt --all -- --check, cargo clippy --all-targets -- -D warnings, and cargo test for code validation, or document which checks were skipped and why

Files:

  • clients/agent-runtime/src/gateway/mod.rs
  • clients/agent-runtime/src/gateway/admin.rs
clients/agent-runtime/src/{security,gateway,tools,config}/**/*.rs

📄 CodeRabbit inference engine (clients/agent-runtime/AGENTS.md)

Do not silently weaken security policy or access constraints; keep default behavior secure-by-default with deny-by-default where applicable

Files:

  • clients/agent-runtime/src/gateway/mod.rs
  • clients/agent-runtime/src/gateway/admin.rs
**/*.rs

⚙️ CodeRabbit configuration file

**/*.rs: Focus on Rust idioms, memory safety, and ownership/borrowing correctness.
Flag unnecessary clones, unchecked panics in production paths, and weak error context.
Prioritize unsafe blocks, FFI boundaries, concurrency races, and secret handling.

Files:

  • clients/agent-runtime/src/gateway/mod.rs
  • clients/agent-runtime/src/gateway/admin.rs
**/*

⚙️ CodeRabbit configuration file

**/*: Security first, performance second.
Validate input boundaries, auth/authz implications, and secret management.
Look for behavioral regressions, missing tests, and contract breaks across modules.

Files:

  • clients/agent-runtime/src/gateway/mod.rs
  • clients/agent-runtime/src/gateway/admin.rs
🧠 Learnings (10)
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/main.rs : Preserve CLI contract unless change is intentional and documented; prefer explicit errors over silent fallback for unsupported critical paths

Applied to files:

  • clients/agent-runtime/src/gateway/mod.rs
  • clients/agent-runtime/src/gateway/admin.rs
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/**/*.rs : Run `cargo fmt --all -- --check`, `cargo clippy --all-targets -- -D warnings`, and `cargo test` for code validation, or document which checks were skipped and why

Applied to files:

  • clients/agent-runtime/src/gateway/mod.rs
  • .github/workflows/sonarqube-analysis.yml
  • clients/agent-runtime/src/gateway/admin.rs
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/{security,gateway,tools,config}/**/*.rs : Do not silently weaken security policy or access constraints; keep default behavior secure-by-default with deny-by-default where applicable

Applied to files:

  • clients/agent-runtime/src/gateway/mod.rs
  • clients/agent-runtime/src/gateway/admin.rs
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/main.rs : Keep startup path lean and avoid heavy initialization in command parsing flow

Applied to files:

  • clients/agent-runtime/src/gateway/mod.rs
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/**/Cargo.toml : Do not add heavy dependencies for minor convenience; justify new crate additions

Applied to files:

  • clients/agent-runtime/src/gateway/mod.rs
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/**/*.rs : Avoid unnecessary allocations, clones, and blocking operations to maintain performance and efficiency

Applied to files:

  • clients/agent-runtime/src/gateway/mod.rs
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/channels/**/*.rs : Implement `Channel` trait in `src/channels/` with consistent `send`, `listen`, and `health_check` semantics and cover auth/allowlist/health behavior with tests

Applied to files:

  • clients/agent-runtime/src/gateway/mod.rs
  • clients/agent-runtime/src/gateway/admin.rs
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/{security,gateway,tools}/**/*.rs : Treat `src/security/`, `src/gateway/`, `src/tools/` as high-risk surfaces and never broaden filesystem/network execution scope without explicit policy checks

Applied to files:

  • clients/agent-runtime/src/gateway/mod.rs
  • clients/agent-runtime/src/gateway/admin.rs
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/tools/**/*.rs : Implement `Tool` trait in `src/tools/` with strict parameter schema, validate and sanitize all inputs, and return structured `ToolResult` without panics in runtime path

Applied to files:

  • clients/agent-runtime/src/gateway/mod.rs
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/**/Cargo.toml : Preserve release-size profile assumptions in `Cargo.toml` and avoid adding heavy dependencies unless clearly justified

Applied to files:

  • clients/agent-runtime/src/gateway/mod.rs
🔇 Additional comments (4)
.agents/skills/android-expert/scripts/analyze-apk-size.sh (1)

24-24: LGTM!

All [ ] to [[ ]] conversions are correct. This improves consistency and provides safer behavior (no word splitting, better operator support). Logic is preserved throughout.

Also applies to: 61-61, 77-77, 88-88, 102-102, 108-108, 126-126, 140-140, 143-143, 153-153, 195-195, 203-203, 210-210

.github/workflows/sonarqube-analysis.yml (1)

74-74: Good hardening: action is pinned to an immutable SHA.

Line 74 improves CI supply-chain stability and reproducibility by avoiding floating action refs.

clients/agent-runtime/src/gateway/mod.rs (1)

2589-2594: Non-behavioral formatting change looks good.

This reflow is readability-only and does not alter test behavior.

clients/agent-runtime/src/gateway/admin.rs (1)

766-775: Please attach validation run results for this refactor.

I don’t see evidence in PR context that required Rust checks were run after this modular split. Please share results (or explicitly document what was skipped and why).

As per coding guidelines: clients/agent-runtime/**/*.rs: Run cargo fmt --all -- --check, cargo clippy --all-targets -- -D warnings, and cargo test for code validation, or document which checks were skipped and why.

Comment on lines +10 to +15
return 1
}

if [[ -z "$1" ]]; then
usage
exit 1
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

exit 1 on line 15 is unreachable due to set -e.

With set -e (line 2), when usage returns 1, the script terminates immediately—exit 1 never executes. The exit behavior is correct, but the code is misleading.

Either remove return 1 (rely on the explicit exit 1) or remove exit 1 (rely on set -e propagating the return code).

Option A: Remove redundant exit
 if [[ -z "$1" ]]; then
   usage
-  exit 1
 fi
Option B: Keep explicit exit, remove return
 usage() {
   echo "Usage: $0 <file_or_directory>" >&2
   echo "Analyzes HTML files for web quality issues." >&2
-  return 1
 }
📝 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
return 1
}
if [[ -z "$1" ]]; then
usage
exit 1
return 1
}
if [[ -z "$1" ]]; then
usage
fi
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.agents/skills/web-quality-audit/scripts/analyze.sh around lines 10 - 15,
The usage function currently does "return 1" and later the script calls usage
followed by "exit 1", but with "set -e" the "return 1" causes the script to exit
immediately making "exit 1" unreachable; pick one approach and make it
consistent: either remove the "return 1" from the usage function and keep the
explicit "exit 1" after the usage call, or remove the trailing "exit 1" and let
"set -e" propagate the non-zero return from usage—update the usage function and
the conditional that calls it accordingly (referencing the usage function and
the if [[ -z "$1" ]] check).

Comment on lines 78 to +80
- name: 📦 Install cargo-llvm-cov
if: env.SONAR_TOKEN != ''
uses: taiki-e/install-action@cargo-llvm-cov
uses: taiki-e/install-action@2834d6555cea49e0759c35c2a877ea0228e73e06 # cargo-llvm-cov
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cat -n .github/workflows/sonarqube-analysis.yml | sed -n '75,85p'

Repository: dallay/corvus

Length of output: 453


🌐 Web query:

taiki-e/install-action documentation cargo-llvm-cov version pinning with: syntax

💡 Result:

To pin cargo-llvm-cov when using taiki-e/install-action, use the action normally (e.g. @v2) and specify the tool version in with.tool via tool@version syntax:

- uses: taiki-e/install-action@v2
  with:
    tool: cargo-llvm-cov@0.6.14

You can also pin only major/minor (patch optional; and minor optional if major ≥ 1):

- uses: taiki-e/install-action@v2
  with:
    tool: cargo-llvm-cov@0.6

The shorthand form:

- uses: taiki-e/install-action@cargo-llvm-cov

installs the latest and does not pin the action/tool versions. [1][2]


Pin cargo-llvm-cov version explicitly to ensure deterministic builds.

The action SHA is pinned, but the installed tool version is not. Without a with: block specifying the tool version, the action installs the latest release on each run, causing potential CI drift.

Suggested change
       - name: 📦 Install cargo-llvm-cov
         if: env.SONAR_TOKEN != ''
         uses: taiki-e/install-action@2834d6555cea49e0759c35c2a877ea0228e73e06 # cargo-llvm-cov
+        with:
+          tool: cargo-llvm-cov@0.6.14

Replace 0.6.14 with the desired version or use 0.6 for major.minor pinning.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/sonarqube-analysis.yml around lines 78 - 80, The workflow
currently uses the
taiki-e/install-action@2834d6555cea49e0759c35c2a877ea0228e73e06 invocation to
install cargo-llvm-cov but does not pin the tool version; update the step that
uses taiki-e/install-action (the "📦 Install cargo-llvm-cov" step) to add a
with: block specifying the desired cargo-llvm-cov version (e.g., version:
'0.6.14' or '0.6') so the installed tool is deterministic across runs.

Comment on lines +537 to +541
collect_core_restart_fields(cfg, patch, &mut fields);
collect_runtime_identity_restart_fields(cfg, patch, &mut fields);
collect_scheduler_gateway_restart_fields(cfg, patch, &mut fields);
collect_webhook_restart_fields(cfg, patch, &mut fields);
collect_secret_restart_fields(cfg, patch, &mut fields);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Restart gate and patch application are out of sync for runtime-bound fields.

restart_required_updates no longer tracks several fields that apply_patch mutates (observability.*, autonomy.*, gateway.trust_forwarded_headers, gateway.rate_limit_max_keys, gateway.idempotency_*, memory.backend). Those values are wired into long-lived gateway state at startup, so accepting these updates without restart conflict is a behavioral regression.

🔧 Suggested parity fix
 fn collect_core_restart_fields(...) {
   ...
+  if let Some(obs) = patch.observability.as_ref() {
+      if let Some(backend) = obs.backend.as_ref() {
+          if backend.trim().to_ascii_lowercase() != cfg.observability.backend {
+              fields.push("observability.backend");
+          }
+      }
+      if normalize_optional_string_opt(obs.otel_endpoint.as_ref()) != cfg.observability.otel_endpoint {
+          fields.push("observability.otel_endpoint");
+      }
+      if normalize_optional_string_opt(obs.otel_service_name.as_ref()) != cfg.observability.otel_service_name {
+          fields.push("observability.otel_service_name");
+      }
+  }
+  if let Some(memory) = patch.memory.as_ref() {
+      if let Some(backend) = memory.backend.as_ref() {
+          if backend.trim().to_ascii_lowercase() != cfg.memory.backend {
+              fields.push("memory.backend");
+          }
+      }
+  }
 }
 
 fn collect_runtime_identity_restart_fields(...) {
   ...
+  if let Some(aut) = patch.autonomy.as_ref() {
+      if aut.level.is_some_and(|v| v != cfg.autonomy.level) { fields.push("autonomy.level"); }
+      if aut.workspace_only.is_some_and(|v| v != cfg.autonomy.workspace_only) { fields.push("autonomy.workspace_only"); }
+      if aut.max_actions_per_hour.is_some_and(|v| v != cfg.autonomy.max_actions_per_hour) { fields.push("autonomy.max_actions_per_hour"); }
+      if aut.max_cost_per_day_cents.is_some_and(|v| v != cfg.autonomy.max_cost_per_day_cents) { fields.push("autonomy.max_cost_per_day_cents"); }
+      if aut.require_approval_for_medium_risk.is_some_and(|v| v != cfg.autonomy.require_approval_for_medium_risk) { fields.push("autonomy.require_approval_for_medium_risk"); }
+      if aut.block_high_risk_commands.is_some_and(|v| v != cfg.autonomy.block_high_risk_commands) { fields.push("autonomy.block_high_risk_commands"); }
+  }
 }
 
 fn collect_scheduler_gateway_restart_fields(...) {
   ...
+  if let Some(v) = gateway.trust_forwarded_headers {
+      if v != cfg.gateway.trust_forwarded_headers { fields.push("gateway.trust_forwarded_headers"); }
+  }
+  if let Some(v) = gateway.rate_limit_max_keys {
+      if v != cfg.gateway.rate_limit_max_keys { fields.push("gateway.rate_limit_max_keys"); }
+  }
+  if let Some(v) = gateway.idempotency_ttl_secs {
+      if v != cfg.gateway.idempotency_ttl_secs { fields.push("gateway.idempotency_ttl_secs"); }
+  }
+  if let Some(v) = gateway.idempotency_max_keys {
+      if v != cfg.gateway.idempotency_max_keys { fields.push("gateway.idempotency_max_keys"); }
+  }
 }

As per coding guidelines: clients/agent-runtime/src/{security,gateway,tools,config}/**/*.rs: Do not silently weaken security policy or access constraints; keep default behavior secure-by-default with deny-by-default where applicable.

Also applies to: 767-773

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@clients/agent-runtime/src/gateway/admin.rs` around lines 537 - 541, The patch
application is allowing updates to runtime-bound fields without marking a
restart required; ensure parity between apply_patch and restart detection by
adding the same fields to restart_required_updates that apply_patch mutates:
include observability.* and autonomy.* keys, gateway.trust_forwarded_headers,
gateway.rate_limit_max_keys, all gateway.idempotency_* keys, and memory.backend;
update the logic that builds fields (see collect_core_restart_fields,
collect_runtime_identity_restart_fields,
collect_scheduler_gateway_restart_fields, collect_webhook_restart_fields,
collect_secret_restart_fields) or the restart_required_updates predicate so
these mutations trigger a restart and preserve secure-by-default behavior.

@yacosta738 yacosta738 merged commit 7151777 into main Mar 4, 2026
19 checks passed
@yacosta738 yacosta738 deleted the cqa branch March 4, 2026 23:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant