perf: Enhance Docker setup for dashboard with improved config bindings#151
Conversation
✅ Contributor ReportUser: @yacosta738
Contributor Report evaluates based on public GitHub activity. Analysis period: 2025-03-06 to 2026-03-06 |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Deploying corvus with
|
| Latest commit: |
ffcb725
|
| Status: | 🚫 Build failed. |
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughSummary by CodeRabbitRelease Notes
WalkthroughAdds a local-first dashboard (compose, Dockerfile, dev tooling, docs), extends CI to build/publish a separate dashboard multi-arch image, and applies multiple internal refactors across Rust, shell, and frontend code for helper extraction and normalization. Changes
Sequence Diagram(s)sequenceDiagram
participant Release as "Release trigger"
participant Publish as "publish job"
participant DockerBuild as "Buildx/QEMU"
participant Registry as "GHCR / Docker Hub"
participant DashboardJob as "docker-dashboard-image job"
Release->>Publish: trigger publish workflow
Publish->>DockerBuild: build runtime multi-arch images
DockerBuild->>Registry: push runtime images
Publish->>DashboardJob: notify/require publish completion
DashboardJob->>DockerBuild: setup QEMU & build dashboard image
DashboardJob->>Registry: push dashboard multi-arch image
DashboardJob->>Release: create GitHub release (artifact metadata)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/_publish.yml:
- Around line 305-340: The dashboard metadata and push currently always include
docker.io which produces malformed names when DOCKERHUB_USERNAME/DOCKERHUB_TOKEN
are missing; modify the workflow so the dashboard-docker-meta
(docker/metadata-action) images list only includes docker.io/${{
secrets.DOCKERHUB_USERNAME }}/corvus-dashboard when both DOCKERHUB_USERNAME and
DOCKERHUB_TOKEN are present (use a prior step to compute an output or
conditional step to build the images string), and update the
docker/build-push-action usage to only attempt pushing to Docker Hub when those
secrets exist (either gate a second push step with if:
secrets.DOCKERHUB_USERNAME && secrets.DOCKERHUB_TOKEN or have the
build-push-action use the metadata output from the conditional metadata step),
leaving GHCR tagging/pushing unconditional via the existing GitHub Container
Registry login step.
- Around line 299-317: The docker-dashboard-image job is using mutable refs
(`@v3`) for docker/setup-qemu-action, docker/setup-buildx-action, and
docker/login-action; update those uses to the same verified commit SHAs used in
the docker-image job (replace the `@v3` refs with the exact commit SHA pins for
docker/setup-qemu-action, docker/setup-buildx-action, and docker/login-action)
so the workflow matches the immutable, hardened pins in the existing
docker-image job.
In `@clients/agent-runtime/docker-compose.yml`:
- Around line 65-78: The dashboard service block (service name "dashboard") uses
a repo-relative build context ("context: ../web" and "dockerfile:
apps/dashboard/Dockerfile") which breaks copy-paste usage; replace the build
block with a published image (e.g., keep "image: corvus-dashboard:local" or
change to a released tag like "corvus-dashboard:latest") and remove the
build/context/dockerfile lines so "docker compose --profile dashboard up" works
from the example file, or alternatively add a clear comment above the
"dashboard" service explaining it only functions from a full repository
checkout; update the "dashboard" service accordingly and keep existing fields
like "container_name", "restart", "depends_on", "profiles", and "ports".
In `@clients/agent-runtime/src/agent/classifier.rs`:
- Around line 29-31: The current any(...) call allocates a new String per
keyword via keyword.to_lowercase(); to fix, precompute and store lowercased
keywords once (e.g., when deserializing or constructing ClassificationRule) and
replace runtime calls with a non-allocating contains check; update the code
paths that populate the keywords collection so the field (or a new field like
keywords_lower) holds the lowercased Strings, and change the classifier code
that uses keywords and lower_message (the line using keywords.iter().any(...))
to iterate over the pre-lowercased keywords without calling to_lowercase() each
invocation.
In `@clients/agent-runtime/src/channels/mod.rs`:
- Around line 1198-1266: The functions maybe_restart_launchd_daemon_service and
maybe_restart_systemd_daemon_service currently hardcode the daemon identifiers
("com.corvus.daemon" and "corvus.service"); update them to import and use a
shared pub(crate) helper from crate::service (add a pub(crate) fn or const in
service/mod.rs that returns the LaunchAgent plist name and the systemd unit
name/path), then replace the literal strings in these functions with
calls/values from that helper so both the installer (service/mod.rs) and these
restart helpers remain in sync.
- Around line 1210-1217: The probe currently only checks spawn success and then
treats any non-successful exit as "no service" by returning Ok(false); update
the Command::new("launchctl") probe (the code that assigns list_output and
listed) to inspect list_output.status (e.g., status.success()) before using
stdout—if the process exited non-zero, return an Err with context (or propagate
a contextualized error) instead of Ok(false); make the analogous change for the
systemctl probe in the block around the systemctl --user is-active call (the
variables and Command invocation in the 1247-1254 region) so non-zero exits are
treated as failures and not silently reported as "no managed service".
In `@clients/agent-runtime/src/gateway/admin.rs`:
- Around line 1496-1509: The current code calls ensure_webhook_config(cfg) and
then applies port/secret patches even when the webhook is disabled, which can
re-enable it unintentionally; change the logic in the webhook patch handling so
that if patch.port.is_some() or patch.secret.is_some() you only proceed when
cfg.channels_config.webhook.is_some() OR the patch explicitly sets enabled to
Some(true) (i.e., patch.enabled == Some(true)); otherwise reject the request
(return an appropriate error) instead of calling ensure_webhook_config or
applying apply_webhook_port_patch/apply_webhook_secret_patch; update the unit
tests to add a regression test covering the disabled webhook + secret/port
update cases and verify restart_required_updates() semantics remain correct.
In `@clients/agent-runtime/src/tools/git_operations.rs`:
- Around line 129-148: parse_ordinary_changed_entry is extracting the wrong
token as the file path by using splitn(3, ' '), which captures the <sub> field
instead of the actual path in porcelain v2 (path is the 8th token in the "1 <XY>
<sub> <mH> <mI> <mW> <hH> <hI> <path>" format); update parsing to split into at
least 9 fields (or use split_whitespace() and collect) and read the path from
the final field, then pass that path into Self::push_status_if_changed for
staged and unstaged status checks (keep staging extraction from the second token
<XY>), and add unit tests using real porcelain v2 lines to cover
parse_ordinary_changed_entry and ensure paths are correct.
In `@clients/agent-runtime/src/tools/mod.rs`:
- Around line 203-206: The code builds delegate_agents by manually iterating and
cloning each key/value pair; replace that with a direct clone of the HashMap
(e.g., change the expression to let delegate_agents: HashMap<String,
DelegateAgentConfig> = agents.clone(); or let delegate_agents =
agents.to_owned();), so the entire map is cloned in one call instead of mapping
over agents.iter() and cloning keys/values manually.
In `@clients/web/apps/dashboard/Dockerfile`:
- Line 24: Replace the hard-coded binary invocation in the Dockerfile RUN step
that currently calls ../../node_modules/.bin/vite with a pnpm-based invocation
to avoid relying on hoisted node_modules; update the RUN command to use either
"pnpm exec vite build" or "pnpm --filter dashboard build" so the build uses
pnpm's execution context and remains resilient to linker/hoisting changes.
In `@clients/web/packages/locales/src/parity.spec.ts`:
- Around line 25-45: The extractPlaceholders function currently resets start on
every '{', collapsing double-brace "{{name}}" into "{name}"; change the logic in
extractPlaceholders so that encountering '{' does not overwrite an existing
start index (i.e., only set start when start < 0) so the slice preserves the
exact brace shape (e.g., "{{name}}"), and update the closing logic to capture
the full bracketed sequence accordingly; also add a regression test that asserts
"{{name}}" and nested-brace inputs are returned as distinct placeholders (not
reduced) to prevent parity guards from passing incorrectly.
In `@dev/cli.sh`:
- Around line 24-40: The wait_http_ok loop can exceed the overall timeout
because each curl call can block; modify wait_http_ok to compute the remaining
seconds before each curl (remaining = timeout_secs - (now_ts - start_ts)), if
remaining <= 0 return 1, and pass that remaining value to curl's timeout options
(e.g., --max-time and/or --connect-timeout) so each attempt is bounded by the
remaining time; update the curl invocation in the wait_http_ok function to use
those options and ensure remaining is an integer >= 1 before calling curl.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4f14f2f3-d0b5-4bc3-8557-90b305460feb
📒 Files selected for processing (26)
.github/workflows/README.md.github/workflows/_publish.yml.github/workflows/fix-renovate.yml.github/workflows/shell/check-commit-msg.sh.github/workflows/sonarqube-analysis.ymlMakefileREADME.mdclients/agent-runtime/README.mdclients/agent-runtime/docker-compose.ymlclients/agent-runtime/npm/corvus-cli/scripts/postinstall.jsclients/agent-runtime/scripts/test_dockerignore.shclients/agent-runtime/src/agent/classifier.rsclients/agent-runtime/src/channels/mod.rsclients/agent-runtime/src/gateway/admin.rsclients/agent-runtime/src/tools/browser.rsclients/agent-runtime/src/tools/git_operations.rsclients/agent-runtime/src/tools/mod.rsclients/web/apps/dashboard/Dockerfileclients/web/apps/dashboard/README.mdclients/web/apps/dashboard/src/App.vueclients/web/packages/locales/src/parity.spec.tsclients/web/packages/shared/index.tsdev/README.mddev/cli.shdev/docker-compose.ymlsync-version-with-tag.sh
💤 Files with no reviewable changes (1)
- clients/web/packages/shared/index.ts
📜 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: Cloudflare Pages
- GitHub Check: pr-checks
🧰 Additional context used
📓 Path-based instructions (10)
**/*.{md,mdx}
⚙️ CodeRabbit configuration file
**/*.{md,mdx}: Verify technical accuracy and that docs stay aligned with code changes.
For user-facing docs, check EN/ES parity or explicitly note pending translation gaps.
Files:
dev/README.mdREADME.mdclients/web/apps/dashboard/README.mdclients/agent-runtime/README.md
**/*
⚙️ 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:
dev/README.mdREADME.mdclients/agent-runtime/scripts/test_dockerignore.shclients/agent-runtime/docker-compose.ymlclients/agent-runtime/src/channels/mod.rsclients/agent-runtime/src/agent/classifier.rsclients/web/apps/dashboard/README.mdclients/agent-runtime/src/tools/git_operations.rsclients/web/packages/locales/src/parity.spec.tsdev/docker-compose.ymlclients/agent-runtime/npm/corvus-cli/scripts/postinstall.jsclients/agent-runtime/src/tools/browser.rsclients/web/apps/dashboard/Dockerfiledev/cli.shclients/agent-runtime/src/gateway/admin.rsclients/web/apps/dashboard/src/App.vueclients/agent-runtime/src/tools/mod.rsclients/agent-runtime/README.mdsync-version-with-tag.shMakefile
clients/agent-runtime/src/channels/**/*.rs
📄 CodeRabbit inference engine (clients/agent-runtime/AGENTS.md)
Implement
Channeltrait insrc/channels/with consistentsend,listen, andhealth_checksemantics and cover auth/allowlist/health behavior with tests
Files:
clients/agent-runtime/src/channels/mod.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/channels/mod.rsclients/agent-runtime/src/agent/classifier.rsclients/agent-runtime/src/tools/git_operations.rsclients/agent-runtime/src/tools/browser.rsclients/agent-runtime/src/gateway/admin.rsclients/agent-runtime/src/tools/mod.rs
clients/agent-runtime/**/*.rs
📄 CodeRabbit inference engine (clients/agent-runtime/AGENTS.md)
Run
cargo fmt --all -- --check,cargo clippy --all-targets -- -D warnings, andcargo testfor code validation, or document which checks were skipped and why
Files:
clients/agent-runtime/src/channels/mod.rsclients/agent-runtime/src/agent/classifier.rsclients/agent-runtime/src/tools/git_operations.rsclients/agent-runtime/src/tools/browser.rsclients/agent-runtime/src/gateway/admin.rsclients/agent-runtime/src/tools/mod.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/channels/mod.rsclients/agent-runtime/src/agent/classifier.rsclients/agent-runtime/src/tools/git_operations.rsclients/agent-runtime/src/tools/browser.rsclients/agent-runtime/src/gateway/admin.rsclients/agent-runtime/src/tools/mod.rs
clients/agent-runtime/src/tools/**/*.rs
📄 CodeRabbit inference engine (clients/agent-runtime/AGENTS.md)
Implement
Tooltrait insrc/tools/with strict parameter schema, validate and sanitize all inputs, and return structuredToolResultwithout panics in runtime path
Files:
clients/agent-runtime/src/tools/git_operations.rsclients/agent-runtime/src/tools/browser.rsclients/agent-runtime/src/tools/mod.rs
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/tools/git_operations.rsclients/agent-runtime/src/tools/browser.rsclients/agent-runtime/src/gateway/admin.rsclients/agent-runtime/src/tools/mod.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/tools/git_operations.rsclients/agent-runtime/src/tools/browser.rsclients/agent-runtime/src/gateway/admin.rsclients/agent-runtime/src/tools/mod.rs
**/*.vue
⚙️ CodeRabbit configuration file
**/*.vue: Enforce Vue 3 Composition API with <script setup>.
Ensure accessibility (A11y) and proper use of Tailwind CSS classes.
Check for proper prop validation and emitted events documentation.
Files:
clients/web/apps/dashboard/src/App.vue
🧠 Learnings (11)
📚 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/channels/mod.rsclients/agent-runtime/src/agent/classifier.rsclients/agent-runtime/src/tools/git_operations.rsclients/agent-runtime/src/tools/browser.rsclients/agent-runtime/src/gateway/admin.rsclients/agent-runtime/src/tools/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/channels/mod.rsclients/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/channels/mod.rsclients/agent-runtime/src/tools/browser.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/channels/mod.rsclients/agent-runtime/src/tools/browser.rsclients/agent-runtime/src/gateway/admin.rsclients/agent-runtime/src/tools/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/**/*.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/channels/mod.rsclients/agent-runtime/src/tools/browser.rsclients/agent-runtime/src/gateway/admin.rsclients/agent-runtime/src/tools/mod.rsclients/agent-runtime/README.mdMakefile
📚 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/.github/**/*.{yml,yaml} : For workflow/template-only changes, ensure YAML/template syntax validity
Applied to files:
.github/workflows/fix-renovate.yml
📚 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/tools/browser.rsclients/agent-runtime/src/tools/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/{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/tools/browser.rsclients/agent-runtime/src/gateway/admin.rsclients/agent-runtime/src/tools/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/tools/browser.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/tools/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/providers/**/*.rs : Implement `Provider` trait in `src/providers/` and register in `src/providers/mod.rs` factory when adding a new provider
Applied to files:
clients/agent-runtime/src/tools/mod.rs
🪛 Checkov (3.2.334)
clients/web/apps/dashboard/Dockerfile
[low] 1-30: Ensure that HEALTHCHECK instructions have been added to container images
(CKV_DOCKER_2)
[low] 1-30: Ensure that a user for the container has been created
(CKV_DOCKER_3)
🪛 LanguageTool
clients/web/apps/dashboard/README.md
[grammar] ~28-~28: Elimina la palabra o signo.
Context: ...en http://127.0.0.1:3000 y completa el pairing en /pair para obtener bearer t...
(QB_NEW_ES_OTHER_ERROR_IDS_UNNECESSARY_SPACE)
🪛 Trivy (0.69.2)
clients/web/apps/dashboard/Dockerfile
[error] 1-1: Image user should not be 'root'
Specify at least 1 USER command in Dockerfile with non-root user as argument
Rule: DS-0002
(IaC/Dockerfile)
[info] 1-1: No HEALTHCHECK defined
Add HEALTHCHECK instruction in your Dockerfile
Rule: DS-0026
(IaC/Dockerfile)
🔇 Additional comments (33)
clients/agent-runtime/npm/corvus-cli/scripts/postinstall.js (1)
5-16: LGTM: safer catch handling, no behavior regression.This keeps postinstall non-fatal while handling non-
Errorthrow values cleanly and preserving the existing flow.clients/agent-runtime/scripts/test_dockerignore.sh (3)
18-23: LGTM – defensive improvements.Using
localfor the message parameter prevents accidental global variable pollution. The explicitreturn 0adds clarity, though technically redundant since arithmetic expansion returns 0 on success.
25-30: LGTM – consistent withlog_pass.Same pattern applied here. Clean and correct.
102-103: Good fix: proper trap quoting.Single quotes delay
$TEMP_DIRexpansion until trap execution, which is the correct pattern for cleanup traps. This ensures the variable holds its value even if the shell context changes before EXIT fires..github/workflows/sonarqube-analysis.yml (2)
108-109: LGTM — Test discovery configured correctly.Setting
sonar.tests=.withsonar.test.inclusionsproperly identifies test files across TypeScript and Rust codebases. The patterns cover the expected conventions (*.spec.ts,*.test.ts,*_test.rs,tests/**).
111-111: Coverage exclusions look appropriate, minor consistency note.TypeScript test patterns are correctly excluded from coverage. The Rust test patterns (
**/*_test.rs,**/tests/**) aren't listed here, but this is fine since Rust coverage is generated viacargo-llvm-covwhich handles exclusions at the instrumentation level.The specific file exclusions (
App.vue,index.ts,postinstall.js) appear to be entry points or bootstrap files — reasonable to exclude if they're impractical to unit test.sync-version-with-tag.sh (1)
37-37: Explicit exit-status propagation looks good.These returns preserve the underlying command status for callers and do not introduce a behavioral regression.
Also applies to: 138-138
clients/agent-runtime/src/agent/classifier.rs (2)
3-21: LGTM — clean helper with correct semantics.Idiomatic use of early returns, no allocations, and handles
Optionbounds cleanly.
54-62: LGTM — refactoring improves clarity.Helper usage is correct. The
rule.hint.clone()is necessary for the owned return type, and the call site inagent.rshandles ownership properly.clients/agent-runtime/src/tools/browser.rs (5)
510-580: Clean extraction of command argument builders.These helpers centralize argument construction for agent-browser CLI commands. The logic is straightforward and correctly handles optional parameters with conditional pushes.
1811-1822: Well-structured action parsing dispatcher.The
parse_browser_actionfunction cleanly routes to specialized parsers based on action type. Grouping related actions (e.g., selector-based actions on line 1813) reduces duplication.
1830-1845: Snapshot defaults are sensible.
interactive_onlyandcompactdefault totrue, which aligns with typical agent workflows (pruning the DOM tree for faster, relevant snapshots). The saturatingu32::try_from(d).unwrap_or(u32::MAX)on line 1843 safely handles out-of-range depth values without panicking.
1847-1891: Selector and value parsing helpers follow trait guidelines.
parse_selector_actionandparse_selector_value_actionvalidate required fields viarequired_action_strand return structured errors—no panics on missing input.
1954-1958: Reusable required-field extractor.
required_action_strprovides consistent error messages across all parsing helpers. Good pattern for maintaining uniform error UX.clients/agent-runtime/src/tools/mod.rs (4)
105-136: Browser tool registration properly gates on config.Early return when
enabledis false prevents unnecessary object construction. TheComputerUseConfigstruct is fully populated from the config schema, preserving all security-relevant fields (endpoint validation, coordinate limits, window allowlist).
173-191: Composio guard handles empty-key edge case.The double-check (Option unwrap then empty-string check) correctly prevents registration when the key is present but blank—a common config mistake.
218-255: MCP collision handling is fail-closed—good security posture.On detecting any tool-name collision (MCP vs. core, or MCP vs. MCP), the entire MCP batch is rejected with a warning. This prevents ambiguous dispatch at runtime. Error messages are redacted via
redact_runtime_error(line 248), avoiding credential leakage.Based on learnings: "Treat
src/security/,src/gateway/,src/tools/as high-risk surfaces and never broaden filesystem/network execution scope without explicit policy checks."
328-341: Refactored registration callsites are clean.The helper calls replace inline logic, improving readability while preserving identical behavior. Feature-gated
extend_with_mcp_tools(line 341) ensures no overhead whenmcp-runtimeis disabled.clients/agent-runtime/src/channels/mod.rs (1)
1268-1275: LGTM: the platform dispatch is easier to follow now.Keeping the OS selection in one helper makes the bind flow simpler and keeps the platform-specific restart logic isolated.
clients/agent-runtime/src/tools/git_operations.rs (3)
112-119: LGTM!Clean extraction of the branch head with proper early return pattern.
121-127: LGTM!Correctly filters unchanged status entries before pushing to the result vectors.
98-105: Clean delegation to helper functions.The call sites properly delegate parsing to the new helpers. The correctness issue lies in
parse_ordinary_changed_entry(addressed above).README.md (1)
23-23: LGTM!Dynamic version badge using
github/v/tagwithsort=semvercorrectly reflects the latest release tag. Aligns well with the expanded release workflow..github/workflows/fix-renovate.yml (2)
123-149: Good security hardening: TOCTOU mitigation before write operations.Re-validating the PR head SHA immediately before committing prevents a race condition where an attacker could force-push malicious commits after the initial approval check but before the workflow writes. This is solid defense-in-depth for a workflow with
contents: writepermissions.
151-163: LGTM!Scoping
git logto the explicit SHA (line 153) and usingHEAD:branchfor push (line 163) are both correct and align with the validated commit..github/workflows/shell/check-commit-msg.sh (2)
29-37: LGTM!Here-strings (
<<<) are cleaner and avoid an extra subshell compared toecho | grep. Good refactor.
39-49: LGTM!Proper stderr redirection for error messages. This allows CI systems to capture errors separately and avoids polluting stdout used for data flow.
clients/web/apps/dashboard/README.md (1)
20-29: LGTM!Documentation accurately reflects the Docker Compose profile usage and port bindings. The pairing flow guidance is correct per the gateway API.
clients/agent-runtime/README.md (1)
68-88: LGTM!Documentation is clear and actionable. The local-copy pattern (
docker-compose.local.yml) is a good practice for keeping secrets out of version control. Port bindings and profile name align with the compose file changes.clients/web/apps/dashboard/Dockerfile (1)
26-30: LGTM - nginx-unprivileged runs as non-root.The static analysis warnings about root user are false positives.
nginxinc/nginx-unprivilegedruns as UID 101 by default, satisfying the non-root requirement in the runtime stage. The build stage running as root is standard practice.Regarding the missing healthcheck: nginx-unprivileged doesn't include curl, so adding one would require installing additional packages. For a static file server behind compose/orchestration health checks, this is acceptable.
dev/README.md (2)
28-34: LGTM!Documentation accurately reflects the
up-dashboardcommand and dashboard URL. Aligns with the docker-compose.yml service definition.
83-93: LGTM!Commands match the cli.sh implementation. The
smokecommand correctly documents gateway and optional dashboard health checks.dev/docker-compose.yml (1)
57-71: LGTM!Good setup:
- Localhost-only port binding (
127.0.0.1:4324:8080) prevents accidental exposure.- Profile-gated (
["dashboard"]) keeps it opt-in.depends_onensures proper startup order.The lack of a healthcheck is mitigated by
wait_http_okpolling in cli.sh's smoke command.
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
|
1 participant
This pull request introduces several improvements and new features across the project, with a focus on enhanced Docker and dashboard support, developer experience, and code maintainability. The most notable changes are the addition of a Docker Compose profile and image for the local dashboard, improvements to the release workflow to publish dashboard images, and code refactoring for clarity and modularity. There are also updates to documentation and developer tooling for better usability.
Docker and Dashboard Enhancements:
dashboardservice toclients/agent-runtime/docker-compose.yml, with a new Docker Compose profile for running the dashboard locally alongside the gateway, and updated port bindings for improved security. [1] [2] [3]clients/agent-runtime/README.mdto describe how to use Docker Compose for local dashboard and gateway development, including port and pairing instructions.Release and CI Workflow Improvements:
Code Quality and Refactoring:
clients/agent-runtime/src/agent/classifier.rsby extracting logic for length constraints and keyword/pattern matching into helper functions for better readability and testability. [1] [2]clients/agent-runtime/src/channels/mod.rsto split platform-specific daemon restart logic into separate functions for macOS (launchd) and Linux (systemd), improving maintainability. [1] [2] [3]clients/agent-runtime/src/gateway/admin.rsinto smaller, focused functions for binding, security, limits, and idempotency. [1] [2]Developer Tooling and Documentation:
README.mdto dynamically reflect the latest GitHub tag.These changes collectively improve the developer experience, streamline local and CI workflows, and lay the groundwork for a more robust dashboard deployment and management process.