ENG-2098: disable temporal connect if workflows false #3436
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Summary by CodeRabbit
WalkthroughMakes Temporal configuration optional across the stack by conditionally generating CLI arguments based on feature flags in Rust, parsing them as optional flags in Python, and making temporalConfig undefined when temporal URL is absent in TypeScript runners. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 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}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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)
packages/ts-moose-lib/src/scripts/runner.ts (1)
66-67: 🧹 Nitpick | 🔵 TrivialPre-existing:
awaitis unnecessary withreadFileSync.
readFileSyncis synchronous — theawaitis a no-op. UsereadFile(async) or drop theawait.Suggested fix
- const cert = await fs.readFileSync(temporalConfig.clientCert); - const key = await fs.readFileSync(temporalConfig.clientKey); + const cert = fs.readFileSync(temporalConfig.clientCert); + const key = fs.readFileSync(temporalConfig.clientKey);
🤖 Fix all issues with AI agents
In `@apps/framework-cli/src/framework/python/wrappers/consumption_runner.py`:
- Line 472: The assignment to server_port uses a redundant getattr call; replace
the getattr usage with a direct access to args.proxy_port (i.e., server_port =
args.proxy_port) since proxy_port already defaults to 4001 elsewhere, and remove
the unnecessary getattr(..., "proxy_port", 4001) expression to avoid duplication
(refer to the server_port assignment and args.proxy_port in
consumption_runner.py).
- Around line 73-77: The four uses of getattr to pull values from args
(temporal_url, temporal_namespace, client_cert, client_key, api_key) are
redundant; replace lines using getattr(args, "temporal_url", "") etc. with
direct attribute access (args.temporal_url, args.temporal_namespace,
args.client_cert, args.client_key, args.api_key) since argparse supplies
defaults and the attributes always exist.
📜 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/src/framework/python/consumption.rsapps/framework-cli/src/framework/python/wrappers/consumption_runner.pypackages/ts-moose-lib/src/moose-runner.tspackages/ts-moose-lib/src/scripts/runner.ts
🧰 Additional context used
📓 Path-based instructions (7)
**/*.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/python/consumption.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/python/consumption.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/python/consumption.rsapps/framework-cli/src/framework/python/wrappers/consumption_runner.py
**/*.{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
Format TypeScript/JavaScript code with Prettier usingexperimentalTernaries: true; auto-formats on commit via Husky + lint-staged
Files:
packages/ts-moose-lib/src/scripts/runner.tspackages/ts-moose-lib/src/moose-runner.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Prefer interfaces for objects, types for unions/intersections in TypeScript; explicit return types on public APIs
Files:
packages/ts-moose-lib/src/scripts/runner.tspackages/ts-moose-lib/src/moose-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.
- In addition to reviewing for docs discrepancies, you should also review the code as per usual guidelines.
Files:
packages/ts-moose-lib/src/scripts/runner.tspackages/ts-moose-lib/src/moose-runner.ts
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Follow PEP 8 style guide in Python; use snake_case for functions/variables, PascalCase for classes, UPPER_SNAKE_CASE for constants
Format Python code with Black using line-length 88; auto-formats on commit via Husky + lint-staged
Use type hints for function signatures and public APIs in Python
Files:
apps/framework-cli/src/framework/python/wrappers/consumption_runner.py
🧠 Learnings (3)
📓 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.
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.
📚 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/python/consumption.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/framework/python/consumption.rspackages/ts-moose-lib/src/scripts/runner.tspackages/ts-moose-lib/src/moose-runner.ts
🪛 Ruff (0.14.14)
apps/framework-cli/src/framework/python/wrappers/consumption_runner.py
[warning] 465-465: Do not catch blind exception: Exception
(BLE001)
⏰ 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). (3)
- GitHub Check: Test E2E TypeScript Tests Template (Node 24, npm)
- GitHub Check: Test E2E TypeScript Tests Template (Node 20, npm)
- GitHub Check: Test E2E TypeScript Tests Template (Node 22, npm)
🔇 Additional comments (8)
packages/ts-moose-lib/src/scripts/runner.ts (2)
25-25: Good: Optional Temporal config aligns with PR objective.The change to make
temporalConfigoptional enables disabling workflows when Temporal isn't configured.
119-123: LGTM: Clean early guard.Proper null check returns early when Temporal is disabled. Logging is helpful for debugging.
packages/ts-moose-lib/src/moose-runner.ts (2)
132-141: Conditional Temporal config: consistent pattern.Correctly gates config object creation on
temporalUrlpresence. Same pattern applied to bothconsumption-apisandscriptscommands.
202-211: LGTM: Matches consumption-apis pattern.apps/framework-cli/src/framework/python/consumption.rs (2)
65-76: Good: Mutable args vector for conditional extension.Clean setup for conditionally appending Temporal arguments.
78-89: Conditional Temporal args when workflows enabled.Correctly gates Temporal-related CLI flags behind
project.features.workflows. This ensures the Python consumption runner won't attempt Temporal connection when workflows are disabled.apps/framework-cli/src/framework/python/wrappers/consumption_runner.py (2)
50-57: Good: Temporal flags now optional with defaults.Changed from positional args to optional flags. This allows the CLI to omit them entirely when workflows are disabled.
457-466: Conditional Temporal connection: correct guard.Temporal connection attempted only when
temporal_urlis provided. The broadExceptioncatch (flagged by Ruff) is acceptable here since it allows the server to start gracefully when Temporal is unavailable or misconfigured.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Actionable comments posted: 1
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/framework/python/consumption.rs (1)
65-93:⚠️ Potential issue | 🟠 MajorDocument workflows-gated Temporal args and verify Clippy compliance.
The code change conditionally adds Temporal arguments only when
project.features.workflowsis enabled, which is a user-facing behavior change that affects how consumption APIs operate with workflows. Per coding guidelines, user-facing feature changes require documentation updates. The documentation inapps/framework-docs-v2/content/moosestack/apis/analytics-api.mdxand related consumption API docs do not currently explain this workflows/Temporal relationship. Additionally,cargo clippy --all-targets -- -D warningsmust be run before commit with no warnings remaining.
🤖 Fix all issues with AI agents
In `@apps/framework-cli/src/framework/python/wrappers/consumption_runner.py`:
- Around line 457-467: Add a BLE001 suppression on the broad exception catch in
the Temporal connection block: when calling create_temporal_connection (used to
set temporal_client if temporal_url is present), append a noqa: BLE001 with a
short justification comment indicating that Temporal initialization is optional
and we intentionally catch all exceptions to allow graceful server degradation;
keep the existing print of the error and do not change the exception handling
logic beyond adding the suppression comment.
📜 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/src/framework/python/consumption.rsapps/framework-cli/src/framework/python/wrappers/consumption_runner.pypackages/ts-moose-lib/src/moose-runner.tspackages/ts-moose-lib/src/scripts/runner.ts
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{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
Format TypeScript/JavaScript code with Prettier usingexperimentalTernaries: true; auto-formats on commit via Husky + lint-staged
Files:
packages/ts-moose-lib/src/scripts/runner.tspackages/ts-moose-lib/src/moose-runner.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Prefer interfaces for objects, types for unions/intersections in TypeScript; explicit return types on public APIs
Files:
packages/ts-moose-lib/src/scripts/runner.tspackages/ts-moose-lib/src/moose-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.
- In addition to reviewing for docs discrepancies, you should also review the code as per usual guidelines.
Files:
packages/ts-moose-lib/src/scripts/runner.tspackages/ts-moose-lib/src/moose-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 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/python/consumption.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/python/consumption.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/python/consumption.rsapps/framework-cli/src/framework/python/wrappers/consumption_runner.py
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Follow PEP 8 style guide in Python; use snake_case for functions/variables, PascalCase for classes, UPPER_SNAKE_CASE for constants
Format Python code with Black using line-length 88; auto-formats on commit via Husky + lint-staged
Use type hints for function signatures and public APIs in Python
Files:
apps/framework-cli/src/framework/python/wrappers/consumption_runner.py
🧠 Learnings (2)
📓 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:
packages/ts-moose-lib/src/scripts/runner.tsapps/framework-cli/src/framework/python/consumption.rspackages/ts-moose-lib/src/moose-runner.ts
🧬 Code graph analysis (1)
apps/framework-cli/src/framework/python/wrappers/consumption_runner.py (1)
apps/framework-cli/src/framework/python/utils/temporal.py (1)
create_temporal_connection(6-40)
🪛 Ruff (0.14.14)
apps/framework-cli/src/framework/python/wrappers/consumption_runner.py
[warning] 465-465: Do not catch blind exception: Exception
(BLE001)
⏰ 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). (5)
- GitHub Check: Test E2E TypeScript Tests Template (Node 24, npm)
- GitHub Check: Test E2E TypeScript Tests Template (Node 20, npm)
- GitHub Check: Test E2E TypeScript Tests Template (Node 22, npm)
- GitHub Check: Test E2E Python Tests Template (Python 3.13)
- GitHub Check: Test E2E Python Tests Template (Python 3.12)
🔇 Additional comments (7)
packages/ts-moose-lib/src/scripts/runner.ts (2)
119-123: Guard looks correct for workflows-disabled mode.
24-26: ScriptsConfig is not exported as public API. SinceScriptsConfigis not re-exported from the public API (index.ts) andrunScriptsis only used internally bymoose-runner, no documentation update is required for this change.Likely an incorrect or invalid review comment.
packages/ts-moose-lib/src/moose-runner.ts (2)
132-141: Confirm TS lint/Prettier checks were run.Please verify linting/formatting ran for this TS change set.
202-211: LGTM for optional scripts temporalConfig.apps/framework-cli/src/framework/python/wrappers/consumption_runner.py (3)
50-57: Optional flags with defaults look good.
73-77: Good use of safe getattr defaults.
472-472: Server port fallback is clear.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Merge activity
|
LucioFranco
deleted the
02-02-eng-2098_disable_temporal_connect_if_workflows_false
branch
Note
Medium Risk
Moderate risk because it changes CLI argument shapes and when Temporal connections/workflow registration occur, which could break existing invocations or unintentionally disable workflow functionality if flags/config are mis-set.
Overview
Makes Temporal integration conditional on workflows being enabled, rather than always attempting to configure/connect.
The Rust CLI now only passes Temporal parameters to the Python
consumption_runnerwhenproject.features.workflowsis true, and always passes--proxy-port. The Python runner switches Temporal/port args from required positionals to optional flags and only connects to Temporal when--temporal-urlis provided.In the TS runner,
temporalConfigis now optional (forconsumption-apisandscripts), and the scripts worker skips workflow registration entirely when Temporal config isn’t provided.Written by Cursor Bugbot for commit 5d3529e. This will update automatically on new commits. Configure here.