Cugaflo#271
Open
liorlimonad wants to merge 74 commits into
Open
Conversation
- Add cuga_flow module: BPMN parser, flow agent, langgraph engine, workflow engine, task/decision agents, hook manager, process registry - Add cuga_flo_mcp server bridge - Add flow_agent_app examples (loan approval, receive order, trip planner) - Add lifecycle diagrams - Patch supervisor graph, CLI, SDK, server routes, and frontend to wire in flow engine support Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- README.md: add missing CUGA FLO Nodes section (FlowAgent, TaskAgent, DecisionAgent, Hook Node documentation) - local_executor.py: replace hardcoded _timeout=30 with @Property reading settings.advanced_features.code_executor_timeout; default timeout=None - code_executor.py: replace hardcoded timeout=30 with configurable code_executor_timeout in eval_with_tools_async and _execute_locally - llm/models.py: add httpx.Timeout to http_client/http_async_client when ssl_verify is not True, preventing connections hanging indefinitely - manage_routes.py: add /flow/policies GET and PATCH endpoints plus _flow_policies_dir and _policy_display_name helpers - supervisor_config.py: add flow_agent type handling for inline FlowAgent instantiation; update import_from check to accept CugaAgent or FlowAgent Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| result_1 = await flow_agent.invoke(input_data_1) | ||
|
|
||
| print("\nResult:") | ||
| print(f" Credit Score: {result_1.get('credit_score', 'N/A')}") |
| result_2 = await flow_agent.invoke(input_data_2) | ||
|
|
||
| print("\nResult:") | ||
| print(f" Credit Score: {result_2.get('credit_score', 'N/A')}") |
|
|
||
| target = Path(policies_dir) / filename | ||
| # Prevent path traversal | ||
| if not target.resolve().parent == Path(policies_dir).resolve(): |
| if not target.resolve().parent == Path(policies_dir).resolve(): | ||
| raise HTTPException(status_code=400, detail="Invalid filename") | ||
|
|
||
| target.write_text(content, encoding="utf-8") |
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds CUGA FLO: BPMN→LangGraph flow orchestration with FlowState, BPMN parser, Task/Decision agents, hook system, YAML FlowConfig loader, LangGraph engine, MCP bridge, ProcessRegistry, example flows, supervisor integration, and management endpoints. ChangesCUGA FLO Core Framework
Examples, Supervisor Integration, and Management APIs
Sequence Diagram(s)sequenceDiagram
participant Application
participant ProcessRegistry
participant FlowAgent
participant MCPFlowBridge
participant LangGraphWorkflowEngine
participant FlowState
participant TaskAgent
participant DecisionAgent
Application->>ProcessRegistry: discover/register flows
ProcessRegistry->>FlowAgent: provide process definition
FlowAgent->>MCPFlowBridge: register tools (execute_task, route_gateway, evaluate_hook, get_static_config)
Application->>FlowAgent: invoke(input)
FlowAgent->>MCPFlowBridge: run_process(process_key, inputs)
MCPFlowBridge->>LangGraphWorkflowEngine: _run_via_mcp(process, inputs, mcp_server)
LangGraphWorkflowEngine->>MCPFlowBridge: get_static_config()
MCPFlowBridge-->>LangGraphWorkflowEngine: static_config
LangGraphWorkflowEngine->>FlowState: initialize(start metadata)
LangGraphWorkflowEngine->>MCPFlowBridge: execute_task(ctx)
MCPFlowBridge->>FlowAgent: _handle_task(ctx)
FlowAgent->>TaskAgent: execute(task_input)
TaskAgent-->>FlowAgent: task_result
FlowAgent-->>MCPFlowBridge: task_result
LangGraphWorkflowEngine->>MCPFlowBridge: route_gateway(ctx)
MCPFlowBridge->>FlowAgent: _handle_gateway(ctx)
FlowAgent->>DecisionAgent: route(flows, state)
DecisionAgent-->>FlowAgent: chosen_flow_id
FlowAgent-->>MCPFlowBridge: gateway_result
LangGraphWorkflowEngine->>MCPFlowBridge: evaluate_hook(edge_ctx)
MCPFlowBridge->>FlowAgent: _handle_hook(ctx)
FlowAgent-->>MCPFlowBridge: HookResult
LangGraphWorkflowEngine-->>FlowAgent: final FlowState (json)
FlowAgent-->>Application: validated FlowState
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 12
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/cuga/backend/cuga_graph/nodes/cuga_lite/executors/local/local_executor.py (1)
38-42:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMake
timeoutexplicitly optional to satisfy RUF013
Insrc/cuga/backend/cuga_graph/nodes/cuga_lite/executors/local/local_executor.py,LocalExecutor.executeusestimeout: int = None(RuffRUF013) at line 42. Update the annotation to an explicit optional type.Proposed fix
- timeout: int = None, + timeout: int | None = None,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/cuga/backend/cuga_graph/nodes/cuga_lite/executors/local/local_executor.py` around lines 38 - 42, The parameter annotation for timeout on LocalExecutor.execute should be made explicitly optional to satisfy RUF013: change the signature to use an optional type (e.g., timeout: Optional[int] or timeout: int | None) and update imports if necessary (add from typing import Optional when using Optional[int]); locate the symbol LocalExecutor.execute in local_executor.py and update the parameter annotation and any related type hints accordingly.
🟡 Minor comments (13)
src/cuga/backend/cuga_graph/nodes/cuga_flow/README.md-163-172 (1)
163-172:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd a language tag to the fenced module-structure block.
This block triggers markdownlint MD040 (
fenced-code-language).📝 Proposed fix
-``` +```text cuga_flow/ ├── flow_agent.py # FlowAgent — compiles BPMN → LangGraph, meta-agent ├── flow_agent_state.py # FlowState — process-aware agent state ├── flow_config.py # FlowConfig — YAML-based instantiation ├── bpmn_parser.py # BPMN 2.0 XML parser → BPMNProcess ├── task_agent.py # TaskAgent — CugaAgent wrapper for task nodes ├── decision_agent.py # DecisionAgent — two-node gateway router └── hook_manager.py # Hook, HookManager, HookAction, HookResult</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@src/cuga/backend/cuga_graph/nodes/cuga_flow/README.mdaround lines 163 -
172, Add a fenced-code language tag (e.g., "text") to the module-structure code
block that lists files like flow_agent.py, flow_agent_state.py, flow_config.py,
bpmn_parser.py, task_agent.py, decision_agent.py, and hook_manager.py in the
README module-structure block so it satisfies markdownlint MD040; locate the
triple-backtick fence surrounding that listing and change it fromtotext
(keeping the block content unchanged).</details> </blockquote></details> <details> <summary>src/cuga/backend/cuga_graph/nodes/cuga_flow/__init__.py-53-86 (1)</summary><blockquote> `53-86`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Sort `__all__` to satisfy Ruff RUF022.** `__all__` ordering currently violates the configured lint rule and can fail CI. <details> <summary>🔧 Proposed fix</summary> ```diff __all__ = [ - # State - "FlowState", - "HookEvaluation", - "GraphModification", - # Hooks - "Hook", - "HookType", - "HookAction", - "HookResult", - "HookManager", - "GraphChangeSpec", - "create_simple_hook", - # BPMN Parser - "BPMNParser", - "BPMNProcess", - "BPMNElement", - "BPMNFlow", - # Agents - "DecisionAgent", - "TaskAgent", - "TaskAgentFactory", - "FlowAgent", - # Configuration - "FlowConfig", - "load_flow_from_yaml", - # Registry & Engine (new) - "ProcessRegistry", - "ProcessDefinition", - "WorkflowEngine", - "ControlPointContext", - "MCPFlowBridge", - "LangGraphWorkflowEngine", + "BPMNElement", + "BPMNFlow", + "BPMNParser", + "BPMNProcess", + "ControlPointContext", + "DecisionAgent", + "FlowAgent", + "FlowConfig", + "FlowState", + "GraphChangeSpec", + "GraphModification", + "Hook", + "HookAction", + "HookEvaluation", + "HookManager", + "HookResult", + "HookType", + "LangGraphWorkflowEngine", + "MCPFlowBridge", + "ProcessDefinition", + "ProcessRegistry", + "TaskAgent", + "TaskAgentFactory", + "WorkflowEngine", + "create_simple_hook", + "load_flow_from_yaml", ] ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/cuga/backend/cuga_graph/nodes/cuga_flow/__init__.py` around lines 53 - 86, The __all__ list in this module is not alphabetized and triggers Ruff RUF022; reorder the entries in the __all__ variable so they are sorted lexicographically (ASCII order) while preserving the same exported symbols (no additions/removals), and ensure there are no duplicate names; update the existing __all__ declaration (the list containing "FlowState", "HookEvaluation", ... "LangGraphWorkflowEngine") to the sorted order to satisfy the linter. ``` </details> </blockquote></details> <details> <summary>src/cuga/backend/server/manage_routes.py-1442-1458 (1)</summary><blockquote> `1442-1458`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Restrict PATCH to actual flow policy filenames.** This accepts any `.md` under `policies/`, and `write_text()` will create it if it does not exist. That means the “flow policies” endpoint can mutate markdown files the GET endpoint never exposes. Limit `filename` to the `task-`, `decision-`, and `hook-` patterns, and reject unknown files instead of creating them. <details> <summary>Proposed fix</summary> ```diff + import re + body = await request.json() filename: str = body.get("filename", "") content: str = body.get("content", "") - if not filename or not filename.endswith(".md"): + if not isinstance(filename, str) or not re.fullmatch( + r"(task|decision|hook)-[^/\\]+\.md", filename + ): raise HTTPException(status_code=400, detail="Invalid filename") + if not isinstance(content, str): + raise HTTPException(status_code=400, detail="Invalid content") policies_dir = _flow_policies_dir() if not policies_dir: raise HTTPException(status_code=404, detail="No flow policies directory found") - target = Path(policies_dir) / filename + base_dir = Path(policies_dir).resolve() + target = (base_dir / filename).resolve() # Prevent path traversal - if not target.resolve().parent == Path(policies_dir).resolve(): + if target.parent != base_dir: raise HTTPException(status_code=400, detail="Invalid filename") + if not target.is_file(): + raise HTTPException(status_code=404, detail="Policy not found") target.write_text(content, encoding="utf-8") ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/cuga/backend/server/manage_routes.py` around lines 1442 - 1458, The PATCH handler currently accepts any .md and will create new files; restrict allowed filenames by validating the filename string (variable filename) against the allowed prefixes (e.g., startswith "task-", "decision-", or "hook-") and the expected pattern (only alphanumeric/-, no extra path separators), reject others with HTTPException(400). After resolving the target Path (variable target) keep the existing path-traversal check, then require that target.exists() is True before calling target.write_text(content, ...); if the file does not exist return HTTPException(404) so new files cannot be created. Use the existing helper _flow_policies_dir() to locate policies_dir and perform these checks before writing. ``` </details> </blockquote></details> <details> <summary>docs/examples/flow_agent_app/config/loan_approval_config.yaml-55-55 (1)</summary><blockquote> `55-55`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Gateway threshold excludes the documented 0.6 boundary.** Line 55 uses `> 0.6`, but your score guidance marks `0.6` as approvable. A score of exactly `0.6` will currently route to rejection. <details> <summary>🔧 Suggested fix</summary> ```diff - condition: "${credit_score} > 0.6" # evaluated by eval_condition; result forwarded to LLM + policy + condition: "${credit_score} >= 0.6" # evaluated by eval_condition; result forwarded to LLM + policy ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/examples/flow_agent_app/config/loan_approval_config.yaml` at line 55, The gateway condition currently uses a strict greater-than which excludes the documented 0.6 boundary; update the condition expression (the YAML key "condition" evaluated by eval_condition) from "${credit_score} > 0.6" to include equality so a score of exactly 0.6 is treated as approvable (i.e., use >= instead of >). ``` </details> </blockquote></details> <details> <summary>docs/examples/flow_agent_app/simple_loan_demo.py-71-83 (1)</summary><blockquote> `71-83`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Config introspection reads the wrong structure and always under-reports.** Lines 71-83 traverse `flow.tasks`/`flow.gateways`, but this PR’s YAML defines `tasks` and `gateways` at the top level. The demo will print `0` even when config is populated. <details> <summary>🔧 Suggested fix</summary> ```diff - tasks_config = flow_config.config.get('flow', {}).get('tasks', {}) - print(f" Tasks Configured: {len(tasks_config)}") - for task_id, task_config in tasks_config.items(): - agent = task_config.get('agent', 'N/A') - print(f" - {task_id}: {agent}") + tasks_config = flow_config.config.get("tasks", []) + print(f" Tasks Configured: {len(tasks_config)}") + for task in tasks_config: + task_id = task.get("id", "N/A") + mode = task.get("mode", "N/A") + print(f" - {task_id}: {mode}") - gateways_config = flow_config.config.get('flow', {}).get('gateways', {}) + gateways_config = flow_config.config.get("gateways", {}) print(f" Gateways Configured: {len(gateways_config)}") for gw_id, gw_config in gateways_config.items(): - gw_type = gw_config.get('type', 'N/A') + gw_type = gw_config.get("mode", "N/A") print(f" - {gw_id}: {gw_type}") ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/examples/flow_agent_app/simple_loan_demo.py` around lines 71 - 83, The code is reading tasks/gateways under flow_config.config['flow'] but the YAML in this PR places 'tasks' and 'gateways' at the top level; update the lookups that set tasks_config and gateways_config to use flow_config.config.get('tasks', {}) and flow_config.config.get('gateways', {}) respectively so the counts and loops (the for loops over tasks_config.items() and gateways_config.items()) reflect the actual config entries; keep the existing agent/gw_type extraction and print statements unchanged. ``` </details> </blockquote></details> <details> <summary>docs/examples/flow_agent_app/simple_loan_demo.py-57-57 (1)</summary><blockquote> `57-57`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Narrow broad exception handlers to avoid swallowing unexpected errors.** `ruff` flags BLE001 for `except Exception as e` at lines 57, 91, and 109 in `docs/examples/flow_agent_app/simple_loan_demo.py`. Catch more specific exception types and re-raise unexpected ones instead of handling all `Exception`. Also applies to: 91-91, 109-109 <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/examples/flow_agent_app/simple_loan_demo.py` at line 57, Replace the broad "except Exception as e" handlers in docs/examples/flow_agent_app/simple_loan_demo.py with specific exception types relevant to the surrounding code (for example ValueError, KeyError, IOError, or whatever the called APIs raise) and only handle those cases; for any other unexpected exceptions, log if needed and re-raise instead of swallowing (i.e., catch specific exceptions in the blocks where "except Exception as e" appears, handle them, and add a final "except Exception: raise" or simply omit the broad handler so unexpected errors propagate). Ensure you update each occurrence of the "except Exception as e" pattern in the file accordingly so you don't suppress unrelated failures. ``` </details> </blockquote></details> <details> <summary>docs/examples/flow_agent_app_inline/loan_approval/policies/task-credit_check.md-9-17 (1)</summary><blockquote> `9-17`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Add blank lines around the scoring table (MD058).** Markdownlint warning is valid here; add a blank line before and after the table block to avoid lint noise. <details> <summary>Suggested markdown fix</summary> ```diff ## Scoring Criteria + | Factor | Weight | |--------|--------| | Payment history (on-time vs late) | 35 % | | Outstanding debt relative to income | 30 % | | Length of credit history | 15 % | | Credit mix (loans, cards, etc.) | 10 % | | Recent credit enquiries | 10 % | + ## Edge Cases ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/examples/flow_agent_app_inline/loan_approval/policies/task-credit_check.md` around lines 9 - 17, The Markdown table under the "Scoring Criteria" heading needs blank lines before and after to satisfy MD058; edit the file section containing the "Scoring Criteria" heading and the following table (the pipe-delimited block starting with "| Factor | Weight |") and insert one blank line immediately before the table and one blank line immediately after the table. ``` </details> </blockquote></details> <details> <summary>docs/examples/flow_agent_app_inline/receive_order/policies/task-receive_order.md-24-30 (1)</summary><blockquote> `24-30`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Add a language to the fenced block (MD040).** Line 24 opens a fenced block without a language, which will fail markdownlint. <details> <summary>Suggested fix</summary> ```diff -``` +```text Order registered successfully. - Order ID: <order_id> - Customer: <customer_name> (<customer_id>) - Items: <items> - Total: <total_amount> ``` ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@docs/examples/flow_agent_app_inline/receive_order/policies/task-receive_order.md
around lines 24 - 30, Add a language identifier to the fenced code block that
begins with "Order registered successfully." — change the opening triple
backticks to include a language (e.g., text) so the block reads ```text and the
rest of the content remains unchanged to satisfy markdownlint MD040.</details> </blockquote></details> <details> <summary>docs/examples/flow_agent_app_inline/receive_order/policies/task-generate_invoice.md-28-39 (1)</summary><blockquote> `28-39`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Add a language to the fenced block (MD040).** Line 28 opens a fenced block without a language, which will fail markdownlint. <details> <summary>Suggested fix</summary> ```diff -``` +```text INVOICE INV-<order_id> Issued to: <customer_name> (<customer_id>) Date: <today> @@ Thank you for your order. ``` ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@docs/examples/flow_agent_app_inline/receive_order/policies/task-generate_invoice.md
around lines 28 - 39, The fenced code block in task-generate_invoice.md is
missing a language tag which triggers markdownlint MD040; update the opening
fence fromto include a language (for exampletext) so the invoice block
is properly annotated and linting passes—ensure the closing fence remains ```
and that only the fence line is changed in the policy file.</details> </blockquote></details> <details> <summary>docs/examples/flow_agent_app_inline/receive_order/policies/task-reserve_inventory.md-22-25 (1)</summary><blockquote> `22-25`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Add a language to the fenced block (MD040).** Line 22 opens a fenced block without a language, which will fail markdownlint. <details> <summary>Suggested fix</summary> ```diff -``` +```text Inventory reserved for order <order_id>. Reserved items: <list of items and quantities> ``` ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@docs/examples/flow_agent_app_inline/receive_order/policies/task-reserve_inventory.md
around lines 22 - 25, The fenced code block in task-reserve_inventory.md that
starts with triple backticks and contains "Inventory reserved for order
<order_id>." is missing a language specifier (MD040); update that fenced block
to include a language token such as text (e.g., changetotext) so
markdownlint passes and the block is treated as plain text.</details> </blockquote></details> <details> <summary>src/cuga/backend/cuga_graph/nodes/cuga_flow/flow_config.py-100-106 (1)</summary><blockquote> `100-106`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Validate YAML root type before building `FlowConfig`.** At Line 101, `yaml.safe_load()` can return `None`/non-dict values; that currently fails later with less clear errors. Fail fast with a clear message here. <details> <summary>Proposed fix</summary> ```diff - with open(yaml_file, 'r') as f: - config_dict = yaml.safe_load(f) + with open(yaml_file, "r", encoding="utf-8") as f: + config_dict = yaml.safe_load(f) + if not isinstance(config_dict, dict): + raise ValueError(f"Flow config must be a YAML mapping at top level: {yaml_file}") ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/cuga/backend/cuga_graph/nodes/cuga_flow/flow_config.py` around lines 100 - 106, The code reads YAML via yaml.safe_load into config_dict in the FlowConfig loader but doesn't validate its type; add a fail-fast check after yaml.safe_load() to ensure config_dict is a dict (not None or another type) and raise a clear exception (or ValueError) with a descriptive message if it isn't. Update the loader that returns cls(config_dict, config_file_dir) (the FlowConfig constructor call) to only proceed when the validated dict is present; reference yaml.safe_load, config_dict, config_file_dir and cls (FlowConfig) so the validation is added immediately after loading the YAML. ``` </details> </blockquote></details> <details> <summary>src/cuga/backend/cuga_graph/nodes/cuga_flow/flow_config.py-543-549 (1)</summary><blockquote> `543-549`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Don’t silently ignore invalid YAML during config discovery.** At Lines 548-549, `except Exception: pass` can mask malformed files and lead to misleading discovery errors. Log and narrow exceptions. <details> <summary>Proposed fix</summary> ```diff - except Exception: - pass + except yaml.YAMLError as e: + logger.warning(f"Skipping invalid YAML file '{yf}': {e}") + except OSError as e: + logger.warning(f"Could not read YAML file '{yf}': {e}") ``` </details> As per coding guidelines, "Run ruff format and ruff check on all Python changes in the cuga_graph/nodes/ directory before committing." <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/cuga/backend/cuga_graph/nodes/cuga_flow/flow_config.py` around lines 543 - 549, Replace the bare except that swallows YAML parse errors when loading yf: catch yaml.YAMLError and OSError (or Exception as last resort) instead of a blanket except, and log the failure including yf and the exception details (use the module logger via logging.getLogger(__name__) or the existing logger in this module) rather than pass; ensure flow_yamls only appends when load succeeds and the doc contains "flow". After changes, run ruff format and ruff check on the cuga_graph/nodes/ directory before committing. ``` </details> </blockquote></details> <details> <summary>src/cuga/backend/cuga_graph/nodes/cuga_flow/flow_agent.py-471-474 (1)</summary><blockquote> `471-474`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Log definition lookup failures in `get_process_info`.** At Lines 473-474, silent `except/pass` hides registry issues and makes debugging harder. Keep fallback behavior, but log at debug level. <details> <summary>Proposed fix</summary> ```diff - except Exception: - pass + except Exception as e: + logger.debug(f"Could not load definition for process '{self.process_key}': {e}") ``` </details> As per coding guidelines, "Run ruff format and ruff check on all Python changes in the cuga_graph/nodes/ directory before committing." <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/cuga/backend/cuga_graph/nodes/cuga_flow/flow_agent.py` around lines 471 - 474, The try/except in get_process_info that calls self.registry.get_definition(self.process_key) currently swallows all exceptions; change it to catch Exception as e and log the failure at debug level (including the exception and context like self.process_key and registry) while keeping the fallback behavior, i.e. replace the bare except/pass with a processLogger.debug (or self.logger.debug) call that includes the exception details; after making the change, run ruff format and ruff check on the cuga_graph/nodes/ directory before committing. ``` </details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.Inline comments:
In
@docs/examples/flow_agent_app_inline/loan_approval/config/loan_approval_config.yaml:
- Around line 53-62: Gateway_09ad5fc's decision flows never set the approved
flag, so even when routing to Flow_0ybszcv ("Approve — credit score sufficient")
approved remains false and breaks the loan processor precondition; update the
Gateway_09ad5fc config so the approval branch (Flow_0ybszcv) explicitly sets
approved = true (e.g., add an action/variable assignment in that flow) and
ensure the rejection branch leaves approved false or sets it false to keep
behavior consistent.In
@docs/examples/flow_agent_app_inline/loan_approval/policies/decision-credit_decision.md:
- Around line 13-19: The table and the primary threshold disagree for score
0.60—update them to be consistent: either change the routing table ranges so the
second row starts at 0.61 (e.g., "| 0.61 – 0.79 | Good | Approve (yes) |" and
leave the primary thresholdcredit_score > 0.6), or change the primary
threshold tocredit_score >= 0.6and adjust the table label for the second row
to "0.60 – 0.79". Pick one approach, apply it consistently to both the routing
table and the Primary threshold line, and ensure the Markdown numeric
formatting matches the chosen inclusive/exclusive boundary.In
@docs/examples/flow_agent_app_inline/receive_order/policies/hook-post_receival.md:
- Around line 4-8: The current skip_to in hook-post_receival.md sends order_id
== 1212 straight to Activity_1h9ix55, which will deadlock at the parallel join
Gateway_15y9k7z that expects both branches; change the policy so the skip_to
target is a node after the parallel join (the post-join node) instead of
Activity_1h9ix55, or alternatively remodel the split/join (e.g., make the split
an inclusive/merge-friendly pattern) so a single-branch completion is valid;
update the action/target_node in the policy (or the process model’s split/join
semantics) accordingly and ensure references to Activity_1h9ix55 and
Gateway_15y9k7z remain consistent.In
@docs/examples/flow_agent_app_inline/trip_planner/config/trip_planner_config.yaml:
- Around line 40-43: The gateway currently checks planning_preference only for
exact "agent" and "human" branches (Flow_0g3h7rx and Flow_0xkvb29), which can
leave no matching outgoing flow for other values; add a default/fallback flow
(e.g., Flow_default) or add a normalization/validation step before the gateway
that maps/validates planning_preference to "agent" or "human" so the gateway
always has a deterministic outgoing route. Ensure the new fallback flow is
placed as an outgoing branch of the same gateway and/or update the input
processing logic to coerce invalid values into a known default.In
@docs/examples/flow_agent_app/simple_loan_demo.py:
- Around line 91-123: The current exception handlers allow execution to continue
with invalid state: when flow_config fails to load or flow_agent instantiation
fails the script still prints success messages; update the error handling around
the flow_config load and flow_agent creation (the except blocks catching
Exception for flow_config and the try around flow_config.to_flow_agent()) to
stop further execution on failure—either by raising/returning/exiting (e.g.,
sys.exit(1)) immediately after logging the error or by setting boolean flags
(e.g., config_loaded, agent_instantiated) and gating subsequent steps and the
final summary on those flags; ensure you reference flow_config,
flow_config.to_flow_agent(), and flow_agent.process when implementing the
guard/early-exit so no code runs with undefined objects and the final summary
only reports checks that actually succeeded.In
@src/cuga/backend/cuga_graph/nodes/cuga_flow/bpmn_parser.py:
- Line 122: The end_events declaration uses a None default while typing it as
List[str]; replace the None with an actual list to match the type and avoid
runtime errors—if this is a dataclass use end_events: List[str] =
field(default_factory=list) (importing dataclasses.field), otherwise initialize
end_events to an empty list in the class init (or change the annotation to
Optional[List[str]] if you intend to keep None semantics); target the end_events
symbol in bpmn_parser.py when making the change.- Around line 17-18: The parser currently imports xml.etree.ElementTree and
calls ET.parse(...) which is unsafe for untrusted BPMN XML; replace that import
and parsing usage with defusedxml.ElementTree (use defusedxml.ElementTree as ET)
so parsing uses the secure implementation. Also fix the BPMNProcess.end_events
typing and default: change the annotation from List[str] = None to either
Optional[List[str]] with guards where used or set a default empty list (e.g.,
end_events: List[str] = field(default_factory=list) / or initialize as [] in
init), and update usages referenced by list(process.end_events) and
flow.target_ref in process.end_events to assume a proper iterable (or guard when
Optional). Ensure references to ET.parse and any ElementTree functions in this
module use the defusedxml ET import and update BPMNProcess.end_events
declaration and any checks that iterate or test membership accordingly.In
@src/cuga/backend/cuga_graph/nodes/cuga_flow/decision_agent.py:
- Around line 238-240: The selection logic uses substring matching (next((fid
for fid in available_ids if fid in text), ...)) which can misroute when IDs are
substrings of others; change chosen to match whole tokens only by parsing text
into exact ID tokens or using a word-boundary regex match for each fid (e.g.,
use re.search with re.escape and \b) against the incoming text before falling
back to available_ids[0]; update the logic around available_ids and chosen so
only exact ID matches from state["available_flows"] are accepted.In
@src/cuga/backend/cuga_graph/nodes/cuga_flow/flow_agent_state.py:
- Around line 141-147: The TypeError occurs because FlowState.start_time is
timezone-aware (set with datetime.now(timezone.utc)) but end_time is assigned
timezone-naive via datetime.utcnow(); update all places that set end_time (e.g.,
in the halt method and any other assignments) to use timezone-aware timestamps
such as datetime.now(timezone.utc) so that FlowState.get_execution_summary() can
safely compute (self.end_time - self.start_time); ensure you update the symbol
references: FlowState.end_time assignment in halt() (and wherever end_time is
set).In
@src/cuga/backend/cuga_graph/nodes/cuga_flow/flow_agent.py:
- Around line 149-153: The hook policy LLM is always initialized with
settings.agent.planner.model; change the initialization in flow_agent.py so
FlowAgent uses the flow-selected model name (self.model_name) when present and
falls back to settings.agent.planner.model otherwise: update the line that
assigns self._llm (where LLMManager().get_model is called) to call
get_model(self.model_name or settings.agent.planner.model) so hook reasoning
respects the YAML/direct-config-selected model.- Around line 218-220: The except block in DecisionAgent that catches Exception
as e and returns flows[0].id on error (using gateway_id and flows) must stop
silently falling back to the first flow; instead remove the implicit fallback
and propagate the failure so the engine can handle it. Replace the return of
flows[0].id with either re-raising the caught exception (raise) or raising a
descriptive custom exception (including gateway_id and e) after logging the
error with logger.error, so callers can explicitly halt/recover rather than
proceeding down an incorrect branch.In
@src/cuga/backend/cuga_graph/nodes/cuga_flow/process_registry.py:
- Around line 72-79: The discovery loop currently swallows all errors with a
bare except, hiding malformed YAML or IO problems; update the try/except around
opening and yaml.safe_load(yf) in process_registry.py so it catches and handles
specific exceptions (FileNotFoundError, PermissionError, yaml.YAMLError, and
optionally UnicodeDecodeError), log an error including the filename (yf) and
exception details using the module/class logger (e.g., logger or self.logger)
and continue the loop rather than silently passing; keep the existing logic that
sets flow_yaml and breaks only on a valid dict with "flow".
Outside diff comments:
In
@src/cuga/backend/cuga_graph/nodes/cuga_lite/executors/local/local_executor.py:
- Around line 38-42: The parameter annotation for timeout on
LocalExecutor.execute should be made explicitly optional to satisfy RUF013:
change the signature to use an optional type (e.g., timeout: Optional[int] or
timeout: int | None) and update imports if necessary (add from typing import
Optional when using Optional[int]); locate the symbol LocalExecutor.execute in
local_executor.py and update the parameter annotation and any related type hints
accordingly.
Minor comments:
In
@docs/examples/flow_agent_app_inline/loan_approval/policies/task-credit_check.md:
- Around line 9-17: The Markdown table under the "Scoring Criteria" heading
needs blank lines before and after to satisfy MD058; edit the file section
containing the "Scoring Criteria" heading and the following table (the
pipe-delimited block starting with "| Factor | Weight |") and insert one blank
line immediately before the table and one blank line immediately after the
table.In
@docs/examples/flow_agent_app_inline/receive_order/policies/task-generate_invoice.md:
- Around line 28-39: The fenced code block in task-generate_invoice.md is
missing a language tag which triggers markdownlint MD040; update the opening
fence fromto include a language (for exampletext) so the invoice block
is properly annotated and linting passes—ensure the closing fence remains ```
and that only the fence line is changed in the policy file.In
@docs/examples/flow_agent_app_inline/receive_order/policies/task-receive_order.md:
- Around line 24-30: Add a language identifier to the fenced code block that
begins with "Order registered successfully." — change the opening triple
backticks to include a language (e.g., text) so the block reads ```text and the
rest of the content remains unchanged to satisfy markdownlint MD040.In
@docs/examples/flow_agent_app_inline/receive_order/policies/task-reserve_inventory.md:
- Around line 22-25: The fenced code block in task-reserve_inventory.md that
starts with triple backticks and contains "Inventory reserved for order
<order_id>." is missing a language specifier (MD040); update that fenced block
to include a language token such as text (e.g., changetotext) so
markdownlint passes and the block is treated as plain text.In
@docs/examples/flow_agent_app/config/loan_approval_config.yaml:
- Line 55: The gateway condition currently uses a strict greater-than which
excludes the documented 0.6 boundary; update the condition expression (the YAML
key "condition" evaluated by eval_condition) from "${credit_score} > 0.6" to
include equality so a score of exactly 0.6 is treated as approvable (i.e., use= instead of >).
In
@docs/examples/flow_agent_app/simple_loan_demo.py:
- Around line 71-83: The code is reading tasks/gateways under
flow_config.config['flow'] but the YAML in this PR places 'tasks' and 'gateways'
at the top level; update the lookups that set tasks_config and gateways_config
to use flow_config.config.get('tasks', {}) and
flow_config.config.get('gateways', {}) respectively so the counts and loops (the
for loops over tasks_config.items() and gateways_config.items()) reflect the
actual config entries; keep the existing agent/gw_type extraction and print
statements unchanged.- Line 57: Replace the broad "except Exception as e" handlers in
docs/examples/flow_agent_app/simple_loan_demo.py with specific exception types
relevant to the surrounding code (for example ValueError, KeyError, IOError, or
whatever the called APIs raise) and only handle those cases; for any other
unexpected exceptions, log if needed and re-raise instead of swallowing (i.e.,
catch specific exceptions in the blocks where "except Exception as e" appears,
handle them, and add a final "except Exception: raise" or simply omit the broad
handler so unexpected errors propagate). Ensure you update each occurrence of
the "except Exception as e" pattern in the file accordingly so you don't
suppress unrelated failures.In
@src/cuga/backend/cuga_graph/nodes/cuga_flow/__init__.py:
- Around line 53-86: The all list in this module is not alphabetized and
triggers Ruff RUF022; reorder the entries in the all variable so they are
sorted lexicographically (ASCII order) while preserving the same exported
symbols (no additions/removals), and ensure there are no duplicate names; update
the existing all declaration (the list containing "FlowState",
"HookEvaluation", ... "LangGraphWorkflowEngine") to the sorted order to satisfy
the linter.In
@src/cuga/backend/cuga_graph/nodes/cuga_flow/flow_agent.py:
- Around line 471-474: The try/except in get_process_info that calls
self.registry.get_definition(self.process_key) currently swallows all
exceptions; change it to catch Exception as e and log the failure at debug level
(including the exception and context like self.process_key and registry) while
keeping the fallback behavior, i.e. replace the bare except/pass with a
processLogger.debug (or self.logger.debug) call that includes the exception
details; after making the change, run ruff format and ruff check on the
cuga_graph/nodes/ directory before committing.In
@src/cuga/backend/cuga_graph/nodes/cuga_flow/flow_config.py:
- Around line 100-106: The code reads YAML via yaml.safe_load into config_dict
in the FlowConfig loader but doesn't validate its type; add a fail-fast check
after yaml.safe_load() to ensure config_dict is a dict (not None or another
type) and raise a clear exception (or ValueError) with a descriptive message if
it isn't. Update the loader that returns cls(config_dict, config_file_dir) (the
FlowConfig constructor call) to only proceed when the validated dict is present;
reference yaml.safe_load, config_dict, config_file_dir and cls (FlowConfig) so
the validation is added immediately after loading the YAML.- Around line 543-549: Replace the bare except that swallows YAML parse errors
when loading yf: catch yaml.YAMLError and OSError (or Exception as last resort)
instead of a blanket except, and log the failure including yf and the exception
details (use the module logger via logging.getLogger(name) or the existing
logger in this module) rather than pass; ensure flow_yamls only appends when
load succeeds and the doc contains "flow". After changes, run ruff format and
ruff check on the cuga_graph/nodes/ directory before committing.In
@src/cuga/backend/cuga_graph/nodes/cuga_flow/README.md:
- Around line 163-172: Add a fenced-code language tag (e.g., "text") to the
module-structure code block that lists files like flow_agent.py,
flow_agent_state.py, flow_config.py, bpmn_parser.py, task_agent.py,
decision_agent.py, and hook_manager.py in the README module-structure block so
it satisfies markdownlint MD040; locate the triple-backtick fence surrounding
that listing and change it fromtotext (keeping the block content
unchanged).In
@src/cuga/backend/server/manage_routes.py:
- Around line 1442-1458: The PATCH handler currently accepts any .md and will
create new files; restrict allowed filenames by validating the filename string
(variable filename) against the allowed prefixes (e.g., startswith "task-",
"decision-", or "hook-") and the expected pattern (only alphanumeric/-, no extra
path separators), reject others with HTTPException(400). After resolving the
target Path (variable target) keep the existing path-traversal check, then
require that target.exists() is True before calling target.write_text(content,
...); if the file does not exist return HTTPException(404) so new files cannot
be created. Use the existing helper _flow_policies_dir() to locate policies_dir
and perform these checks before writing.</details> <details> <summary>🪄 Autofix (Beta)</summary> Fix all unresolved CodeRabbit comments on this PR: - [ ] <!-- {"checkboxId": "4b0d0e0a-96d7-4f10-b296-3a18ea78f0b9"} --> Push a commit to this branch (recommended) - [ ] <!-- {"checkboxId": "ff5b1114-7d8c-49e6-8ac1-43f82af23a33"} --> Create a new PR with the fixes </details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: defaults **Review profile**: CHILL **Plan**: Pro **Run ID**: `824b77da-3cca-4409-b2e5-1083a3772f3b` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 27bd35e013369cfbc4f1d79c0cca74f156f573bf and bd23f1636ae59b88c6b2e03d5b6aff6f9f84b4c3. </details> <details> <summary>⛔ Files ignored due to path filters (2)</summary> * `src/cuga/frontend/dist/index.html` is excluded by `!**/dist/**` * `src/cuga/frontend/dist/main.5600d468a5c5189e68bc.js` is excluded by `!**/dist/**` </details> <details> <summary>📒 Files selected for processing (49)</summary> * `docs/diagrams/cuga-flo-lifecycle.md` * `docs/diagrams/cuga-flo-lifecycle.mmd` * `docs/examples/flow_agent_app/agents/__init__.py` * `docs/examples/flow_agent_app/agents/credit_checker.py` * `docs/examples/flow_agent_app/agents/loan_processor.py` * `docs/examples/flow_agent_app/agents/rejection_handler.py` * `docs/examples/flow_agent_app/config/loan_approval_config.yaml` * `docs/examples/flow_agent_app/flow_agent_instance.py` * `docs/examples/flow_agent_app/loan_demo.py` * `docs/examples/flow_agent_app/simple_loan_demo.py` * `docs/examples/flow_agent_app_inline/loan_approval/config/BPMNdiagram.bpmn` * `docs/examples/flow_agent_app_inline/loan_approval/config/loan_approval_config.yaml` * `docs/examples/flow_agent_app_inline/loan_approval/config/supervisor_loan_approval.yaml` * `docs/examples/flow_agent_app_inline/loan_approval/policies/decision-credit_decision.md` * `docs/examples/flow_agent_app_inline/loan_approval/policies/hook-audit_credit_decision.md` * `docs/examples/flow_agent_app_inline/loan_approval/policies/task-credit_check.md` * `docs/examples/flow_agent_app_inline/loan_approval/policies/task-loan_processor.md` * `docs/examples/flow_agent_app_inline/loan_approval/policies/task-rejection_handler.md` * `docs/examples/flow_agent_app_inline/receive_order/config/BPMNdiagram.bpmn` * `docs/examples/flow_agent_app_inline/receive_order/config/receive_order_config.yaml` * `docs/examples/flow_agent_app_inline/receive_order/config/supervisor_receive_order.yaml` * `docs/examples/flow_agent_app_inline/receive_order/policies/hook-post_receival.md` * `docs/examples/flow_agent_app_inline/receive_order/policies/task-generate_invoice.md` * `docs/examples/flow_agent_app_inline/receive_order/policies/task-receive_order.md` * `docs/examples/flow_agent_app_inline/receive_order/policies/task-reserve_inventory.md` * `docs/examples/flow_agent_app_inline/trip_planner/config/supervisor_trip_planner.yaml` * `docs/examples/flow_agent_app_inline/trip_planner/config/trip-planner.bpmn` * `docs/examples/flow_agent_app_inline/trip_planner/config/trip_planner_config.yaml` * `docs/examples/flow_agent_app_inline/trip_planner/policies/task-plan_trip_by_agent.md` * `src/cuga/backend/cuga_graph/README.md` * `src/cuga/backend/cuga_graph/nodes/cuga_flow/README.md` * `src/cuga/backend/cuga_graph/nodes/cuga_flow/__init__.py` * `src/cuga/backend/cuga_graph/nodes/cuga_flow/bpmn_parser.py` * `src/cuga/backend/cuga_graph/nodes/cuga_flow/decision_agent.py` * `src/cuga/backend/cuga_graph/nodes/cuga_flow/flow_agent.py` * `src/cuga/backend/cuga_graph/nodes/cuga_flow/flow_agent_state.py` * `src/cuga/backend/cuga_graph/nodes/cuga_flow/flow_config.py` * `src/cuga/backend/cuga_graph/nodes/cuga_flow/hook_manager.py` * `src/cuga/backend/cuga_graph/nodes/cuga_flow/langgraph_engine.py` * `src/cuga/backend/cuga_graph/nodes/cuga_flow/process_registry.py` * `src/cuga/backend/cuga_graph/nodes/cuga_flow/task_agent.py` * `src/cuga/backend/cuga_graph/nodes/cuga_flow/workflow_engine.py` * `src/cuga/backend/cuga_graph/nodes/cuga_lite/executors/code_executor.py` * `src/cuga/backend/cuga_graph/nodes/cuga_lite/executors/local/local_executor.py` * `src/cuga/backend/llm/models.py` * `src/cuga/backend/server/cuga_flo_mcp/__init__.py` * `src/cuga/backend/server/cuga_flo_mcp/bridge.py` * `src/cuga/backend/server/manage_routes.py` * `src/cuga/supervisor_utils/supervisor_config.py` </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
Comment on lines
+53
to
+62
| Gateway_09ad5fc: | ||
| mode: decision_agent | ||
| condition: "${credit_score} > 0.6" # evaluated by eval_condition; result forwarded to LLM + policy | ||
| policy: "../policies/decision-credit_decision.md" | ||
| flows: | ||
| Flow_0ybszcv: | ||
| decision: "Approve — credit score sufficient" | ||
| Flow_1jgea85: | ||
| decision: "Reject — credit score insufficient" | ||
|
|
Contributor
There was a problem hiding this comment.
Define who sets approved during gateway routing.
approved is initialized as false, but this config never sets it on the approval path. That conflicts with the loan processor precondition (approved == true) and can produce contradictory behavior on successful routing.
Suggested config-level fix
gateways:
Gateway_09ad5fc:
mode: decision_agent
condition: "${credit_score} > 0.6" # evaluated by eval_condition; result forwarded to LLM + policy
policy: "../policies/decision-credit_decision.md"
flows:
Flow_0ybszcv:
decision: "Approve — credit score sufficient"
+ state_updates:
+ approved: true
+ rejection_reason: ""
Flow_1jgea85:
decision: "Reject — credit score insufficient"
+ state_updates:
+ approved: false📝 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.
Suggested change
| Gateway_09ad5fc: | |
| mode: decision_agent | |
| condition: "${credit_score} > 0.6" # evaluated by eval_condition; result forwarded to LLM + policy | |
| policy: "../policies/decision-credit_decision.md" | |
| flows: | |
| Flow_0ybszcv: | |
| decision: "Approve — credit score sufficient" | |
| Flow_1jgea85: | |
| decision: "Reject — credit score insufficient" | |
| Gateway_09ad5fc: | |
| mode: decision_agent | |
| condition: "${credit_score} > 0.6" # evaluated by eval_condition; result forwarded to LLM + policy | |
| policy: "../policies/decision-credit_decision.md" | |
| flows: | |
| Flow_0ybszcv: | |
| decision: "Approve — credit score sufficient" | |
| state_updates: | |
| approved: true | |
| rejection_reason: "" | |
| Flow_1jgea85: | |
| decision: "Reject — credit score insufficient" | |
| state_updates: | |
| approved: false |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@docs/examples/flow_agent_app_inline/loan_approval/config/loan_approval_config.yaml`
around lines 53 - 62, Gateway_09ad5fc's decision flows never set the approved
flag, so even when routing to Flow_0ybszcv ("Approve — credit score sufficient")
approved remains false and breaks the loan processor precondition; update the
Gateway_09ad5fc config so the approval branch (Flow_0ybszcv) explicitly sets
approved = true (e.g., add an action/variable assignment in that flow) and
ensure the rejection branch leaves approved false or sets it false to keep
behavior consistent.
Comment on lines
+13
to
+19
| | 0.80 – 1.00 | Excellent | Approve (yes) | | ||
| | 0.60 – 0.79 | Good | Approve (yes) | | ||
| | 0.40 – 0.59 | Fair | Reject (no) | | ||
| | 0.00 – 0.39 | Poor | Reject (no) | | ||
|
|
||
| **Primary threshold**: `credit_score > 0.6` → approve; `credit_score ≤ 0.6` → reject. | ||
|
|
Contributor
There was a problem hiding this comment.
Fix threshold contradiction for score 0.60.
The routing table approves 0.60, but the primary rule rejects <= 0.6. This ambiguity can make gateway decisions nondeterministic.
Suggested edit (pick one consistent rule)
-**Primary threshold**: `credit_score > 0.6` → approve; `credit_score ≤ 0.6` → reject.
+**Primary threshold**: `credit_score >= 0.6` → approve; `credit_score < 0.6` → reject.📝 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.
Suggested change
| | 0.80 – 1.00 | Excellent | Approve (yes) | | |
| | 0.60 – 0.79 | Good | Approve (yes) | | |
| | 0.40 – 0.59 | Fair | Reject (no) | | |
| | 0.00 – 0.39 | Poor | Reject (no) | | |
| **Primary threshold**: `credit_score > 0.6` → approve; `credit_score ≤ 0.6` → reject. | |
| | 0.80 – 1.00 | Excellent | Approve (yes) | | |
| | 0.60 – 0.79 | Good | Approve (yes) | | |
| | 0.40 – 0.59 | Fair | Reject (no) | | |
| | 0.00 – 0.39 | Poor | Reject (no) | | |
| **Primary threshold**: `credit_score >= 0.6` → approve; `credit_score < 0.6` → reject. | |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@docs/examples/flow_agent_app_inline/loan_approval/policies/decision-credit_decision.md`
around lines 13 - 19, The table and the primary threshold disagree for score
0.60—update them to be consistent: either change the routing table ranges so the
second row starts at 0.61 (e.g., "| 0.61 – 0.79 | Good | Approve (yes) |" and
leave the primary threshold `credit_score > 0.6`), or change the primary
threshold to `credit_score >= 0.6` and adjust the table label for the second row
to "0.60 – 0.79". Pick one approach, apply it consistently to both the routing
table and the **Primary threshold** line, and ensure the Markdown numeric
formatting matches the chosen inclusive/exclusive boundary.
Comment on lines
+4
to
+8
| If the current order's `order_id` is exactly `1212`, redirect the flow directly to the | ||
| Reserve Inventory task (`Activity_1h9ix55`), bypassing the parallel gateway and the | ||
| Generate Invoice task entirely. | ||
|
|
||
| Use action `skip_to` with `target_node = "Activity_1h9ix55"`. |
Contributor
There was a problem hiding this comment.
Parallel-join stall risk in the skip_to target.
On Line 4–Line 8, the policy skips directly to Activity_1h9ix55, but the process later synchronizes at a parallel join (Gateway_15y9k7z) that expects both incoming branches. This can block completion for the 1212 path. Please either route to a post-join node for this override path or remodel split/join semantics so single-branch completion is valid.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@docs/examples/flow_agent_app_inline/receive_order/policies/hook-post_receival.md`
around lines 4 - 8, The current skip_to in hook-post_receival.md sends order_id
== 1212 straight to Activity_1h9ix55, which will deadlock at the parallel join
Gateway_15y9k7z that expects both branches; change the policy so the skip_to
target is a node after the parallel join (the post-join node) instead of
Activity_1h9ix55, or alternatively remodel the split/join (e.g., make the split
an inclusive/merge-friendly pattern) so a single-branch completion is valid;
update the action/target_node in the policy (or the process model’s split/join
semantics) accordingly and ensure references to Activity_1h9ix55 and
Gateway_15y9k7z remain consistent.
Comment on lines
+91
to
+123
| except Exception as e: | ||
| print(f"✗ Error loading configuration: {e}") | ||
| import traceback | ||
|
|
||
| traceback.print_exc() | ||
| print() | ||
|
|
||
| # Step 4: Instantiate FlowAgent | ||
| print("Step 4: Instantiate FlowAgent") | ||
| print("-" * 60) | ||
| try: | ||
| flow_agent = flow_config.to_flow_agent() | ||
| print("✓ FlowAgent instantiated successfully") | ||
| print(f" Process: {flow_agent.process.name}") | ||
| print(f" Elements: {len(flow_agent.process.elements)}") | ||
| print(f" Flows: {len(flow_agent.process.flows)}") | ||
| print() | ||
|
|
||
| except Exception as e: | ||
| print(f"✗ Error instantiating FlowAgent: {e}") | ||
| import traceback | ||
|
|
||
| traceback.print_exc() | ||
| print() | ||
|
|
||
| print("=" * 60) | ||
| print("Test Complete!") | ||
| print("=" * 60) | ||
| print() | ||
| print("Summary:") | ||
| print(" ✓ BPMN process parsed successfully") | ||
| print(" ✓ YAML configuration loaded") | ||
| print(" ✓ FlowAgent instantiated") |
Contributor
There was a problem hiding this comment.
Failure paths are reported as success and execution continues with invalid state.
If config loading fails, Line 102 can execute with an undefined flow_config, and Lines 121-123 still print all checks as successful. This masks real failures.
🔧 Suggested fix
+ bpmn_ok = False
+ config_ok = False
+ instantiate_ok = False
+ flow_config = None
@@
try:
parser = BPMNParser()
process = parser.parse_file(str(bpmn_file))
+ bpmn_ok = True
@@
try:
flow_config = FlowConfig.from_yaml(str(config_file))
+ config_ok = True
@@
- try:
- flow_agent = flow_config.to_flow_agent()
+ if config_ok and flow_config is not None:
+ try:
+ flow_agent = flow_config.to_flow_agent()
+ instantiate_ok = True
+ print("✓ FlowAgent instantiated successfully")
+ print(f" Process: {flow_agent.process.name}")
+ print(f" Elements: {len(flow_agent.process.elements)}")
+ print(f" Flows: {len(flow_agent.process.flows)}")
+ print()
+ except Exception as e:
+ print(f"✗ Error instantiating FlowAgent: {e}")
+ import traceback
+ traceback.print_exc()
+ print()
+ else:
+ print("✗ Skipping FlowAgent instantiation because configuration failed.")
+ print()
- print("✓ FlowAgent instantiated successfully")
- print(f" Process: {flow_agent.process.name}")
- print(f" Elements: {len(flow_agent.process.elements)}")
- print(f" Flows: {len(flow_agent.process.flows)}")
- print()
-
- except Exception as e:
- print(f"✗ Error instantiating FlowAgent: {e}")
- import traceback
-
- traceback.print_exc()
- print()
@@
- print(" ✓ BPMN process parsed successfully")
- print(" ✓ YAML configuration loaded")
- print(" ✓ FlowAgent instantiated")
+ print(f" {'✓' if bpmn_ok else '✗'} BPMN process parsed")
+ print(f" {'✓' if config_ok else '✗'} YAML configuration loaded")
+ print(f" {'✓' if instantiate_ok else '✗'} FlowAgent instantiated")🧰 Tools
🪛 Ruff (0.15.13)
[warning] 91-91: Do not catch blind exception: Exception
(BLE001)
[warning] 109-109: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/examples/flow_agent_app/simple_loan_demo.py` around lines 91 - 123, The
current exception handlers allow execution to continue with invalid state: when
flow_config fails to load or flow_agent instantiation fails the script still
prints success messages; update the error handling around the flow_config load
and flow_agent creation (the except blocks catching Exception for flow_config
and the try around flow_config.to_flow_agent()) to stop further execution on
failure—either by raising/returning/exiting (e.g., sys.exit(1)) immediately
after logging the error or by setting boolean flags (e.g., config_loaded,
agent_instantiated) and gating subsequent steps and the final summary on those
flags; ensure you reference flow_config, flow_config.to_flow_agent(), and
flow_agent.process when implementing the guard/early-exit so no code runs with
undefined objects and the final summary only reports checks that actually
succeeded.
Comment on lines
+238
to
+240
| available_ids = [f["id"] for f in state["available_flows"]] | ||
| chosen = next((fid for fid in available_ids if fid in text), available_ids[0]) | ||
|
|
Contributor
There was a problem hiding this comment.
Avoid substring-based flow ID selection; it can misroute gateways.
fid in text is ambiguous (e.g., flow_1 matches inside flow_10). Parse an exact ID token instead.
Proposed fix
+import re
...
available_ids = [f["id"] for f in state["available_flows"]]
- chosen = next((fid for fid in available_ids if fid in text), available_ids[0])
+ id_set = set(available_ids)
+ tokens = re.findall(r"[A-Za-z0-9_.:-]+", text)
+ chosen = next((tok for tok in tokens if tok in id_set), available_ids[0])📝 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.
Suggested change
| available_ids = [f["id"] for f in state["available_flows"]] | |
| chosen = next((fid for fid in available_ids if fid in text), available_ids[0]) | |
| available_ids = [f["id"] for f in state["available_flows"]] | |
| id_set = set(available_ids) | |
| tokens = re.findall(r"[A-Za-z0-9_.:-]+", text) | |
| chosen = next((tok for tok in tokens if tok in id_set), available_ids[0]) | |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/cuga/backend/cuga_graph/nodes/cuga_flow/decision_agent.py` around lines
238 - 240, The selection logic uses substring matching (next((fid for fid in
available_ids if fid in text), ...)) which can misroute when IDs are substrings
of others; change chosen to match whole tokens only by parsing text into exact
ID tokens or using a word-boundary regex match for each fid (e.g., use re.search
with re.escape and \b) against the incoming text before falling back to
available_ids[0]; update the logic around available_ids and chosen so only exact
ID matches from state["available_flows"] are accepted.
Comment on lines
+149
to
+153
| from cuga.backend.llm.models import LLMManager | ||
| from cuga.config import settings | ||
|
|
||
| self._llm = LLMManager().get_model(settings.agent.planner.model) | ||
|
|
Contributor
There was a problem hiding this comment.
Use the flow-selected model for hook policy LLM decisions.
At Line 152, hook reasoning always uses settings.agent.planner.model, ignoring self.model_name set from YAML/direct config.
Proposed fix
- self._llm = LLMManager().get_model(settings.agent.planner.model)
+ selected_model = self.model_name or settings.agent.planner.model
+ self._llm = LLMManager().get_model(selected_model)
+ self.model_name = selected_model📝 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.
Suggested change
| from cuga.backend.llm.models import LLMManager | |
| from cuga.config import settings | |
| self._llm = LLMManager().get_model(settings.agent.planner.model) | |
| from cuga.backend.llm.models import LLMManager | |
| from cuga.config import settings | |
| selected_model = self.model_name or settings.agent.planner.model | |
| self._llm = LLMManager().get_model(selected_model) | |
| self.model_name = selected_model |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/cuga/backend/cuga_graph/nodes/cuga_flow/flow_agent.py` around lines 149 -
153, The hook policy LLM is always initialized with
settings.agent.planner.model; change the initialization in flow_agent.py so
FlowAgent uses the flow-selected model name (self.model_name) when present and
falls back to settings.agent.planner.model otherwise: update the line that
assigns self._llm (where LLMManager().get_model is called) to call
get_model(self.model_name or settings.agent.planner.model) so hook reasoning
respects the YAML/direct-config-selected model.
Comment on lines
+218
to
+220
| except Exception as e: | ||
| logger.error(f" DecisionAgent error for {gateway_id}: {e}") | ||
| return flows[0].id if flows else "" |
Contributor
There was a problem hiding this comment.
Avoid defaulting to the first flow when gateway routing fails.
At Lines 218-220, falling back to flows[0] on routing errors can send execution down an incorrect branch. Propagate failure so the engine can halt/recover explicitly.
Proposed fix
- except Exception as e:
- logger.error(f" DecisionAgent error for {gateway_id}: {e}")
- return flows[0].id if flows else ""
+ except Exception as e:
+ logger.error(f"DecisionAgent error for gateway '{gateway_id}': {e}")
+ raise RuntimeError(f"Gateway routing failed for '{gateway_id}'") from e📝 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.
Suggested change
| except Exception as e: | |
| logger.error(f" DecisionAgent error for {gateway_id}: {e}") | |
| return flows[0].id if flows else "" | |
| except Exception as e: | |
| logger.error(f"DecisionAgent error for gateway '{gateway_id}': {e}") | |
| raise RuntimeError(f"Gateway routing failed for '{gateway_id}'") from e |
🧰 Tools
🪛 Ruff (0.15.13)
[warning] 218-218: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/cuga/backend/cuga_graph/nodes/cuga_flow/flow_agent.py` around lines 218 -
220, The except block in DecisionAgent that catches Exception as e and returns
flows[0].id on error (using gateway_id and flows) must stop silently falling
back to the first flow; instead remove the implicit fallback and propagate the
failure so the engine can handle it. Replace the return of flows[0].id with
either re-raising the caught exception (raise) or raising a descriptive custom
exception (including gateway_id and e) after logging the error with
logger.error, so callers can explicitly halt/recover rather than proceeding down
an incorrect branch.
…nd ProcessRegistry Reframes cuga-flo as a process harness decoupled from its execution engine via MCPFlowBridge. Adds sections for WorkflowEngine ABC (LangGraph as demo engine), MCPFlowBridge integration contract, ProcessRegistry auto-discovery, demo app invocation, and updated module structure.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/cuga/backend/cuga_graph/nodes/cuga_flow/README.md`:
- Around line 271-286: The fenced code block in README.md for the module
structure is missing a language tag (triggering markdownlint MD040); update the
opening fence for the module-structure snippet in
src/cuga/backend/cuga_graph/nodes/cuga_flow/README.md from ``` to a specific
language (e.g., ```text) so the block is lint-clean and preserves formatting for
the tree listing.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d0799675-68a3-4ec7-ab65-6871f28247a6
⛔ Files ignored due to path filters (1)
docs/images/cugaflo-logo.pngis excluded by!**/*.png
📒 Files selected for processing (1)
src/cuga/backend/cuga_graph/nodes/cuga_flow/README.md
…LO reconciliation
…ODE hook actions REMOVE_NODE and ADD_NODE now trigger a recompile-and-replay cycle: - Hook node sets _graph_rebuild_spec and _replay_completed_nodes, routes to END - _run_graph rebuild loop applies the structural change via _apply_graph_modification - REMOVE_NODE: removes element, rewires predecessor→successor bypass flows - ADD_NODE: inserts element, redirects incoming flows, adds MCP-backed task handler - All previously-executed nodes fast-forward as no-ops on the next graph pass - Updates README with new actions and rebuild semantics
…E graph rebuild Replace the replay-of-executed-nodes approach with a conditional START edge that routes directly to the correct resumption point: - ADD_NODE resumes at the newly inserted node - REMOVE_NODE resumes at the removed node's successor (or normal_target if the removed node is not the immediate next) - _build_graph now owns START routing; _add_edges_with_hooks skips start-event flows - Node factories no longer carry replay fast-forward logic
…reference extraction [skip ci]
…ck to process variables [skip ci]
…s variables [skip ci]
…ing logs [skip ci] - _process_output: use JSONDecoder.raw_decode to find JSON anywhere in content, handles preamble text that would previously cause silent parse failure - _tool_route_gateway: log process_variables and per-flow condition results so routing decisions are visible in the output
… visible [skip ci] Empty starters caused the Carbon Chat component to set starters.isOn=false, hiding the entire homescreen including the greeting text.
…tate [skip ci] - Add Supervisor delegation section: delegation.py detects FlowAgent via isinstance, calls invoke(input_data, process_variables), extracts answer - Task node: document output_mapping — TaskAgent._process_output parses JSON output and calls state.set_process_variable for each mapped key - Gateway node: split into two paths — mode:decision_agent (MCP round-trip through route_gateway → DecisionAgent) vs mode:tool (engine calls _tool_route_gateway with eval_condition directly, no MCP) - Hook node: clarify that condition guard lives in engine (pre-evaluator call), state_updates are applied in engine after HookResult returned, and SKIP_TO issues Command(goto=skip_to_node) - Invocation: note str vs dict input_data handling (_user_message key)
… [skip ci] CLI sets supervisor config path → supervisor_config.py reads YAML → load_flow_from_yaml → FlowConfig.create_flow_agent() → FlowAgent(process_key, registry)
…Bridge [skip ci] Add SC (supervisor_config.py) and FC (FlowConfig) participants to make the full startup chain explicit: SC → FC → ProcessRegistry → FlowAgent → MCPFlowBridge → LangGraphWorkflowEngine
… creation to FlowConfig [skip ci] FlowConfig.create_flow_agent() now creates the bridge and engine before instantiating FlowAgent, passing the bridge in. FlowAgent receives the bridge, registers itself, and leaves engine registration to the caller. Diagram updated to match.
…th_bridge [skip ci] Add LangGraphWorkflowEngine.register_with_bridge(bridge, registry) so the engine is responsible for its own registration. FlowConfig now calls engine.register_with_bridge(bridge, registry) instead of bridge.register_engine(engine, registry). Diagram updated to match.
…on creation [skip ci] Add __init__(bridge, registry) to LangGraphWorkflowEngine — passes both to bridge.register_engine(self, registry) immediately on construction. FlowConfig and FlowAgent (standalone path) now just instantiate the engine with bridge+registry; no separate registration call needed.
…iagram [skip ci] Three missing consumers added: - FlowAgent.__init__ → Reg.get_flow_config(process_key) - Bridge.run_process closure → Reg.get_bpmn_process(process_key) - FlowAgent.invoke (completion) → Reg.get_bpmn_process(process_key)
ProcessRegistry now registers itself with MCPFlowBridge via register_registry(), exposing get_bpmn_process as an MCP tool. All external access to the registry is mediated through the bridge: - FlowAgent.invoke() fetches BPMNProcess via MCP call - LangGraphWorkflowEngine.run_process uses self._registry internally - register_engine() no longer takes a registry parameter BPMNProcess gains to_dict/from_dict for MCP serialisation.
…r_flow() [skip ci] ProcessRegistry.register_flow(flow_config_path) now owns all parsing: loads FlowConfig from YAML, parses BPMN, builds ProcessDefinition, and populates its caches in one call. FlowConfig.to_flow_agent() is reduced to forwarding self.config_path and wiring the bridge/FA/engine stack. The old preloaded-objects path is retained as _register_preloaded() for the backward-compat bpmn_file path in FlowAgent.
FlowConfig no longer calls ProcessRegistry.register_flow() directly. Instead: bridge.register_registry() now also registers a register_flow MCP tool; bridge.load_flow(config_path) is the sync bridge method that mediates the call (FlowConfig → Bridge → Registry). Bridge is created before load_flow so the mediation is in place from the start.
…ip ci] ProcessRegistry.__init__(bridge) calls bridge.register_registry(self) immediately if a bridge is provided, so the bridge is always created before the registry. FlowConfig.to_flow_agent() and FlowAgent.__init__ (backward-compat path) both create the bridge first, then pass it to ProcessRegistry(bridge=bridge). Removes all explicit register_registry() call sites outside of the constructor.
…ip ci] All ProcessRegistry access from FlowAgent is now exclusively via MCPFlowBridge. Adds FlowConfig.to_dict() for MCP serialisation, sync bridge proxy get_flow_annotations(), and the matching async MCP tool. Diagrams updated.
… {state, bpmn} [skip ci]
BPMNProcess was fetched twice per invoke() — once by FlowAgent for the completion
message, once by the engine inside run_process. run_process now returns both payloads
in a single response, eliminating the extra registry round-trip.
…e [skip ci] run_process tool now passes process_key directly to the engine. The engine fetches BPMNProcess via get_bpmn_process MCP call before building the graph, keeping all registry access behind the bridge. Returns (FlowState, BPMNProcess).
…lowAgent [skip ci] get_static_config (FlowAgent→Bridge→Engine) replaced with get_flow_annotations (Bridge→Registry→Engine). FlowConfig.to_engine_config() derives the engine- consumable dict directly from YAML. FlowAgent._get_static_config() removed.
… [skip ci] run_process is now a proxy method on MCPFlowBridge (same pattern as load_flow and get_flow_annotations), so FlowAgent never needs to open a transport client. The MCP tool registration is kept for future remote/cross-process use.
Locally-modified flow_config.py was passing registry=registry to FlowAgent, but that argument was removed in an earlier commit. The resulting TypeError was silently caught in graph.py, causing the supervisor to fall back to the default CRM/Email/Filesystem agents instead of loading the loan_flow_agent from supervisor_loan_approval.yaml.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Documentation
Improvements