fix(#5339): skip disabled workflows gracefully in per-org dispatch#5340
fix(#5339): skip disabled workflows gracefully in per-org dispatch#5340fullsend-ai-coder[bot] wants to merge 1 commit into
Conversation
The dispatch script treated all gh workflow run failures as fatal, including HTTP 422 when a target workflow is intentionally disabled. Detect 'disabled workflow' in the error output and emit a warning instead of failing, matching the pattern already used in the prioritize-scheduler. When all matched workflows are disabled, exit cleanly with a warning rather than erroring on zero dispatches. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
🤖 Finished Review · ✅ Success · Started 6:36 AM UTC · Completed 6:50 AM UTC |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
ReviewFindingsMedium
Low
|
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
| echo "Scanned $scanned workflow(s), skipped $skipped, dispatched $dispatched" | ||
| echo "Scanned $scanned workflow(s), skipped $skipped, disabled $disabled_skipped, dispatched $dispatched" | ||
|
|
||
| if [[ $dispatched -eq 0 ]]; then |
There was a problem hiding this comment.
[medium] logic-error
When all stage-matched workflows are disabled (dispatched==0, disabled_skipped>0), the warning branch does not exit or skip the final echo. Execution falls through to 'Successfully dispatched 0 workflow(s) for stage ...' — a misleading success message. The control flow is fragile: future code appended after the success echo would execute in the all-disabled case without a guard.
Suggested fix: Add exit 0 after the all-disabled warning (inside the if [[ $disabled_skipped -gt 0 ]] branch), or restructure as if/elif/else so the success message only prints when dispatched>0.
| if ! output=$(gh workflow run "$workflow_name" "${DISPATCH_ARGS[@]}" 2>&1); then | ||
| echo "::error::Failed to dispatch $workflow_name: $output" | ||
| exit 1 | ||
| if echo "$output" | grep -qi 'disabled workflow'; then |
There was a problem hiding this comment.
[low] edge-case
Disabled-workflow detection relies on grep -qi 'disabled workflow' against gh CLI stderr. This matches the current error format but depends on an uncontrolled third-party string that could change between CLI versions. If the wording changes, the fallback is fail-closed (exit 1), which is safe.
| echo "Scanned $scanned workflow(s), skipped $skipped, dispatched $dispatched" | ||
| echo "Scanned $scanned workflow(s), skipped $skipped, disabled $disabled_skipped, dispatched $dispatched" | ||
|
|
||
| if [[ $dispatched -eq 0 ]]; then |
There was a problem hiding this comment.
[low] fail-open
When all workflows for a stage are disabled, dispatch exits 0 with only a ::warning::. The stage's event is effectively dropped — upstream callers see success though no agent ran. Intentional per #5339 but creates a silent-failure path.
Suggested fix: Consider emitting an output variable (e.g., dispatched_count=0) so upstream callers can detect the all-disabled case.
| disabled_skipped=$((disabled_skipped + 1)) | ||
| continue | ||
| else | ||
| echo "::error::Failed to dispatch $workflow_name: $output" |
There was a problem hiding this comment.
[low] injection
Pre-existing (not introduced by this PR): $output (gh CLI stderr) is interpolated unsanitized into ::error:: and ::notice:: workflow commands. The new disabled-workflow code path does not interpolate $output into any new commands.
fullsend-ai-review
Bot
added
the
requires-manual-review
Summary
dispatch.yml) now gracefully handles disabled target workflows instead of failing the entire dispatch jobdisabled workflowingh workflow runstderr and emits::warning::+ continues, matching the pattern already used inprioritize-scheduler.ymlexit 1Fixes #5339
Changes
internal/scaffold/fullsend-repo/.github/workflows/dispatch.ymldisabled_skippedcounter to track workflows skipped due to being disableddisabled workflowingh workflow runerror output — on match, emit::warning::andcontinueinstead ofexit 1::warning::instead of::error::+exit 1; if no workflows matched at all (genuine config problem), keep the errorlint-workflow-sizefrom 500 to 510 to accommodate the additional error handling linesTesting
::warning::annotation when target workflow is disabled (HTTP 422Cannot trigger a 'workflow_dispatch' on a disabled workflow)::error::+exit 1for non-disabled errors (e.g., bad credentials, network errors)::warning::(notexit 1)::error::+exit 1🤖 Generated with Claude Code
Closes #5339
Post-script verification
agent/5339-graceful-disabled-workflow-dispatch)bb253104277632085f4fb0cb434a77587dd30a0a..HEAD)