-
Notifications
You must be signed in to change notification settings - Fork 26
Add initContainer support for moose prod #3358
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Add initContainer support for moose prod #3358
Conversation
Created using jj-spr 0.1.0
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughA new Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/framework-cli/src/cli/routines/mod.rs (1)
709-725: Spawned listeners may leak on early exit.
spawn_api_update_listenerandspawn_webapp_update_listenerspawn async tasks (lines 709-713). Whenexit_after_initis true, these tasks are abandoned. For init container use case this is acceptable since process terminates anyway.If cleaner shutdown is desired, consider moving listener spawning after the
exit_after_initcheck, or explicitly canceling them before early return.
🤖 Fix all issues with AI agents
In `@apps/framework-cli-e2e/test/init-container.test.ts`:
- Around line 1-3: Prettier formatting errors were detected in the test header
comments; run the project's Prettier formatter on this file (e.g., `npx prettier
--write`) to reformat the file so it matches the repository style, then stage
the updated file and update the PR; confirm the triple-slash reference comments
(/// <reference types="node" />, mocha, chai) are preserved and line
endings/spacing are corrected by the formatter.
- Line 20: The import line brings in createClient and ClickHouseClient from
`@clickhouse/client` but they are unused in init-container.test.ts; remove
createClient and ClickHouseClient from the import (or remove the entire import
statement if nothing else is imported) to eliminate the unused-import warning
and keep the test file clean.
In `@apps/framework-cli/src/cli/commands.rs`:
- Around line 133-136: Add user-facing documentation for the new CLI flag
--exit-after-init (struct field exit_after_init in commands.rs): create a short
entry in the CLI reference/docs site explaining the flag's purpose ("Exit after
infrastructure initialization, for Kubernetes init containers"), its
type/default (boolean), usage example (how to invoke the CLI with
--exit-after-init) and any caveats; also ensure the new doc is linked from the
CLI/flags index or TOC so it appears in the generated docs.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
apps/framework-cli-e2e/test/init-container.test.tsapps/framework-cli/src/cli.rsapps/framework-cli/src/cli/commands.rsapps/framework-cli/src/cli/routines/mod.rs
🧰 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 the fallibility unit with NO globalErrortype
NEVER useanyhow::Resultin Rust; define custom error types instead
Use snake_case for functions and variables, PascalCase for types and traits, SCREAMING_SNAKE_CASE for constants in Rust
Place constants inconstants.rsat the appropriate module level in Rust
Use tuple structs with validation constructors as newtypes in Rust (e.g.,struct UserId(String))
Write inline tests with#[cfg(test)]modules in Rust
Documentation is required for all public APIs in Rust
Files:
apps/framework-cli/src/cli/commands.rsapps/framework-cli/src/cli/routines/mod.rsapps/framework-cli/src/cli.rs
apps/framework-cli/**/*.rs
📄 CodeRabbit inference engine (apps/framework-cli/AGENTS.md)
apps/framework-cli/**/*.rs: Always runcargo clippy --all-targets -- -D warningsbefore commits; fix all warnings - no Clippy warnings may remain (treat warnings as errors)
Userustfmt --edition 2021for consistent formatting
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/commands.rsapps/framework-cli/src/cli/routines/mod.rsapps/framework-cli/src/cli.rs
**/framework-cli/src/**
⚙️ CodeRabbit configuration file
**/framework-cli/src/**: When reviewing changes to Moose CLI:
- Check if any user-facing changes were made (commands, flags, configs, apis, etc)
- If yes, verify the documentation for THAT SPECIFIC feature is updated in apps/framework-docs-v2/content
- If docs for that feature doesn't exist yet, it should be added. If the change removes public apis, the documentation for those should also be removed. Changing unrelated docs doesn't satisfy this requirement.
- In addition to reviewing for docs discrepancies, you should also review the code as per usual guidelines.
Files:
apps/framework-cli/src/cli/commands.rsapps/framework-cli/src/cli/routines/mod.rsapps/framework-cli/src/cli.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 dependencies, internal modules, and types; use named exports from barrel files (index.ts)
Use camelCase for variables and functions, PascalCase for types/classes/components, UPPER_SNAKE_CASE for constants in TypeScript/JavaScript
Prefix unused variables with underscore (e.g.,_unusedParam) to bypass linting errors in TypeScript/JavaScript
Files:
apps/framework-cli-e2e/test/init-container.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Prefer interfaces for objects, types for unions/intersections; use explicit return types on public APIs in TypeScript
Files:
apps/framework-cli-e2e/test/init-container.test.ts
🧠 Learnings (6)
📚 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/cli/commands.rsapps/framework-cli/src/cli/routines/mod.rsapps/framework-cli/src/cli.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 : Use `rustfmt --edition 2021` for consistent formatting
Applied to files:
apps/framework-cli/src/cli/commands.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 : Test error conditions and edge cases
Applied to files:
apps/framework-cli/src/cli/commands.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 : Group related constants together
Applied to files:
apps/framework-cli/src/cli/commands.rs
📚 Learning: 2025-12-16T23:09:00.546Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-16T23:09:00.546Z
Learning: When changing MooseStack functionality, ALWAYS run end-to-end tests located in `apps/framework-cli-e2e`
Applied to files:
apps/framework-cli-e2e/test/init-container.test.ts
📚 Learning: 2025-12-16T23:09:00.546Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-16T23:09:00.546Z
Learning: When changing user-facing features, add E2E tests to `python-tests` and `typescript-tests` templates AND audit documentation
Applied to files:
apps/framework-cli-e2e/test/init-container.test.ts
🧬 Code graph analysis (3)
apps/framework-cli-e2e/test/init-container.test.ts (4)
apps/framework-cli-e2e/test/utils/file-utils.ts (1)
createTempTestDirectory(41-52)apps/framework-cli-e2e/test/constants.ts (2)
TIMEOUTS(7-45)SERVER_CONFIG(90-94)apps/framework-cli-e2e/test/utils/cleanup-utils.ts (2)
performGlobalCleanup(92-115)cleanupTestSuite(37-87)apps/framework-cli-e2e/test/utils/process-utils.ts (2)
waitForServerStart(82-171)stopDevProcess(36-77)
apps/framework-cli/src/cli/routines/mod.rs (1)
apps/framework-cli/src/cli/display/message_display.rs (1)
show_message_wrapper(61-64)
apps/framework-cli/src/cli.rs (1)
apps/framework-cli/src/cli/routines/mod.rs (1)
start_production_mode(642-768)
🪛 GitHub Actions: Format
apps/framework-cli-e2e/test/init-container.test.ts
[warning] 1-1: Code style issues found in the file. Run Prettier with --write to fix.
🪛 GitHub Actions: Test Framework CLI
apps/framework-cli/src/cli.rs
[error] 1-1: cargo test failed. Test 'cli::tests::test_list_templates' failed: assertion 'result.is_ok()' did not hold. See test logs for details.
🔇 Additional comments (10)
apps/framework-cli-e2e/test/init-container.test.ts (5)
56-89: Helper function looks correct.
waitForProcessExitproperly cleans up timeout on exit/error and handles stdout/stderr capture.
94-126: Project setup follows established patterns.Creates temp directory, copies template, updates package.json name for unique Docker project, installs deps. Consistent with other E2E tests.
143-223: Basic flow test validates the three-phase init container pattern.Test correctly:
- Runs init container with
--exit-after-init- Verifies main container starts with infrastructure intact
- Checks health endpoint and plan shows no pending changes
Cleanup in
finallyblock ensures resources are released.
225-275: Idempotency test validates repeated --exit-after-init calls.Second call omits
--start-include-dependenciessince Docker is already running. Good coverage for real-world scenarios.
278-321: Backwards compatibility test ensures normal prod flow still works.Validates that
moose prod --start-include-dependencieswithout--exit-after-initoperates as expected.apps/framework-cli/src/cli.rs (3)
878-881: Flag extraction from command variant is correct.Pattern matches destructure to extract
exit_after_initalongsidestart_include_dependencies.
940-953: Production mode invocation correctly passes exit_after_init.The flag is threaded through to
start_production_modeas expected.
1525-1540: Test failure is unrelated to--exit-after-initfeature.
test_list_templatestests thetemplate listcommand path (lines 1273–1294), which callslist_available_templates(). The--exit-after-initflag is only used in theProdcommand handler (lines 880, 940–945) viaroutines::start_production_mode(). These are completely separate command paths with no interaction.apps/framework-cli/src/cli/routines/mod.rs (2)
638-648: Function signature updated with documentation.Public API change documented in doc comment. Parameter naming is clear.
731-742: Early exit logic placed correctly after infrastructure setup.Exit occurs after:
- State storage created
- Plan computed and validated
- Migration executed (if applicable)
- Initial infra changes executed
- Infrastructure map stored
Both
info!log and user-facing message emitted for observability.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| /// <reference types="node" /> | ||
| /// <reference types="mocha" /> | ||
| /// <reference types="chai" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Formatting issue detected by pipeline.
Pipeline reports Prettier found code style issues. Run npx prettier --write on this file.
🧰 Tools
🪛 GitHub Actions: Format
[warning] 1-1: Code style issues found in the file. Run Prettier with --write to fix.
🤖 Prompt for AI Agents
In `@apps/framework-cli-e2e/test/init-container.test.ts` around lines 1 - 3,
Prettier formatting errors were detected in the test header comments; run the
project's Prettier formatter on this file (e.g., `npx prettier --write`) to
reformat the file so it matches the repository style, then stage the updated
file and update the PR; confirm the triple-slash reference comments (///
<reference types="node" />, mocha, chai) are preserved and line endings/spacing
are corrected by the formatter.
| import * as fs from "fs"; | ||
| import * as path from "path"; | ||
| import { promisify } from "util"; | ||
| import { createClient, ClickHouseClient } from "@clickhouse/client"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Unused import.
createClient and ClickHouseClient from @clickhouse/client are imported but never used in this file.
Proposed fix
-import { createClient, ClickHouseClient } from "@clickhouse/client";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { createClient, ClickHouseClient } from "@clickhouse/client"; |
🤖 Prompt for AI Agents
In `@apps/framework-cli-e2e/test/init-container.test.ts` at line 20, The import
line brings in createClient and ClickHouseClient from `@clickhouse/client` but
they are unused in init-container.test.ts; remove createClient and
ClickHouseClient from the import (or remove the entire import statement if
nothing else is imported) to eliminate the unused-import warning and keep the
test file clean.
|
|
||
| /// Exit after infrastructure initialization (for Kubernetes init containers) | ||
| #[arg(long)] | ||
| exit_after_init: bool, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
rg -i "exit.?after.?init" apps/framework-docs-v2/content --type mdRepository: 514-labs/moosestack
Length of output: 45
Documentation required for new --exit-after-init flag.
The --exit-after-init flag is a user-facing CLI change and must be documented in apps/framework-docs-v2/content before merge. Documentation for this flag does not currently exist.
🤖 Prompt for AI Agents
In `@apps/framework-cli/src/cli/commands.rs` around lines 133 - 136, Add
user-facing documentation for the new CLI flag --exit-after-init (struct field
exit_after_init in commands.rs): create a short entry in the CLI reference/docs
site explaining the flag's purpose ("Exit after infrastructure initialization,
for Kubernetes init containers"), its type/default (boolean), usage example (how
to invoke the CLI with --exit-after-init) and any caveats; also ensure the new
doc is linked from the CLI/flags index or TOC so it appears in the generated
docs.
Note
Introduces init-container support for production runs by allowing infra setup to complete and the process to exit cleanly.
--exit-after-initflag tomoose prod(CLI and command wiring) and plumbs through toroutines::start_production_modestart_production_modenow stores the infra map and, whenexit_after_initis true, returns early after initialization with a success messageinit-container.test.ts) cover basic flow, idempotent repeated runs, and backward compatibility with normalprodWritten by Cursor Bugbot for commit 0d80245. This will update automatically on new commits. Configure here.