Skip to content

ci(npm-publish): group event guards so manual publish is not skipped - #5226

Draft
eliseo-juan wants to merge 1 commit into
mainfrom
fix/npm-publish-dispatch-condition
Draft

ci(npm-publish): group event guards so manual publish is not skipped#5226
eliseo-juan wants to merge 1 commit into
mainfrom
fix/npm-publish-dispatch-condition

Conversation

@eliseo-juan

Copy link
Copy Markdown
Contributor

Description

Manual runs of Publish packages were always skipped, so when the release-triggered publish broke there was no way to retry it by hand. Discovered while investigating why @factorialco/f0-react has been stuck on 6.38.1 on npm since 19 Aug while releases kept being cut up to 6.51.1.

&& binds tighter than || in GitHub Actions expressions, so the flat condition on each job parsed as:

(contains(release.tag_name, 'react-v') && head_ref != … && !contains(labels, …) && event_name == 'workflow_dispatch')
  || event_name == 'release'
  || (workflow_run.conclusion == 'success' && workflow_run.outputs.new_version)

On a workflow_dispatch there is no github.event.release, so contains(null, 'react-v') is false and the whole first group collapses. The second branch needs event_name == 'release' and the third needs a workflow_run payload, so neither can match a dispatch either → all three jobs skip. Example: run 32831792413, 3/3 jobs skipped in 2s. The release_tag input the checkout step already reads was never reachable.

Type of change

  • Bug fix
  • Refactor / internal change (no API or behavior change) — CI only, no package code touched

Implementation details

Parenthesised the per-event branches into one group ANDed with the shared guards, and gave each branch its own tag filter:

  • release — filters on github.event.release.tag_name, as before. New: this now actually filters. Previously event_name == 'release' alone satisfied the condition, so every release started all three jobs and two of them no-op'd with a misleading green tick (published=false, Slack step skipped). Only the job for the released package starts now.
  • workflow_dispatch — filters on the release_tag input, consistent with what actions/checkout already resolves. This is the path that was dead.
  • workflow_run — unchanged logic, now explicitly gated on event_name too.

The tag filters discriminate cleanly, so no dispatch can fan out to the wrong package:

tag react-v react-native-v core-v
f0-react-v6.51.1
f0-react-native-v0.59.0
f0-core-v2.1.0

Replaced the two stale comments with one line recording the precedence trap, since that is the part a future edit could silently undo.

Not addressed here (flagging for the owning team)

Two pre-existing issues found while tracing this, left out to keep the diff to the reported bug:

  1. github.event.workflow_run.outputs does not exist. The workflow_run payload does not carry a called workflow's outputs, so that branch can never be true — which is why every push to main leaves a skipped Publish packages run behind. The pipeline runs entirely off the release trigger. Given release-please authenticates with RELEASE_PLEASE_GH_TOKEN (a PAT, not GITHUB_TOKEN), release events do fire and the workflow_run trigger the comment in on: justifies looks redundant. Worth deleting, but that changes release behaviour and did not belong in a hotfix.
  2. github.head_ref and github.event.pull_request.labels are always empty here. This workflow has no pull_request trigger, so both release-please guards are inert. Kept them rather than widen the blast radius.

Verification

  • npm-publish.yaml parses as valid YAML; all three if expressions render as intended.
  • Tag-filter substring matrix above verified.
  • Not exercised end to end: a dispatch can only be tested once this is on main. Worth a manual dispatch with release_tag: f0-react-v6.51.1 after merge to confirm the path is live.

Context

The npm side of the incident is separate and not fixed by this PR: the publish itself was failing with npm error code E404 … PUT https://registry.npmjs.org/@factorialco%2ff0-react, which is npm's response to a token without write access to the scoped package. NPMJS_TOKEN was rotated on 25 Aug 09:10 UTC and the v6.51.1 release run has been re-run to pick it up.

🤖 Generated with Claude Code

`&&` binds tighter than `||` in GitHub Actions expressions, so the flat
condition parsed as:

  (contains(release.tag_name, 'react-v') && … && event_name == 'workflow_dispatch')
  || event_name == 'release'
  || (workflow_run.conclusion == 'success' && workflow_run.outputs.new_version)

On a workflow_dispatch `github.event.release` is null, so the tag check
fails and the first group is false while the other two branches cannot
match either — every manual run of this workflow was skipped, leaving no
escape hatch when the release-triggered publish fails.

Parenthesise the per-event branches and give each one its own tag filter:
dispatch now matches on the `release_tag` input it already checks out, and
a release event only starts the job for the package it belongs to instead
of starting all three and letting two no-op.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@github-actions github-actions Bot added the ci label Aug 25, 2026
@github-actions

Copy link
Copy Markdown
Contributor

🔍 Review policy: Code change

Default rule: any other change needs one approval from f0-devs (rule 4).

Required approvals

Team Why Status
@factorialco/f0-devs Every code change needs a dev approval ⏳ pending
How this was decided
  • PRs touching only sds/ modules require their owners and nothing else.
  • Otherwise, docs-only changes (*.md, *.mdx, *.stories.tsx, anything in __stories__/) → one f0-general approval.
  • Otherwise, feat: titles → one f0-devs and one f0-designers approval. Not a feature? Fix the title prefix.
  • Anything else → one f0-devs approval.
  • Add the needs-design-review label to also request a design approval on any PR.
  • Creating a new sds/ module (new package.yml) additionally requires an f0-general approval.

Policy source: ownership/review-policy.ts · Team members: ownership/teams.yml

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant