Skip to content

ensure actions don't break the graph in modules with no instances#38089

Merged
DanielMSchmidt merged 2 commits intomainfrom
fix-zero-count-module-action-bug
Jan 23, 2026
Merged

ensure actions don't break the graph in modules with no instances#38089
DanielMSchmidt merged 2 commits intomainfrom
fix-zero-count-module-action-bug

Conversation

@DanielMSchmidt
Copy link
Contributor

Before we only added the root graph node on each module iteration, but we want the root node to always be present in the sub-graph so we moved this call to just before the return statement.

Fixes #38088

Target Release

1.15.x

Rollback Plan

  • If a change needs to be reverted, we will roll out an update to the code within 7 days.

Changes to Security Controls

Are there any changes to security controls (access controls, encryption, logging) in this pull request? If so, explain.

CHANGELOG entry

  • This change is user-facing and I added a changelog entry.
  • This change is not user-facing.

@DanielMSchmidt DanielMSchmidt requested a review from a team as a code owner January 23, 2026 09:32
@DanielMSchmidt DanielMSchmidt force-pushed the fix-zero-count-module-action-bug branch 2 times, most recently from 84a4765 to 8180837 Compare January 23, 2026 09:38
@DanielMSchmidt DanielMSchmidt added the 1.14-backport If you add this label to a PR before merging, backport-assistant will open a new PR once merged label Jan 23, 2026
@DanielMSchmidt DanielMSchmidt force-pushed the fix-zero-count-module-action-bug branch from 8180837 to 500a822 Compare January 23, 2026 09:48
Copy link
Member

@dbanck dbanck left a comment

Choose a reason for hiding this comment

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

Nice!

One question (not intended for this PR): I see that most of the other node expansion logic uses the forEachModuleInstance helper. Would that make sense in the actions context as well?

@DanielMSchmidt
Copy link
Contributor Author

@dbanck Yeah I think that could make sense, I believe I just copied, I mean was inspired by parts of the code base that didn't use the helper yet. Normally I would be in favor of making that small adjustment but @mildwonkey is currently working on rewriting part of the actions scheduling logic and I don't want refactoring changes to make their life harder so maybe it's sth to keep in mind for after that effort concludes 👍

@DanielMSchmidt DanielMSchmidt merged commit 492d342 into main Jan 23, 2026
11 checks passed
@DanielMSchmidt DanielMSchmidt deleted the fix-zero-count-module-action-bug branch January 23, 2026 12:29
@github-actions
Copy link
Contributor

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 23, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

1.14-backport If you add this label to a PR before merging, backport-assistant will open a new PR once merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Invalid dynamic subgraph when using action in module with count=0

2 participants