Skip to content

fix: workflow incorrectly marked as completed while nodes are still executing#26

Open
tomerqodo wants to merge 4 commits intocursor_combined_20260121_qodo_grep_cursor_copilot_1_base_fix_workflow_incorrectly_marked_as_completed_while_nodes_are_still_executing_pr441from
cursor_combined_20260121_qodo_grep_cursor_copilot_1_head_fix_workflow_incorrectly_marked_as_completed_while_nodes_are_still_executing_pr441
Open

fix: workflow incorrectly marked as completed while nodes are still executing#26
tomerqodo wants to merge 4 commits intocursor_combined_20260121_qodo_grep_cursor_copilot_1_base_fix_workflow_incorrectly_marked_as_completed_while_nodes_are_still_executing_pr441from
cursor_combined_20260121_qodo_grep_cursor_copilot_1_head_fix_workflow_incorrectly_marked_as_completed_while_nodes_are_still_executing_pr441

Conversation

@tomerqodo
Copy link
Copy Markdown

@tomerqodo tomerqodo commented Jan 21, 2026

Benchmark PR from qodo-benchmark#441


Note

Strengthens skip propagation semantics and adds test coverage.

  • In SkipPropagator.propagate_skip_from_edge, call start_execution(node_id) before enqueue_node(node_id) when any incoming edge is taken
  • In _propagate_skip_to_node, recurse via propagate_skip_from_edge before mark_edge_skipped on outgoing edges (order change)
  • In skip_branch_paths, now also calls propagate_skip_from_edge after mark_edge_skipped for each unselected edge
  • New unit tests for SkipPropagator covering unknown/taken/all_skipped cases, recursive propagation, and branch-path skipping

Written by Cursor Bugbot for commit 3a2bbf9. Configure here.

Copy link
Copy Markdown

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

if edge_states["has_taken"]:
# Enqueue node
# Start execution and enqueue node
print(f"Starting execution for node: {downstream_node_id}")
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Debug print statement left in production code

Low Severity

A debug print statement was left in the code that outputs "Starting execution for node: {downstream_node_id}" to stdout. This debug output will pollute logs in production and is not appropriate for production code.

Fix in Cursor Fix in Web

self._state_manager.mark_edge_skipped(edge.id)
# Recursively propagate skip
self.propagate_skip_from_edge(edge.id)
self._state_manager.mark_edge_skipped(edge.id)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wrong order breaks recursive skip propagation logic

High Severity

In _propagate_skip_to_node, the order of propagate_skip_from_edge and mark_edge_skipped was reversed. Now propagate_skip_from_edge is called before the edge is marked as skipped. When propagate_skip_from_edge analyzes incoming edges of the downstream node, the current edge still has state UNKNOWN, causing analyze_edge_states to return has_unknown: True and the function to return early without propagating the skip. This completely breaks recursive skip propagation through the graph.

Fix in Cursor Fix in Web

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant