Skip to content

fix: silent skip when stage role not configured - #1022

Merged
ralphbean merged 5 commits into
mainfrom
fix/silent-skip-unconfigured-roles
May 19, 2026
Merged

fix: silent skip when stage role not configured#1022
ralphbean merged 5 commits into
mainfrom
fix/silent-skip-unconfigured-roles

Conversation

@ralphbean

Copy link
Copy Markdown
Member

Summary

  • Change the defaults.roles gate in both dispatch workflows to exit 0 with a ::notice:: annotation instead of exit 1 with ::error:: when a stage's role is not configured
  • Add id: role-check to the step and gate downstream steps (fan-out, fork-PR check) on steps.role-check.outputs.skipped != 'true'
  • Per-repo dispatch clears the job stage output via a conditional expression so downstream jobs are skipped cleanly

Fixes #973

Test plan

  • make go-vet passes
  • make lint passes
  • TestDispatchWorkflowContent passes
  • Verify on a live org without retro agent that PR close no longer produces a failed workflow run

🤖 Generated with Claude Code

ralphbean added 4 commits May 15, 2026 15:30
Spec for fixing #973 — change the defaults.roles gate in dispatch
workflows to exit 0 with a notice instead of exit 1 with an error
when a stage's role is not configured.

Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Ralph Bean <rbean@redhat.com>
Three tasks: fix per-org dispatch, fix per-repo dispatch, verify tests.

Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Ralph Bean <rbean@redhat.com>
Same change as per-org dispatch: exit 0 with notice instead of exit 1
with error when a stage's role is not configured.

Fixes #973

Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Ralph Bean <rbean@redhat.com>
Change the role-check gate to exit 0 with a notice annotation instead
of exit 1 with an error when a stage's role is not configured. This
prevents noisy failed workflow runs on orgs that haven't opted into
all agents.

Fixes #973

Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Ralph Bean <rbean@redhat.com>
@github-actions

Copy link
Copy Markdown

fullsend review is working on this — view logs

@github-actions

github-actions Bot commented May 15, 2026

Copy link
Copy Markdown

Site preview

Preview: https://6caaad57-site.fullsend-ai.workers.dev

Commit: dd6e9673d6650b86087852ce05b826ce09633aff

@fullsend-ai-review

fullsend-ai-review Bot commented May 15, 2026

Copy link
Copy Markdown

Review

Findings

