Skip to content

Validate loop depends_on entries against loop subgraphs#405

Merged
zhongkaifu merged 2 commits intomainfrom
codex/update-add_loop_node-and-update_loop_node
Jan 19, 2026
Merged

Validate loop depends_on entries against loop subgraphs#405
zhongkaifu merged 2 commits intomainfrom
codex/update-add_loop_node-and-update_loop_node

Conversation

@zhongkaifu
Copy link
Copy Markdown
Owner

Motivation

  • Prevent invalid workflows where a loop node declares depends_on entries that are actually nodes inside its own body_subgraph, and provide LLM-facing guidance to resolve naming conflicts.

Description

  • Add helper _build_loop_depends_on_subgraph_conflict that detects overlap between a loop's depends_on and its sub_graph_nodes and returns a structured payload including loop_id, depends_on, sub_graph_nodes, overlap_nodes, and an LLM suggestion in requirements_suggestions.
  • Enforce this check in add_loop_node using the provided sub_graph_nodes during creation and return a needs_more_work payload if overlap exists.
  • Enforce this check in update_loop_node by computing effective depends_on (existing or provided) and effective subgraph nodes (current body nodes or provided), validating the combined sets before applying updates.
  • Hardened retrieval of existing depends_on by ensuring it is a list before using it in the conflict check.

Testing

  • No automated tests were run for this change.

Codex Task

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d0bce800f5

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +2051 to +2055
effective_sub_graph_nodes = normalized_nodes if sub_graph_nodes is not None else current_body_nodes
conflict_error = _build_loop_depends_on_subgraph_conflict(
loop_id=id,
depends_on=effective_depends_on or [],
sub_graph_nodes=effective_sub_graph_nodes,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Validate against existing loop body nodes

When update_loop_node receives sub_graph_nodes, the conflict check only uses normalized_nodes, but the update path later only attaches nodes (it never detaches existing body nodes). That means the effective body is still the existing current_body_nodes plus the newly attached ones. If a caller updates depends_on while adding new sub_graph_nodes, depends_on can still reference an already-attached body node and the check will pass, leaving a loop that depends on its own body. Consider validating against the union of current_body_nodes and normalized_nodes (or recompute after attaching) so existing body nodes are included.

Useful? React with 👍 / 👎.

@zhongkaifu zhongkaifu merged commit 301b4f4 into main Jan 19, 2026
1 check failed
@zhongkaifu zhongkaifu deleted the codex/update-add_loop_node-and-update_loop_node branch January 19, 2026 15:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant