fix(cli): clearer container runtime errors and infra failure formatting - #4062
Merged
Conversation
- Expose container_cli on DockerClient; map spawn NotFound to guidance with
install, config, and --dockerless options
- Preserve RoutineFailure details in run_local_infrastructure_with_timeout
- Multi-line user-facing errors; use Display ({e:#}) for infrastructure failures
- Add unit tests for missing CLI and format helper
Made-with: Cursor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📜 Recent review details⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
🧰 Additional context used📓 Path-based instructions (3)**/*.rs📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
apps/framework-cli/**/*.rs📄 CodeRabbit inference engine (apps/framework-cli/AGENTS.md)
Files:
**/framework-cli/src/**⚙️ CodeRabbit configuration file
Files:
🧠 Learnings (13)📓 Common learnings📚 Learning: 2026-04-22T18:10:52.989ZApplied to files:
📚 Learning: 2026-02-06T01:42:40.104ZApplied to files:
📚 Learning: 2026-04-08T00:08:36.251ZApplied to files:
📚 Learning: 2026-02-18T00:09:56.326ZApplied to files:
📚 Learning: 2026-02-07T04:42:43.495ZApplied to files:
📚 Learning: 2026-03-04T00:09:14.325ZApplied to files:
📚 Learning: 2026-02-07T06:10:32.927ZApplied to files:
📚 Learning: 2026-04-18T22:43:44.130ZApplied to files:
📚 Learning: 2026-02-08T22:31:22.237ZApplied to files:
📚 Learning: 2026-04-09T15:11:59.525ZApplied to files:
📚 Learning: 2026-02-06T01:43:06.078ZApplied to files:
📚 Learning: 2026-02-07T04:42:43.608ZApplied to files:
🔇 Additional comments (2)
Summary by CodeRabbit
WalkthroughAdds a private formatter for rendering Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/framework-cli/src/cli.rs`:
- Around line 2493-2511: The test suite is missing coverage for the "no-source"
branch of format_infrastructure_routine_failure; add a unit test that constructs
a RoutineFailure using RoutineFailure::error (i.e., an error instance without an
underlying source) and asserts the formatted string from
format_infrastructure_routine_failure contains the message details and the error
text, thereby exercising the branch at line 433; place the new test alongside
format_infrastructure_routine_failure_preserves_details_and_error and reference
the same Message and format_infrastructure_routine_failure symbols so behavior
is locked in.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: a0d7b1b9-5519-4163-a1c7-a477e2d45aa0
📒 Files selected for processing (3)
apps/framework-cli/src/cli.rsapps/framework-cli/src/cli/routines/util.rsapps/framework-cli/src/utilities/docker.rs
📜 Review details
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: Test CLI (blacksmith-4vcpu-ubuntu-2404)
- GitHub Check: Test CLI (macos-latest-large)
- GitHub Check: Test TS Moose Lib (Node 22)
- GitHub Check: Check
- GitHub Check: Lints
- GitHub Check: Analyze (rust)
- GitHub Check: Build CLI Binaries (linux-arm64-glibc, ubuntu-22-8-core, aarch64-unknown-linux-gnu)
- GitHub Check: Build CLI Binaries (linux-x64-glibc, ubuntu-22-8-core, x86_64-unknown-linux-gnu)
- GitHub Check: Build CLI Binaries (darwin-arm64, macos-14-large, aarch64-apple-darwin)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.rs
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Run
cargo clippyto ensure Rust code passes Clippy's linting standards before each commit
**/*.rs: Usethiserrorwith#[derive(thiserror::Error)]for error handling in Rust; define errors near fallibility unit (NO globalErrortype); NEVER useanyhow::Result
Use snake_case for functions/variables, PascalCase for types/traits, SCREAMING_SNAKE_CASE for constants in Rust
Use tuple structs with validation constructors for newtypes in Rust (e.g.,struct UserId(String))
Write tests inline with#[cfg(test)]modules in Rust
Documentation is required for all public APIs in Rust
Runcargo clippy --all-targets -- -D warningspre-commit; no warnings allowed in Rust
Files:
apps/framework-cli/src/utilities/docker.rsapps/framework-cli/src/cli.rsapps/framework-cli/src/cli/routines/util.rs
apps/framework-cli/**/*.rs
📄 CodeRabbit inference engine (apps/framework-cli/AGENTS.md)
apps/framework-cli/**/*.rs: Always runcargo clippy --all-targets -- -D warningsbefore commits; fix all warnings - no Clippy warnings may remain (treat warnings as errors)
Userustfmt --edition 2021for consistent formatting
Prefer top-of-fileusestatements over inline ones in function bodies; adduseimports for fully-qualified paths; localusefor disambiguation (e.g., name collisions) is fine
Write meaningful names for functions, variables, and types
Keep functions focused and modular
Document all public APIs and breaking changes
Usethiserrorcrate for error handling instead ofanyhow::Result
Define errors near their unit of fallibility (no global Error types)
Use#[derive(thiserror::Error)]with#[error()]messages for error structs
Structure error types as: context struct + error enum +#[source]chaining
Define newtypes as tuple structs:struct UserId(u64);
Add validation constructors for newtypes:UserId::new(id: u64) -> Result<Self, Error>
Derive standard traits for newtypes:#[derive(Debug, Clone, PartialEq)]
ImplementFrom/TryFromfor newtype conversions
Usederive_moreornutypecrates to reduce boilerplate in newtype definitions
Useconstfor static values (prefer overstatic)
UseUPPER_SNAKE_CASEnaming for constants
Scope constant visibility:pub(crate)>pub(super)>pub
Group related constants together
Write unit tests for all public functions
Test error conditions and edge cases in unit and integration tests
Files:
apps/framework-cli/src/utilities/docker.rsapps/framework-cli/src/cli.rsapps/framework-cli/src/cli/routines/util.rs
**/framework-cli/src/**
⚙️ CodeRabbit configuration file
**/framework-cli/src/**: When reviewing changes to Moose CLI:
- Check if any user-facing changes were made (commands, flags, configs, apis, etc)
- If yes, verify the documentation for THAT SPECIFIC feature is updated in apps/framework-docs-v2/content
- If docs for that feature doesn't exist yet, it should be added. If the change removes public apis, the documentation for those should also be removed. Changing unrelated docs doesn't satisfy this requirement.
Files:
apps/framework-cli/src/utilities/docker.rsapps/framework-cli/src/cli.rsapps/framework-cli/src/cli/routines/util.rs
🧠 Learnings (23)
📓 Common learnings
Learnt from: callicles
Repo: 514-labs/moosestack PR: 4041
File: apps/framework-cli/src/cli/logger.rs:490-494
Timestamp: 2026-04-23T01:59:27.626Z
Learning: In 514-labs/moosestack `apps/framework-cli/src/cli/logger.rs`, recursive `sweep_logs` for nested `log_file_date_format` paths (containing `/`) was explicitly considered and rejected as scope creep in PR `#4041`. The PR's intended fix is only `create_dir_all(parent)` in `open_log_writer` to prevent panics when `~/.moose/` vanishes mid-run. If nested date-format paths are ever supported, the correct approach is config-load validation (reject `/` in `log_file_date_format`), not recursive log cleanup. Do not flag absence of recursive cleanup as a bug in this or similar PRs.
Learnt from: LucioFranco
Repo: 514-labs/moosestack PR: 3936
File: apps/framework-cli/src/cli/local_webserver.rs:3088-3090
Timestamp: 2026-04-09T15:11:17.424Z
Learning: In 514-labs/moosestack, `moose dev --alpha` (NativeInfraProvider) starts ONLY ClickHouse and Temporal as native binaries — no Docker containers are launched at all (Kafka/Redis native support is not yet implemented). Skipping Docker teardown wholesale in `apps/framework-cli/src/cli/local_webserver.rs` when `project.use_native_infra == true` is intentional and correct for the current scope. This will need revisiting when native Kafka/Redis support is added.
Learnt from: DatGuyJonathan
Repo: 514-labs/moosestack PR: 3400
File: apps/framework-cli/src/framework/core/infrastructure_map.rs:1238-1298
Timestamp: 2026-01-26T00:56:27.011Z
Learning: Repo 514-labs/moosestack — Workflows: The CLI’s workflow diff is intended to detect only Temporal schedule–affecting changes. In apps/framework-cli/src/framework/core/infrastructure_map.rs, workflows_config_equal should compare schedule, retries, and timeout only; it must not include tasks. Task code/config changes are picked up automatically when the orchestration worker restarts and should not trigger a WorkflowChange.
Learnt from: oatsandsugar
Repo: 514-labs/moosestack PR: 3462
File: apps/framework-cli/src/cli/routines/feedback.rs:107-119
Timestamp: 2026-02-06T01:42:40.104Z
Learning: In apps/framework-cli/src/cli/routines/**/*.rs (Moose CLI): Raw `println!` is acceptable for multi-line help text and formatted usage examples. The display wrapper (display::show_message_wrapper) is intended for status messages, not help text output.
Learnt from: LucioFranco
Repo: 514-labs/moosestack PR: 3932
File: apps/framework-cli/src/cli.rs:0-0
Timestamp: 2026-04-10T14:30:34.800Z
Learning: In 514-labs/moosestack, `moose clean` calling `shutdown_embedded_servers()` in `apps/framework-cli/src/cli.rs` only closes in-memory handles in the current process. It cannot stop embedded servers (devredis, devkafka, ClickHouse/Temporal) owned by a separate `moose dev --alpha` process. Cross-process cleanup via PID files or a control socket is a known architectural gap shared across all embedded servers, intentionally deferred and out of scope for individual feature PRs. Do not flag this as a bug requiring immediate fix.
📚 Learning: 2026-04-22T18:10:52.989Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: apps/framework-cli/AGENTS.md:0-0
Timestamp: 2026-04-22T18:10:52.989Z
Learning: Applies to apps/framework-cli/**/*.rs : Document all public APIs and breaking changes
Applied to files:
apps/framework-cli/src/utilities/docker.rsapps/framework-cli/src/cli.rs
📚 Learning: 2026-02-06T01:43:06.078Z
Learnt from: oatsandsugar
Repo: 514-labs/moosestack PR: 3462
File: apps/framework-cli/src/cli/routines/feedback.rs:40-72
Timestamp: 2026-02-06T01:43:06.078Z
Learning: In the Moose framework-cli (Rust), the telemetry functions capture_usage and wait_for_usage_capture use a fire-and-forget pattern: errors are logged internally and not surfaced to users. This behavior is intentional across all commands. Do not change error propagation for individual commands; any modification requires a broad refactor across the CLI telemetry layer. During reviews, treat changes to telemetry error reporting as high risk and scope changes to capture_usage/wait_for_usage_capture to the entire framework-cli, not single commands.
Applied to files:
apps/framework-cli/src/utilities/docker.rsapps/framework-cli/src/cli.rsapps/framework-cli/src/cli/routines/util.rs
📚 Learning: 2026-02-07T04:42:43.608Z
Learnt from: oatsandsugar
Repo: 514-labs/moosestack PR: 3468
File: apps/framework-cli/src/cli/routines/docs.rs:878-887
Timestamp: 2026-02-07T04:42:43.608Z
Learning: For the Moose CLI under apps/framework-cli, document and implement that Windows support is best-effort only, while macOS (open) and Linux (xdg-open) are the primary targets. Treat Windows-specific code paths (e.g., using cmd /c start) as fallback implementations and ensure they are not required to be as robust as the primary platforms. When adding Windows fallbacks, include clear caveats, avoid blocking on Windows-specific behavior, and add appropriate runtime checks, logs, and TODOs. This guidance applies to Rust source files under apps/framework-cli/src (and adjacent Rust files in the same module) to keep Windows handling clearly scoped and maintainable.
Applied to files:
apps/framework-cli/src/utilities/docker.rsapps/framework-cli/src/cli.rsapps/framework-cli/src/cli/routines/util.rs
📚 Learning: 2026-04-22T18:10:52.989Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: apps/framework-cli/AGENTS.md:0-0
Timestamp: 2026-04-22T18:10:52.989Z
Learning: Applies to apps/framework-cli/**/*.rs : Define errors near their unit of fallibility (no global Error types)
Applied to files:
apps/framework-cli/src/cli.rs
📚 Learning: 2026-04-22T18:10:52.989Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: apps/framework-cli/AGENTS.md:0-0
Timestamp: 2026-04-22T18:10:52.989Z
Learning: Applies to apps/framework-cli/**/*.rs : Use `thiserror` crate for error handling instead of `anyhow::Result`
Applied to files:
apps/framework-cli/src/cli.rsapps/framework-cli/src/cli/routines/util.rs
📚 Learning: 2026-04-22T18:10:52.989Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: apps/framework-cli/AGENTS.md:0-0
Timestamp: 2026-04-22T18:10:52.989Z
Learning: Applies to apps/framework-cli/**/*.rs : Test error conditions and edge cases in unit and integration tests
Applied to files:
apps/framework-cli/src/cli.rsapps/framework-cli/src/cli/routines/util.rs
📚 Learning: 2026-04-22T18:10:52.989Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: apps/framework-cli/AGENTS.md:0-0
Timestamp: 2026-04-22T18:10:52.989Z
Learning: Applies to apps/framework-cli/**/*.rs : Structure error types as: context struct + error enum + `#[source]` chaining
Applied to files:
apps/framework-cli/src/cli.rs
📚 Learning: 2026-01-26T01:14:58.758Z
Learnt from: DatGuyJonathan
Repo: 514-labs/moosestack PR: 3400
File: apps/framework-cli/src/framework/core/execute.rs:141-146
Timestamp: 2026-01-26T01:14:58.758Z
Learning: In apps/framework-cli/src/infrastructure/orchestration/workflows.rs, workflow scheduling failures should be non-fatal. The workflows::execute_changes function should return () and log errors rather than returning Result and propagating failures. Failing to schedule a workflow is not a reason to fail the deploy.
Applied to files:
apps/framework-cli/src/cli.rs
📚 Learning: 2026-02-06T01:42:40.104Z
Learnt from: oatsandsugar
Repo: 514-labs/moosestack PR: 3462
File: apps/framework-cli/src/cli/routines/feedback.rs:107-119
Timestamp: 2026-02-06T01:42:40.104Z
Learning: In apps/framework-cli/src/cli/routines/**/*.rs (Moose CLI): Raw `println!` is acceptable for multi-line help text and formatted usage examples. The display wrapper (display::show_message_wrapper) is intended for status messages, not help text output.
Applied to files:
apps/framework-cli/src/cli.rs
📚 Learning: 2026-04-22T18:10:52.989Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: apps/framework-cli/AGENTS.md:0-0
Timestamp: 2026-04-22T18:10:52.989Z
Learning: Applies to apps/framework-cli/**/*.rs : Use `#[derive(thiserror::Error)]` with `#[error()]` messages for error structs
Applied to files:
apps/framework-cli/src/cli.rs
📚 Learning: 2026-04-22T18:10:52.989Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: apps/framework-cli/AGENTS.md:0-0
Timestamp: 2026-04-22T18:10:52.989Z
Learning: Applies to apps/framework-cli/**/*.rs : Keep functions focused and modular
Applied to files:
apps/framework-cli/src/cli.rs
📚 Learning: 2026-03-09T21:19:28.424Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-09T21:19:28.424Z
Learning: Applies to **/*.rs : Use `thiserror` with `#[derive(thiserror::Error)]` for error handling in Rust; define errors near fallibility unit (NO global `Error` type); NEVER use `anyhow::Result`
Applied to files:
apps/framework-cli/src/cli.rs
📚 Learning: 2026-04-09T15:11:17.424Z
Learnt from: LucioFranco
Repo: 514-labs/moosestack PR: 3936
File: apps/framework-cli/src/cli/local_webserver.rs:3088-3090
Timestamp: 2026-04-09T15:11:17.424Z
Learning: In 514-labs/moosestack, `moose dev --alpha` (NativeInfraProvider) starts ONLY ClickHouse and Temporal as native binaries — no Docker containers are launched at all (Kafka/Redis native support is not yet implemented). Skipping Docker teardown wholesale in `apps/framework-cli/src/cli/local_webserver.rs` when `project.use_native_infra == true` is intentional and correct for the current scope. This will need revisiting when native Kafka/Redis support is added.
Applied to files:
apps/framework-cli/src/cli/routines/util.rs
📚 Learning: 2026-02-08T22:31:17.000Z
Learnt from: oatsandsugar
Repo: 514-labs/moosestack PR: 3468
File: apps/framework-cli/src/cli/routines/docs.rs:0-0
Timestamp: 2026-02-08T22:31:17.000Z
Learning: In Moose CLI (apps/framework-cli/src/cli/routines/...), when implementing raw output mode (--raw), ensure public entry points show_toc, fetch_page, search_toc, browse_docs return an empty Message (Message::new("".to_string(), "".to_string())) to avoid polluting piped output. Also wrap show_message! calls with if !raw guards to prevent intermediate messages from appearing in piped output. This pattern applies to all Rust files under apps/framework-cli/src/cli/routines/ and should be checked during reviews.
Applied to files:
apps/framework-cli/src/cli/routines/util.rs
📚 Learning: 2026-04-18T22:43:44.130Z
Learnt from: callicles
Repo: 514-labs/moosestack PR: 4024
File: apps/framework-cli/src/utilities/native_infra/preflight.rs:53-95
Timestamp: 2026-04-18T22:43:44.130Z
Learning: In `apps/framework-cli/src/utilities/native_infra/preflight.rs` (`PortConflictError::fmt`), `same_project_instance` uses `.any(|c| c.owner_pid.is_some())` intentionally. Only ClickHouse and Temporal write PID files; devredis, devkafka, http, management, and proxy_port ports are never attributed. Switching to `.all()` makes the check structurally unachievable for any realistic conflict set, causing the "another moose project" footer to always render even when attributed PIDs are present — a self-contradictory message. Do not suggest changing `.any()` to `.all()` here.
Applied to files:
apps/framework-cli/src/cli/routines/util.rs
📚 Learning: 2026-01-26T00:56:27.011Z
Learnt from: DatGuyJonathan
Repo: 514-labs/moosestack PR: 3400
File: apps/framework-cli/src/framework/core/infrastructure_map.rs:1238-1298
Timestamp: 2026-01-26T00:56:27.011Z
Learning: Repo 514-labs/moosestack — Workflows: The CLI’s workflow diff is intended to detect only Temporal schedule–affecting changes. In apps/framework-cli/src/framework/core/infrastructure_map.rs, workflows_config_equal should compare schedule, retries, and timeout only; it must not include tasks. Task code/config changes are picked up automatically when the orchestration worker restarts and should not trigger a WorkflowChange.
Applied to files:
apps/framework-cli/src/cli/routines/util.rs
📚 Learning: 2026-02-08T22:20:04.881Z
Learnt from: oatsandsugar
Repo: 514-labs/moosestack PR: 3468
File: apps/framework-cli/src/cli/routines/docs.rs:1590-1838
Timestamp: 2026-02-08T22:20:04.881Z
Learning: For HTTP-dependent functionality under apps/framework-cli/src/cli/routines (e.g., fetch_docs), prefer E2E or integration tests over unit tests. Unit tests for HTTP fetching can be flaky due to network variability and typically add less value than integration tests. When adding tests in this directory, categorize tests as integration/E2E where they exercise real HTTP behavior or client-server interactions; mock or stub HTTP only when necessary to test internal logic, not the HTTP layer itself.
Applied to files:
apps/framework-cli/src/cli/routines/util.rs
📚 Learning: 2026-04-12T12:58:18.735Z
Learnt from: 514Ben
Repo: 514-labs/moosestack PR: 3954
File: apps/framework-cli/src/framework/core/infrastructure_map.rs:2039-2042
Timestamp: 2026-04-12T12:58:18.735Z
Learning: 514-labs/moosestack — apps/framework-cli/src/framework/core/infrastructure_map.rs: InfrastructureMap::diff_dictionaries() uses dicts_equal_ignore_metadata(a, b) to ignore metadata when detecting dictionary updates. Treat metadata-only changes as no-ops in future reviews.
Applied to files:
apps/framework-cli/src/cli/routines/util.rs
📚 Learning: 2026-02-06T01:42:35.875Z
Learnt from: oatsandsugar
Repo: 514-labs/moosestack PR: 3462
File: apps/framework-cli/src/cli/routines/feedback.rs:107-119
Timestamp: 2026-02-06T01:42:35.875Z
Learning: In modules under apps/framework-cli/src/cli/routines (any Rust file under subdirectories) use raw println! for multi-line help text and formatted usage examples. Do not rely on display::show_message_wrapper for help output; reserve it for status messages and user-visible progress updates. Ensure help text remains readable and properly formatted.
Applied to files:
apps/framework-cli/src/cli/routines/util.rs
📚 Learning: 2026-04-14T16:21:40.959Z
Learnt from: 514Ben
Repo: 514-labs/moosestack PR: 3997
File: apps/framework-cli/src/framework/core/infrastructure_map.rs:0-0
Timestamp: 2026-04-14T16:21:40.959Z
Learning: Repo 514-labs/moosestack — apps/framework-cli/src/framework/core/infrastructure_map.rs: InfrastructureMap::diff_dictionaries() must gate UPDATE filtering on target_dict.life_cycle (not the current dict.life_cycle). ExternallyManaged blocks CREATE/UPDATE; DeletionProtected blocks DROP only.
Applied to files:
apps/framework-cli/src/cli/routines/util.rs
📚 Learning: 2026-02-07T04:42:46.739Z
Learnt from: oatsandsugar
Repo: 514-labs/moosestack PR: 3468
File: apps/framework-cli/src/cli/routines/docs.rs:40-48
Timestamp: 2026-02-07T04:42:46.739Z
Learning: In Rust files under apps/framework-cli/src/cli/routines/, prefer using inherent from_str methods for conversions when the conversion is only used in a single place and you are not using .parse() elsewhere. Avoid implementing std::str::FromStr purely for stylistic reasons if the conversion isn’t reused with .parse()."
Applied to files:
apps/framework-cli/src/cli/routines/util.rs
📚 Learning: 2026-02-07T06:10:34.807Z
Learnt from: oatsandsugar
Repo: 514-labs/moosestack PR: 3468
File: apps/framework-cli/src/cli/routines/docs.rs:896-942
Timestamp: 2026-02-07T06:10:34.807Z
Learning: In apps/framework-cli/src/cli/routines/**/*.rs (Rust code for Moose CLI), variable shadowing is acceptable and idiomatic when progressively refining a value (e.g., from &str to String or from raw input to processed output). Consider this a preferred pattern in this codebase, but ensure shadowing is intentional and improves readability rather than obscuring the data flow.
Applied to files:
apps/framework-cli/src/cli/routines/util.rs
🔇 Additional comments (4)
apps/framework-cli/src/utilities/docker.rs (1)
63-75: Accessor + test constructor are clean and scoped.
container_cli()stays read-only, andnew_for_testis correctly constrained to tests.apps/framework-cli/src/cli.rs (1)
427-435: Infrastructure error formatting change is solid.This preserves actionable routine context and renders multiline source errors clearly in both dev and prod paths.
Also applies to: 450-452, 750-751, 1033-1034
apps/framework-cli/src/cli/routines/util.rs (2)
4-19: Missing-CLI handling is clear and actionable.The
ErrorKind::NotFoundmapping is targeted, and the guidance covers install, config override, and dockerless fallback.Also applies to: 22-30
44-73: New unit test is focused and valuable.It asserts the exact user-facing guidance contract for the missing-container-runtime path.
… error Addresses CodeRabbit: lock None branch (RoutineFailure::error) for line 433. Made-with: Cursor
okane16
marked this pull request as ready for review
April 29, 2026 15:57
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/framework-cli/src/cli.rs`:
- Around line 2495-2496: Move the test-local `use` statements (e.g., `use
crate::cli::display::Message;`) out of individual test functions and place them
at the top of the `mod tests` test module so they are module-scoped; locate the
tests that construct `RoutineFailure::new` (references: RoutineFailure::new and
Message) and remove the inline `use` lines inside those functions (also apply
the same change for the other case around the second occurrence at the lines
noted) so the imports are declared once for the entire `mod tests`.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: e62b4893-db03-4725-92cf-2274a253681e
📒 Files selected for processing (1)
apps/framework-cli/src/cli.rs
📜 Review details
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (28)
- GitHub Check: Test E2E Prod Docker Mode
- GitHub Check: Test E2E TypeScript Default Template (Node 20, pnpm-10)
- GitHub Check: Test E2E TS Tests: db-pull (Node 22)
- GitHub Check: Test E2E TypeScript Default Template (Node 22, pnpm-10)
- GitHub Check: Test E2E TS Tests: core (Node 24)
- GitHub Check: Test E2E TypeScript Default Template (Node 24, npm)
- GitHub Check: Test E2E TypeScript Default Template (Node 24, pnpm-10)
- GitHub Check: Test E2E TS Tests: s3 (Node 22)
- GitHub Check: Test E2E TS Tests: s3 (Node 20)
- GitHub Check: Test E2E PY Tests: core (Python 3.12)
- GitHub Check: Test E2E TypeScript Default Template (Node 20, npm)
- GitHub Check: Test E2E PY Tests: s3 (Python 3.13)
- GitHub Check: Test E2E TS Tests: s3 (Node 24)
- GitHub Check: Test E2E TS Tests: db-pull (Node 24)
- GitHub Check: Test E2E TS Tests: core (Node 22)
- GitHub Check: Test E2E TS Tests: core (Node 20)
- GitHub Check: Test E2E PY Tests: core (Python 3.13)
- GitHub Check: Test E2E Cluster Support - Python (Python 3.13)
- GitHub Check: Test E2E PY Tests: s3 (Python 3.12)
- GitHub Check: Test E2E Backward Compatibility - Python (Python 3.13)
- GitHub Check: Test E2E OTLP Log Export (TypeScript)
- GitHub Check: Test E2E Backward Compatibility - TypeScript (Node 20)
- GitHub Check: Test E2E Cluster Support - TypeScript (Node 20)
- GitHub Check: Test E2E Python Default Template (Python 3.13)
- GitHub Check: Build CLI Binaries (darwin-arm64, macos-14-large, aarch64-apple-darwin)
- GitHub Check: Build CLI Binaries (linux-arm64-glibc, ubuntu-22-8-core, aarch64-unknown-linux-gnu)
- GitHub Check: Build CLI Binaries (linux-x64-glibc, ubuntu-22-8-core, x86_64-unknown-linux-gnu)
- GitHub Check: Analyze (rust)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.rs
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Run
cargo clippyto ensure Rust code passes Clippy's linting standards before each commit
**/*.rs: Usethiserrorwith#[derive(thiserror::Error)]for error handling in Rust; define errors near fallibility unit (NO globalErrortype); NEVER useanyhow::Result
Use snake_case for functions/variables, PascalCase for types/traits, SCREAMING_SNAKE_CASE for constants in Rust
Use tuple structs with validation constructors for newtypes in Rust (e.g.,struct UserId(String))
Write tests inline with#[cfg(test)]modules in Rust
Documentation is required for all public APIs in Rust
Runcargo clippy --all-targets -- -D warningspre-commit; no warnings allowed in Rust
Files:
apps/framework-cli/src/cli.rs
apps/framework-cli/**/*.rs
📄 CodeRabbit inference engine (apps/framework-cli/AGENTS.md)
apps/framework-cli/**/*.rs: Always runcargo clippy --all-targets -- -D warningsbefore commits; fix all warnings - no Clippy warnings may remain (treat warnings as errors)
Userustfmt --edition 2021for consistent formatting
Prefer top-of-fileusestatements over inline ones in function bodies; adduseimports for fully-qualified paths; localusefor disambiguation (e.g., name collisions) is fine
Write meaningful names for functions, variables, and types
Keep functions focused and modular
Document all public APIs and breaking changes
Usethiserrorcrate for error handling instead ofanyhow::Result
Define errors near their unit of fallibility (no global Error types)
Use#[derive(thiserror::Error)]with#[error()]messages for error structs
Structure error types as: context struct + error enum +#[source]chaining
Define newtypes as tuple structs:struct UserId(u64);
Add validation constructors for newtypes:UserId::new(id: u64) -> Result<Self, Error>
Derive standard traits for newtypes:#[derive(Debug, Clone, PartialEq)]
ImplementFrom/TryFromfor newtype conversions
Usederive_moreornutypecrates to reduce boilerplate in newtype definitions
Useconstfor static values (prefer overstatic)
UseUPPER_SNAKE_CASEnaming for constants
Scope constant visibility:pub(crate)>pub(super)>pub
Group related constants together
Write unit tests for all public functions
Test error conditions and edge cases in unit and integration tests
Files:
apps/framework-cli/src/cli.rs
**/framework-cli/src/**
⚙️ CodeRabbit configuration file
**/framework-cli/src/**: When reviewing changes to Moose CLI:
- Check if any user-facing changes were made (commands, flags, configs, apis, etc)
- If yes, verify the documentation for THAT SPECIFIC feature is updated in apps/framework-docs-v2/content
- If docs for that feature doesn't exist yet, it should be added. If the change removes public apis, the documentation for those should also be removed. Changing unrelated docs doesn't satisfy this requirement.
Files:
apps/framework-cli/src/cli.rs
🧠 Learnings (18)
📓 Common learnings
Learnt from: callicles
Repo: 514-labs/moosestack PR: 4041
File: apps/framework-cli/src/cli/logger.rs:490-494
Timestamp: 2026-04-23T01:59:27.626Z
Learning: In 514-labs/moosestack `apps/framework-cli/src/cli/logger.rs`, recursive `sweep_logs` for nested `log_file_date_format` paths (containing `/`) was explicitly considered and rejected as scope creep in PR `#4041`. The PR's intended fix is only `create_dir_all(parent)` in `open_log_writer` to prevent panics when `~/.moose/` vanishes mid-run. If nested date-format paths are ever supported, the correct approach is config-load validation (reject `/` in `log_file_date_format`), not recursive log cleanup. Do not flag absence of recursive cleanup as a bug in this or similar PRs.
Learnt from: LucioFranco
Repo: 514-labs/moosestack PR: 3936
File: apps/framework-cli/src/cli/local_webserver.rs:3088-3090
Timestamp: 2026-04-09T15:11:17.424Z
Learning: In 514-labs/moosestack, `moose dev --alpha` (NativeInfraProvider) starts ONLY ClickHouse and Temporal as native binaries — no Docker containers are launched at all (Kafka/Redis native support is not yet implemented). Skipping Docker teardown wholesale in `apps/framework-cli/src/cli/local_webserver.rs` when `project.use_native_infra == true` is intentional and correct for the current scope. This will need revisiting when native Kafka/Redis support is added.
Learnt from: DatGuyJonathan
Repo: 514-labs/moosestack PR: 3400
File: apps/framework-cli/src/framework/core/infrastructure_map.rs:1238-1298
Timestamp: 2026-01-26T00:56:27.011Z
Learning: Repo 514-labs/moosestack — Workflows: The CLI’s workflow diff is intended to detect only Temporal schedule–affecting changes. In apps/framework-cli/src/framework/core/infrastructure_map.rs, workflows_config_equal should compare schedule, retries, and timeout only; it must not include tasks. Task code/config changes are picked up automatically when the orchestration worker restarts and should not trigger a WorkflowChange.
Learnt from: LucioFranco
Repo: 514-labs/moosestack PR: 3932
File: apps/framework-cli/src/cli.rs:0-0
Timestamp: 2026-04-10T14:30:34.800Z
Learning: In 514-labs/moosestack, `moose clean` calling `shutdown_embedded_servers()` in `apps/framework-cli/src/cli.rs` only closes in-memory handles in the current process. It cannot stop embedded servers (devredis, devkafka, ClickHouse/Temporal) owned by a separate `moose dev --alpha` process. Cross-process cleanup via PID files or a control socket is a known architectural gap shared across all embedded servers, intentionally deferred and out of scope for individual feature PRs. Do not flag this as a bug requiring immediate fix.
Learnt from: oatsandsugar
Repo: 514-labs/moosestack PR: 3462
File: apps/framework-cli/src/cli/routines/feedback.rs:107-119
Timestamp: 2026-02-06T01:42:40.104Z
Learning: In apps/framework-cli/src/cli/routines/**/*.rs (Moose CLI): Raw `println!` is acceptable for multi-line help text and formatted usage examples. The display wrapper (display::show_message_wrapper) is intended for status messages, not help text output.
Learnt from: oatsandsugar
Repo: 514-labs/moosestack PR: 3468
File: apps/framework-cli/src/cli/routines/docs.rs:1590-1838
Timestamp: 2026-02-08T22:20:08.757Z
Learning: In apps/framework-cli/src/cli/routines/**/*.rs (Moose CLI): For HTTP-dependent functionality like `fetch_docs`, prefer E2E or integration tests over unit tests. Unit testing HTTP fetching adds little value compared to integration testing and can be flaky due to network dependencies.
Learnt from: oatsandsugar
Repo: 514-labs/moosestack PR: 3468
File: apps/framework-cli/src/cli/routines/docs.rs:0-0
Timestamp: 2026-02-08T22:31:22.237Z
Learning: In apps/framework-cli/src/cli/routines/**/*.rs (Moose CLI): When implementing raw output mode (--raw flag), public entry points (show_toc, fetch_page, search_toc, browse_docs) must return an empty Message (Message::new("".to_string(), "".to_string())) to prevent success messages from polluting piped output. Additionally, wrap show_message! calls in `if !raw` guards to keep intermediate messages from appearing in piped output.
Learnt from: DatGuyJonathan
Repo: 514-labs/moosestack PR: 3830
File: apps/framework-cli/src/framework/core/infrastructure_map.rs:596-598
Timestamp: 2026-03-24T19:10:26.494Z
Learning: Repo 514-labs/moosestack — Moose CLI: Row policies always reference existing tables. apps/framework-cli/src/framework/core/plan_validator.rs::validate_row_policy_columns rejects policies whose tables are missing or columns don’t exist. Therefore, InfrastructureMap::uses_olap() not checking select_row_policies is acceptable because any map with policies will also have non-empty tables.
Learnt from: callicles
Repo: 514-labs/moosestack PR: 4024
File: apps/framework-cli/src/utilities/native_infra/preflight.rs:53-95
Timestamp: 2026-04-18T22:43:44.130Z
Learning: In `apps/framework-cli/src/utilities/native_infra/preflight.rs` (`PortConflictError::fmt`), `same_project_instance` uses `.any(|c| c.owner_pid.is_some())` intentionally. Only ClickHouse and Temporal write PID files; devredis, devkafka, http, management, and proxy_port ports are never attributed. Switching to `.all()` makes the check structurally unachievable for any realistic conflict set, causing the "another moose project" footer to always render even when attributed PIDs are present — a self-contradictory message. Do not suggest changing `.any()` to `.all()` here.
📚 Learning: 2026-04-22T18:10:52.989Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: apps/framework-cli/AGENTS.md:0-0
Timestamp: 2026-04-22T18:10:52.989Z
Learning: Applies to apps/framework-cli/**/*.rs : Use `thiserror` crate for error handling instead of `anyhow::Result`
Applied to files:
apps/framework-cli/src/cli.rs
📚 Learning: 2026-04-22T18:10:52.989Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: apps/framework-cli/AGENTS.md:0-0
Timestamp: 2026-04-22T18:10:52.989Z
Learning: Applies to apps/framework-cli/**/*.rs : Define errors near their unit of fallibility (no global Error types)
Applied to files:
apps/framework-cli/src/cli.rs
📚 Learning: 2026-02-06T01:42:40.104Z
Learnt from: oatsandsugar
Repo: 514-labs/moosestack PR: 3462
File: apps/framework-cli/src/cli/routines/feedback.rs:107-119
Timestamp: 2026-02-06T01:42:40.104Z
Learning: In apps/framework-cli/src/cli/routines/**/*.rs (Moose CLI): Raw `println!` is acceptable for multi-line help text and formatted usage examples. The display wrapper (display::show_message_wrapper) is intended for status messages, not help text output.
Applied to files:
apps/framework-cli/src/cli.rs
📚 Learning: 2026-04-22T18:10:52.989Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: apps/framework-cli/AGENTS.md:0-0
Timestamp: 2026-04-22T18:10:52.989Z
Learning: Applies to apps/framework-cli/**/*.rs : Structure error types as: context struct + error enum + `#[source]` chaining
Applied to files:
apps/framework-cli/src/cli.rs
📚 Learning: 2026-04-22T18:10:52.989Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: apps/framework-cli/AGENTS.md:0-0
Timestamp: 2026-04-22T18:10:52.989Z
Learning: Applies to apps/framework-cli/**/*.rs : Test error conditions and edge cases in unit and integration tests
Applied to files:
apps/framework-cli/src/cli.rs
📚 Learning: 2026-04-22T18:10:52.989Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: apps/framework-cli/AGENTS.md:0-0
Timestamp: 2026-04-22T18:10:52.989Z
Learning: Applies to apps/framework-cli/**/*.rs : Use `#[derive(thiserror::Error)]` with `#[error()]` messages for error structs
Applied to files:
apps/framework-cli/src/cli.rs
📚 Learning: 2026-02-07T04:42:50.188Z
Learnt from: oatsandsugar
Repo: 514-labs/moosestack PR: 3468
File: apps/framework-cli/src/cli/routines/docs.rs:40-48
Timestamp: 2026-02-07T04:42:50.188Z
Learning: In apps/framework-cli/src/cli/routines/**/*.rs (Moose CLI Rust code): Prefer inherent `from_str` methods over implementing `std::str::FromStr` trait when the conversion is only used in one place and not used with `.parse()` syntax—stylistic preference.
Applied to files:
apps/framework-cli/src/cli.rs
📚 Learning: 2026-04-22T18:10:52.989Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: apps/framework-cli/AGENTS.md:0-0
Timestamp: 2026-04-22T18:10:52.989Z
Learning: Applies to apps/framework-cli/**/*.rs : Write meaningful names for functions, variables, and types
Applied to files:
apps/framework-cli/src/cli.rs
📚 Learning: 2026-04-22T18:10:52.989Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: apps/framework-cli/AGENTS.md:0-0
Timestamp: 2026-04-22T18:10:52.989Z
Learning: Applies to apps/framework-cli/**/*.rs : Document all public APIs and breaking changes
Applied to files:
apps/framework-cli/src/cli.rs
📚 Learning: 2026-03-09T21:19:28.424Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-09T21:19:28.424Z
Learning: Applies to **/*.rs : Use `thiserror` with `#[derive(thiserror::Error)]` for error handling in Rust; define errors near fallibility unit (NO global `Error` type); NEVER use `anyhow::Result`
Applied to files:
apps/framework-cli/src/cli.rs
📚 Learning: 2026-04-22T18:10:52.989Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: apps/framework-cli/AGENTS.md:0-0
Timestamp: 2026-04-22T18:10:52.989Z
Learning: Applies to apps/framework-cli/**/*.rs : Write unit tests for all public functions
Applied to files:
apps/framework-cli/src/cli.rs
📚 Learning: 2026-03-24T19:15:00.795Z
Learnt from: DatGuyJonathan
Repo: 514-labs/moosestack PR: 3830
File: apps/framework-cli/src/infrastructure/olap/clickhouse/mod.rs:1638-1644
Timestamp: 2026-03-24T19:15:00.795Z
Learning: In `apps/framework-cli/src/infrastructure/olap/clickhouse/mod.rs`, `rls_bootstrap` (public) and `parse_row_policy_filter` (private) are intentionally covered by E2E tests in `apps/framework-cli-e2e/test/row-level-security.test.ts` (the "Row-Level Security" suite) rather than by inline `#[cfg(test)]` unit tests. Do not request additional unit tests for these functions in this file.
Applied to files:
apps/framework-cli/src/cli.rs
📚 Learning: 2026-03-24T19:13:18.829Z
Learnt from: DatGuyJonathan
Repo: 514-labs/moosestack PR: 3830
File: apps/framework-cli/src/cli/routines/migrate.rs:363-366
Timestamp: 2026-03-24T19:13:18.829Z
Learning: In `apps/framework-cli/src/cli/routines/migrate.rs`, the `CreateRowPolicy`/`DropRowPolicy` branches in `validate_table_databases_and_clusters` (which skip cluster validation) are intentionally covered by E2E tests in `apps/framework-cli-e2e/test/row-level-security.test.ts` rather than by dedicated unit tests in `migrate.rs`. Do not request additional unit tests for these branches.
Applied to files:
apps/framework-cli/src/cli.rs
📚 Learning: 2026-03-24T19:14:21.620Z
Learnt from: DatGuyJonathan
Repo: 514-labs/moosestack PR: 3830
File: apps/framework-cli/src/framework/core/plan.rs:876-884
Timestamp: 2026-03-24T19:14:21.620Z
Learning: In `apps/framework-cli/src/framework/core/plan.rs`, the `MockOlapClient::list_row_policies` stub in the `#[cfg(test)]` module intentionally returns `Ok(vec![])`. The row-policy reconciliation branches inside `reconcile_with_reality()` are covered by E2E tests in `apps/framework-cli-e2e/test/row-level-security.test.ts` rather than by additional unit tests in `plan.rs`. Do not request extra unit tests for these branches in this file.
Applied to files:
apps/framework-cli/src/cli.rs
📚 Learning: 2026-04-09T15:11:59.525Z
Learnt from: LucioFranco
Repo: 514-labs/moosestack PR: 3936
File: apps/framework-cli/src/utilities/native_infra/clickhouse.rs:223-245
Timestamp: 2026-04-09T15:11:59.525Z
Learning: In `apps/framework-cli/src/utilities/native_infra/clickhouse.rs`, `clickhouse_download_url()` uses `unreachable!()` as its final else branch. `detect_platform()` in `binary_manager.rs` returns `NativeInfraError::UnsupportedPlatform` for unsupported OS/arch combinations before artifact selection is reached, so the unreachable branch is correct. Do not flag the final else as a silent fallback.
Applied to files:
apps/framework-cli/src/cli.rs
📚 Learning: 2026-02-06T01:43:06.078Z
Learnt from: oatsandsugar
Repo: 514-labs/moosestack PR: 3462
File: apps/framework-cli/src/cli/routines/feedback.rs:40-72
Timestamp: 2026-02-06T01:43:06.078Z
Learning: In the Moose framework-cli (Rust), the telemetry functions capture_usage and wait_for_usage_capture use a fire-and-forget pattern: errors are logged internally and not surfaced to users. This behavior is intentional across all commands. Do not change error propagation for individual commands; any modification requires a broad refactor across the CLI telemetry layer. During reviews, treat changes to telemetry error reporting as high risk and scope changes to capture_usage/wait_for_usage_capture to the entire framework-cli, not single commands.
Applied to files:
apps/framework-cli/src/cli.rs
📚 Learning: 2026-02-07T04:42:43.608Z
Learnt from: oatsandsugar
Repo: 514-labs/moosestack PR: 3468
File: apps/framework-cli/src/cli/routines/docs.rs:878-887
Timestamp: 2026-02-07T04:42:43.608Z
Learning: For the Moose CLI under apps/framework-cli, document and implement that Windows support is best-effort only, while macOS (open) and Linux (xdg-open) are the primary targets. Treat Windows-specific code paths (e.g., using cmd /c start) as fallback implementations and ensure they are not required to be as robust as the primary platforms. When adding Windows fallbacks, include clear caveats, avoid blocking on Windows-specific behavior, and add appropriate runtime checks, logs, and TODOs. This guidance applies to Rust source files under apps/framework-cli/src (and adjacent Rust files in the same module) to keep Windows handling clearly scoped and maintainable.
Applied to files:
apps/framework-cli/src/cli.rs
🔇 Additional comments (4)
apps/framework-cli/src/cli.rs (4)
427-435: Formatter behavior is solid and readable.Good extraction. This keeps routine action/details and only appends the source block when present, which avoids noisy output in no-source failures.
450-451: Error mapping now preserves failure context correctly.Routing
run_local_infrastructurefailures through the formatter before converting toanyhowaligns with the intended UX improvement.
2493-2526: Test coverage for both formatter branches looks good.Nice addition: both
Some(err)andNonepaths are now locked with assertions tailored to expected output structure.
750-751: Update troubleshooting docs to document the "Local infrastructure could not start" error.The error message at lines 750-751 and 1033-1034 is user-facing and not currently documented. While
apps/framework-docs-v2/content/moosestack/help/troubleshooting.mdxcovers dev infrastructure startup issues, it should explicitly mention this error message and guide users to interpret the detailed error output shown via the{e:#}formatting.
HDIBen
reviewed
Apr 29, 2026
HDIBen
previously requested changes
Apr 29, 2026
- Move Message import to mod tests scope (CodeRabbit) - Mention nerdctl in install-options bullet (514Ben) Made-with: Cursor
callicles
approved these changes
Apr 29, 2026
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/framework-cli/src/cli.rs (1)
427-452:⚠️ Potential issue | 🟠 MajorDon't flatten the local-infra failure into a string before returning it.
format_infrastructure_routine_failurerenders the text correctly, butanyhow!(...)turns it into a string-only error, so the original source chain is lost before the caller formats{e:#}. That defeats the stated goal of preserving underlying error context.Keep the structured error intact until the final display boundary, or wrap it in a typed error that preserves
source().Also applies to: 747-752, 1030-1035
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/framework-cli/src/cli.rs` around lines 427 - 452, The current code turns RoutineFailure into a plain string via anyhow!(format_infrastructure_routine_failure(&e)), which drops the original error chain; instead preserve the source by returning an anyhow::Error that wraps the RoutineFailure and attaches the formatted message as context (e.g. use anyhow::Error::new(e).context(format_infrastructure_routine_failure(&e)) or anyhow::Context::with_context) in run_local_infrastructure_with_timeout (the spawn_blocking map_err closure) and the other similar map_err sites so the underlying RoutineFailure remains the source and the pretty formatter can still use {e:#}.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@apps/framework-cli/src/cli.rs`:
- Around line 427-452: The current code turns RoutineFailure into a plain string
via anyhow!(format_infrastructure_routine_failure(&e)), which drops the original
error chain; instead preserve the source by returning an anyhow::Error that
wraps the RoutineFailure and attaches the formatted message as context (e.g. use
anyhow::Error::new(e).context(format_infrastructure_routine_failure(&e)) or
anyhow::Context::with_context) in run_local_infrastructure_with_timeout (the
spawn_blocking map_err closure) and the other similar map_err sites so the
underlying RoutineFailure remains the source and the pretty formatter can still
use {e:#}.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 60a50fbd-dc88-42ff-bb2b-eb17dfbca967
📒 Files selected for processing (2)
apps/framework-cli/src/cli.rsapps/framework-cli/src/cli/routines/util.rs
📜 Review details
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Build CLI Binaries (linux-arm64-glibc, ubuntu-22-8-core, aarch64-unknown-linux-gnu)
- GitHub Check: Build CLI Binaries (linux-x64-glibc, ubuntu-22-8-core, x86_64-unknown-linux-gnu)
- GitHub Check: Package and Publish Independant TS Package
- GitHub Check: Build CLI Binaries (darwin-arm64, macos-14-large, aarch64-apple-darwin)
- GitHub Check: Test CLI (macos-latest-large)
- GitHub Check: Cursor Bugbot
- GitHub Check: Analyze (rust)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.rs
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Run
cargo clippyto ensure Rust code passes Clippy's linting standards before each commit
**/*.rs: Usethiserrorwith#[derive(thiserror::Error)]for error handling in Rust; define errors near fallibility unit (NO globalErrortype); NEVER useanyhow::Result
Use snake_case for functions/variables, PascalCase for types/traits, SCREAMING_SNAKE_CASE for constants in Rust
Use tuple structs with validation constructors for newtypes in Rust (e.g.,struct UserId(String))
Write tests inline with#[cfg(test)]modules in Rust
Documentation is required for all public APIs in Rust
Runcargo clippy --all-targets -- -D warningspre-commit; no warnings allowed in Rust
Files:
apps/framework-cli/src/cli/routines/util.rsapps/framework-cli/src/cli.rs
apps/framework-cli/**/*.rs
📄 CodeRabbit inference engine (apps/framework-cli/AGENTS.md)
apps/framework-cli/**/*.rs: Always runcargo clippy --all-targets -- -D warningsbefore commits; fix all warnings - no Clippy warnings may remain (treat warnings as errors)
Userustfmt --edition 2021for consistent formatting
Prefer top-of-fileusestatements over inline ones in function bodies; adduseimports for fully-qualified paths; localusefor disambiguation (e.g., name collisions) is fine
Write meaningful names for functions, variables, and types
Keep functions focused and modular
Document all public APIs and breaking changes
Usethiserrorcrate for error handling instead ofanyhow::Result
Define errors near their unit of fallibility (no global Error types)
Use#[derive(thiserror::Error)]with#[error()]messages for error structs
Structure error types as: context struct + error enum +#[source]chaining
Define newtypes as tuple structs:struct UserId(u64);
Add validation constructors for newtypes:UserId::new(id: u64) -> Result<Self, Error>
Derive standard traits for newtypes:#[derive(Debug, Clone, PartialEq)]
ImplementFrom/TryFromfor newtype conversions
Usederive_moreornutypecrates to reduce boilerplate in newtype definitions
Useconstfor static values (prefer overstatic)
UseUPPER_SNAKE_CASEnaming for constants
Scope constant visibility:pub(crate)>pub(super)>pub
Group related constants together
Write unit tests for all public functions
Test error conditions and edge cases in unit and integration tests
Files:
apps/framework-cli/src/cli/routines/util.rsapps/framework-cli/src/cli.rs
**/framework-cli/src/**
⚙️ CodeRabbit configuration file
**/framework-cli/src/**: When reviewing changes to Moose CLI:
- Check if any user-facing changes were made (commands, flags, configs, apis, etc)
- If yes, verify the documentation for THAT SPECIFIC feature is updated in apps/framework-docs-v2/content
- If docs for that feature doesn't exist yet, it should be added. If the change removes public apis, the documentation for those should also be removed. Changing unrelated docs doesn't satisfy this requirement.
Files:
apps/framework-cli/src/cli/routines/util.rsapps/framework-cli/src/cli.rs
🧠 Learnings (32)
📓 Common learnings
Learnt from: callicles
Repo: 514-labs/moosestack PR: 4041
File: apps/framework-cli/src/cli/logger.rs:490-494
Timestamp: 2026-04-23T01:59:27.626Z
Learning: In 514-labs/moosestack `apps/framework-cli/src/cli/logger.rs`, recursive `sweep_logs` for nested `log_file_date_format` paths (containing `/`) was explicitly considered and rejected as scope creep in PR `#4041`. The PR's intended fix is only `create_dir_all(parent)` in `open_log_writer` to prevent panics when `~/.moose/` vanishes mid-run. If nested date-format paths are ever supported, the correct approach is config-load validation (reject `/` in `log_file_date_format`), not recursive log cleanup. Do not flag absence of recursive cleanup as a bug in this or similar PRs.
Learnt from: DatGuyJonathan
Repo: 514-labs/moosestack PR: 3400
File: apps/framework-cli/src/framework/core/infrastructure_map.rs:1238-1298
Timestamp: 2026-01-26T00:56:27.011Z
Learning: Repo 514-labs/moosestack — Workflows: The CLI’s workflow diff is intended to detect only Temporal schedule–affecting changes. In apps/framework-cli/src/framework/core/infrastructure_map.rs, workflows_config_equal should compare schedule, retries, and timeout only; it must not include tasks. Task code/config changes are picked up automatically when the orchestration worker restarts and should not trigger a WorkflowChange.
Learnt from: LucioFranco
Repo: 514-labs/moosestack PR: 3936
File: apps/framework-cli/src/cli/local_webserver.rs:3088-3090
Timestamp: 2026-04-09T15:11:17.424Z
Learning: In 514-labs/moosestack, `moose dev --alpha` (NativeInfraProvider) starts ONLY ClickHouse and Temporal as native binaries — no Docker containers are launched at all (Kafka/Redis native support is not yet implemented). Skipping Docker teardown wholesale in `apps/framework-cli/src/cli/local_webserver.rs` when `project.use_native_infra == true` is intentional and correct for the current scope. This will need revisiting when native Kafka/Redis support is added.
Learnt from: oatsandsugar
Repo: 514-labs/moosestack PR: 3468
File: apps/framework-cli/src/cli/routines/docs.rs:0-0
Timestamp: 2026-02-08T22:31:22.237Z
Learning: In apps/framework-cli/src/cli/routines/**/*.rs (Moose CLI): When implementing raw output mode (--raw flag), public entry points (show_toc, fetch_page, search_toc, browse_docs) must return an empty Message (Message::new("".to_string(), "".to_string())) to prevent success messages from polluting piped output. Additionally, wrap show_message! calls in `if !raw` guards to keep intermediate messages from appearing in piped output.
Learnt from: LucioFranco
Repo: 514-labs/moosestack PR: 3932
File: apps/framework-cli/src/cli.rs:0-0
Timestamp: 2026-04-10T14:30:34.800Z
Learning: In 514-labs/moosestack, `moose clean` calling `shutdown_embedded_servers()` in `apps/framework-cli/src/cli.rs` only closes in-memory handles in the current process. It cannot stop embedded servers (devredis, devkafka, ClickHouse/Temporal) owned by a separate `moose dev --alpha` process. Cross-process cleanup via PID files or a control socket is a known architectural gap shared across all embedded servers, intentionally deferred and out of scope for individual feature PRs. Do not flag this as a bug requiring immediate fix.
Learnt from: oatsandsugar
Repo: 514-labs/moosestack PR: 3462
File: apps/framework-cli/src/cli/routines/feedback.rs:107-119
Timestamp: 2026-02-06T01:42:40.104Z
Learning: In apps/framework-cli/src/cli/routines/**/*.rs (Moose CLI): Raw `println!` is acceptable for multi-line help text and formatted usage examples. The display wrapper (display::show_message_wrapper) is intended for status messages, not help text output.
Learnt from: oatsandsugar
Repo: 514-labs/moosestack PR: 3468
File: apps/framework-cli/src/cli/routines/docs.rs:1590-1838
Timestamp: 2026-02-08T22:20:08.757Z
Learning: In apps/framework-cli/src/cli/routines/**/*.rs (Moose CLI): For HTTP-dependent functionality like `fetch_docs`, prefer E2E or integration tests over unit tests. Unit testing HTTP fetching adds little value compared to integration testing and can be flaky due to network dependencies.
Learnt from: phiSgr
Repo: 514-labs/moosestack PR: 3930
File: apps/framework-cli/src/cli/local_webserver.rs:3524-3535
Timestamp: 2026-04-08T00:08:36.251Z
Learning: In 514-labs/moosestack, the `/admin/inframap` and related `/admin/*` endpoints (e.g., `/admin/plan`, `/admin/integrate-changes`, `/admin/reality-check`) in `apps/framework-cli/src/cli/local_webserver.rs` are internal CLI-to-server communication endpoints. They are consumed programmatically by CLI routines (e.g., migration, remote-plan), not by end users. Do not flag missing framework-docs-v2 documentation for changes to these admin endpoints — the coding guideline requiring docs updates applies only to user-facing APIs, commands, flags, and configs.
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: apps/framework-cli/AGENTS.md:0-0
Timestamp: 2026-04-22T18:10:52.989Z
Learning: Applies to apps/framework-cli/**/*.rs : Use `thiserror` crate for error handling instead of `anyhow::Result`
📚 Learning: 2026-04-22T18:10:52.989Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: apps/framework-cli/AGENTS.md:0-0
Timestamp: 2026-04-22T18:10:52.989Z
Learning: Applies to apps/framework-cli/**/*.rs : Test error conditions and edge cases in unit and integration tests
Applied to files:
apps/framework-cli/src/cli/routines/util.rsapps/framework-cli/src/cli.rs
📚 Learning: 2026-04-09T15:11:17.424Z
Learnt from: LucioFranco
Repo: 514-labs/moosestack PR: 3936
File: apps/framework-cli/src/cli/local_webserver.rs:3088-3090
Timestamp: 2026-04-09T15:11:17.424Z
Learning: In 514-labs/moosestack, `moose dev --alpha` (NativeInfraProvider) starts ONLY ClickHouse and Temporal as native binaries — no Docker containers are launched at all (Kafka/Redis native support is not yet implemented). Skipping Docker teardown wholesale in `apps/framework-cli/src/cli/local_webserver.rs` when `project.use_native_infra == true` is intentional and correct for the current scope. This will need revisiting when native Kafka/Redis support is added.
Applied to files:
apps/framework-cli/src/cli/routines/util.rs
📚 Learning: 2026-02-08T22:31:17.000Z
Learnt from: oatsandsugar
Repo: 514-labs/moosestack PR: 3468
File: apps/framework-cli/src/cli/routines/docs.rs:0-0
Timestamp: 2026-02-08T22:31:17.000Z
Learning: In Moose CLI (apps/framework-cli/src/cli/routines/...), when implementing raw output mode (--raw), ensure public entry points show_toc, fetch_page, search_toc, browse_docs return an empty Message (Message::new("".to_string(), "".to_string())) to avoid polluting piped output. Also wrap show_message! calls with if !raw guards to prevent intermediate messages from appearing in piped output. This pattern applies to all Rust files under apps/framework-cli/src/cli/routines/ and should be checked during reviews.
Applied to files:
apps/framework-cli/src/cli/routines/util.rs
📚 Learning: 2026-04-18T22:43:44.130Z
Learnt from: callicles
Repo: 514-labs/moosestack PR: 4024
File: apps/framework-cli/src/utilities/native_infra/preflight.rs:53-95
Timestamp: 2026-04-18T22:43:44.130Z
Learning: In `apps/framework-cli/src/utilities/native_infra/preflight.rs` (`PortConflictError::fmt`), `same_project_instance` uses `.any(|c| c.owner_pid.is_some())` intentionally. Only ClickHouse and Temporal write PID files; devredis, devkafka, http, management, and proxy_port ports are never attributed. Switching to `.all()` makes the check structurally unachievable for any realistic conflict set, causing the "another moose project" footer to always render even when attributed PIDs are present — a self-contradictory message. Do not suggest changing `.any()` to `.all()` here.
Applied to files:
apps/framework-cli/src/cli/routines/util.rs
📚 Learning: 2026-04-09T15:11:59.525Z
Learnt from: LucioFranco
Repo: 514-labs/moosestack PR: 3936
File: apps/framework-cli/src/utilities/native_infra/clickhouse.rs:223-245
Timestamp: 2026-04-09T15:11:59.525Z
Learning: In `apps/framework-cli/src/utilities/native_infra/clickhouse.rs`, `clickhouse_download_url()` uses `unreachable!()` as its final else branch. `detect_platform()` in `binary_manager.rs` returns `NativeInfraError::UnsupportedPlatform` for unsupported OS/arch combinations before artifact selection is reached, so the unreachable branch is correct. Do not flag the final else as a silent fallback.
Applied to files:
apps/framework-cli/src/cli/routines/util.rsapps/framework-cli/src/cli.rs
📚 Learning: 2026-02-06T01:42:35.875Z
Learnt from: oatsandsugar
Repo: 514-labs/moosestack PR: 3462
File: apps/framework-cli/src/cli/routines/feedback.rs:107-119
Timestamp: 2026-02-06T01:42:35.875Z
Learning: In modules under apps/framework-cli/src/cli/routines (any Rust file under subdirectories) use raw println! for multi-line help text and formatted usage examples. Do not rely on display::show_message_wrapper for help output; reserve it for status messages and user-visible progress updates. Ensure help text remains readable and properly formatted.
Applied to files:
apps/framework-cli/src/cli/routines/util.rs
📚 Learning: 2026-04-18T22:28:28.229Z
Learnt from: callicles
Repo: 514-labs/moosestack PR: 4024
File: apps/framework-cli/src/utilities/native_infra/mod.rs:524-550
Timestamp: 2026-04-18T22:28:28.229Z
Learning: In `apps/framework-cli/src/utilities/native_infra/mod.rs` (514-labs/moosestack), `kill_pid_file` uses `kill -0` to poll liveness after SIGTERM. Zombie state is unreachable here: ClickHouse and Temporal are spawned with `kill_on_drop(false)` and the `Child` handle is never retained across CLI invocations. A subsequent `moose dev`/`moose clean` is a different process, so the kernel reparents the orphaned child to PID 1 (launchd/init), which reaps zombies immediately. Do not suggest replacing `kill -0` with `ps -o stat=` zombie detection — it adds parsing overhead for a scenario that cannot occur.
Applied to files:
apps/framework-cli/src/cli/routines/util.rs
📚 Learning: 2026-01-26T00:56:27.011Z
Learnt from: DatGuyJonathan
Repo: 514-labs/moosestack PR: 3400
File: apps/framework-cli/src/framework/core/infrastructure_map.rs:1238-1298
Timestamp: 2026-01-26T00:56:27.011Z
Learning: Repo 514-labs/moosestack — Workflows: The CLI’s workflow diff is intended to detect only Temporal schedule–affecting changes. In apps/framework-cli/src/framework/core/infrastructure_map.rs, workflows_config_equal should compare schedule, retries, and timeout only; it must not include tasks. Task code/config changes are picked up automatically when the orchestration worker restarts and should not trigger a WorkflowChange.
Applied to files:
apps/framework-cli/src/cli/routines/util.rs
📚 Learning: 2026-02-08T22:20:04.881Z
Learnt from: oatsandsugar
Repo: 514-labs/moosestack PR: 3468
File: apps/framework-cli/src/cli/routines/docs.rs:1590-1838
Timestamp: 2026-02-08T22:20:04.881Z
Learning: For HTTP-dependent functionality under apps/framework-cli/src/cli/routines (e.g., fetch_docs), prefer E2E or integration tests over unit tests. Unit tests for HTTP fetching can be flaky due to network variability and typically add less value than integration tests. When adding tests in this directory, categorize tests as integration/E2E where they exercise real HTTP behavior or client-server interactions; mock or stub HTTP only when necessary to test internal logic, not the HTTP layer itself.
Applied to files:
apps/framework-cli/src/cli/routines/util.rs
📚 Learning: 2026-04-22T18:10:52.989Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: apps/framework-cli/AGENTS.md:0-0
Timestamp: 2026-04-22T18:10:52.989Z
Learning: Applies to apps/framework-cli/**/*.rs : Use `thiserror` crate for error handling instead of `anyhow::Result`
Applied to files:
apps/framework-cli/src/cli/routines/util.rsapps/framework-cli/src/cli.rs
📚 Learning: 2026-02-07T04:42:46.739Z
Learnt from: oatsandsugar
Repo: 514-labs/moosestack PR: 3468
File: apps/framework-cli/src/cli/routines/docs.rs:40-48
Timestamp: 2026-02-07T04:42:46.739Z
Learning: In Rust files under apps/framework-cli/src/cli/routines/, prefer using inherent from_str methods for conversions when the conversion is only used in a single place and you are not using .parse() elsewhere. Avoid implementing std::str::FromStr purely for stylistic reasons if the conversion isn’t reused with .parse()."
Applied to files:
apps/framework-cli/src/cli/routines/util.rs
📚 Learning: 2026-02-07T06:10:34.807Z
Learnt from: oatsandsugar
Repo: 514-labs/moosestack PR: 3468
File: apps/framework-cli/src/cli/routines/docs.rs:896-942
Timestamp: 2026-02-07T06:10:34.807Z
Learning: In apps/framework-cli/src/cli/routines/**/*.rs (Rust code for Moose CLI), variable shadowing is acceptable and idiomatic when progressively refining a value (e.g., from &str to String or from raw input to processed output). Consider this a preferred pattern in this codebase, but ensure shadowing is intentional and improves readability rather than obscuring the data flow.
Applied to files:
apps/framework-cli/src/cli/routines/util.rs
📚 Learning: 2026-02-06T01:43:06.078Z
Learnt from: oatsandsugar
Repo: 514-labs/moosestack PR: 3462
File: apps/framework-cli/src/cli/routines/feedback.rs:40-72
Timestamp: 2026-02-06T01:43:06.078Z
Learning: In the Moose framework-cli (Rust), the telemetry functions capture_usage and wait_for_usage_capture use a fire-and-forget pattern: errors are logged internally and not surfaced to users. This behavior is intentional across all commands. Do not change error propagation for individual commands; any modification requires a broad refactor across the CLI telemetry layer. During reviews, treat changes to telemetry error reporting as high risk and scope changes to capture_usage/wait_for_usage_capture to the entire framework-cli, not single commands.
Applied to files:
apps/framework-cli/src/cli/routines/util.rsapps/framework-cli/src/cli.rs
📚 Learning: 2026-02-07T04:42:43.608Z
Learnt from: oatsandsugar
Repo: 514-labs/moosestack PR: 3468
File: apps/framework-cli/src/cli/routines/docs.rs:878-887
Timestamp: 2026-02-07T04:42:43.608Z
Learning: For the Moose CLI under apps/framework-cli, document and implement that Windows support is best-effort only, while macOS (open) and Linux (xdg-open) are the primary targets. Treat Windows-specific code paths (e.g., using cmd /c start) as fallback implementations and ensure they are not required to be as robust as the primary platforms. When adding Windows fallbacks, include clear caveats, avoid blocking on Windows-specific behavior, and add appropriate runtime checks, logs, and TODOs. This guidance applies to Rust source files under apps/framework-cli/src (and adjacent Rust files in the same module) to keep Windows handling clearly scoped and maintainable.
Applied to files:
apps/framework-cli/src/cli/routines/util.rsapps/framework-cli/src/cli.rs
📚 Learning: 2026-04-22T18:10:52.989Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: apps/framework-cli/AGENTS.md:0-0
Timestamp: 2026-04-22T18:10:52.989Z
Learning: Applies to apps/framework-cli/**/*.rs : Define errors near their unit of fallibility (no global Error types)
Applied to files:
apps/framework-cli/src/cli.rs
📚 Learning: 2026-02-06T01:42:40.104Z
Learnt from: oatsandsugar
Repo: 514-labs/moosestack PR: 3462
File: apps/framework-cli/src/cli/routines/feedback.rs:107-119
Timestamp: 2026-02-06T01:42:40.104Z
Learning: In apps/framework-cli/src/cli/routines/**/*.rs (Moose CLI): Raw `println!` is acceptable for multi-line help text and formatted usage examples. The display wrapper (display::show_message_wrapper) is intended for status messages, not help text output.
Applied to files:
apps/framework-cli/src/cli.rs
📚 Learning: 2026-04-22T18:10:52.989Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: apps/framework-cli/AGENTS.md:0-0
Timestamp: 2026-04-22T18:10:52.989Z
Learning: Applies to apps/framework-cli/**/*.rs : Use `#[derive(thiserror::Error)]` with `#[error()]` messages for error structs
Applied to files:
apps/framework-cli/src/cli.rs
📚 Learning: 2026-04-22T18:10:52.989Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: apps/framework-cli/AGENTS.md:0-0
Timestamp: 2026-04-22T18:10:52.989Z
Learning: Applies to apps/framework-cli/**/*.rs : Structure error types as: context struct + error enum + `#[source]` chaining
Applied to files:
apps/framework-cli/src/cli.rs
📚 Learning: 2026-02-07T04:42:50.188Z
Learnt from: oatsandsugar
Repo: 514-labs/moosestack PR: 3468
File: apps/framework-cli/src/cli/routines/docs.rs:40-48
Timestamp: 2026-02-07T04:42:50.188Z
Learning: In apps/framework-cli/src/cli/routines/**/*.rs (Moose CLI Rust code): Prefer inherent `from_str` methods over implementing `std::str::FromStr` trait when the conversion is only used in one place and not used with `.parse()` syntax—stylistic preference.
Applied to files:
apps/framework-cli/src/cli.rs
📚 Learning: 2026-03-09T21:19:28.424Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-09T21:19:28.424Z
Learning: Applies to **/*.rs : Use `thiserror` with `#[derive(thiserror::Error)]` for error handling in Rust; define errors near fallibility unit (NO global `Error` type); NEVER use `anyhow::Result`
Applied to files:
apps/framework-cli/src/cli.rs
📚 Learning: 2026-01-26T01:14:58.758Z
Learnt from: DatGuyJonathan
Repo: 514-labs/moosestack PR: 3400
File: apps/framework-cli/src/framework/core/execute.rs:141-146
Timestamp: 2026-01-26T01:14:58.758Z
Learning: In apps/framework-cli/src/infrastructure/orchestration/workflows.rs, workflow scheduling failures should be non-fatal. The workflows::execute_changes function should return () and log errors rather than returning Result and propagating failures. Failing to schedule a workflow is not a reason to fail the deploy.
Applied to files:
apps/framework-cli/src/cli.rs
📚 Learning: 2026-04-22T18:10:52.989Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: apps/framework-cli/AGENTS.md:0-0
Timestamp: 2026-04-22T18:10:52.989Z
Learning: Applies to apps/framework-cli/**/*.rs : Use `rustfmt --edition 2021` for consistent formatting
Applied to files:
apps/framework-cli/src/cli.rs
📚 Learning: 2026-04-22T18:10:52.989Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: apps/framework-cli/AGENTS.md:0-0
Timestamp: 2026-04-22T18:10:52.989Z
Learning: Applies to apps/framework-cli/**/*.rs : Write unit tests for all public functions
Applied to files:
apps/framework-cli/src/cli.rs
📚 Learning: 2026-03-24T19:15:00.795Z
Learnt from: DatGuyJonathan
Repo: 514-labs/moosestack PR: 3830
File: apps/framework-cli/src/infrastructure/olap/clickhouse/mod.rs:1638-1644
Timestamp: 2026-03-24T19:15:00.795Z
Learning: In `apps/framework-cli/src/infrastructure/olap/clickhouse/mod.rs`, `rls_bootstrap` (public) and `parse_row_policy_filter` (private) are intentionally covered by E2E tests in `apps/framework-cli-e2e/test/row-level-security.test.ts` (the "Row-Level Security" suite) rather than by inline `#[cfg(test)]` unit tests. Do not request additional unit tests for these functions in this file.
Applied to files:
apps/framework-cli/src/cli.rs
📚 Learning: 2026-03-24T19:13:18.829Z
Learnt from: DatGuyJonathan
Repo: 514-labs/moosestack PR: 3830
File: apps/framework-cli/src/cli/routines/migrate.rs:363-366
Timestamp: 2026-03-24T19:13:18.829Z
Learning: In `apps/framework-cli/src/cli/routines/migrate.rs`, the `CreateRowPolicy`/`DropRowPolicy` branches in `validate_table_databases_and_clusters` (which skip cluster validation) are intentionally covered by E2E tests in `apps/framework-cli-e2e/test/row-level-security.test.ts` rather than by dedicated unit tests in `migrate.rs`. Do not request additional unit tests for these branches.
Applied to files:
apps/framework-cli/src/cli.rs
📚 Learning: 2026-03-24T19:14:21.620Z
Learnt from: DatGuyJonathan
Repo: 514-labs/moosestack PR: 3830
File: apps/framework-cli/src/framework/core/plan.rs:876-884
Timestamp: 2026-03-24T19:14:21.620Z
Learning: In `apps/framework-cli/src/framework/core/plan.rs`, the `MockOlapClient::list_row_policies` stub in the `#[cfg(test)]` module intentionally returns `Ok(vec![])`. The row-policy reconciliation branches inside `reconcile_with_reality()` are covered by E2E tests in `apps/framework-cli-e2e/test/row-level-security.test.ts` rather than by additional unit tests in `plan.rs`. Do not request extra unit tests for these branches in this file.
Applied to files:
apps/framework-cli/src/cli.rs
📚 Learning: 2026-04-22T18:10:52.989Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: apps/framework-cli/AGENTS.md:0-0
Timestamp: 2026-04-22T18:10:52.989Z
Learning: Applies to apps/framework-cli/**/*.rs : Prefer top-of-file `use` statements over inline ones in function bodies; add `use` imports for fully-qualified paths; local `use` for disambiguation (e.g., name collisions) is fine
Applied to files:
apps/framework-cli/src/cli.rs
📚 Learning: 2026-04-22T18:10:52.989Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: apps/framework-cli/AGENTS.md:0-0
Timestamp: 2026-04-22T18:10:52.989Z
Learning: Applies to apps/framework-cli/**/*.rs : Keep functions focused and modular
Applied to files:
apps/framework-cli/src/cli.rs
📚 Learning: 2026-03-09T21:19:28.424Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-09T21:19:28.424Z
Learning: Applies to **/*.rs : Write tests inline with `#[cfg(test)]` modules in Rust
Applied to files:
apps/framework-cli/src/cli.rs
📚 Learning: 2026-02-08T22:20:08.757Z
Learnt from: oatsandsugar
Repo: 514-labs/moosestack PR: 3468
File: apps/framework-cli/src/cli/routines/docs.rs:1590-1838
Timestamp: 2026-02-08T22:20:08.757Z
Learning: In apps/framework-cli/src/cli/routines/**/*.rs (Moose CLI): For HTTP-dependent functionality like `fetch_docs`, prefer E2E or integration tests over unit tests. Unit testing HTTP fetching adds little value compared to integration testing and can be flaky due to network dependencies.
Applied to files:
apps/framework-cli/src/cli.rs
📚 Learning: 2026-04-22T18:10:52.989Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: apps/framework-cli/AGENTS.md:0-0
Timestamp: 2026-04-22T18:10:52.989Z
Learning: Applies to apps/framework-cli/**/tests/**/*.rs : Use integration tests for CLI commands
Applied to files:
apps/framework-cli/src/cli.rs
🔇 Additional comments (2)
apps/framework-cli/src/cli/routines/util.rs (1)
4-72: Looks good.The missing-CLI path is handled cleanly, and the test covers the new guidance text without introducing any obvious regression risk.
apps/framework-cli/src/cli.rs (1)
2496-2527: Good coverage on both formatter branches.The two tests lock in the with-source and without-source formatting behavior, which is exactly what changed here.
okane16
dismissed
HDIBen’s stale review
April 29, 2026 16:08
Addressed comments and got Nico's approval
okane16
enabled auto-merge
April 29, 2026 16:08
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Doc comment omits nerdctl from supported CLI list
- Updated the doc comment on container_cli() method to include nerdctl alongside docker and finch, making it consistent with the user-facing error messages.
Or push these changes by commenting:
@cursor push 4367d1e663
Preview (4367d1e663)
diff --git a/apps/framework-cli/src/utilities/docker.rs b/apps/framework-cli/src/utilities/docker.rs
--- a/apps/framework-cli/src/utilities/docker.rs
+++ b/apps/framework-cli/src/utilities/docker.rs
@@ -60,7 +60,7 @@
Self { cli_command }
}
- /// Returns the configured container runtime executable name or path (e.g. `docker`, `finch`).
+ /// Returns the configured container runtime executable name or path (e.g. `docker`, `finch`, `nerdctl`).
#[must_use]
pub fn container_cli(&self) -> &str {
&self.cli_commandYou can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit af66ea1. Configure here.
Made-with: Cursor
|
Found 2 test failures on Blacksmith runners: Failures
|
Contributor
Graphite Automations"engineering review - commercial [copy]" took an action on this PR • (04/29/26)1 reviewer was added to this PR based on Mitchell Suzuki's automation. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Made-with: Cursor
Note
Low Risk
Primarily adjusts error formatting/propagation and adds tests; behavior changes are limited to user-facing messages when Docker/container runtime startup fails.
Overview
Improves CLI infrastructure startup failures by preserving
RoutineFailuredetails (and any source error) when converting toanyhow, and surfaces multi-line, user-friendly messages indev/prodwhen local infrastructure cannot start.Adds explicit guidance when the configured container runtime CLI is missing (
ErrorKind::NotFound), including install/config options and a--dockerlesshint;DockerClientnow exposes the configured CLI viacontainer_cli()plus a test-only constructor. Includes new unit tests covering both the infra error formatting helper and missing-container-CLI messaging.Reviewed by Cursor Bugbot for commit 671a7f0. Bugbot is set up for automated code reviews on this repo. Configure here.