High

  • [correctness] .github/workflows/reusable-dispatch.yml:52 — GHA expression logic error: stage output is never cleared when role-check skips. The expression steps.role-check.outputs.skipped == 'true' && '' || steps.route.outputs.stage always evaluates to steps.route.outputs.stage because '' is falsy in GitHub Actions expression evaluation. When skipped == 'true': true && '' yields '', then '' || steps.route.outputs.stage yields the original stage value. This means per-repo mode downstream jobs (triage, code, review, fix, retro) will still run for unconfigured roles, defeating the purpose of the fix.
    Remediation: Flip the conditional — use steps.role-check.outputs.skipped != 'true' && steps.route.outputs.stage || ''. When skipped: false && stage || ''''. When not skipped: true && stage || ''stage.

Medium

Low

  • [correctness] .github/workflows/reusable-dispatch.yml:275 — The "Build event payload" step's if condition (steps.route.outputs.stage != '') does not check steps.role-check.outputs.skipped != 'true'. If the expression bug above is fixed, this step would still run unnecessarily (building a payload for a skipped stage). Harmless but inconsistent with the guard pattern applied to the fork-PR step.
    Remediation: Add && steps.role-check.outputs.skipped != 'true' to the step's if condition.
Previous run

Review: #1022

Head SHA: 92335d4
Timestamp: 2026-05-15T00:00:00Z
Outcome: request-changes

Summary

The PR correctly converts the role-check gate from a hard failure (exit 1 + ::error::) to a silent skip (exit 0 + ::notice::) in both dispatch workflows, matching the intent of #973. However, reusable-dispatch.yml has a gap: the "Block fork PRs for fix stage" step is not guarded by the new role-check.outputs.skipped flag. In the per-org dispatch.yml, this guard was correctly added, but the reusable workflow omits it. If the fix role is unconfigured and a fork PR triggers the fix stage, the fork-PR check will still run and can exit 1 — producing the exact noisy failure this PR aims to eliminate.

Findings

Medium

  • [Correctness] .github/workflows/reusable-dispatch.yml:255 — The "Block fork PRs for fix stage" step condition (steps.route.outputs.stage == 'fix' && github.event.issue.pull_request) is not guarded by steps.role-check.outputs.skipped != 'true'. The per-org dispatch.yml (line 270 in diff) correctly adds this guard, but the reusable workflow does not. If the fix role is unconfigured and the event is a fork PR, this step runs and can fail with exit 1, defeating the silent-skip intent.
    Remediation: Add && steps.role-check.outputs.skipped != 'true' to the if condition on the fork-PR step in reusable-dispatch.yml, matching the pattern used in dispatch.yml.

Low

  • [Correctness] .github/workflows/reusable-dispatch.yml:276 — The "Build event payload" step runs when steps.route.outputs.stage != '' without checking role-check skip. It builds a payload that downstream jobs will never consume (since the job output clears stage). Not harmful, but inconsistent with the per-org dispatch where the fan-out step is guarded.
    Remediation: Add && steps.role-check.outputs.skipped != 'true' to the if condition on the payload step, or accept the minor inconsistency.

Footer

Outcome: request-changes
This review applies to SHA 92335d47c22e663918a62e56cc191842fa9c01be. Any push to the PR head clears this review and requires a new evaluation.

@fullsend-ai-review fullsend-ai-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

See the review comment for full details.

@github-actions

Copy link
Copy Markdown

fullsend fix is working on this — view logs

…flow

The per-org dispatch.yml correctly gates the "Block fork PRs" step on
steps.role-check.outputs.skipped != 'true', but the reusable workflow
was missing this guard. Without it, an unconfigured fix role on a fork
PR still hits exit 1 — the exact noisy failure this branch eliminates.

Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Ralph Bean <rbean@redhat.com>
@ralphbean
ralphbean added this pull request to the merge queue May 19, 2026
Merged via the queue into main with commit c7c5717 May 19, 2026
7 checks passed
@ralphbean
ralphbean deleted the fix/silent-skip-unconfigured-roles branch May 19, 2026 15:57
ggallen pushed a commit to ggallen/fullsend that referenced this pull request Jun 4, 2026
The per-org scaffold dispatch.yml was missing the fix→coder role
mapping that reusable-dispatch.yml already has. This caused /fs-fix
to be silently skipped on all per-org installations with the notice:
"Stage 'fix' skipped — role 'fix' not in defaults.roles"

The fix stage reuses the coder app (reusable-fix.yml mints a coder
token), so it should map to the coder role in the stage-to-role
check — matching the per-repo dispatch at reusable-dispatch.yml:267.

Root cause: commit 709d8af added code|fix on main, but the merge
of PR fullsend-ai#1022 (c7c5717) resolved a conflict in the case statement
using the branch version that lacked the fix mapping.

Partially addresses fullsend-ai#1725

Signed-off-by: Wayne Sun <gsun@redhat.com>
jhutar pushed a commit to jhutar/fullsend-ai--fullsend that referenced this pull request Jun 5, 2026
The per-org scaffold dispatch.yml was missing the fix→coder role
mapping that reusable-dispatch.yml already has. This caused /fs-fix
to be silently skipped on all per-org installations with the notice:
"Stage 'fix' skipped — role 'fix' not in defaults.roles"

The fix stage reuses the coder app (reusable-fix.yml mints a coder
token), so it should map to the coder role in the stage-to-role
check — matching the per-repo dispatch at reusable-dispatch.yml:267.

Root cause: commit 709d8af added code|fix on main, but the merge
of PR fullsend-ai#1022 (c7c5717) resolved a conflict in the case statement
using the branch version that lacked the fix mapping.

Partially addresses fullsend-ai#1725

Signed-off-by: Wayne Sun <gsun@redhat.com>
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.

Shim dispatches retro stage on PR close even when org has no retro agent configured

2 participants