ReviewOS

also looking at this

stacks/buddy

fix(setup): let the generated workflow's jobs actually start

#1418
Merged glennmichael123 wants to merge fix/workflow-reachability into main
3 files +224 -3
9 days ago

setup is a dependency of every job in the generated workflow, and a job whose needs was skipped is skipped with it. Its condition named four of the eleven gates:

setup:
  if: ${{ run_check == 'true' || run_update == 'true'
       || run_dashboard == 'true' || run_report == 'true' }}

The other seven could never start.

JobGateReachable before
reviewrun_review❌
commandrun_command❌
fix-cirun_fixci❌
gategithub.event_name❌
touchgithub.event_name❌
post-mergegithub.event_name❌
issue-linksgithub.event_name❌

On a pull request being opened, determine-jobs sets run_review=true and nothing else → setup skipped → the review skipped. Same for @buddy comments, CI repair, the pre-merge gate, finishing touches, post-merge, and issue quick-links.

Every one appears in the Actions run as skipped, with no error to notice.

This repo didn't catch it because its own committed buddy.yml is an older generation with no review job at all — the generator has never been dogfooded against these events.

I introduced half of this

#1412 fixed exactly this defect for the health report — by adding run_report to that same condition — and I did not check whether the report's siblings had it too. Seven of them did.

Naming outputs there one at a time is the bug. So setup now gates on run_any, derived in the router from what it just decided rather than restated:

if grep -q '=true$' "$GITHUB_OUTPUT"; then
  echo "run_any=true" >> $GITHUB_OUTPUT

plus the two event names whose jobs gate on github.* directly. A new output can't be added and forgotten here again.

run_report is also now defaulted to false alongside the others — it was the only one left unset, so it read as the empty string.

The test is the point

test/workflow-reachability.test.ts evaluates the real if expressions against a fixture per event and asserts the jobs that should start do. Reverting this fix turns 7 of its 12 cases red.

The test it replaces asserted that run_report appeared in a string — the mechanism rather than the property — and would not have caught any of this. I've rewritten that one too.

Tests

2059 on this branch (2072 with #1417 stacked). Found by a multi-agent sweep for the "declared but never wired" pattern; I verified the finding by generating the YAML and walking the job graph before writing anything.

Lint ✓ · typecheck ✓ · check:docs ✓ · 0 fail

🤖 Generated with Claude Code

https://claude.ai/code/session_01UWouahBJT3msK7V2VDYY6T

3 changed files on the files tab, with 0 review threads.

Sign in to comment on this pull request.