ENG-1760: add moose versions to inframap #3435
Conversation
Created using jj-spr 0.1.0
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Summary by CodeRabbit
WalkthroughAdds a new optional Changes
Sequence DiagramsequenceDiagram
rect rgba(200,200,255,0.5)
participant CLI as CLI Process
end
rect rgba(200,255,200,0.5)
participant InfraMap as InfrastructureMap
end
rect rgba(255,200,200,0.5)
participant Storage as State Storage (Redis / ClickHouse)
end
CLI->>InfraMap: create/load map (moose_version: None)
InfraMap-->>CLI: return map
CLI->>Storage: store_infrastructure_map(map)
activate Storage
Storage->>Storage: clone map -> versioned_map
Storage->>Storage: set versioned_map.moose_version = CLI_VERSION
Storage->>Storage: serialize versioned_map (proto/JSON)
Storage->>Storage: persist to backend
deactivate Storage
Storage-->>CLI: ack stored
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (9)
🧰 Additional context used📓 Path-based instructions (4)**/*.rs📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
apps/framework-cli/**/*.rs📄 CodeRabbit inference engine (apps/framework-cli/AGENTS.md)
Files:
**/framework-cli/src/**⚙️ CodeRabbit configuration file
Files:
packages/protobuf/**/*.proto📄 CodeRabbit inference engine (packages/protobuf/AGENTS.md)
Files:
🧠 Learnings (12)📓 Common learnings📚 Learning: 2026-01-26T00:56:27.011ZApplied to files:
📚 Learning: 2025-12-16T23:09:39.883ZApplied to files:
📚 Learning: 2025-12-16T23:09:39.883ZApplied to files:
📚 Learning: 2025-12-16T23:09:39.883ZApplied to files:
📚 Learning: 2025-12-16T23:09:39.883ZApplied to files:
📚 Learning: 2025-12-16T23:09:39.883ZApplied to files:
📚 Learning: 2025-12-16T23:09:39.883ZApplied to files:
📚 Learning: 2025-12-16T23:09:39.883ZApplied to files:
📚 Learning: 2025-12-16T23:09:39.883ZApplied to files:
📚 Learning: 2025-12-16T23:09:39.883ZApplied to files:
📚 Learning: 2025-12-16T23:09:10.917ZApplied to files:
🧬 Code graph analysis (1)apps/framework-cli/src/cli/local_webserver.rs (1)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
🔇 Additional comments (13)
✏️ Tip: You can disable this entire section by setting 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}
LucioFranco
force-pushed
the
spr/lucio/eng-1760-add-moose-version-to-inframap-proto
branch
from
66d0467 to
18ecc36
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/framework-cli/src/framework/core/infrastructure_map.rs`:
- Around line 7791-7865: The custom Serialize implementation for
InfrastructureMap is omitting the moose_version field so serde_json never emits
it; update the serializer (the shadow/aux struct used in InfrastructureMap's
Serialize impl) to include a moose_version entry that is serialized only when
Some, mirroring how other optional fields are handled — locate the Serialize
impl for InfrastructureMap and add the moose_version field to the temporary
serializable struct and to the sequence/map of fields emitted so that
serde_json::to_string includes "\"moose_version\":\"...\"" when moose_version is
Some and skips it when None.
📜 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 (8)
apps/framework-cli/src/cli/routines/peek.rsapps/framework-cli/src/cli/routines/seed_data.rsapps/framework-cli/src/framework/core/infra_reality_checker.rsapps/framework-cli/src/framework/core/infrastructure_map.rsapps/framework-cli/src/framework/core/partial_infrastructure_map.rsapps/framework-cli/src/framework/core/plan_validator.rsapps/framework-cli/src/framework/core/state_storage.rspackages/protobuf/infrastructure_map.proto
🧰 Additional context used
📓 Path-based instructions (4)
packages/protobuf/**/*.proto
📄 CodeRabbit inference engine (packages/protobuf/AGENTS.md)
packages/protobuf/**/*.proto: NEVER make breaking changes to existing.protofiles, including removing or renaming fields, changing field numbers or types, or changing message/service names in public APIs
Runbuf lintbefore committing Protocol Buffer files
Runbuf breakingto check for breaking changes in Protocol Buffer files
Always use explicit field numbers in Protocol Buffer definitions (never rely on auto-assignment)
Reserve removed Protocol Buffer field numbers using the syntax:reserved 2, 15, 9 to 11;
Reserve removed Protocol Buffer field names using the syntax:reserved "foo", "bar";
Useoptionalfor all Protocol Buffer fields unless required by business logic
Usesnake_casefor Protocol Buffer field names (e.g.,user_id,created_at)
UsePascalCasefor Protocol Buffer message and service names (e.g.,UserProfile,DataService)
UseUPPER_SNAKE_CASEfor Protocol Buffer enum values (e.g.,STATUS_ACTIVE,TYPE_PREMIUM)
Document all messages and fields in Protocol Buffer files with comments
Group related messages in the same Protocol Buffer file
Use imports for shared types across Protocol Buffer files
Mark deprecated Protocol Buffer fields withdeprecated = trueand add a comment explaining the replacement
Files:
packages/protobuf/infrastructure_map.proto
**/*.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, never useanyhow::Result
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 (newtypes) for Rust type safety (e.g.,struct UserId(String))
Write inline tests with#[cfg(test)]modules in Rust
Documentation is required for all public APIs in Rust
Runcargo clippy --all-targets -- -D warningsbefore committing Rust code; no warnings allowed
Files:
apps/framework-cli/src/framework/core/plan_validator.rsapps/framework-cli/src/cli/routines/peek.rsapps/framework-cli/src/framework/core/state_storage.rsapps/framework-cli/src/framework/core/infrastructure_map.rsapps/framework-cli/src/framework/core/partial_infrastructure_map.rsapps/framework-cli/src/cli/routines/seed_data.rsapps/framework-cli/src/framework/core/infra_reality_checker.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/framework/core/plan_validator.rsapps/framework-cli/src/cli/routines/peek.rsapps/framework-cli/src/framework/core/state_storage.rsapps/framework-cli/src/framework/core/infrastructure_map.rsapps/framework-cli/src/framework/core/partial_infrastructure_map.rsapps/framework-cli/src/cli/routines/seed_data.rsapps/framework-cli/src/framework/core/infra_reality_checker.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/framework/core/plan_validator.rsapps/framework-cli/src/cli/routines/peek.rsapps/framework-cli/src/framework/core/state_storage.rsapps/framework-cli/src/framework/core/infrastructure_map.rsapps/framework-cli/src/framework/core/partial_infrastructure_map.rsapps/framework-cli/src/cli/routines/seed_data.rsapps/framework-cli/src/framework/core/infra_reality_checker.rs
🧠 Learnings (11)
📓 Common learnings
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.
📚 Learning: 2025-12-16T23:09:39.883Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: packages/protobuf/AGENTS.md:0-0
Timestamp: 2025-12-16T23:09:39.883Z
Learning: Applies to packages/protobuf/**/*.proto : NEVER make breaking changes to existing `.proto` files, including removing or renaming fields, changing field numbers or types, or changing message/service names in public APIs
Applied to files:
packages/protobuf/infrastructure_map.proto
📚 Learning: 2025-12-16T23:09:39.883Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: packages/protobuf/AGENTS.md:0-0
Timestamp: 2025-12-16T23:09:39.883Z
Learning: Applies to packages/protobuf/**/*.proto : Mark deprecated Protocol Buffer fields with `deprecated = true` and add a comment explaining the replacement
Applied to files:
packages/protobuf/infrastructure_map.proto
📚 Learning: 2025-12-16T23:09:39.883Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: packages/protobuf/AGENTS.md:0-0
Timestamp: 2025-12-16T23:09:39.883Z
Learning: Applies to packages/protobuf/**/*.proto : Reserve removed Protocol Buffer field numbers using the syntax: `reserved 2, 15, 9 to 11;`
Applied to files:
packages/protobuf/infrastructure_map.proto
📚 Learning: 2025-12-16T23:09:39.883Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: packages/protobuf/AGENTS.md:0-0
Timestamp: 2025-12-16T23:09:39.883Z
Learning: Version Protocol Buffer API changes carefully and test generated code in target languages before merging
Applied to files:
packages/protobuf/infrastructure_map.protoapps/framework-cli/src/framework/core/infrastructure_map.rs
📚 Learning: 2025-12-16T23:09:39.883Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: packages/protobuf/AGENTS.md:0-0
Timestamp: 2025-12-16T23:09:39.883Z
Learning: Applies to packages/protobuf/**/*.proto : Reserve removed Protocol Buffer field names using the syntax: `reserved "foo", "bar";`
Applied to files:
packages/protobuf/infrastructure_map.proto
📚 Learning: 2025-12-16T23:09:39.883Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: packages/protobuf/AGENTS.md:0-0
Timestamp: 2025-12-16T23:09:39.883Z
Learning: Applies to packages/protobuf/**/*.proto : Always use explicit field numbers in Protocol Buffer definitions (never rely on auto-assignment)
Applied to files:
packages/protobuf/infrastructure_map.proto
📚 Learning: 2025-12-16T23:09:39.883Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: packages/protobuf/AGENTS.md:0-0
Timestamp: 2025-12-16T23:09:39.883Z
Learning: Applies to packages/protobuf/**/*.proto : Run `buf breaking` to check for breaking changes in Protocol Buffer files
Applied to files:
packages/protobuf/infrastructure_map.proto
📚 Learning: 2025-12-16T23:09:39.883Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: packages/protobuf/AGENTS.md:0-0
Timestamp: 2025-12-16T23:09:39.883Z
Learning: Applies to packages/protobuf/**/*.proto : Document all messages and fields in Protocol Buffer files with comments
Applied to files:
packages/protobuf/infrastructure_map.proto
📚 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/framework/core/plan_validator.rsapps/framework-cli/src/cli/routines/peek.rsapps/framework-cli/src/framework/core/state_storage.rsapps/framework-cli/src/framework/core/partial_infrastructure_map.rsapps/framework-cli/src/cli/routines/seed_data.rsapps/framework-cli/src/framework/core/infra_reality_checker.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/framework/core/plan_validator.rs
🧬 Code graph analysis (1)
apps/framework-cli/src/cli/routines/seed_data.rs (1)
apps/framework-cli/src/utilities/validate_passthrough.rs (1)
None(515-515)
🪛 GitHub Actions: Test Framework CLI
apps/framework-cli/src/framework/core/infrastructure_map.rs
[error] 7864-7864: Test 'test_version_serialization_includes_some' failed: expected JSON to contain '"moose_version":"1.2.3"' during version serialization.
🔇 Additional comments (20)
packages/protobuf/infrastructure_map.proto (1)
26-28:⚠️ Potential issue | 🟡 MinorAdd
optionaltomoose_versionfield to match schema policy.Field violates guideline: "Use
optionalfor all Protocol Buffer fields unless required by business logic." Addoptionalfor presence semantics.✅ Suggested fix
- string moose_version = 14; + optional string moose_version = 14;⛔ Skipped due to learnings
Learnt from: CR Repo: 514-labs/moosestack PR: 0 File: packages/protobuf/AGENTS.md:0-0 Timestamp: 2025-12-16T23:09:39.883Z Learning: Applies to packages/protobuf/**/*.proto : Use `optional` for all Protocol Buffer fields unless required by business logicLearnt from: CR Repo: 514-labs/moosestack PR: 0 File: packages/protobuf/AGENTS.md:0-0 Timestamp: 2025-12-16T23:09:39.883Z Learning: Applies to packages/protobuf/**/*.proto : Always use explicit field numbers in Protocol Buffer definitions (never rely on auto-assignment)Learnt from: CR Repo: 514-labs/moosestack PR: 0 File: packages/protobuf/AGENTS.md:0-0 Timestamp: 2025-12-16T23:09:39.883Z Learning: Applies to packages/protobuf/**/*.proto : Reserve removed Protocol Buffer field numbers using the syntax: `reserved 2, 15, 9 to 11;`Learnt from: CR Repo: 514-labs/moosestack PR: 0 File: packages/protobuf/AGENTS.md:0-0 Timestamp: 2025-12-16T23:09:39.883Z Learning: Applies to packages/protobuf/**/*.proto : Mark deprecated Protocol Buffer fields with `deprecated = true` and add a comment explaining the replacementLearnt from: CR Repo: 514-labs/moosestack PR: 0 File: packages/protobuf/AGENTS.md:0-0 Timestamp: 2025-12-16T23:09:39.883Z Learning: Applies to packages/protobuf/**/*.proto : NEVER make breaking changes to existing `.proto` files, including removing or renaming fields, changing field numbers or types, or changing message/service names in public APIsLearnt from: DatGuyJonathan Repo: 514-labs/moosestack PR: 3400 File: packages/protobuf/infrastructure_map.proto:507-518 Timestamp: 2026-01-26T21:29:55.084Z Learning: In packages/protobuf/infrastructure_map.proto, the Workflow message fields (name, schedule, retries, timeout, language) intentionally use non-optional scalars because default values have semantic meaning: retries: 0 means no retries (1 attempt only), empty schedule means manual trigger only, and empty timeout means use default. The runtime applies these defaults appropriately, so the unset vs empty distinction is unnecessary.Learnt from: CR Repo: 514-labs/moosestack PR: 0 File: packages/protobuf/AGENTS.md:0-0 Timestamp: 2025-12-16T23:09:39.883Z Learning: Applies to packages/protobuf/**/*.proto : Reserve removed Protocol Buffer field names using the syntax: `reserved "foo", "bar";`Learnt from: CR Repo: 514-labs/moosestack PR: 0 File: packages/protobuf/AGENTS.md:0-0 Timestamp: 2025-12-16T23:09:39.883Z Learning: Applies to packages/protobuf/**/*.proto : Document all messages and fields in Protocol Buffer files with commentsLearnt from: CR Repo: 514-labs/moosestack PR: 0 File: packages/protobuf/CLAUDE.md:0-0 Timestamp: 2025-12-16T23:08:46.806Z Learning: Applies to packages/protobuf/**/AGENTS.md : Review AGENTS.md for coding standards and guidelinesLearnt from: CR Repo: 514-labs/moosestack PR: 0 File: packages/protobuf/AGENTS.md:0-0 Timestamp: 2025-12-16T23:09:39.883Z Learning: Version Protocol Buffer API changes carefully and test generated code in target languages before mergingapps/framework-cli/src/framework/core/infrastructure_map.rs (5)
588-593: moose_version field docs/serde attributes look good.
2660-2665: Proto schema change: confirm codegen/compat in all target languages.Ensure regenerated protobufs and tests are updated for the new field.
Based on learnings: Version Protocol Buffer API changes carefully and test generated code in target languages before merging.
2814-2818: Backward-compat empty-string → None mapping is correct.
3593-3594: Default keeps moose_version unset until storage.
614-632: Verify clippy + rustfmt before merge.Please run:
cargo clippy --all-targets -- -D warnings rustfmt --edition 2021As per coding guidelines: Always run
cargo clippy --all-targets -- -D warningsbefore commits; userustfmt --edition 2021for consistent formatting.apps/framework-cli/src/cli/routines/seed_data.rs (1)
691-692: LGTM: fixture includesmoose_version: None.
Keeps test infra maps aligned with the new optional field without changing behavior.apps/framework-cli/src/framework/core/partial_infrastructure_map.rs (1)
683-702: LGTM: defaultmoose_version: Noneon infra map creation.
Aligns construction with the new optional field while keeping behavior unchanged.apps/framework-cli/src/cli/routines/peek.rs (1)
405-422: LGTM: test infra map now initializesmoose_version: None.
Keeps fixtures consistent with the new field.apps/framework-cli/src/framework/core/infra_reality_checker.rs (8)
962-979: LGTM: test infra map includesmoose_version: None.
Fixture alignment with new schema is correct.
1035-1052: LGTM:moose_version: Noneadded in test infra map.
No behavior change; keeps tests compiling with new field.
1114-1131: LGTM: consistentmoose_version: Nonein test infra.
Fixture update is correct.
1184-1201: LGTM:moose_versiondefaulted to None in test infra.
Keeps schema compatibility.
1256-1273: LGTM: test infra map includesmoose_version: None.
Consistent with new optional field.
1344-1361: LGTM:moose_version: Noneadded for test infra map.
No impact beyond fixture completeness.
1549-1566: LGTM:moose_versioninitialized to None in test infra.
Aligned with updated InfrastructureMap struct.
1616-1633: LGTM:moose_version: Nonein test infra map.
Fixture update is correct.apps/framework-cli/src/framework/core/state_storage.rs (2)
62-66: LGTM: versioned infra map stored to Redis.
Cloning then settingmoose_versionpreserves caller state and adds the intended tracking.
189-195: LGTM:moose_versionset before ClickHouse serialization.
Ensures stored state is tagged with the CLI version consistently.apps/framework-cli/src/framework/core/plan_validator.rs (1)
186-204: LGTM: test InfraPlan now includesmoose_version: None.
Keeps tests aligned with the updated InfraPlan shape.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
LucioFranco
force-pushed
the
spr/lucio/eng-1760-add-moose-version-to-inframap-proto
branch
from
18ecc36 to
cb6a4d4
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/protobuf/infrastructure_map.proto`:
- Around line 26-28: The moose_version field is currently a plain string but
should be declared optional to match Rust Option<String> handling and be
consistent with other version fields; update the infrastructure_map.proto
definition for the field named moose_version to use optional string (preserving
the field number 14) so generated code supports presence checks like
has_moose_version().
📜 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 (8)
apps/framework-cli/src/cli/routines/peek.rsapps/framework-cli/src/cli/routines/seed_data.rsapps/framework-cli/src/framework/core/infra_reality_checker.rsapps/framework-cli/src/framework/core/infrastructure_map.rsapps/framework-cli/src/framework/core/partial_infrastructure_map.rsapps/framework-cli/src/framework/core/plan_validator.rsapps/framework-cli/src/framework/core/state_storage.rspackages/protobuf/infrastructure_map.proto
🧰 Additional context used
📓 Path-based instructions (4)
**/*.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, never useanyhow::Result
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 (newtypes) for Rust type safety (e.g.,struct UserId(String))
Write inline tests with#[cfg(test)]modules in Rust
Documentation is required for all public APIs in Rust
Runcargo clippy --all-targets -- -D warningsbefore committing Rust code; no warnings allowed
Files:
apps/framework-cli/src/framework/core/infra_reality_checker.rsapps/framework-cli/src/cli/routines/peek.rsapps/framework-cli/src/cli/routines/seed_data.rsapps/framework-cli/src/framework/core/partial_infrastructure_map.rsapps/framework-cli/src/framework/core/state_storage.rsapps/framework-cli/src/framework/core/infrastructure_map.rsapps/framework-cli/src/framework/core/plan_validator.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/framework/core/infra_reality_checker.rsapps/framework-cli/src/cli/routines/peek.rsapps/framework-cli/src/cli/routines/seed_data.rsapps/framework-cli/src/framework/core/partial_infrastructure_map.rsapps/framework-cli/src/framework/core/state_storage.rsapps/framework-cli/src/framework/core/infrastructure_map.rsapps/framework-cli/src/framework/core/plan_validator.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/framework/core/infra_reality_checker.rsapps/framework-cli/src/cli/routines/peek.rsapps/framework-cli/src/cli/routines/seed_data.rsapps/framework-cli/src/framework/core/partial_infrastructure_map.rsapps/framework-cli/src/framework/core/state_storage.rsapps/framework-cli/src/framework/core/infrastructure_map.rsapps/framework-cli/src/framework/core/plan_validator.rs
packages/protobuf/**/*.proto
📄 CodeRabbit inference engine (packages/protobuf/AGENTS.md)
packages/protobuf/**/*.proto: NEVER make breaking changes to existing.protofiles, including removing or renaming fields, changing field numbers or types, or changing message/service names in public APIs
Runbuf lintbefore committing Protocol Buffer files
Runbuf breakingto check for breaking changes in Protocol Buffer files
Always use explicit field numbers in Protocol Buffer definitions (never rely on auto-assignment)
Reserve removed Protocol Buffer field numbers using the syntax:reserved 2, 15, 9 to 11;
Reserve removed Protocol Buffer field names using the syntax:reserved "foo", "bar";
Useoptionalfor all Protocol Buffer fields unless required by business logic
Usesnake_casefor Protocol Buffer field names (e.g.,user_id,created_at)
UsePascalCasefor Protocol Buffer message and service names (e.g.,UserProfile,DataService)
UseUPPER_SNAKE_CASEfor Protocol Buffer enum values (e.g.,STATUS_ACTIVE,TYPE_PREMIUM)
Document all messages and fields in Protocol Buffer files with comments
Group related messages in the same Protocol Buffer file
Use imports for shared types across Protocol Buffer files
Mark deprecated Protocol Buffer fields withdeprecated = trueand add a comment explaining the replacement
Files:
packages/protobuf/infrastructure_map.proto
🧠 Learnings (8)
📓 Common learnings
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.
📚 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/framework/core/infra_reality_checker.rsapps/framework-cli/src/cli/routines/peek.rsapps/framework-cli/src/cli/routines/seed_data.rsapps/framework-cli/src/framework/core/partial_infrastructure_map.rsapps/framework-cli/src/framework/core/state_storage.rsapps/framework-cli/src/framework/core/plan_validator.rs
📚 Learning: 2025-12-16T23:09:39.883Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: packages/protobuf/AGENTS.md:0-0
Timestamp: 2025-12-16T23:09:39.883Z
Learning: Applies to packages/protobuf/**/*.proto : NEVER make breaking changes to existing `.proto` files, including removing or renaming fields, changing field numbers or types, or changing message/service names in public APIs
Applied to files:
packages/protobuf/infrastructure_map.proto
📚 Learning: 2025-12-16T23:09:39.883Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: packages/protobuf/AGENTS.md:0-0
Timestamp: 2025-12-16T23:09:39.883Z
Learning: Applies to packages/protobuf/**/*.proto : Mark deprecated Protocol Buffer fields with `deprecated = true` and add a comment explaining the replacement
Applied to files:
packages/protobuf/infrastructure_map.proto
📚 Learning: 2025-12-16T23:09:39.883Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: packages/protobuf/AGENTS.md:0-0
Timestamp: 2025-12-16T23:09:39.883Z
Learning: Applies to packages/protobuf/**/*.proto : Reserve removed Protocol Buffer field numbers using the syntax: `reserved 2, 15, 9 to 11;`
Applied to files:
packages/protobuf/infrastructure_map.proto
📚 Learning: 2025-12-16T23:09:39.883Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: packages/protobuf/AGENTS.md:0-0
Timestamp: 2025-12-16T23:09:39.883Z
Learning: Version Protocol Buffer API changes carefully and test generated code in target languages before merging
Applied to files:
packages/protobuf/infrastructure_map.protoapps/framework-cli/src/framework/core/infrastructure_map.rs
📚 Learning: 2025-12-16T23:09:39.883Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: packages/protobuf/AGENTS.md:0-0
Timestamp: 2025-12-16T23:09:39.883Z
Learning: Applies to packages/protobuf/**/*.proto : Reserve removed Protocol Buffer field names using the syntax: `reserved "foo", "bar";`
Applied to files:
packages/protobuf/infrastructure_map.proto
📚 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/routines/seed_data.rsapps/framework-cli/src/framework/core/plan_validator.rs
🧬 Code graph analysis (1)
apps/framework-cli/src/framework/core/plan_validator.rs (1)
apps/framework-cli/src/utilities/validate_passthrough.rs (1)
None(515-515)
⏰ 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). (1)
- GitHub Check: Test E2E TypeScript Tests Template (Node 20, npm)
🔇 Additional comments (14)
apps/framework-cli/src/framework/core/infrastructure_map.rs (7)
588-593: Moose version field is clear and optional.
631-632: Empty map defaults the version correctly.
2814-2818: Backward-compat version parsing looks good.
3593-3593: Default impl keeps version unset.
3628-3653: Shadow struct update looks good.
7794-7868: Version handling tests cover the new field.
2664-2665: No breaking changes to regenerate; field addition is forward-compatible in proto3.The
moose_versionfield (line 28 of infrastructure_map.proto) is newly added as field#14(not modifying existing fields). Proto3 forward-compatibility means this is not a breaking change. Rust conversion correctly usesunwrap_or_default()with backward-compat handling for empty strings. Tests verify roundtrip serialization. If multi-language code generation is part of your build workflow, regenerate and verify; if not committed, no action required.apps/framework-cli/src/framework/core/partial_infrastructure_map.rs (1)
683-702: LGTM: infra map initializesmoose_versionas unset.
Keeps the optional field explicit and backward compatible.apps/framework-cli/src/cli/routines/peek.rs (1)
405-422: LGTM: test infra map includesmoose_version: None.
Fixture stays consistent with the new optional field.apps/framework-cli/src/cli/routines/seed_data.rs (1)
673-692: LGTM: test infra map includesmoose_version: None.
Keeps fixtures aligned with the new optional field.apps/framework-cli/src/framework/core/state_storage.rs (2)
62-65: Good: Redis persistence now stamps the CLI version.
Please confirmcargo clippy --all-targets -- -D warningsandrustfmt --edition 2021were run for this change.
As per coding guidelines, “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). Use rustfmt --edition 2021 for consistent formatting.”
185-195: LGTM: ClickHouse storage serializes the versioned map.
Ensures the persisted protobuf includesmoose_version.apps/framework-cli/src/framework/core/plan_validator.rs (1)
186-204: LGTM: test plan initializesmoose_version: None.
Fixtures stay aligned with the updated InfrastructureMap.apps/framework-cli/src/framework/core/infra_reality_checker.rs (1)
978-979: LGTM: test infra maps now includemoose_version: None.
All fixtures stay consistent with the new optional field.Also applies to: 1051-1052, 1130-1131, 1200-1201, 1272-1273, 1360-1361, 1565-1566, 1632-1633
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| // Version of Moose CLI that created or last updated this infrastructure map | ||
| // Helps with debugging, compatibility tracking, and migration planning | ||
| string moose_version = 14; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider using optional string for consistency and explicit presence tracking.
The Rust side treats this as Option<String>, and other version fields in this proto (e.g., Topic.version, Table.version) use optional. Using optional string enables has_moose_version() checks and aligns with the guideline to use optional unless required by business logic.
Suggested change
// Version of Moose CLI that created or last updated this infrastructure map
// Helps with debugging, compatibility tracking, and migration planning
- string moose_version = 14;
+ optional string moose_version = 14;As per coding guidelines: "Use optional for all Protocol Buffer fields unless required by business logic."
📝 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.
| // Version of Moose CLI that created or last updated this infrastructure map | |
| // Helps with debugging, compatibility tracking, and migration planning | |
| string moose_version = 14; | |
| // Version of Moose CLI that created or last updated this infrastructure map | |
| // Helps with debugging, compatibility tracking, and migration planning | |
| optional string moose_version = 14; |
🤖 Prompt for AI Agents
In `@packages/protobuf/infrastructure_map.proto` around lines 26 - 28, The
moose_version field is currently a plain string but should be declared optional
to match Rust Option<String> handling and be consistent with other version
fields; update the infrastructure_map.proto definition for the field named
moose_version to use optional string (preserving the field number 14) so
generated code supports presence checks like has_moose_version().
There was a problem hiding this comment.
Why not use optional here? that will make sure we have an Option in the Rust code I believe
There was a problem hiding this comment.
@callicles You're absolutely right! Using optional string here is the correct approach and would indeed ensure clean Option<String> generation in Rust with proper has_moose_version() presence checks.
Currently, the code works around the non-optional field by treating empty strings as None in the Rust conversion (if proto.moose_version.is_empty() { None } else { Some(...) }), but this is less idiomatic than proto3's built-in optional support.
Using optional would:
- Match the Rust
Option<String>semantics directly - Align with other version fields like
Topic.versionandTable.version - Follow the coding guideline: "Use
optionalfor all Protocol Buffer fields unless required by business logic" - Enable cleaner presence detection via
has_moose_version()
The suggestion to change it to optional string moose_version = 14; is spot on.
🧠 Learnings used
Learnt from: DatGuyJonathan
Repo: 514-labs/moosestack PR: 3400
File: packages/protobuf/infrastructure_map.proto:507-518
Timestamp: 2026-01-26T21:29:55.084Z
Learning: In packages/protobuf/infrastructure_map.proto, the Workflow message fields (name, schedule, retries, timeout, language) intentionally use non-optional scalars because default values have semantic meaning: retries: 0 means no retries (1 attempt only), empty schedule means manual trigger only, and empty timeout means use default. The runtime applies these defaults appropriately, so the unset vs empty distinction is unnecessary.
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: packages/protobuf/AGENTS.md:0-0
Timestamp: 2025-12-16T23:09:39.883Z
Learning: Applies to packages/protobuf/**/*.proto : Use `optional` for all Protocol Buffer fields unless required by business logic
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: packages/protobuf/AGENTS.md:0-0
Timestamp: 2025-12-16T23:09:39.883Z
Learning: Applies to packages/protobuf/**/*.proto : NEVER make breaking changes to existing `.proto` files, including removing or renaming fields, changing field numbers or types, or changing message/service names in public APIs
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: packages/protobuf/AGENTS.md:0-0
Timestamp: 2025-12-16T23:09:39.883Z
Learning: Version Protocol Buffer API changes carefully and test generated code in target languages before merging
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: packages/protobuf/AGENTS.md:0-0
Timestamp: 2025-12-16T23:09:39.883Z
Learning: Applies to packages/protobuf/**/*.proto : Mark deprecated Protocol Buffer fields with `deprecated = true` and add a comment explaining the replacement
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.
There was a problem hiding this comment.
So by default strings with an empty value already represent an optional empty state. This is the same as if we got a proto message with no message field as well when we try to deserailize. Optional is a bad paradigm to use in protobuf so we should stick away from that as there is no actual advantage. We already handle it correctly above.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
There was a problem hiding this comment.
I believe we had that impression with @phiSgr and we actually had an issue in prod. That's why we are putting optional
There was a problem hiding this comment.
this was added in a way to be backwards compatible since protobuf comes with strong support for that without optional. In reality this item is not actually optional its always required its just that previous versions of the message may not have it hence why it got tag 14 which is new allowing protobuf to skip over it if the code doesn't support tag 14 yet or to decode it either via a default value if it doesn't exist or the real value. If we had issues with protobuf in prod happy to take a peek at why.
…d-moose-version-to-inframap-proto # Conflicts: # apps/framework-cli/src/framework/core/infrastructure_map.rs
LucioFranco
force-pushed
the
spr/lucio/eng-1760-add-moose-version-to-inframap-proto
branch
from
cb6a4d4 to
5d38b64
Compare
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.
There was a problem hiding this comment.
Actionable comments posted: 0
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/local_webserver.rs (1)
3294-3310:⚠️ Potential issue | 🟠 MajorAdd documentation for the
/admin/inframapendpoint and its response structure.This change exposes
moose_versionin theInfraMapResponsevia theGET /admin/inframapendpoint, which is user-facing. The endpoint itself and its response structure are not currently documented inapps/framework-docs-v2/content. Per guidelines, documentation for this endpoint should be added (or expanded if partial docs exist elsewhere).
📜 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 (9)
apps/framework-cli/src/cli/local_webserver.rsapps/framework-cli/src/cli/routines/peek.rsapps/framework-cli/src/cli/routines/seed_data.rsapps/framework-cli/src/framework/core/infra_reality_checker.rsapps/framework-cli/src/framework/core/infrastructure_map.rsapps/framework-cli/src/framework/core/partial_infrastructure_map.rsapps/framework-cli/src/framework/core/plan_validator.rsapps/framework-cli/src/framework/core/state_storage.rspackages/protobuf/infrastructure_map.proto
🧰 Additional context used
📓 Path-based instructions (4)
**/*.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, never useanyhow::Result
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 (newtypes) for Rust type safety (e.g.,struct UserId(String))
Write inline tests with#[cfg(test)]modules in Rust
Documentation is required for all public APIs in Rust
Runcargo clippy --all-targets -- -D warningsbefore committing Rust code; no warnings allowed
Files:
apps/framework-cli/src/framework/core/state_storage.rsapps/framework-cli/src/cli/routines/peek.rsapps/framework-cli/src/framework/core/infra_reality_checker.rsapps/framework-cli/src/cli/local_webserver.rsapps/framework-cli/src/framework/core/plan_validator.rsapps/framework-cli/src/cli/routines/seed_data.rsapps/framework-cli/src/framework/core/partial_infrastructure_map.rsapps/framework-cli/src/framework/core/infrastructure_map.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/framework/core/state_storage.rsapps/framework-cli/src/cli/routines/peek.rsapps/framework-cli/src/framework/core/infra_reality_checker.rsapps/framework-cli/src/cli/local_webserver.rsapps/framework-cli/src/framework/core/plan_validator.rsapps/framework-cli/src/cli/routines/seed_data.rsapps/framework-cli/src/framework/core/partial_infrastructure_map.rsapps/framework-cli/src/framework/core/infrastructure_map.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/framework/core/state_storage.rsapps/framework-cli/src/cli/routines/peek.rsapps/framework-cli/src/framework/core/infra_reality_checker.rsapps/framework-cli/src/cli/local_webserver.rsapps/framework-cli/src/framework/core/plan_validator.rsapps/framework-cli/src/cli/routines/seed_data.rsapps/framework-cli/src/framework/core/partial_infrastructure_map.rsapps/framework-cli/src/framework/core/infrastructure_map.rs
packages/protobuf/**/*.proto
📄 CodeRabbit inference engine (packages/protobuf/AGENTS.md)
packages/protobuf/**/*.proto: NEVER make breaking changes to existing.protofiles, including removing or renaming fields, changing field numbers or types, or changing message/service names in public APIs
Runbuf lintbefore committing Protocol Buffer files
Runbuf breakingto check for breaking changes in Protocol Buffer files
Always use explicit field numbers in Protocol Buffer definitions (never rely on auto-assignment)
Reserve removed Protocol Buffer field numbers using the syntax:reserved 2, 15, 9 to 11;
Reserve removed Protocol Buffer field names using the syntax:reserved "foo", "bar";
Useoptionalfor all Protocol Buffer fields unless required by business logic
Usesnake_casefor Protocol Buffer field names (e.g.,user_id,created_at)
UsePascalCasefor Protocol Buffer message and service names (e.g.,UserProfile,DataService)
UseUPPER_SNAKE_CASEfor Protocol Buffer enum values (e.g.,STATUS_ACTIVE,TYPE_PREMIUM)
Document all messages and fields in Protocol Buffer files with comments
Group related messages in the same Protocol Buffer file
Use imports for shared types across Protocol Buffer files
Mark deprecated Protocol Buffer fields withdeprecated = trueand add a comment explaining the replacement
Files:
packages/protobuf/infrastructure_map.proto
🧠 Learnings (12)
📓 Common learnings
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.
📚 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/framework/core/state_storage.rsapps/framework-cli/src/cli/routines/peek.rsapps/framework-cli/src/framework/core/infra_reality_checker.rsapps/framework-cli/src/cli/local_webserver.rsapps/framework-cli/src/framework/core/plan_validator.rsapps/framework-cli/src/cli/routines/seed_data.rsapps/framework-cli/src/framework/core/partial_infrastructure_map.rsapps/framework-cli/src/framework/core/infrastructure_map.rs
📚 Learning: 2025-12-16T23:09:39.883Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: packages/protobuf/AGENTS.md:0-0
Timestamp: 2025-12-16T23:09:39.883Z
Learning: Applies to packages/protobuf/**/*.proto : NEVER make breaking changes to existing `.proto` files, including removing or renaming fields, changing field numbers or types, or changing message/service names in public APIs
Applied to files:
packages/protobuf/infrastructure_map.proto
📚 Learning: 2025-12-16T23:09:39.883Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: packages/protobuf/AGENTS.md:0-0
Timestamp: 2025-12-16T23:09:39.883Z
Learning: Applies to packages/protobuf/**/*.proto : Mark deprecated Protocol Buffer fields with `deprecated = true` and add a comment explaining the replacement
Applied to files:
packages/protobuf/infrastructure_map.proto
📚 Learning: 2025-12-16T23:09:39.883Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: packages/protobuf/AGENTS.md:0-0
Timestamp: 2025-12-16T23:09:39.883Z
Learning: Applies to packages/protobuf/**/*.proto : Reserve removed Protocol Buffer field numbers using the syntax: `reserved 2, 15, 9 to 11;`
Applied to files:
packages/protobuf/infrastructure_map.proto
📚 Learning: 2025-12-16T23:09:39.883Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: packages/protobuf/AGENTS.md:0-0
Timestamp: 2025-12-16T23:09:39.883Z
Learning: Version Protocol Buffer API changes carefully and test generated code in target languages before merging
Applied to files:
packages/protobuf/infrastructure_map.proto
📚 Learning: 2025-12-16T23:09:39.883Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: packages/protobuf/AGENTS.md:0-0
Timestamp: 2025-12-16T23:09:39.883Z
Learning: Applies to packages/protobuf/**/*.proto : Reserve removed Protocol Buffer field names using the syntax: `reserved "foo", "bar";`
Applied to files:
packages/protobuf/infrastructure_map.proto
📚 Learning: 2025-12-16T23:09:39.883Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: packages/protobuf/AGENTS.md:0-0
Timestamp: 2025-12-16T23:09:39.883Z
Learning: Applies to packages/protobuf/**/*.proto : Run `buf breaking` to check for breaking changes in Protocol Buffer files
Applied to files:
packages/protobuf/infrastructure_map.proto
📚 Learning: 2025-12-16T23:09:39.883Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: packages/protobuf/AGENTS.md:0-0
Timestamp: 2025-12-16T23:09:39.883Z
Learning: Applies to packages/protobuf/**/*.proto : Document all messages and fields in Protocol Buffer files with comments
Applied to files:
packages/protobuf/infrastructure_map.proto
📚 Learning: 2025-12-16T23:09:39.883Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: packages/protobuf/AGENTS.md:0-0
Timestamp: 2025-12-16T23:09:39.883Z
Learning: Applies to packages/protobuf/**/*.proto : Always use explicit field numbers in Protocol Buffer definitions (never rely on auto-assignment)
Applied to files:
packages/protobuf/infrastructure_map.proto
📚 Learning: 2025-12-16T23:09:39.883Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: packages/protobuf/AGENTS.md:0-0
Timestamp: 2025-12-16T23:09:39.883Z
Learning: Applies to packages/protobuf/**/*.proto : Use `optional` for all Protocol Buffer fields unless required by business logic
Applied to files:
packages/protobuf/infrastructure_map.proto
📚 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/local_webserver.rsapps/framework-cli/src/framework/core/plan_validator.rsapps/framework-cli/src/cli/routines/seed_data.rsapps/framework-cli/src/framework/core/infrastructure_map.rs
🧬 Code graph analysis (1)
apps/framework-cli/src/cli/local_webserver.rs (1)
apps/framework-cli/src/framework/core/state_storage.rs (1)
redis_client(444-447)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Test E2E TypeScript Default Template (Node 20, pnpm-9)
- GitHub Check: Test E2E TypeScript Tests Template (Node 20, npm)
- GitHub Check: Test E2E Backward Compatibility - TypeScript (Node 20)
- GitHub Check: Test E2E Python Tests Template (Python 3.12)
- GitHub Check: Test E2E TypeScript Tests Template (Node 22, npm)
- GitHub Check: Test E2E TypeScript Tests Template (Node 24, npm)
- GitHub Check: Test E2E Python Tests Template (Python 3.13)
🔇 Additional comments (13)
apps/framework-cli/src/framework/core/partial_infrastructure_map.rs (1)
701-701: LGTM!Correct initialization to
None. The version is stamped by storage layer on write.apps/framework-cli/src/cli/routines/peek.rs (1)
421-421: LGTM!Test fixture correctly updated for the new field.
apps/framework-cli/src/framework/core/plan_validator.rs (1)
203-203: LGTM!Test helper correctly updated.
apps/framework-cli/src/framework/core/infra_reality_checker.rs (1)
978-978: LGTM!Test fixtures correctly updated throughout.
apps/framework-cli/src/framework/core/state_storage.rs (2)
63-65: LGTM!Correct pattern: clone to avoid mutating input, stamp version, then store.
189-194: LGTM!Symmetric implementation with Redis storage. Version tracking applied consistently before serialization.
apps/framework-cli/src/cli/routines/seed_data.rs (1)
674-692: LGTM: test infra map initializesmoose_version.apps/framework-cli/src/framework/core/infrastructure_map.rs (6)
588-632: Confirm clippy/rustfmt were run for this framework-cli change.
Please verifycargo clippy --all-targets -- -D warningsandrustfmt --edition 2021were executed with no warnings.
As per coding guidelines: “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). Use rustfmt --edition 2021 for consistent formatting.”
2664-2666: Proto serialization handles unset version cleanly.
Using the empty-string default forNonekeeps proto backward-compatible as intended.
2814-2818: Backward-compat deserialization looks correct.
Treating empty proto strings asNoneavoids false version attribution.
3593-3594: Default test map initialization updated correctly.
moose_version: Noneis consistent with storage-time population.
3628-3652: Custom serializer now round-tripsmoose_version.
Skip-when-None preserves old JSON shape while keeping deterministic output.
7794-7868: Version roundtrip/back-compat tests are solid.
Covers proto and JSON behaviors forSomeandNone.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
callicles
left a comment
callicles
left a comment
There was a problem hiding this comment.
Not sure about the proto change
| // Version of Moose CLI that created or last updated this infrastructure map | ||
| // Helps with debugging, compatibility tracking, and migration planning | ||
| string moose_version = 14; |
There was a problem hiding this comment.
Why not use optional here? that will make sure we have an Option in the Rust code I believe
[jj-spr] initial version
Created using jj-spr 0.1.0
Merge remote-tracking branch 'origin/main' into spr/lucio/eng-1760-add-moose-version-to-inframap-proto
Conflicts:
apps/framework-cli/src/framework/core/infrastructure_map.rs
Note
Medium Risk
Touches persisted state formats (protobuf/JSON) and state storage write paths; while backward compatibility is handled, mistakes could break reading older state or introduce unexpected diffs in stored maps.
Overview
Adds CLI version tracking to the persisted
InfrastructureMapvia a newmoose_versionfield (Rust struct +infrastructure_map.proto).Ensures
moose_versionis automatically populated withCLI_VERSIONwhenever the infra map is stored (Redis, ClickHouse, and the adminintegrate-changespath), with backward-compatible parsing (empty/absent values becomeNone) and serialization that omits the field when unset. Includes tests covering proto roundtrips and JSON backward compatibility, plus updates to test fixtures to initialize the new field.Written by Cursor Bugbot for commit 5d38b64. This will update automatically on new commits. Configure here.