Merged
Conversation
|
The latest updates on your projects. Learn more about Boreal.
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdded preflight port conflict detection during native dev startup, retry logic for consumption API upstream requests (targeting connection-only failures), and a circuit-breaker for rapid process restart failures. Integrated port specs validation, enhanced process termination, and improved error attribution to guide users toward conflict resolution. Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI (start)
participant Preflight as Preflight<br/>Port Check
participant NativeInfra as NativeInfra<br/>Provider
participant Services as Services<br/>(Redis/Kafka/<br/>ClickHouse/Temporal)
participant User as User
CLI->>Preflight: check_ports(specs, native_dir)
activate Preflight
Preflight->>Preflight: bind TcpListener to each port
alt Port Conflict Detected
Preflight->>Preflight: attribute PIDs from .moose/native_infra/*.pid
Preflight->>Preflight: format conflict error with guidance
Preflight-->>NativeInfra: Err(PortConflictError)
else All Ports Free
Preflight-->>NativeInfra: Ok(())
end
deactivate Preflight
alt Conflict Error
NativeInfra-->>User: RoutineFailure (with suggested fix)
else Success
NativeInfra->>Services: spawn devredis, devkafka, etc.
activate Services
Services-->>NativeInfra: success
deactivate Services
NativeInfra-->>CLI: ready
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 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. Comment |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![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: 11
🤖 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/utilities/native_infra/preflight.rs`:
- Line 113: The probe uses TcpListener::bind((spec.host, spec.port)) which will
miss a service bound only to IPv6 loopback (::1); update the preflight to also
try binding the IPv6 loopback when spec.host is "127.0.0.1" (or when probing
localhost) by attempting a secondary bind to ("::1", spec.port) and treating
either bind success as port-in-use, or add a short comment explaining this
limitation; reference TcpListener::bind, spec.host and spec.port when locating
where to add the additional "::1" probe or the clarifying comment.
- Around line 16-31: PortSpec's field declaration order (port, service, host)
doesn't match the new() parameter order (host, port, service), which is a
footgun; update new() so its parameters mirror the struct field order (e.g.,
new(port: u16, service: &'static str, host: &'static str)) or change the struct
declaration to match the existing parameter order, and ensure the new()
constructor uses explicit field names in the returned PortSpec (port:, service:,
host:) referencing the PortSpec type and its new() method so future callers are
unambiguous.
- Around line 53-95: The current Display impl for PortConflictError uses
same_project_instance (computed as self.conflicts.iter().any(|c|
c.owner_pid.is_some())) which over-triggers when only some conflicts are
attributed; update the heuristic in the fmt implementation so it requires all
conflicts to be attributed (use all(|c| c.owner_pid.is_some())) or instead print
per-conflict guidance: for each Conflict report whether it is "owned by prior
moose dev (PID ...)" or "held by another process (stop it)"; adjust the footer
logic in the fmt implementation of PortConflictError to rely on the new
all-attributed check or to emit a mixed-summary when some are attributed and
some are not, referencing the same display code paths and the
same_project_instance variable.
- Around line 195-215: project.clickhouse_config fields (host_port, native_port,
keeper_port, keeper_raft_port) are i32 and are being cast to u16 in the
PortSpec::new calls causing silent truncation; replace the direct casts with
validated conversions (e.g. use
u16::try_from(project.clickhouse_config.host_port) etc.) and handle the Result
by returning or bubbling a clear error from the preflight routine when a port is
out of range (or change the ClickHouseConfig port types to u16 and update
callers). Ensure all four uses (host_port, native_port, keeper_port,
keeper_raft_port) are validated before constructing PortSpec::new so
invalid/negative or >65535 values produce an explicit error instead of
truncating.
In `@apps/framework-cli/src/utilities/system.rs`:
- Around line 266-315: The test relies on hardcoded backoff constants inside
RestartingProcess::create which forces long real sleeps; add a RestartConfig
(fields: initial_delay, max_delay, min_runtime_for_reset,
max_consecutive_rapid_failures) with Default mapping to the current constants
and a new constructor create_with_config(name, start_fn, policy, cfg) (keep
existing create delegating to Default::default()), then modify the test to call
create_with_config with millisecond-scale durations so the breaker trips quickly
(and optionally tighten the assertion on calls); update references to the
circuit-breaker logic inside RestartingProcess (where
INITIAL_DELAY_MS/MAX_DELAY_MS/MIN_RUNTIME_FOR_RESET/MAX_CONSECUTIVE_RAPID_FAILURES
are used) to read from the RestartConfig instance instead of consts.
In `@Cross.toml`:
- Around line 12-15: Replace the mutable ":main" image reference with an
immutable digest-pinned reference to ensure reproducible builds: locate the
image line (image = "ghcr.io/cross-rs/aarch64-unknown-linux-gnu:main") in
Cross.toml and replace the tag with the corresponding `@sha256`:<digest> string
obtained from the GHCR registry for that exact image/version, so the build
always uses the same Ubuntu/OpenSSL binary instead of the mutable :main tag.
- Line 31: The current symlink command ("ln -sfn /usr/include/openssl
/usr/aarch64-linux-gnu/include/openssl") misses the target-specific
opensslconf.h; update the Cross.toml step that creates the OpenSSL include
symlink to also create or copy a link for the architecture-specific config
header (opensslconf.h) from the multiarch path (e.g.,
/usr/include/aarch64-linux-gnu/openssl/opensslconf.h) into the target include
dir (e.g., /usr/aarch64-linux-gnu/include/openssl/opensslconf.h), ensuring the
target-specific header exists in the sysroot before building.
- Line 9: The passthrough list currently forwards host OPENSSL_LIB_DIR and
OPENSSL_INCLUDE_DIR which can leak host native paths into container builds;
change the entries in the passthrough array (symbol: passthrough) to use
target-qualified variables AARCH64_UNKNOWN_LINUX_GNU_OPENSSL_LIB_DIR and
AARCH64_UNKNOWN_LINUX_GNU_OPENSSL_INCLUDE_DIR (keep OPENSSL_STATIC or replace
with AARCH64_UNKNOWN_LINUX_GNU_OPENSSL_STATIC if you need target-specific static
selection) so the ARM64 build uses target-specific OpenSSL env vars rather than
host values.
In `@packages/ts-moose-lib/src/consumption-apis/runner.ts`:
- Around line 620-635: The current server "error" handler (attached via
server.on("error", ...)) causes any post-listen socket error to exit the worker;
change this to only cover the listen/startup phase by making the handler
temporary: either use server.once("error", ...) for the startup attach and/or
install the handler and then remove it inside the server.on("listening", ...)
callback (or downgrade to calling workerStop there) so runtime socket errors
after listening don't trigger process.exit(1); reference the existing server
error handler and the "listening" event and ensure process.exit(1) remains only
for startup failures like EADDRINUSE.
- Around line 637-644: The change from server.listen(port, ...) to omitting the
host inadvertently binds to all interfaces (dual‑stack ::) and exposes the dev
service; revert to an explicit loopback bind by calling server.listen with the
loopback host (e.g., "127.0.0.1" or "localhost") in the runner where
server.listen is invoked so the consumption API remains local only, keeping the
existing cluster/RoundRobinHandle usage intact; update the server.listen
invocation in the same function that defines server and uses port to include the
loopback host argument and preserve the callback logging.
🪄 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: 9dae8bc0-0fcf-4cdc-9269-eaa7927f2aae
⛔ Files ignored due to path filters (2)
Cargo.lockis excluded by!**/*.lockpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (8)
Cross.tomlapps/framework-cli/Cargo.tomlapps/framework-cli/src/cli/local_webserver.rsapps/framework-cli/src/utilities/native_infra/errors.rsapps/framework-cli/src/utilities/native_infra/mod.rsapps/framework-cli/src/utilities/native_infra/preflight.rsapps/framework-cli/src/utilities/system.rspackages/ts-moose-lib/src/consumption-apis/runner.ts
📜 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: 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: Package and Publish Independant TS Package
- GitHub Check: Test CLI (blacksmith-4vcpu-ubuntu-2404)
- GitHub Check: Lints
- GitHub Check: Check
- GitHub Check: Test CLI (macos-latest-large)
- GitHub Check: Cursor Bugbot
- GitHub Check: Analyze (rust)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Run linting checks before submitting PRs for TypeScript/JavaScript code
**/*.{ts,tsx,js,jsx}: Group imports by external deps, internal modules, types; use named exports from barrel files (index.ts)
Use camelCase for variable and function names, PascalCase for types/classes/components, UPPER_SNAKE_CASE for constants in TypeScript/JavaScript
Prefix unused variables with_(e.g.,_unusedParam) to bypass linting errors in TypeScript/JavaScript
Format with Prettier usingexperimentalTernaries: true; auto-formats on commit via Husky + lint-staged in TypeScript/JavaScript
ESLint extends Next.js, Turbo, TypeScript recommended;@typescript-eslint/no-explicit-anydisabled
Files:
packages/ts-moose-lib/src/consumption-apis/runner.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Prefer interfaces for objects, types for unions/intersections; explicit return types on public APIs in TypeScript
Files:
packages/ts-moose-lib/src/consumption-apis/runner.ts
**/ts-moose-lib/src/**/*.ts
⚙️ CodeRabbit configuration file
**/ts-moose-lib/src/**/*.ts: When reviewing changes to typescript moose lib:
- Check if any public apis were changed (class, method, type, config, 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:
packages/ts-moose-lib/src/consumption-apis/runner.ts
**/*.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/local_webserver.rsapps/framework-cli/src/utilities/native_infra/mod.rsapps/framework-cli/src/utilities/native_infra/errors.rsapps/framework-cli/src/utilities/system.rsapps/framework-cli/src/utilities/native_infra/preflight.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
Write meaningful names for functions, variables, and types
Document all public APIs and breaking changes
Usethiserrorcrate instead ofanyhow::Resultfor error handling
Define errors near their unit of fallibility (no global Error types)
Use#[derive(thiserror::Error)]with#[error()]messages for error definitions
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_moreornutypeto reduce newtype boilerplate
Useconstfor static values (prefer overstatic)
UseUPPER_SNAKE_CASEnaming for constants
Scope constant visibility with preference:pub(crate)>pub(super)>pub
Group related constants together
Write unit tests for all public functions
Test error conditions and edge cases
Files:
apps/framework-cli/src/cli/local_webserver.rsapps/framework-cli/src/utilities/native_infra/mod.rsapps/framework-cli/src/utilities/native_infra/errors.rsapps/framework-cli/src/utilities/system.rsapps/framework-cli/src/utilities/native_infra/preflight.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/local_webserver.rsapps/framework-cli/src/utilities/native_infra/mod.rsapps/framework-cli/src/utilities/native_infra/errors.rsapps/framework-cli/src/utilities/system.rsapps/framework-cli/src/utilities/native_infra/preflight.rs
🧠 Learnings (27)
📓 Common learnings
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: 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: CR
Repo: 514-labs/moosestack PR: 0
File: templates/typescript-agent/AGENTS.md:0-0
Timestamp: 2026-04-01T07:34:38.401Z
Learning: Verify ports 3000, 4000, 5001, 7233, 8080, 9000, and 18123 are free and update `packages/moosestack-service/moose.config.toml` if needed
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: CR
Repo: 514-labs/moosestack PR: 0
File: templates/next-app-empty/AGENTS.md:0-0
Timestamp: 2026-03-31T03:45:31.128Z
Learning: Start the Moose backend before the Next.js frontend during development
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-09T21:19:28.424Z
Learning: When changing MooseStack functionality, ALWAYS run end-to-end tests
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: templates/typescript-mcp/AGENTS.md:0-0
Timestamp: 2026-03-31T03:47:14.870Z
Learning: Applies to templates/typescript-mcp/**/moose.config.toml : Verify ports 3000, 4000, 5001, 7233, 8080, 9000, and 18123 are free before development, or update moose.config.toml
📚 Learning: 2025-12-16T23:09:10.917Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: apps/framework-cli/AGENTS.md:0-0
Timestamp: 2025-12-16T23:09:10.917Z
Learning: Applies to apps/framework-cli/**/*.rs : Document all public APIs and breaking changes
Applied to files:
apps/framework-cli/Cargo.tomlapps/framework-cli/src/utilities/native_infra/mod.rs
📚 Learning: 2025-12-16T23:09:10.917Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: apps/framework-cli/AGENTS.md:0-0
Timestamp: 2025-12-16T23:09:10.917Z
Learning: Applies to apps/framework-cli/**/*.rs : Always run `cargo clippy --all-targets -- -D warnings` before commits; fix all warnings - no Clippy warnings may remain (treat warnings as errors)
Applied to files:
apps/framework-cli/Cargo.toml
📚 Learning: 2025-12-16T23:09:10.917Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: apps/framework-cli/AGENTS.md:0-0
Timestamp: 2025-12-16T23:09:10.917Z
Learning: Applies to apps/framework-cli/**/*.rs : Use `rustfmt --edition 2021` for consistent formatting
Applied to files:
apps/framework-cli/Cargo.toml
📚 Learning: 2026-03-31T03:46:50.753Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: templates/typescript-express/AGENTS.md:0-0
Timestamp: 2026-03-31T03:46:50.753Z
Learning: Applies to templates/typescript-express/**/moose.config.toml : Configure port and service settings in `moose.config.toml` instead of hardcoding them
Applied to files:
packages/ts-moose-lib/src/consumption-apis/runner.tsapps/framework-cli/src/utilities/native_infra/preflight.rs
📚 Learning: 2026-03-31T03:46:59.509Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: templates/typescript-fastify/AGENTS.md:0-0
Timestamp: 2026-03-31T03:46:59.509Z
Learning: Applies to templates/typescript-fastify/**/moose.config.toml : Configure port and service settings in `moose.config.toml`
Applied to files:
packages/ts-moose-lib/src/consumption-apis/runner.tsapps/framework-cli/src/utilities/native_infra/preflight.rs
📚 Learning: 2026-04-01T07:34:51.710Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: templates/typescript-agent/packages/moosestack-service/AGENTS.md:0-0
Timestamp: 2026-04-01T07:34:51.710Z
Learning: Applies to templates/typescript-agent/packages/moosestack-service/app/index.ts : Export Moose-discovered primitives in `app/index.ts` as the public service entrypoint
Applied to files:
packages/ts-moose-lib/src/consumption-apis/runner.ts
📚 Learning: 2026-04-01T07:34:38.401Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: templates/typescript-agent/AGENTS.md:0-0
Timestamp: 2026-04-01T07:34:38.401Z
Learning: Applies to templates/typescript-agent/packages/moosestack-service/app/mcp/tools/**/*.ts : Return user-friendly error messages in MCP tool responses and do not expose internal error details or stack traces
Applied to files:
packages/ts-moose-lib/src/consumption-apis/runner.ts
📚 Learning: 2026-03-31T03:47:14.870Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: templates/typescript-mcp/AGENTS.md:0-0
Timestamp: 2026-03-31T03:47:14.870Z
Learning: Applies to templates/typescript-mcp/**/moose.config.toml : Verify ports 3000, 4000, 5001, 7233, 8080, 9000, and 18123 are free before development, or update moose.config.toml
Applied to files:
packages/ts-moose-lib/src/consumption-apis/runner.tsapps/framework-cli/src/utilities/native_infra/preflight.rs
📚 Learning: 2026-04-01T07:34:38.401Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: templates/typescript-agent/AGENTS.md:0-0
Timestamp: 2026-04-01T07:34:38.401Z
Learning: Applies to templates/typescript-agent/packages/moosestack-service/app/mcp/tools/**/*.ts : Return errors in MCP tools via `{ content: [...], isError: true }` instead of throwing exceptions
Applied to files:
packages/ts-moose-lib/src/consumption-apis/runner.ts
📚 Learning: 2026-04-01T07:34:38.401Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: templates/typescript-agent/AGENTS.md:0-0
Timestamp: 2026-04-01T07:34:38.401Z
Learning: Applies to templates/typescript-agent/packages/moosestack-service/app/index.ts : Export new data models, APIs, and MCP tools from `packages/moosestack-service/app/index.ts` so MooseStack discovers them automatically
Applied to files:
packages/ts-moose-lib/src/consumption-apis/runner.ts
📚 Learning: 2026-03-31T03:46:39.081Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: templates/typescript-empty/AGENTS.md:0-0
Timestamp: 2026-03-31T03:46:39.081Z
Learning: Applies to templates/typescript-empty/**/moose.config.toml : Configure port and service settings in `moose.config.toml` — default ports used: 4000, 5001, 7233, 8080, 9000, 18123
Applied to files:
packages/ts-moose-lib/src/consumption-apis/runner.tsapps/framework-cli/src/utilities/native_infra/preflight.rs
📚 Learning: 2026-03-31T03:47:14.870Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: templates/typescript-mcp/AGENTS.md:0-0
Timestamp: 2026-03-31T03:47:14.870Z
Learning: Applies to templates/typescript-mcp/packages/moosestack-service/app/index.ts : Export new primitives (data models, APIs, MCP tools) from app/index.ts so MooseStack discovers them automatically
Applied to files:
packages/ts-moose-lib/src/consumption-apis/runner.ts
📚 Learning: 2026-03-31T03:46:50.753Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: templates/typescript-express/AGENTS.md:0-0
Timestamp: 2026-03-31T03:46:50.753Z
Learning: Applies to templates/typescript-express/app/index.ts : Export all new primitives (data models, pipelines, APIs, workflows) from `app/index.ts` to ensure MooseStack discovery
Applied to files:
packages/ts-moose-lib/src/consumption-apis/runner.ts
📚 Learning: 2026-03-14T08:20:02.295Z
Learnt from: phiSgr
Repo: 514-labs/moosestack PR: 3694
File: apps/framework-cli/src/cli/local_webserver.rs:2035-2035
Timestamp: 2026-03-14T08:20:02.295Z
Learning: In 514-labs/moosestack, WebApps (TypeScript SDK, `packages/ts-moose-lib/src/dmv2/sdk/webApp.ts`) enforce that `mountPath` cannot be `"/"`. Therefore, the `GET /` route in `apps/framework-cli/src/cli/local_webserver.rs` can safely be handled before the WebApp proxy fallback without risk of shadowing a root-mounted WebApp.
Applied to files:
apps/framework-cli/src/cli/local_webserver.rs
📚 Learning: 2026-02-18T00:09:56.326Z
Learnt from: DatGuyJonathan
Repo: 514-labs/moosestack PR: 3596
File: apps/framework-cli/src/infrastructure/olap/clickhouse/queries.rs:376-381
Timestamp: 2026-02-18T00:09:56.326Z
Learning: In apps/framework-cli/src/infrastructure/olap/clickhouse/queries.rs, maintain inline // comments for ClickhouseEngine variants and their fields (no /// rustdoc) to match the file’s existing style unless a repo-wide doc-style change is undertaken.
Applied to files:
apps/framework-cli/src/cli/local_webserver.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/local_webserver.rsapps/framework-cli/src/utilities/native_infra/mod.rsapps/framework-cli/src/utilities/native_infra/errors.rsapps/framework-cli/src/utilities/system.rsapps/framework-cli/src/utilities/native_infra/preflight.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/local_webserver.rsapps/framework-cli/src/utilities/native_infra/mod.rsapps/framework-cli/src/utilities/native_infra/errors.rsapps/framework-cli/src/utilities/system.rsapps/framework-cli/src/utilities/native_infra/preflight.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/utilities/native_infra/mod.rsapps/framework-cli/src/utilities/native_infra/preflight.rs
📚 Learning: 2026-04-10T14:28:37.548Z
Learnt from: LucioFranco
Repo: 514-labs/moosestack PR: 3932
File: apps/devredis/src/lib.rs:18-22
Timestamp: 2026-04-10T14:28:37.548Z
Learning: In 514-labs/moosestack, apps/devredis uses a crate-wide `pub type Error = Box<dyn std::error::Error + Send + Sync>` and `pub type Result<T>` alias intentionally. It is a dev-only embedded Redis server and the boxed error is acceptable here; do not flag it for thiserror/typed-error refactoring.
Applied to files:
apps/framework-cli/src/utilities/native_infra/mod.rs
📚 Learning: 2026-04-10T14:28:45.452Z
Learnt from: LucioFranco
Repo: 514-labs/moosestack PR: 3932
File: apps/devredis/tests/integration.rs:0-0
Timestamp: 2026-04-10T14:28:45.452Z
Learning: In `apps/devredis/tests/integration.rs` (514-labs/moosestack), `handle.abort()` is intentionally used to stop the embedded devredis test server. Graceful shutdown via `server.shutdown()` is deliberately omitted from test helpers because it adds complexity with no test correctness benefit. Do not suggest restructuring test helpers to add graceful shutdown signals.
Applied to files:
apps/framework-cli/src/utilities/system.rsapps/framework-cli/src/utilities/native_infra/preflight.rs
📚 Learning: 2025-12-16T23:08:31.521Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-16T23:08:31.521Z
Learning: Build Rust components using `cargo build` before integration
Applied to files:
Cross.toml
📚 Learning: 2026-04-01T07:34:38.401Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: templates/typescript-agent/AGENTS.md:0-0
Timestamp: 2026-04-01T07:34:38.401Z
Learning: Verify ports 3000, 4000, 5001, 7233, 8080, 9000, and 18123 are free and update `packages/moosestack-service/moose.config.toml` if needed
Applied to files:
apps/framework-cli/src/utilities/native_infra/preflight.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/utilities/native_infra/preflight.rs
📚 Learning: 2026-04-10T15:29:00.915Z
Learnt from: LucioFranco
Repo: 514-labs/moosestack PR: 3933
File: apps/framework-cli/src/utilities/native_infra/devkafka.rs:21-39
Timestamp: 2026-04-10T15:29:00.915Z
Learning: In `apps/framework-cli/src/utilities/native_infra/devkafka.rs` (514-labs/moosestack), `health_check` intentionally uses `std::net::TcpStream::connect_timeout` (blocking) because its callers run inside a synchronous polling loop. Do not flag the blocking TCP connect as a concern; the function is not called from within an async context directly.
Applied to files:
apps/framework-cli/src/utilities/native_infra/preflight.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/utilities/native_infra/preflight.rs
📚 Learning: 2026-04-10T14:30:34.800Z
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.
Applied to files:
apps/framework-cli/src/utilities/native_infra/preflight.rs
🔇 Additional comments (5)
apps/framework-cli/src/utilities/system.rs (1)
135-201: Circuit breaker logic — LGTM.Counter increments on both rapid-exit and spawn-failure paths, resets only on a healthy run (≥10s). Semantics are coherent: "5 consecutive attempts without a healthy run." Log messages at lines 196-199 and 230-233 correctly distinguish the two break sites.
One minor note (non-blocking):
consecutive_rapid_failuresis not cleared on a successfulstart()— it's only cleared after the new child proves itself by running ≥MIN_RUNTIME_FOR_RESET. That's the right invariant for "persistently unhealthy," just worth a one-line comment near line 146 so future readers don't mistake it for an oversight.apps/framework-cli/src/utilities/native_infra/errors.rs (1)
104-105: Clean#[from]wiring — nothing to nitpick here.The delegation via
#[error("{0}")]is fine sincePortConflictError'sDisplayis already user-facing.apps/framework-cli/src/cli/local_webserver.rs (1)
2701-2709: Comment-only refresh — LGTM.Probe still racey by nature (classic bind-and-drop TOCTOU), but that's documented as "defense in depth" and the real guard now lives in
preflight::check_ports.apps/framework-cli/src/utilities/native_infra/mod.rs (1)
130-142: Preflight wiring is correct and ordered properly.Runs before any embedded/native process spawn, and
include_webserver = truecoversproxy_port/http/managementbeforelocal_webserverlater tries to bind them — no wasted rollback if the webserver ports are taken.apps/framework-cli/Cargo.toml (1)
21-21: No action needed.
openssl = { version = "0.10" }does not remove a vendored feature—none exists in the codebase. System OpenSSL is already the configured approach: Cross.toml passesOPENSSL_LIB_DIR,OPENSSL_INCLUDE_DIR, and installslibssl-dev:arm64for cross-compilation; ubuntu-latest runners include libssl-dev by default.> Likely an incorrect or invalid review comment.
This comment has been minimized.
This comment has been minimized.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
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/infrastructure/processes/consumption_registry.rs (1)
76-96:⚠️ Potential issue | 🟠 MajorReplace blocking sleep with async alternative or wrap in
spawn_blocking.
start()is called fromexecute_changes(async fn), but useswait_for_port_freewhich blocks the Tokio runtime withstd::thread::sleepfor up to 5s. This starves other tasks on the same thread.Use
tokio::net::TcpListener::bind+tokio::time::sleep, or wrapwait_for_port_freeintokio::task::spawn_blockingat the call site.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/framework-cli/src/infrastructure/processes/consumption_registry.rs` around lines 76 - 96, The start() method currently calls the blocking wait_for_port_free which uses std::thread::sleep and can block the Tokio runtime since start() is invoked from the async execute_changes; replace the blocking wait by either (A) making the wait async (use tokio::net::TcpListener::bind probe attempts plus tokio::time::sleep in an async loop) and call that from start(), or (B) wrap the existing wait_for_port_free call in tokio::task::spawn_blocking when invoked from start() so the blocking work runs off the async runtime thread; update references to wait_for_port_free in start() accordingly to avoid blocking the Tokio reactor.
🤖 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/local_webserver.rs`:
- Around line 600-609: The body-read can fail after headers are received causing
a hard error instead of triggering the retry loop; in the match handling the
result of http_client.execute(client_req).await (and before building the
Response with add_cors_headers/Response::builder()/Full::new), capture the
result of res.bytes().await into a match/result variable and if it Errs and the
reqwest::Error is a body/connection-class error (use e.is_body() and related
connection checks) return an Err so the existing retry logic can retry,
otherwise continue to build and return the response as before.
In `@apps/framework-cli/src/infrastructure/processes/consumption_registry.rs`:
- Around line 164-168: The comment above the probe that calls
std::net::TcpListener::bind is misleading about SO_REUSEADDR; update the comment
to say the port probe works because the temporary listener never accepts
connections (so the kernel does not create a TIME_WAIT for that socket) and is
dropped immediately, allowing the real Node worker bind to succeed a few
milliseconds later, rather than claiming TcpListener::bind sets SO_REUSEADDR by
default; reference the probe TcpListener::bind and the subsequent Node worker
bind in the comment for clarity.
---
Outside diff comments:
In `@apps/framework-cli/src/infrastructure/processes/consumption_registry.rs`:
- Around line 76-96: The start() method currently calls the blocking
wait_for_port_free which uses std::thread::sleep and can block the Tokio runtime
since start() is invoked from the async execute_changes; replace the blocking
wait by either (A) making the wait async (use tokio::net::TcpListener::bind
probe attempts plus tokio::time::sleep in an async loop) and call that from
start(), or (B) wrap the existing wait_for_port_free call in
tokio::task::spawn_blocking when invoked from start() so the blocking work runs
off the async runtime thread; update references to wait_for_port_free in start()
accordingly to avoid blocking the Tokio reactor.
🪄 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: c93dd58e-2429-481c-802c-17925aa35b95
📒 Files selected for processing (2)
apps/framework-cli/src/cli/local_webserver.rsapps/framework-cli/src/infrastructure/processes/consumption_registry.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: Build CLI Binaries (linux-arm64-glibc, ubuntu-22-8-core, aarch64-unknown-linux-gnu)
- GitHub Check: Build CLI Binaries (darwin-arm64, macos-14-large, aarch64-apple-darwin)
- 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: Test CLI (blacksmith-4vcpu-ubuntu-2404)
- GitHub Check: Test CLI (macos-latest-large)
- GitHub Check: Check
- 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/infrastructure/processes/consumption_registry.rsapps/framework-cli/src/cli/local_webserver.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
Write meaningful names for functions, variables, and types
Document all public APIs and breaking changes
Usethiserrorcrate instead ofanyhow::Resultfor error handling
Define errors near their unit of fallibility (no global Error types)
Use#[derive(thiserror::Error)]with#[error()]messages for error definitions
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_moreornutypeto reduce newtype boilerplate
Useconstfor static values (prefer overstatic)
UseUPPER_SNAKE_CASEnaming for constants
Scope constant visibility with preference:pub(crate)>pub(super)>pub
Group related constants together
Write unit tests for all public functions
Test error conditions and edge cases
Files:
apps/framework-cli/src/infrastructure/processes/consumption_registry.rsapps/framework-cli/src/cli/local_webserver.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/infrastructure/processes/consumption_registry.rsapps/framework-cli/src/cli/local_webserver.rs
🧠 Learnings (5)
📓 Common learnings
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: 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: CR
Repo: 514-labs/moosestack PR: 0
File: templates/typescript-agent/AGENTS.md:0-0
Timestamp: 2026-04-01T07:34:38.401Z
Learning: Verify ports 3000, 4000, 5001, 7233, 8080, 9000, and 18123 are free and update `packages/moosestack-service/moose.config.toml` if needed
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: templates/typescript-mcp/AGENTS.md:0-0
Timestamp: 2026-03-31T03:47:14.870Z
Learning: Applies to templates/typescript-mcp/**/moose.config.toml : Verify ports 3000, 4000, 5001, 7233, 8080, 9000, and 18123 are free before development, or update moose.config.toml
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: CR
Repo: 514-labs/moosestack PR: 0
File: templates/typescript-express/AGENTS.md:0-0
Timestamp: 2026-03-31T03:46:50.753Z
Learning: Applies to templates/typescript-express/**/moose.config.toml : Configure port and service settings in `moose.config.toml` instead of hardcoding them
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: templates/typescript-empty/AGENTS.md:0-0
Timestamp: 2026-03-31T03:46:39.081Z
Learning: Applies to templates/typescript-empty/**/moose.config.toml : Configure port and service settings in `moose.config.toml` — default ports used: 4000, 5001, 7233, 8080, 9000, 18123
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: templates/typescript-fastify/AGENTS.md:0-0
Timestamp: 2026-03-31T03:46:59.509Z
Learning: Applies to templates/typescript-fastify/**/moose.config.toml : Configure port and service settings in `moose.config.toml`
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.
📚 Learning: 2026-04-10T15:29:00.915Z
Learnt from: LucioFranco
Repo: 514-labs/moosestack PR: 3933
File: apps/framework-cli/src/utilities/native_infra/devkafka.rs:21-39
Timestamp: 2026-04-10T15:29:00.915Z
Learning: In `apps/framework-cli/src/utilities/native_infra/devkafka.rs` (514-labs/moosestack), `health_check` intentionally uses `std::net::TcpStream::connect_timeout` (blocking) because its callers run inside a synchronous polling loop. Do not flag the blocking TCP connect as a concern; the function is not called from within an async context directly.
Applied to files:
apps/framework-cli/src/infrastructure/processes/consumption_registry.rs
📚 Learning: 2026-04-10T15:16:06.242Z
Learnt from: LucioFranco
Repo: 514-labs/moosestack PR: 3933
File: apps/devkafka/src/handlers/fetch.rs:42-68
Timestamp: 2026-04-10T15:16:06.242Z
Learning: In 514-labs/moosestack, `apps/devkafka/src/handlers/fetch.rs` — `wait_any_notify` intentionally spawns one task per `Arc<Notify>` rather than using `futures::select_all`. This is an acceptable trade-off for the dev-only embedded Kafka broker (low partition count); adding a `futures` crate dependency just for `select_all` is not desired. Do not suggest replacing the spawn-based approach with `select_all` or similar combinators in future reviews.
Applied to files:
apps/framework-cli/src/infrastructure/processes/consumption_registry.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/infrastructure/processes/consumption_registry.rsapps/framework-cli/src/cli/local_webserver.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/infrastructure/processes/consumption_registry.rsapps/framework-cli/src/cli/local_webserver.rs
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (3)
Cross.toml (3)
21-21: 🧹 Nitpick | 🔵 TrivialPin the cross image by digest for reproducibility.
Line 21 uses mutable
:main; builds can drift over time.Suggested patch
-image = "ghcr.io/cross-rs/aarch64-unknown-linux-gnu:main" +image = "ghcr.io/cross-rs/aarch64-unknown-linux-gnu@sha256:<resolved-digest>"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Cross.toml` at line 21, The image field currently uses the mutable tag "ghcr.io/cross-rs/aarch64-unknown-linux-gnu:main"; replace it with a pinned digest form (sha256 digest) to ensure reproducible builds. Locate the image entry named image in Cross.toml and update the value from the :main tag to the corresponding `@sha256`:<digest> produced by inspecting the registry (e.g., using container registry manifest or docker/ctr pull+inspect) and commit that digest-pinned string so the file references the immutable image digest.
10-15:⚠️ Potential issue | 🟠 MajorUse target-qualified OpenSSL env passthrough variables.
Line 12-Line 14 use generic vars, which can leak host paths into cross builds. Prefer target-qualified vars for
aarch64-unknown-linux-gnu.Suggested patch
passthrough = [ "OPENSSL_NO_VENDOR", - "OPENSSL_LIB_DIR", - "OPENSSL_INCLUDE_DIR", - "OPENSSL_STATIC", + "AARCH64_UNKNOWN_LINUX_GNU_OPENSSL_LIB_DIR", + "AARCH64_UNKNOWN_LINUX_GNU_OPENSSL_INCLUDE_DIR", + "AARCH64_UNKNOWN_LINUX_GNU_OPENSSL_STATIC", ]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Cross.toml` around lines 10 - 15, Replace the generic OpenSSL passthrough env vars with target-qualified variants for aarch64-unknown-linux-gnu: change OPENSSL_LIB_DIR, OPENSSL_INCLUDE_DIR and OPENSSL_STATIC to OPENSSL_LIB_DIR_aarch64-unknown-linux-gnu, OPENSSL_INCLUDE_DIR_aarch64-unknown-linux-gnu and OPENSSL_STATIC_aarch64-unknown-linux-gnu in the passthrough array so host paths aren’t leaked into cross builds while keeping OPENSSL_NO_VENDOR as-is if intended.
37-37:⚠️ Potential issue | 🟠 MajorEnsure target OpenSSL config header is present in sysroot.
Line 37 links only
/usr/include/openssl;opensslconf.his arch-specific on Debian/Ubuntu and may be missing from/usr/aarch64-linux-gnu/include/openssl/.Suggested patch
- "ln -sfn /usr/include/openssl /usr/aarch64-linux-gnu/include/openssl", + "rm -rf /usr/aarch64-linux-gnu/include/openssl", + "cp -a /usr/include/openssl /usr/aarch64-linux-gnu/include/openssl", + "ln -sf /usr/include/aarch64-linux-gnu/openssl/opensslconf.h /usr/aarch64-linux-gnu/include/openssl/opensslconf.h",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Cross.toml` at line 37, The symlink line "ln -sfn /usr/include/openssl /usr/aarch64-linux-gnu/include/openssl" may leave out the architecture-specific opensslconf.h in the sysroot; update the Cross.toml setup so the target sysroot contains the arch-specific OpenSSL config header (opensslconf.h) by ensuring you copy or create a symlink for opensslconf.h into /usr/aarch64-linux-gnu/include/openssl (i.e., detect or reference the host /usr/include/openssl/opensslconf.h or the correct arch-specific variant and place it in the target include dir used by the build), so the build can find the correct config header.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@Cross.toml`:
- Line 21: The image field currently uses the mutable tag
"ghcr.io/cross-rs/aarch64-unknown-linux-gnu:main"; replace it with a pinned
digest form (sha256 digest) to ensure reproducible builds. Locate the image
entry named image in Cross.toml and update the value from the :main tag to the
corresponding `@sha256`:<digest> produced by inspecting the registry (e.g., using
container registry manifest or docker/ctr pull+inspect) and commit that
digest-pinned string so the file references the immutable image digest.
- Around line 10-15: Replace the generic OpenSSL passthrough env vars with
target-qualified variants for aarch64-unknown-linux-gnu: change OPENSSL_LIB_DIR,
OPENSSL_INCLUDE_DIR and OPENSSL_STATIC to
OPENSSL_LIB_DIR_aarch64-unknown-linux-gnu,
OPENSSL_INCLUDE_DIR_aarch64-unknown-linux-gnu and
OPENSSL_STATIC_aarch64-unknown-linux-gnu in the passthrough array so host paths
aren’t leaked into cross builds while keeping OPENSSL_NO_VENDOR as-is if
intended.
- Line 37: The symlink line "ln -sfn /usr/include/openssl
/usr/aarch64-linux-gnu/include/openssl" may leave out the architecture-specific
opensslconf.h in the sysroot; update the Cross.toml setup so the target sysroot
contains the arch-specific OpenSSL config header (opensslconf.h) by ensuring you
copy or create a symlink for opensslconf.h into
/usr/aarch64-linux-gnu/include/openssl (i.e., detect or reference the host
/usr/include/openssl/opensslconf.h or the correct arch-specific variant and
place it in the target include dir used by the build), so the build can find the
correct config header.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 762958fc-aa98-49c5-8840-ff4c226af321
📒 Files selected for processing (1)
Cross.toml
📜 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). (16)
- GitHub Check: Build CLI Binaries (linux-x64-glibc, ubuntu-22-8-core, x86_64-unknown-linux-gnu)
- GitHub Check: Build CLI Binaries (linux-arm64-glibc, ubuntu-22-8-core, aarch64-unknown-linux-gnu)
- GitHub Check: Build CLI Binaries (darwin-arm64, macos-14-large, aarch64-apple-darwin)
- GitHub Check: Package and Publish Templates
- GitHub Check: Package and Publish Independant TS Package
- GitHub Check: Test TS Moose Lib (Node 22)
- GitHub Check: Test TS Moose Lib (Node 20)
- GitHub Check: Test CLI (blacksmith-4vcpu-ubuntu-2404)
- GitHub Check: Test CLI (macos-latest-large)
- GitHub Check: Check
- GitHub Check: Lints
- GitHub Check: Cursor Bugbot
- GitHub Check: Analyze (javascript)
- GitHub Check: Analyze (typescript)
- GitHub Check: Analyze (rust)
- GitHub Check: Analyze (python)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
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: 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: CR
Repo: 514-labs/moosestack PR: 0
File: templates/typescript-agent/AGENTS.md:0-0
Timestamp: 2026-04-01T07:34:38.401Z
Learning: Verify ports 3000, 4000, 5001, 7233, 8080, 9000, and 18123 are free and update `packages/moosestack-service/moose.config.toml` if needed
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: templates/typescript-mcp/AGENTS.md:0-0
Timestamp: 2026-03-31T03:47:14.870Z
Learning: Applies to templates/typescript-mcp/**/moose.config.toml : Verify ports 3000, 4000, 5001, 7233, 8080, 9000, and 18123 are free before development, or update moose.config.toml
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: CR
Repo: 514-labs/moosestack PR: 0
File: templates/typescript-express/AGENTS.md:0-0
Timestamp: 2026-03-31T03:46:50.753Z
Learning: Applies to templates/typescript-express/**/moose.config.toml : Configure port and service settings in `moose.config.toml` instead of hardcoding them
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: templates/typescript-fastify/AGENTS.md:0-0
Timestamp: 2026-03-31T03:46:59.509Z
Learning: Applies to templates/typescript-fastify/**/moose.config.toml : Configure port and service settings in `moose.config.toml`
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: templates/typescript-empty/AGENTS.md:0-0
Timestamp: 2026-03-31T03:46:39.081Z
Learning: Applies to templates/typescript-empty/**/moose.config.toml : Configure port and service settings in `moose.config.toml` — default ports used: 4000, 5001, 7233, 8080, 9000, 18123
Learnt from: cjus
Repo: 514-labs/moosestack PR: 3684
File: apps/framework-cli/src/framework/core/infrastructure_map.rs:0-0
Timestamp: 2026-03-04T22:31:33.997Z
Learning: Repo 514-labs/moosestack — In apps/framework-cli/src/framework/core/infrastructure_map.rs, InfrastructureMap::to_proto serializes select_row_policies and InfrastructureMap::from_proto deserializes them back into the map. Proto/Redis round-trips now preserve row policies; do not flag select_row_policies as being dropped on load in future reviews.
Learnt from: DatGuyJonathan
Repo: 514-labs/moosestack PR: 3830
File: packages/ts-moose-lib/src/consumption-apis/standalone.ts:75-83
Timestamp: 2026-03-24T20:13:13.891Z
Learning: In `packages/ts-moose-lib/src/consumption-apis/standalone.ts`, `getRowPoliciesConfigFromRegistry` does not need a conflict-detection guard for duplicate `MOOSE_RLS_SETTING_PREFIX+column` keys. `validate_row_policy_columns` (Rust, `apps/framework-cli/src/framework/core/plan_validator.rs`) rejects conflicting column/claim mappings at build time, so a deployed standalone app can never have conflicting policies registered at runtime. Do not flag the missing conflict check as a bug.
📚 Learning: 2026-04-09T15:11:18.699Z
Learnt from: LucioFranco
Repo: 514-labs/moosestack PR: 3936
File: apps/framework-cli/src/utilities/native_infra/binary_manager.rs:57-64
Timestamp: 2026-04-09T15:11:18.699Z
Learning: In `apps/framework-cli/src/utilities/native_infra/binary_manager.rs`, SHA-256 checksum / signature verification for downloaded ClickHouse and Temporal binaries is intentionally deferred to GA. Downloads use HTTPS from official GitHub release URLs (TLS provides transport security). The feature is gated behind `--alpha`. Do not re-flag missing checksum verification as a blocker for this alpha feature; flag it again when reviewing GA promotion.
Applied to files:
Cross.toml
📚 Learning: 2025-12-16T23:08:31.521Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-16T23:08:31.521Z
Learning: Build Rust components using `cargo build` before integration
Applied to files:
Cross.toml
`moose dev --dockerless` shutdown was silently leaking ClickHouse. The watchdog installs its own `comm` via prctl(PR_SET_NAME, "clckhouse-watch") — the 'i' is dropped to fit Linux's 15-char TASK_COMM_LEN — so the `process_matches(pid, "clickhouse")` guard in kill_pid_file returned false, the PID file was removed as "stale", and no SIGTERM was ever sent. The ClickHouse child lived on, keeping its ports bound. On the next `moose dev --dockerless` start the new preflight check reported "another moose dev instance may be running" and aborted startup — this is what made the s3 / core E2E buckets fail deterministically on CI once the buckets started sharing the same workspace. - Record the PID file with `clckhouse-watch` so the identity guard matches the watchdog's actual comm. - Make kill_pid_file wait for the process to exit after SIGTERM, with a 10s ceiling before escalating to SIGKILL, so ports are released before the caller returns. - unloaded-files-warning test: pass its offset ports to killRemainingProcesses so cleanup waits for those ports to free before `rm -rf` runs against the test directory.
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/utilities/native_infra/mod.rs`:
- Around line 524-550: Replace the current "kill -0" liveness probe inside the
wait loop with a check that runs "ps -o stat= -p <pid>" (using the existing pid
variable) and treats a missing/empty output or a state beginning with 'Z'
(zombie) as "not alive"; only consider the process alive when ps returns a
non‑zombie state string. Update the map/unwrap logic around the
Command::new("ps") invocation to parse stdout, trim it, and use that result to
decide whether to break, remove pid_path, and return (same behavior as the
existing branch that logs "PID {pid} exited after SIGTERM"), otherwise continue
polling and eventually escalate to SIGKILL as before. Ensure you reference the
same pid and pid_path symbols and keep the existing timeouts and sleep
intervals.
🪄 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: a8ff576c-43f9-4291-b5f5-34d5b070c8a4
📒 Files selected for processing (3)
apps/framework-cli-e2e/test/unloaded-files-warning.test.tsapps/framework-cli/src/utilities/native_infra/clickhouse.rsapps/framework-cli/src/utilities/native_infra/mod.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). (8)
- 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: Check
- GitHub Check: Test CLI (macos-latest-large)
- GitHub Check: Cursor Bugbot
- GitHub Check: Analyze (rust)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.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/native_infra/clickhouse.rsapps/framework-cli/src/utilities/native_infra/mod.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
Write meaningful names for functions, variables, and types
Document all public APIs and breaking changes
Usethiserrorcrate instead ofanyhow::Resultfor error handling
Define errors near their unit of fallibility (no global Error types)
Use#[derive(thiserror::Error)]with#[error()]messages for error definitions
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_moreornutypeto reduce newtype boilerplate
Useconstfor static values (prefer overstatic)
UseUPPER_SNAKE_CASEnaming for constants
Scope constant visibility with preference:pub(crate)>pub(super)>pub
Group related constants together
Write unit tests for all public functions
Test error conditions and edge cases
Files:
apps/framework-cli/src/utilities/native_infra/clickhouse.rsapps/framework-cli/src/utilities/native_infra/mod.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/native_infra/clickhouse.rsapps/framework-cli/src/utilities/native_infra/mod.rs
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Run linting checks before submitting PRs for TypeScript/JavaScript code
**/*.{ts,tsx,js,jsx}: Group imports by external deps, internal modules, types; use named exports from barrel files (index.ts)
Use camelCase for variable and function names, PascalCase for types/classes/components, UPPER_SNAKE_CASE for constants in TypeScript/JavaScript
Prefix unused variables with_(e.g.,_unusedParam) to bypass linting errors in TypeScript/JavaScript
Format with Prettier usingexperimentalTernaries: true; auto-formats on commit via Husky + lint-staged in TypeScript/JavaScript
ESLint extends Next.js, Turbo, TypeScript recommended;@typescript-eslint/no-explicit-anydisabled
Files:
apps/framework-cli-e2e/test/unloaded-files-warning.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Prefer interfaces for objects, types for unions/intersections; explicit return types on public APIs in TypeScript
Files:
apps/framework-cli-e2e/test/unloaded-files-warning.test.ts
🧠 Learnings (22)
📓 Common learnings
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: 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: CR
Repo: 514-labs/moosestack PR: 0
File: templates/typescript-agent/AGENTS.md:0-0
Timestamp: 2026-04-01T07:34:38.401Z
Learning: Verify ports 3000, 4000, 5001, 7233, 8080, 9000, and 18123 are free and update `packages/moosestack-service/moose.config.toml` if needed
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: templates/typescript-mcp/AGENTS.md:0-0
Timestamp: 2026-03-31T03:47:14.870Z
Learning: Applies to templates/typescript-mcp/**/moose.config.toml : Verify ports 3000, 4000, 5001, 7233, 8080, 9000, and 18123 are free before development, or update moose.config.toml
Learnt from: LucioFranco
Repo: 514-labs/moosestack PR: 3932
File: apps/devredis/tests/integration.rs:0-0
Timestamp: 2026-04-10T14:28:45.452Z
Learning: In `apps/devredis/tests/integration.rs` (514-labs/moosestack), `handle.abort()` is intentionally used to stop the embedded devredis test server. Graceful shutdown via `server.shutdown()` is deliberately omitted from test helpers because it adds complexity with no test correctness benefit. Do not suggest restructuring test helpers to add graceful shutdown signals.
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: 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: LucioFranco
Repo: 514-labs/moosestack PR: 3933
File: apps/devkafka/src/groups.rs:65-87
Timestamp: 2026-04-10T15:30:49.933Z
Learning: In `apps/devkafka/src/groups.rs` (514-labs/moosestack PR `#3933`), `choose_protocol()` and `remove_member()` intentionally rely on nondeterministic HashMap iteration order for protocol selection and leader failover. This is acceptable for a dev-only broker with typically 1-2 consumers. Do not flag nondeterministic protocol selection or leader election in these methods in future reviews.
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: templates/typescript-empty/AGENTS.md:0-0
Timestamp: 2026-03-31T03:46:39.081Z
Learning: Applies to templates/typescript-empty/**/moose.config.toml : Configure port and service settings in `moose.config.toml` — default ports used: 4000, 5001, 7233, 8080, 9000, 18123
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: templates/typescript-express/AGENTS.md:0-0
Timestamp: 2026-03-31T03:46:50.753Z
Learning: Applies to templates/typescript-express/**/moose.config.toml : Configure port and service settings in `moose.config.toml` instead of hardcoding them
📚 Learning: 2026-02-18T00:09:56.326Z
Learnt from: DatGuyJonathan
Repo: 514-labs/moosestack PR: 3596
File: apps/framework-cli/src/infrastructure/olap/clickhouse/queries.rs:376-381
Timestamp: 2026-02-18T00:09:56.326Z
Learning: In apps/framework-cli/src/infrastructure/olap/clickhouse/queries.rs, maintain inline // comments for ClickhouseEngine variants and their fields (no /// rustdoc) to match the file’s existing style unless a repo-wide doc-style change is undertaken.
Applied to files:
apps/framework-cli/src/utilities/native_infra/clickhouse.rsapps/framework-cli/src/utilities/native_infra/mod.rs
📚 Learning: 2026-03-04T00:09:14.325Z
Learnt from: phiSgr
Repo: 514-labs/moosestack PR: 3678
File: apps/framework-cli/src/infrastructure/olap/clickhouse/model.rs:483-483
Timestamp: 2026-03-04T00:09:14.325Z
Learning: In apps/framework-cli/src/infrastructure/olap/clickhouse/model.rs, `ClickHouseColumn` and related structs (e.g., `ClickHouseTable`, `ClickHouseIndex`) are internal CLI infrastructure types, not user-facing public APIs. Inline `//` comments are acceptable for their fields; the rustdoc `///` requirement does not apply to these internal structs.
Applied to files:
apps/framework-cli/src/utilities/native_infra/clickhouse.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/utilities/native_infra/clickhouse.rsapps/framework-cli/src/utilities/native_infra/mod.rs
📚 Learning: 2026-03-24T20:28:14.741Z
Learnt from: DatGuyJonathan
Repo: 514-labs/moosestack PR: 3830
File: apps/framework-cli/src/framework/core/infra_reality_checker.rs:793-848
Timestamp: 2026-03-24T20:28:14.741Z
Learning: In `apps/framework-cli/src/framework/core/infra_reality_checker.rs` (514-labs/moosestack), `project.clickhouse_config.db_name` and `infra_map.default_database` are always the same value — `InfrastructureMap` is constructed from the project and sets `default_database` from `clickhouse_config.db_name`. Do not flag uses of `project.clickhouse_config.db_name` as diverging from `infra_map.default_database`.
Applied to files:
apps/framework-cli/src/utilities/native_infra/clickhouse.rs
📚 Learning: 2026-03-24T19:23:44.484Z
Learnt from: DatGuyJonathan
Repo: 514-labs/moosestack PR: 3830
File: apps/framework-cli/src/utilities/docker.rs:502-509
Timestamp: 2026-03-24T19:23:44.484Z
Learning: In `apps/framework-cli/src/utilities/docker.rs`, `generate_clickhouse_clusters_xml` generates ClickHouse clusters XML config only for local dev mode (Docker Compose). The user/password values embedded in the XML are dev-mode credentials controlled by the local user. Do not flag XML-escaping issues for these credentials; the local-dev-only context makes the risk negligible.
Applied to files:
apps/framework-cli/src/utilities/native_infra/clickhouse.rs
📚 Learning: 2026-03-24T19:24:56.751Z
Learnt from: DatGuyJonathan
Repo: 514-labs/moosestack PR: 3830
File: apps/framework-cli/src/framework/typescript/consumption.rs:88-98
Timestamp: 2026-03-24T19:24:56.751Z
Learning: In `apps/framework-cli/src/framework/typescript/consumption.rs`, the `run()` function passes ClickHouse credentials (user, password) via argv (`string_args`) as positional arguments. The `--rls-user` and `--rls-password` flags added for row-level security intentionally follow this same pre-existing pattern. Do not flag passing `effective_rls_password()` via argv as a secret-exposure issue; it is consistent with how the default ClickHouse password is handled in this file.
Applied to files:
apps/framework-cli/src/utilities/native_infra/clickhouse.rs
📚 Learning: 2026-04-13T02:21:24.515Z
Learnt from: 514Ben
Repo: 514-labs/moosestack PR: 3885
File: apps/framework-cli/src/infrastructure/olap/clickhouse/dictionary.rs:1050-1058
Timestamp: 2026-04-13T02:21:24.515Z
Learning: In apps/framework-cli/src/infrastructure/olap/clickhouse/dictionary.rs, `escape_clickhouse_string()` (defined at line ~947) is consistently applied to all string literals embedded in SOURCE DDL clauses, including `where_clause`, `invalidate_query`, `query`, `table`, `db`, `user`, `password`, `host`, `url`, `format`, and `path` fields across all source types (Table, Query, HTTP, ClickHouse, MySQL, PostgreSQL, Redis, MongoDB, Executable, S3). Do not flag these as unescaped.
Applied to files:
apps/framework-cli/src/utilities/native_infra/clickhouse.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/native_infra/clickhouse.rsapps/framework-cli/src/utilities/native_infra/mod.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/native_infra/clickhouse.rsapps/framework-cli/src/utilities/native_infra/mod.rs
📚 Learning: 2026-04-01T07:34:38.401Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: templates/typescript-agent/AGENTS.md:0-0
Timestamp: 2026-04-01T07:34:38.401Z
Learning: Applies to templates/typescript-agent/packages/*/test/**/*.integration.test.ts : Place runtime wiring tests in `packages/*/test/**/*.integration.test.ts` and do not require a prebuilt `dist/` tree
Applied to files:
apps/framework-cli-e2e/test/unloaded-files-warning.test.ts
📚 Learning: 2026-04-01T07:34:38.401Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: templates/typescript-agent/AGENTS.md:0-0
Timestamp: 2026-04-01T07:34:38.401Z
Learning: Applies to templates/typescript-agent/packages/*/test/**/*.unit.test.ts : Place pure helper tests in `packages/*/test/**/*.unit.test.ts` and do not put package-crossing integration coverage there
Applied to files:
apps/framework-cli-e2e/test/unloaded-files-warning.test.ts
📚 Learning: 2026-03-31T03:47:14.870Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: templates/typescript-mcp/AGENTS.md:0-0
Timestamp: 2026-03-31T03:47:14.870Z
Learning: Applies to templates/typescript-mcp/**/moose.config.toml : Verify ports 3000, 4000, 5001, 7233, 8080, 9000, and 18123 are free before development, or update moose.config.toml
Applied to files:
apps/framework-cli-e2e/test/unloaded-files-warning.test.ts
📚 Learning: 2026-01-19T22:36:56.117Z
Learnt from: DatGuyJonathan
Repo: 514-labs/moosestack PR: 3282
File: apps/framework-cli-e2e/test/utils/llm-agent-utils.ts:613-742
Timestamp: 2026-01-19T22:36:56.117Z
Learning: In apps/framework-cli-e2e/test/LLM-related tests (specifically llm-docs-automation.test.ts and llm-agent-utils.ts), token limit exceedances are intentional. The tests should fail when the agent exceeds Claude's context window to reflect real-user failure scenarios. Treat this as a test-quality signal: if the agent can fit within the context window, the documentation or task complexity may be insufficient and needs improvement. This guideline applies to all TypeScript test files under the framework-cli-e2e test directory that exercise LLM context handling.
Applied to files:
apps/framework-cli-e2e/test/unloaded-files-warning.test.ts
📚 Learning: 2026-04-01T17:56:12.400Z
Learnt from: 514Ben
Repo: 514-labs/moosestack PR: 3889
File: apps/framework-cli-e2e/test/dictionary.test.ts:90-91
Timestamp: 2026-04-01T17:56:12.400Z
Learning: In `apps/framework-cli-e2e/test/` E2E tests, a hardcoded 5-second `setTimeout` delay before calling `waitForInfrastructureReady` is an intentional, team-wide pattern (used across suites like kafka, s3, engine, and dictionary). Do not flag it as a bug or premature/inefficient timeout; treat it as acceptable until the planned shared polling helper improvement is implemented.
Applied to files:
apps/framework-cli-e2e/test/unloaded-files-warning.test.ts
📚 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/utilities/native_infra/mod.rs
📚 Learning: 2025-12-16T23:09:10.917Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: apps/framework-cli/AGENTS.md:0-0
Timestamp: 2025-12-16T23:09:10.917Z
Learning: Applies to apps/framework-cli/**/*.rs : Document all public APIs and breaking changes
Applied to files:
apps/framework-cli/src/utilities/native_infra/mod.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/utilities/native_infra/mod.rs
📚 Learning: 2026-04-10T15:29:00.915Z
Learnt from: LucioFranco
Repo: 514-labs/moosestack PR: 3933
File: apps/framework-cli/src/utilities/native_infra/devkafka.rs:21-39
Timestamp: 2026-04-10T15:29:00.915Z
Learning: In `apps/framework-cli/src/utilities/native_infra/devkafka.rs` (514-labs/moosestack), `health_check` intentionally uses `std::net::TcpStream::connect_timeout` (blocking) because its callers run inside a synchronous polling loop. Do not flag the blocking TCP connect as a concern; the function is not called from within an async context directly.
Applied to files:
apps/framework-cli/src/utilities/native_infra/mod.rs
📚 Learning: 2026-04-10T14:28:37.548Z
Learnt from: LucioFranco
Repo: 514-labs/moosestack PR: 3932
File: apps/devredis/src/lib.rs:18-22
Timestamp: 2026-04-10T14:28:37.548Z
Learning: In 514-labs/moosestack, apps/devredis uses a crate-wide `pub type Error = Box<dyn std::error::Error + Send + Sync>` and `pub type Result<T>` alias intentionally. It is a dev-only embedded Redis server and the boxed error is acceptable here; do not flag it for thiserror/typed-error refactoring.
Applied to files:
apps/framework-cli/src/utilities/native_infra/mod.rs
📚 Learning: 2026-04-10T14:30:34.800Z
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.
Applied to files:
apps/framework-cli/src/utilities/native_infra/mod.rs
📚 Learning: 2026-04-10T14:28:45.452Z
Learnt from: LucioFranco
Repo: 514-labs/moosestack PR: 3932
File: apps/devredis/tests/integration.rs:0-0
Timestamp: 2026-04-10T14:28:45.452Z
Learning: In `apps/devredis/tests/integration.rs` (514-labs/moosestack), `handle.abort()` is intentionally used to stop the embedded devredis test server. Graceful shutdown via `server.shutdown()` is deliberately omitted from test helpers because it adds complexity with no test correctness benefit. Do not suggest restructuring test helpers to add graceful shutdown signals.
Applied to files:
apps/framework-cli/src/utilities/native_infra/mod.rs
🔇 Additional comments (5)
apps/framework-cli-e2e/test/unloaded-files-warning.test.ts (1)
74-80: LGTM. PassingPORTSinto both teardown calls aligns cleanup with the dev process's actual ports, addressing theENOTEMPTYrace from ClickHouse's async shutdown.apps/framework-cli/src/utilities/native_infra/mod.rs (3)
6-6: LGTM.The module wiring is straightforward.
219-229: LGTM once the comm value is platform-safe.Persisting the actual ClickHouse watchdog comm with the PID is the right fix for Linux PID identity checks.
130-141: Clarify documentation location for port preflight behavior.The port preflight check is user-facing and affects
moose dev --dockerlessUX, butapps/framework-docs-v2/contentis not set up for CLI command documentation (currently contains only ~2 files).Port configuration is documented in template
AGENTS.mdfiles (e.g.,templates/typescript-mcp/AGENTS.md). Verify whether the port preflight failure behavior should be documented there instead, or if there's a different docs target for this CLI feature.apps/framework-cli/src/utilities/native_infra/clickhouse.rs (1)
12-17: Platform-specific watchdog process name required.
WATCHDOG_COMMdocuments Linuxprctl(PR_SET_NAME)behavior but is used unconditionally on macOS, where ClickHouse likely reports a different process name. This causesprocess_matches()to fail on macOS, treating the watchdog PID as stale and removing the file without sending SIGTERM.Define separate constants for each platform:
Proposed direction
-/// `ps -o comm=` value for the ClickHouse watchdog process. ClickHouse's -/// watchdog calls `prctl(PR_SET_NAME, "clckhouse-watch")` at startup (the 'i' -/// is dropped so the name fits Linux's 15-char `TASK_COMM_LEN`). We spawn the -/// watchdog and persist this name in the PID file so `process_matches` can -/// verify the PID still refers to that watchdog before sending SIGTERM. -pub const WATCHDOG_COMM: &str = "clckhouse-watch"; +/// Linux `ps -o comm=` value for the ClickHouse watchdog process. +#[cfg(target_os = "linux")] +pub const WATCHDOG_COMM: &str = "clckhouse-watch"; + +/// macOS `ps -o comm=` value for the ClickHouse process. +/// Verify actual value against `ps -p <pid> -o comm=` on macOS before merging. +#[cfg(target_os = "macos")] +pub const WATCHDOG_COMM: &str = "clickhouse";
The previous commit unconditionally recorded `clckhouse-watch` in the PID file, but that rename only happens on Linux via `prctl(PR_SET_NAME, …)`. On macOS ClickHouse keeps its original `clickhouse` comm, so `process_matches` would treat the PID as stale-or-reused and skip SIGTERM — the exact regression this PR is trying to prevent, now leaking to macOS dev machines. Gate `WATCHDOG_COMM` on `target_os` so the PID identity check matches what `ps -o comm=` actually returns on each platform.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 48e651a. Configure here.
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
Contributor
Graphite Automations"engineering review - commercial [copy]" took an action on this PR • (04/19/26)1 reviewer was added to this PR based on Mitchell Suzuki's automation. |
LucioFranco
reviewed
Apr 19, 2026
LucioFranco
reviewed
Apr 19, 2026
LucioFranco
reviewed
Apr 19, 2026
LucioFranco
reviewed
Apr 19, 2026
LucioFranco
reviewed
Apr 19, 2026
LucioFranco
approved these changes
Apr 20, 2026
callicles
deleted the
nico/eng-2768-moose-dev-dockerless-crashes-on-re-invocation-with
branch
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.
Note
Medium Risk
Touches dev infrastructure lifecycle and proxying behavior (port preflight, retries, process restart logic), which could affect local startup reliability and error surfacing, but is scoped to dev/docklerless flows.
Overview
Improves dockerless/dev startup stability by adding a preflight port conflict check and better shutdown semantics, reducing EADDRINUSE restart storms and flaky reruns.
The CLI consumption proxy now retries transient connect failures and returns a structured
503withRetry-Afterwhen the consumption API is briefly unavailable. Consumption API startup also waits forproxy_portto become bindable, the Node runner binds explicitly to127.0.0.1and exits cleanly onEADDRINUSE, and the RustRestartingProcessadds a circuit breaker to stop infinite rapid respawns.ClickHouse native infra handling is tightened by persisting the correct watchdog
commin PID files and waiting/escalating on shutdown so ports/files are actually released; E2E suites now pass their offset port sets to cleanup helpers to reliably kill leftover processes. Adds aCross.tomlconvenience config for aarch64 cross-builds and updatespnpm-lock.yamlplatformlibcmetadata.Reviewed by Cursor Bugbot for commit 9427e83. Bugbot is set up for automated code reviews on this repo. Configure here.