chore(ci): open the Vale upgrade PR when the manifest lands on main

[theCodeDrift]

What

vale-upgrade.yml now runs on a push to main that touches .github/scripts/vale-manifest.json, which is the merge of a release-vale.yml detect pull request. That is the same event release-vale.yml publishes on, so the upgrade half starts from the manifest commit itself rather than from a clock. workflow_dispatch stays. The daily 52 7 * * * schedule stays too, re-commented as a backstop rather than the trigger.

Why

The only thing that makes the CLI's six @taskless/vale-* pins fall behind is a republish, and a republish starts exactly when the manifest lands. Until now nothing here reacted to that: the daily cron was the only trigger.

Measured on 3.22.0: the manifest merged at 18:09:13Z, the Release Vale run started at 18:09:16Z, and the packages on npm carry the stamp 3.22.0-20260921180930. The upgrade PR would have been opened by the next 52 7 * * * tick, roughly thirteen hours after the packages were on npm. The pins sat packaged-but-unshipped for that whole window.

How the publish race is handled

Firing on the same push means this workflow and Release Vale start within seconds of each other, and the detect step asks the npm registry what is published. Run immediately, it compares against the old latest, finds nothing to do, and exits clean. Two new steps, both if: github.event_name == 'push', sit before detect:

1. Wait for the Release Vale run for this commit. Polls gh run list --workflow release-vale.yml --event push --commit "$GITHUB_SHA" --json status,conclusion every 30 s, bounded at 25 minutes. Proceeds only once every run for that SHA is completed with conclusion success. Any other conclusion, or no run inside the bound, emits a ::warning:: naming the cron backstop and exits 0. The job gets actions: read for this and nothing else new.
2. Wait for the registry to serve the new set. Up to five probes of vale-upgrade-detect.cjs --json, 60 s apart, until it reports "ahead":true. --json is the script's read-only mode (it refuses --write and --notes-out alongside it). If the registry still serves the pinned version after five minutes: ::warning::, exit 0, detect runs as usual and finds nothing, and the cron picks it up.

Neither step can fail the run. A red here would say "the upgrade is broken" when the truth is "the publish is", and the schedule already covers the cases the push run cannot see: a publish re-run by hand (a re-run is not a new push) and slow registry propagation.

The wait-step shell was exercised locally against a stubbed gh for four outcomes (in progress then success, failure, two runs with one still queued, never registered) and every path returned 0.

Double-opening is not possible: concurrency: vale-upgrade serializes a push run against a scheduled one, and vendor-pr.cjs refuses to push when the rebuilt vendor/vale/upgrade branch is identical to the remote and edits the existing open PR (gh pr list --head) instead of opening a second one.

Not workflow_run: the trigger is deliberately the manifest commit, not the other workflow's completion.

Rollout

The first run on the new trigger is the next manifest merge. Nothing is waiting on this: a workflow_dispatch was run by hand today at 21:16Z for 3.22.0, so the 3.22.0 upgrade PR already exists.

Checks: prettier --check, the YAML parsed with on.push, on.schedule, on.workflow_dispatch present, and pnpm test:scripts (443 pass, including the suite that scans committed workflows). actionlint is not installed locally, so it was not run.

[theCodeDrift]

@claude /review

[claude]

Claude finished @theCodeDrift's task in 4m 52s —— View job

---

Review complete

Review mode: incremental — read 0 prior review thread(s) before reviewing. .prior-review.json contains no threads, reviews, or comments other than the /review trigger itself, so this is the first review of this PR — nothing was treated as already addressed.

• Read .prior-review.json
• Understand the request (code review of the PR diff)
• Fetch PR diff and changed file (gh pr diff 369, gh pr view, full read of .github/workflows/vale-upgrade.yml, .github/workflows/release-vale.yml, .github/scripts/vale-upgrade-detect.cjs)
• Correctness / cleanup / altitude / conventions analysis
• Post inline comments and final review

Summary

The design — fire on the manifest-commit push instead of a daily cron, then wait out the publish race before running detect — is sound and well-reasoned, and the "never fail the run over a race" intent is clearly stated. I found one correctness bug in how that intent is implemented, plus a related but lower-severity diagnostic issue. Posted as inline comments:

1. [New] runs="$(gh run list ...)" (lines 125–151) can still fail the run. That assignment sits directly under set -euo pipefail and is not inside an if/while test, so unlike the if node ... | grep -q construct two steps later (line 170, correctly exempt), a failing gh call here — rate limit, 5xx, transient network error, realistic across up to 50 polls over 25 minutes — trips errexit and fails the step outright. That's exactly the "EVERY EXIT HERE IS 0" / "must not go red over a race it did not cause" guarantee the header comments promise, and it's a gap the local stubbed-gh testing (four status/conclusion outcomes) wouldn't have caught, since it only varied what gh returned, not whether the gh call itself errored.

2. [New] (lines 169–177) A genuine detect-script error is indistinguishable from "still on the old version." If vale-upgrade-detect.cjs --json throws for a reason other than staleness (e.g. a transient registry fetch failure), it prints nothing to stdout and exits 1; grep -q '"ahead":true' treats that identically to a legitimate "not ahead yet," and after 5 attempts the loop emits a warning asserting the registry is slow when the real cause is a script error. Lower severity — it can't turn the run red — but it's a misleading diagnostic.

Also worth a look, not inline since it's a design suggestion rather than a bug: both wait loops are hand-rolled shell polling logic embedded directly in the YAML, unlike every other piece of decision logic in this same pipeline (vale-gate.cjs, vale-upgrade-detect.cjs, etc.), which lives in a .cjs file with a paired .test.cjs exercised by pnpm test:scripts. That's arguably why finding 1 slipped past local testing — it isn't reachable by the project's own automated test suite the way a script would be.

Nothing else stood out — permissions additions (actions: read), the paths: filter mirroring release-vale.yml's, the --commit "$GITHUB_SHA" scoping, and the double-open protection via vendor-pr.cjs all check out against the stated design.

[theCodeDrift]

Re: @claude[bot] — "Claude finished @theCodeDrift's task in 4m 52s — Review complete…"
#369 (comment)

Both inline findings were real and are fixed in 957da48: the gh run list assignment no longer trips errexit on a failed call, and the registry probe distinguishes an erroring probe from a "still current" answer. The suggestion to move the two wait loops into a tested .cjs script is noted as a design question for the maintainer rather than acted on here.

— AI Coding Agent