Repository navigation
Default cargo run to WebUI serve - #6663
Conversation
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
🔎 IronLoop Review StatusHead: Current reviewers:
Reviewer summaries
Recent activity
Available commands
Run metadataAdmission: webhook accepted the request and IronLoop persisted reviewer state before this projection. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe workspace now defaults to the Reborn CLI. Missing CLI subcommands resolve to ChangesReborn CLI dispatch
WebUI build fallback
Workspace and CI targeting
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant RebornCLI
participant Clap
participant Serve
User->>RebornCLI: invoke without subcommand
RebornCLI->>Clap: inject and parse serve
Clap-->>RebornCLI: return Command::Serve
RebornCLI->>Serve: execute serve command
sequenceDiagram
participant BuildScript
participant Corepack
participant NpmExec
BuildScript->>Corepack: run pnpm
alt Corepack unavailable
BuildScript->>NpmExec: run pinned pnpm package
NpmExec-->>BuildScript: return build result
else Corepack available
Corepack-->>BuildScript: return build result
end
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
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 |
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
There was a problem hiding this comment.
❌ IronLoop Review: reviewer
Review at a glance
| Verdict | Blocking | Notes | Inline | Head |
|---|---|---|---|---|
| ❌ Changes requested | 1 | 0 | 1 | 66b151e25326 |
Head: 66b151e25326666ac2112c01cd41019c88ab985a
Next: Fix the blocking findings, push the PR branch, then re-run this reviewer.
Run details
Status: Current
Needs human: no
Needs validation: no
Summary
default-members changes all unqualified workspace Cargo commands, breaking existing root integration-test and pre-push commands rather than only changing cargo run.
Findings
Blocking: 1 / Notes: 0
Blocking findings
1. ❌ [MEDIUM] Preserve root integration-test command selection
Location: Cargo.toml:3
default-members applies to cargo test, cargo check, and cargo clippy too, not only cargo run. The default package becomes ironclaw, which has no library and does not own the root reborn_* test targets. Consequently the installed pre-push hook's cargo test --locked --lib fails (as noted in the PR), while CI's scripts/ci/run-reborn-root-partition.sh and cargo test --test reborn_qa_recorded_behavior select the CLI package and cannot find their root-package tests. Keep those callers explicitly targeting ironclaw_reborn_integration_tests (or use a Cargo configuration that preserves their selection) before changing the workspace default.
Developer follow-up
After fixing this feedback:
- Push the fix to this PR branch.
- Re-run this reviewer with
@ironloopai review --agent reviewerif you only changed this reviewer's findings. - Re-run all reviewers with
@ironloopai reviewwhen the fix may affect multiple areas.
| @@ -1,5 +1,6 @@ | |||
| [workspace] | |||
| members = [".", "crates/ironclaw_common", "crates/ironclaw_observability", "crates/ironclaw_host_api", "crates/ironclaw_host_ingress", "crates/ironclaw_filesystem", "crates/ironclaw_attachments", "crates/ironclaw_extractors", "crates/ironclaw_memory", "crates/ironclaw_memory_native", "crates/ironclaw_memory_mem0", "crates/ironclaw_events", "crates/ironclaw_event_projections", "crates/ironclaw_event_streams", "crates/ironclaw_reborn_event_store", "crates/ironclaw_extensions", "crates/ironclaw_extension_host", "crates/ironclaw_processes", "crates/ironclaw_dispatcher", "crates/ironclaw_scripts", "crates/ironclaw_process_sandbox", "crates/ironclaw_mcp", "crates/ironclaw_wasm", "crates/ironclaw_wasm_limiter", "crates/ironclaw_capabilities", "crates/ironclaw_secrets", "crates/ironclaw_network", "crates/ironclaw_host_runtime", "crates/ironclaw_runtime_policy", "crates/ironclaw_authorization", "crates/ironclaw_run_state", "crates/ironclaw_approvals", "crates/ironclaw_resources", "crates/ironclaw_auth", "crates/ironclaw_trust", "crates/ironclaw_turns", "crates/ironclaw_agent_loop", "crates/ironclaw_threads", "crates/ironclaw_prompt_envelope", "crates/ironclaw_hooks", "crates/ironclaw_loop_host", "crates/ironclaw_runner", "crates/ironclaw_reborn_config", "crates/ironclaw_operator", "crates/ironclaw_reborn_composition", "crates/ironclaw_reborn_identity", "crates/ironclaw_first_party_extensions", "crates/ironclaw_reborn_cli", "crates/ironclaw_reborn_traces", "crates/ironclaw_webui", "crates/ironclaw_reborn_openai_compat", "crates/ironclaw_conversations", "crates/ironclaw_product", "crates/ironclaw_telegram_extension", "crates/ironclaw_telegram_v2_adapter", "crates/ironclaw_slack_extension", "crates/ironclaw_outbound", "crates/ironclaw_triggers", "crates/ironclaw_projects", "crates/ironclaw_architecture", "crates/ironclaw_safety", "crates/ironclaw_skills", "crates/ironclaw_llm", "crates/ironclaw_embeddings", "tools/ironclaw_stress"] | |||
| default-members = ["crates/ironclaw_reborn_cli"] | |||
There was a problem hiding this comment.
default-members applies to every package-unspecified Cargo command, not just cargo run. This switches cargo test --locked --lib to the bin-only CLI and makes root reborn_* test commands in CI select a package that does not own those targets. Please preserve or explicitly set the root integration-test package for those callers.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Cargo.toml`:
- Line 3: Remove the workspace-level default-members setting from Cargo.toml so
root cargo build, test, check, and clippy commands continue targeting the full
workspace. If CLI-only execution is required, replace it with a scope-limited
run alias or equivalent metadata rather than changing workspace defaults.
In `@crates/ironclaw_reborn_cli/src/cli.rs`:
- Around line 8-13: Update the CLI definitions and run() command-selection flow
around Cli, ServeCommand, and Command::Serve so serve-only flags are not
flattened onto the top-level parser. Consolidate those options under
Command::Serve or explicitly reject any top-level serve flags when another
subcommand is selected, returning an error instead of silently ignoring them.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: cc2f0396-377e-4cb1-9f5c-2ba031ae2ac1
📒 Files selected for processing (7)
Cargo.tomlcrates/ironclaw_reborn_cli/src/cli.rscrates/ironclaw_reborn_cli/src/commands/serve.rscrates/ironclaw_reborn_cli/src/commands/traces/tests.rscrates/ironclaw_reborn_cli/tests/smoke.rscrates/ironclaw_webui/build.rscrates/ironclaw_webui/frontend/README.md
| @@ -1,5 +1,6 @@ | |||
| [workspace] | |||
| members = [".", "crates/ironclaw_common", "crates/ironclaw_observability", "crates/ironclaw_host_api", "crates/ironclaw_host_ingress", "crates/ironclaw_filesystem", "crates/ironclaw_attachments", "crates/ironclaw_extractors", "crates/ironclaw_memory", "crates/ironclaw_memory_native", "crates/ironclaw_memory_mem0", "crates/ironclaw_events", "crates/ironclaw_event_projections", "crates/ironclaw_event_streams", "crates/ironclaw_reborn_event_store", "crates/ironclaw_extensions", "crates/ironclaw_extension_host", "crates/ironclaw_processes", "crates/ironclaw_dispatcher", "crates/ironclaw_scripts", "crates/ironclaw_process_sandbox", "crates/ironclaw_mcp", "crates/ironclaw_wasm", "crates/ironclaw_wasm_limiter", "crates/ironclaw_capabilities", "crates/ironclaw_secrets", "crates/ironclaw_network", "crates/ironclaw_host_runtime", "crates/ironclaw_runtime_policy", "crates/ironclaw_authorization", "crates/ironclaw_run_state", "crates/ironclaw_approvals", "crates/ironclaw_resources", "crates/ironclaw_auth", "crates/ironclaw_trust", "crates/ironclaw_turns", "crates/ironclaw_agent_loop", "crates/ironclaw_threads", "crates/ironclaw_prompt_envelope", "crates/ironclaw_hooks", "crates/ironclaw_loop_host", "crates/ironclaw_runner", "crates/ironclaw_reborn_config", "crates/ironclaw_operator", "crates/ironclaw_reborn_composition", "crates/ironclaw_reborn_identity", "crates/ironclaw_first_party_extensions", "crates/ironclaw_reborn_cli", "crates/ironclaw_reborn_traces", "crates/ironclaw_webui", "crates/ironclaw_reborn_openai_compat", "crates/ironclaw_conversations", "crates/ironclaw_product", "crates/ironclaw_telegram_extension", "crates/ironclaw_telegram_v2_adapter", "crates/ironclaw_slack_extension", "crates/ironclaw_outbound", "crates/ironclaw_triggers", "crates/ironclaw_projects", "crates/ironclaw_architecture", "crates/ironclaw_safety", "crates/ironclaw_skills", "crates/ironclaw_llm", "crates/ironclaw_embeddings", "tools/ironclaw_stress"] | |||
| default-members = ["crates/ironclaw_reborn_cli"] | |||
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== candidate files =="
git ls-files | rg '(^|/)(Cargo\.toml|CLAUDE\.md|AGENTS\.md|\.claude/.*rules.*|README\.md)$|\.cargo/config.toml$' | sed -n '1,200p'
echo
echo "== Cargo.toml relevant =="
if [ -f Cargo.toml ]; then
cat -n Cargo.toml | sed -n '1,80p'
fi
echo
echo "== rule mentions =="
for f in CLAUDE.md AGENTS.md; do
if [ -f "$f" ]; then
echo "-- $f --"
rg -n "workspace|workspace-wide|CLIP|Clippy|cargo clippy|default-members|validation|zero-warning|Reborn" "$f" || true
fi
done
if [ -d .claude/rules ]; then
for f in .claude/rules/*; do
echo "-- $f --"
rg -n "workspace|workspace-wide|CLIP|Clippy|cargo clippy|default-members|validation|zero-warning|Reborn" "$f" || true
done
fi
echo
echo "== workspace member count and crate features summary =="
python3 - <<'PY'
import tomllib
from pathlib import Path
cargo = Path('Cargo.toml')
if not cargo.exists():
raise SystemExit(1)
data = tomllib.loads(cargo.read_text())
ws=data.get('workspace',{})
print('members=', ws.get('members'))
print('default-members=', ws.get('default-members'))
print('package=', data.get('package',{}).get('name'))
PY
echo
echo "== deterministic cargo default-members selector probe if cargo/config available =="
if command -v cargo >/dev/null 2>&1; then
tmp="$(mktemp -d)"
mkdir -p "$tmp/proj" "$tmp/proj/a" "$tmp/proj/b"
cat > "$tmp/proj/Cargo.toml" <<'EOF'
[workspace]
members = ["a", "b"]
default-members = ["a"]
EOF
mkdir -p "$tmp/proj/a/src" "$tmp/proj/b/src"
cat > "$tmp/proj/a/src/lib.rs" <<'EOF'
pub fn add(a: i32, b: i32) -> i32 { a + b }
EOF
cat > "$tmp/proj/b/src/lib.rs" <<'EOF'
pub fn sub(x: i32, y: i32) -> i32 { x - y }
EOF
mkdir -p "$tmp/proj/.cargo"
echo 'cargo-default = true' > "$tmp/proj/.cargo/config.toml"
(cd "$tmp/proj" && cargo metadata --format-version 1 --offline >/tmp/cargo_metadata_default.json)
python3 - <<'PY'
import json
from pathlib import Path
data=json.loads(Path('/tmp/cargo_metadata_default.json').read_text())
print("workspace=.", data["workspace_root"])
print("packages=", sorted(p.get("name") for p in data["packages"]))
PY
else
echo "cargo not available"
fiRepository: nearai/ironclaw
Length of output: 24141
Disable default-members unless it is needed for a scoped run command.
default-members = ["crates/ironclaw_reborn_cli"] changes root commands such as cargo build, cargo test, cargo check, and cargo clippy to select only the CLI crate. CLAUDE.md/AGENTS.md and .claude/rules/review-discipline.md require the workspace-wide clippy command for Reborn work; keeping this setting creates a confusing root command that violates that workspace-wide validation invariant. Use a scope-limited run alias/metadata instead of making the whole workspace default to the CLI.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Cargo.toml` at line 3, Remove the workspace-level default-members setting
from Cargo.toml so root cargo build, test, check, and clippy commands continue
targeting the full workspace. If CLI-only execution is required, replace it with
a scope-limited run alias or equivalent metadata rather than changing workspace
defaults.
Source: Coding guidelines
Coverage ratchetReborn integration-tier coverageLine coverage (Reborn crates): 85.96% — 313745 / 365002 lines Per-crate breakdown (61 crates, lowest-covered first)
This table itself is informational and never gates the PR on its own — not the percentage, not the per-crate holes, not the 0-coverage callout. A separate coverage ratchet (dry-run until enforce=true; see tests/integration/coverage-floor.toml) can fail the build on specific configured floors. Exemptions (3 entry/entries excluded from the accounting above)
|
|
🚅 Deployed to the ironclaw-pr-6663 environment in ironclaw-ci-preview
|
Summary
cargo runselect the shipping Reborn CLI package instead of the root integration-test package.ironclawdefault toserve, preserving the existing fail-closed WebUI auth checks.pnpm@11.7.0throughnpm execwhencorepackis not onPATH, and document it.servecommand path.Change Type
Linked Issue
None.
Validation
cargo fmt --all -- --checkcargo clippy --all --benches --tests --examples --all-features -- -D warningscargo buildcargo test -p ironclaw --test smoke workspace_default_cargo_run_targets_reborn_cli;cargo test -p ironclaw --test smoke no_subcommand_defaults_to_serve_commandcargo test --features integrationif database-backed or integration behavior changedcargo run -- --help; no-argcargo runbuilds and reaches the defaultservepath, then fails closed on missing WebUI token as expected.review-prorpr-shepherd --fixwas run before requesting reviewTest Strategy
User behavior:
Root
cargo runshould build and run the Rebornironclawbinary. With no command,ironclawshould behave likeironclaw serve, using the existing WebUI auth setup and fail-closed checks.Risk areas:
Tests added or updated:
workspace_default_cargo_run_targets_reborn_cli;no_subcommand_defaults_to_serve_command; updated traces parser test for optional top-level subcommand.cargo run -- --help; no-argcargo runmanual runtime-entry check; crate clippy forironclawandironclaw_webui.What the tests prove:
The workspace manifest selects
crates/ironclaw_reborn_clifor default Cargo commands, no-subcommand CLI parsing dispatches toserve, and the WebUI build script can complete withoutcorepackwhennpmis available.Commands run:
cargo fmt -p ironclaw_webui -p ironclawcargo fmt --all -- --checkcargo run -- --helpcargo runwith a temporaryIRONCLAW_REBORN_HOMEand no WebUI token, expecting fail-closed auth errorcargo clippy -p ironclaw_webui --all-targets -- -D warningscargo clippy -p ironclaw --all-targets -- -D warningscargo test -p ironclaw --test smoke workspace_default_cargo_run_targets_reborn_clicargo test -p ironclaw --test smoke no_subcommand_defaults_to_serve_commandgit diff --checkno library targets found in package ironclaw); the branch was pushed withIRONCLAW_PREPUSH_TEST=0after the focused tests above passed.Security Impact
No authentication or authorization behavior is weakened. The default command now enters the existing
servepath, which still fails closed if neitherIRONCLAW_REBORN_WEBUI_TOKENnor the onboarded token file exists. The npm fallback fetches the already-pinned pnpm package version when Corepack is unavailable.Reborn Trust-Boundary Checklist
serde(default)fields fail closed or have migration tests. N/A: no serde fields changed.Transient,Permanent,Misconfigured,PolicyDeniedor equivalent). N/A: no runtime error taxonomy changed; existingservemissing-token error is preserved.Database Impact
None.
Blast Radius
Touches workspace Cargo default package selection, Reborn CLI top-level command parsing, WebUI build tooling, and CLI smoke tests. Possible breakage would be local
cargo runworkflows expecting the old no-bin error or a required subcommand; those workflows should now use explicit subcommands when needed.Rollback Plan
Revert the single commit to restore the previous workspace default behavior, required CLI subcommand parsing, and Corepack-only WebUI frontend build.
Review Follow-Through
Reviewer judgment requested on whether defaulting no-argument
ironclawtoserveshould remain the canonical source-development behavior, or whether the default should be revisited before marking the PR ready.Review track: B (feature/maintainer-requested refactor)