Skip to content

Fix format-all sub-workflows silently skipped when called via workflow_call#387

Open
Copilot wants to merge 3 commits intomainfrom
copilot/investigate-clang-format-failure
Open

Fix format-all sub-workflows silently skipped when called via workflow_call#387
Copilot wants to merge 3 commits intomainfrom
copilot/investigate-clang-format-failure

Conversation

Copy link
Contributor

Copilot AI commented Mar 4, 2026

  • Fix pre-check job if conditions in 7 sub-workflow files: replace github.event_name == 'workflow_call' with inputs.ref != ''
  • Fix skip-comment computation in 7 sub-workflow files: replace github.event_name == 'workflow_call' && inputs.skip-comment || 'false' with inputs.skip-comment || 'false'
  • Fix env.local_checkout_path, pre-check.outputs.ref, and pre-check.outputs.repo in 7 sub-workflow files to use inputs.* directly
  • Fix combine-results in format-all.yaml to treat a "skipped" sub-workflow result as a failure
  • Fix reaction steps in sub-workflows to not run when called from format-all.yaml (inputs.skip-comment != 'true')
  • Fix Get PR Info step condition in all 7 sub-workflows: add && inputs.ref == '' to prevent duplicate eyes reactions when called from format-all.yaml
Original prompt

Per #322, @phlexbot format was triggered in a comment and quickly returned successfully with "no automatic format fixes were necessary." However, per https://github.com/Framework-R-D/phlex/actions/runs/22686957615/job/65772403148?pr=322, the clang-format check had failed. Please examine the PR conversation, fix and check action logs, and determine the problem and a reasonable fix. My first thought was that format-all is not waiting for or properly receiving answers from the individual fix workflows, but it's possible they weren't even triggered. Please investigate.

The user has attached the following file paths as relevant context:

  • .github/copilot-instructions.md

Created from VS Code.


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

In GitHub Actions, when a workflow is called via workflow_call, the
github.event_name in the called workflow reflects the CALLING workflow's
event (e.g., issue_comment), NOT 'workflow_call'. All 7 fix sub-workflows
used `github.event_name == 'workflow_call'` in their pre-check job if
conditions, which always evaluated to false when called from format-all.yaml
(triggered by issue_comment). Since the comment body was '@phlexbot format'
(not matching individual bot commands), the entire pre-check condition was
false, causing all sub-workflows to be silently skipped.

Fix:
- Replace `github.event_name == 'workflow_call'` with `inputs.ref != ''`
  in pre-check if conditions (ref is required for workflow_call, so its
  presence correctly identifies that context)
- Simplify env.local_checkout_path, pre-check.outputs.ref, and
  pre-check.outputs.repo to use inputs.* directly
- Fix skip-comment to use `inputs.skip-comment || 'false'` (same root bug
  was preventing comment suppression when called from format-all)
- Guard reaction steps with `inputs.skip-comment != 'true'` to prevent
  duplicate reactions when format-all manages reactions centrally
- Update combine-results in format-all.yaml to treat skipped sub-workflow
  results as failures (defense-in-depth)

Co-authored-by: greenc-FNAL <2372949+greenc-FNAL@users.noreply.github.com>
Copilot AI changed the title [WIP] Investigate clang-format check failure in PR 322 Fix format-all sub-workflows silently skipped when called via workflow_call Mar 4, 2026
@greenc-FNAL greenc-FNAL marked this pull request as ready for review March 4, 2026 20:35
Copilot AI review requested due to automatic review settings March 4, 2026 20:35
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes format-all.yaml orchestrating format sub-workflows (clang-format, cmake-format, etc.) being silently skipped when invoked via workflow_call, due to github.event_name reflecting the calling workflow’s event (e.g., issue_comment) rather than workflow_call.

Changes:

  • Update pre-check.if in all 7 fix workflows to detect workflow_call via required inputs (e.g., inputs.ref != '') instead of github.event_name == 'workflow_call'.
  • Simplify reusable-workflow input selection (ref, repo, local_checkout_path, skip-comment) to use inputs.* directly with fallbacks.
  • Make format-all treat "skipped" sub-workflow results as failures (rather than reporting “No automatic format fixes were necessary”).

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.

Show a summary per file
File Description
.github/workflows/format-all.yaml Fails the combined run when any called formatter workflow is skipped, preventing false “no fixes needed” reports.
.github/workflows/clang-format-fix.yaml Correctly gates execution for workflow_call via required inputs; fixes input selection and skip-comment handling.
.github/workflows/cmake-format-fix.yaml Same workflow_call detection + input/skip-comment fixes as clang-format.
.github/workflows/header-guards-fix.yaml Same workflow_call detection + input/skip-comment fixes as clang-format.
.github/workflows/jsonnet-format-fix.yaml Same workflow_call detection + input/skip-comment fixes as clang-format.
.github/workflows/markdown-fix.yaml Same workflow_call detection + input/skip-comment fixes as clang-format.
.github/workflows/python-fix.yaml Same workflow_call detection + input/skip-comment fixes as clang-format.
.github/workflows/yaml-fix.yaml Same workflow_call detection + input/skip-comment fixes as clang-format.
Comments suppressed due to low confidence (7)

.github/workflows/header-guards-fix.yaml:67

  • With the new pre-check gate (inputs.ref != ''), this workflow will run when called from format-all via workflow_call (where github.event_name is still issue_comment). In that call path, the Get PR Info step (guarded only by github.event_name == 'issue_comment') will still execute and get-pr-info will attempt to add an eyes reaction, even when inputs.skip-comment == 'true'. Consider tightening the Get PR Info step condition to only run when PR info is actually needed (e.g., when inputs.ref/inputs.repo are empty), and/or propagate skip-comment into get-pr-info so it can suppress reactions in the workflow_call path.
    if: >
      github.event_name == 'workflow_dispatch' || inputs.ref != '' || (
        github.event_name == 'issue_comment' &&
        github.event.issue.pull_request &&
        contains(fromJSON('["OWNER", "COLLABORATOR", "MEMBER"]'), github.event.comment.author_association) &&
        startsWith(github.event.comment.body, format('@{0}bot header-guards-fix', github.event.repository.name))

.github/workflows/clang-format-fix.yaml:67

  • With the new pre-check gate (inputs.ref != ''), this workflow will run when called from format-all via workflow_call (where github.event_name is still issue_comment). In that call path, the Get PR Info step (guarded only by github.event_name == 'issue_comment') will still execute and get-pr-info will attempt to add an eyes reaction, even when inputs.skip-comment == 'true'. Consider tightening the Get PR Info step condition to only run when PR info is actually needed (e.g., when inputs.ref/inputs.repo are empty), and/or propagate skip-comment into get-pr-info so it can suppress reactions in the workflow_call path.
    if: >
      github.event_name == 'workflow_dispatch' || inputs.ref != '' || (
        github.event_name == 'issue_comment' &&
        github.event.issue.pull_request &&
        contains(fromJSON('["OWNER", "COLLABORATOR", "MEMBER"]'), github.event.comment.author_association) &&
        startsWith(github.event.comment.body, format('@{0}bot clang-fix', github.event.repository.name))

.github/workflows/cmake-format-fix.yaml:67

  • With the new pre-check gate (inputs.ref != ''), this workflow will run when called from format-all via workflow_call (where github.event_name is still issue_comment). In that call path, the Get PR Info step (guarded only by github.event_name == 'issue_comment') will still execute and get-pr-info will attempt to add an eyes reaction, even when inputs.skip-comment == 'true'. Consider tightening the Get PR Info step condition to only run when PR info is actually needed (e.g., when inputs.ref/inputs.repo are empty), and/or propagate skip-comment into get-pr-info so it can suppress reactions in the workflow_call path.
    if: >
      github.event_name == 'workflow_dispatch' || inputs.ref != '' || (
        github.event_name == 'issue_comment' &&
        github.event.issue.pull_request &&
        contains(fromJSON('["OWNER", "COLLABORATOR", "MEMBER"]'), github.event.comment.author_association) &&
        startsWith(github.event.comment.body, format('@{0}bot cmake-fix', github.event.repository.name))

.github/workflows/python-fix.yaml:67

  • With the new pre-check gate (inputs.ref != ''), this workflow will run when called from format-all via workflow_call (where github.event_name is still issue_comment). In that call path, the Get PR Info step (guarded only by github.event_name == 'issue_comment') will still execute and get-pr-info will attempt to add an eyes reaction, even when inputs.skip-comment == 'true'. Consider tightening the Get PR Info step condition to only run when PR info is actually needed (e.g., when inputs.ref/inputs.repo are empty), and/or propagate skip-comment into get-pr-info so it can suppress reactions in the workflow_call path.
    if: >
      github.event_name == 'workflow_dispatch' || inputs.ref != '' || (
        github.event_name == 'issue_comment' &&
        github.event.issue.pull_request &&
        contains(fromJSON('["OWNER", "COLLABORATOR", "MEMBER"]'), github.event.comment.author_association) &&
        startsWith(github.event.comment.body, format('@{0}bot python-fix', github.event.repository.name))

.github/workflows/markdown-fix.yaml:67

  • With the new pre-check gate (inputs.ref != ''), this workflow will run when called from format-all via workflow_call (where github.event_name is still issue_comment). In that call path, the Get PR Info step (guarded only by github.event_name == 'issue_comment') will still execute and get-pr-info will attempt to add an eyes reaction, even when inputs.skip-comment == 'true'. Consider tightening the Get PR Info step condition to only run when PR info is actually needed (e.g., when inputs.ref/inputs.repo are empty), and/or propagate skip-comment into get-pr-info so it can suppress reactions in the workflow_call path.
    if: >
      github.event_name == 'workflow_dispatch' || inputs.ref != '' || (
        github.event_name == 'issue_comment' &&
        github.event.issue.pull_request &&
        contains(fromJSON('["OWNER", "COLLABORATOR", "MEMBER"]'), github.event.comment.author_association) &&
        startsWith(github.event.comment.body, format('@{0}bot markdown-fix', github.event.repository.name))

.github/workflows/yaml-fix.yaml:67

  • With the new pre-check gate (inputs.ref != ''), this workflow will run when called from format-all via workflow_call (where github.event_name is still issue_comment). In that call path, the Get PR Info step (guarded only by github.event_name == 'issue_comment') will still execute and get-pr-info will attempt to add an eyes reaction, even when inputs.skip-comment == 'true'. Consider tightening the Get PR Info step condition to only run when PR info is actually needed (e.g., when inputs.ref/inputs.repo are empty), and/or propagate skip-comment into get-pr-info so it can suppress reactions in the workflow_call path.
    if: >
      github.event_name == 'workflow_dispatch' || inputs.ref != '' || (
        github.event_name == 'issue_comment' &&
        github.event.issue.pull_request &&
        contains(fromJSON('["OWNER", "COLLABORATOR", "MEMBER"]'), github.event.comment.author_association) &&
        startsWith(github.event.comment.body, format('@{0}bot yaml-fix', github.event.repository.name))

.github/workflows/jsonnet-format-fix.yaml:67

  • With the new pre-check gate (inputs.ref != ''), this workflow will run when called from format-all via workflow_call (where github.event_name is still issue_comment). In that call path, the Get PR Info step (guarded only by github.event_name == 'issue_comment') will still execute and get-pr-info will attempt to add an eyes reaction, even when inputs.skip-comment == 'true'. Consider tightening the Get PR Info step condition to only run when PR info is actually needed (e.g., when inputs.ref/inputs.repo are empty), and/or propagate skip-comment into get-pr-info so it can suppress reactions in the workflow_call path.
    if: >
      github.event_name == 'workflow_dispatch' || inputs.ref != '' || (
        github.event_name == 'issue_comment' &&
        github.event.issue.pull_request &&
        contains(fromJSON('["OWNER", "COLLABORATOR", "MEMBER"]'), github.event.comment.author_association) &&
        startsWith(github.event.comment.body, format('@{0}bot jsonnet-fix', github.event.repository.name))

@greenc-FNAL
Copy link
Contributor

@copilot Please review the "suppressed" comments at #387 (review) for validity, and address them anyway if appropriate.

…_call

When a sub-workflow is called from format-all.yaml via workflow_call,
github.event_name is 'issue_comment' (inherited from the calling workflow).
The Get PR Info step was guarded only by `github.event_name == 'issue_comment'`,
so it was running in the workflow_call path and adding a duplicate eyes
reaction -- while format-all.yaml manages reactions centrally.

Fix: add `&& inputs.ref == ''` to the Get PR Info step condition in all 7
fix sub-workflows. Since ref is a required workflow_call input, its presence
reliably identifies the workflow_call path; the step only needs to run when
the workflow is triggered directly by an issue_comment (where inputs.ref is
empty and PR info must be fetched).

Addresses suppressed review comments in PR 387.

Co-authored-by: greenc-FNAL <2372949+greenc-FNAL@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Mar 4, 2026

@copilot Please review the "suppressed" comments at #387 (review) for validity, and address them anyway if appropriate.

The suppressed comments are valid. When called from format-all.yaml, github.event_name is issue_comment (inherited), so the Get PR Info step was unconditionally running in the workflow_call path and adding a spurious eyes reaction.

Fixed in 3eec214 by adding && inputs.ref == '' to the Get PR Info step condition in all 7 sub-workflows. Since ref is a required: true workflow_call input, its presence reliably identifies the workflow_call path — the step only runs when triggered directly by an issue_comment (where inputs.ref is empty and PR info must actually be fetched).

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.

@greenc-FNAL greenc-FNAL requested a review from knoepfel March 4, 2026 21:24
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.

3 participants