Skip to content

fix(ci): skip npm publish on fork PRs in PR artifact workflow - #671

Merged
ranxianglei merged 1 commit into
masterfrom
2026-09-10_fix-pr-artifact-fork-gating
Sep 10, 2026
Merged

fix(ci): skip npm publish on fork PRs in PR artifact workflow#671
ranxianglei merged 1 commit into
masterfrom
2026-09-10_fix-pr-artifact-fork-gating

Conversation

@ranxianglei

Copy link
Copy Markdown
Owner

Fixes #670

Problem

The PR Build Artifact workflow triggers on every pull_request to master, including fork PRs. Its "Publish to npm with PR tag" step authenticates with secrets.NPM_TOKEN, but GitHub does not pass repository secrets to workflows triggered by fork PRs — so on fork PRs the token is empty and npm publish fails with ENEEDAUTH, failing the job and skipping all subsequent steps (tarball, artifact upload, PR comment).

Observed on #668 (run 34380059571): build succeeded, publish failed with ENEEDAUTH (NODE_AUTH_TOKEN: empty in the publish step), tarball/upload/comment steps skipped. Also affects other fork PRs (e.g. #657, run 34376788600).

Fix

  • Gate the publish step to non-fork PRs:
    if: github.event.pull_request.head.repo.full_name == github.repository
  • The "Comment on PR" step now detects fork PRs and drops the "Option A — Install from npm PR tag" section (replaced with a note that the npm publish was skipped), so the comment no longer advertises a tag that was never published.
  • Tarball creation, artifact upload, and the PR comment still run for fork PRs — fork authors keep the artifact download path.

No credential or workflow permission changes; this is pure workflow gating.

Verification

  • YAML parses (pyyaml); embedded github-script JS passes node --check.
  • Simulated the comment step with stubbed context/github for both cases: non-fork output is byte-identical to the previous comment format; fork output contains only the skip notice + artifact install instructions (no phantom npm tag).
  • Workflow-only change: no typecheck/test/build impact.

@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown

📦 Built Package Artifact

Branch: 2026-09-10_fix-pr-artifact-fork-gating (54253f0)

Option A — Install from npm PR tag (recommended)

npm install -g billion-context@pr-671

Each push to this PR publishes a new version under the pr-671 npm tag.

Option B — Download artifact

  1. Download the artifact from the Actions run
  2. Extract the tarball and install:
tar xzf billion-context-pr671.tgz
npm install -g package

This comment is automatically updated on each push.

@ranxianglei

Copy link
Copy Markdown
Owner Author

🤖 Powered by ework · qwen3.8-27b

[bot] 🏷 Picked up the PR review. Pulling the branch locally to inspect the diff against master, re-run the claimed verifications (YAML parse, JS syntax, comment-step simulation), and screen for duplicates before giving a verdict.

@ranxianglei

Copy link
Copy Markdown
Owner Author

🤖 Powered by ework · qwen3.8-27b

[bot] 🏷 Review complete — approved, ready to merge.

Duplicate screening

No duplicates in this repo: #671 is the sole fix for #670 (properly linked via Fixes #670, which auto-closes on merge). One related item exists in a sibling repo — ranxianglei/opencode-acp#366 tracks the same bug class in that repo's own build-artifact workflow; separate codebase, already tracked there, so no action needed from this PR.

Diff cleanliness

Single file, +10/−1, every line on-topic: .github/workflows/pr-artifact.yml only. No stray files, no reformatting churn, no version bump. Clean.

Independent verification (re-ran all claimed checks)

  • YAML: parses cleanly (pyyaml). Re-diffed merge-base 8c3e81d54253f0: exactly the two changes described, nothing else.
  • JS syntax: extracted the embedded github-script, node --check passes.
  • Comment-step simulation (stubbed context/github, real script body executed):
    • Non-fork output is byte-identical to the pre-change script's output (cmp clean) — also matches the live comment format this PR's own run posted (floor 1).
    • Fork output contains only the skip notice + "Install from artifact" instructions; no phantom pr-N npm tag. Correct.
  • Scope check (that fix(ci): PR Build Artifact publish step fails with ENEEDAUTH on fork PRs #670's claim holds): pr-artifact.yml is the only workflow that is both pull_request-triggered AND uses secrets. ci-e2e.yml is workflow_dispatch-only, release.yml is push-to-master-only, ci.yml uses no secrets — so no other workflow needs this gate. Confirmed.

Correctness notes

  • The gate idiom head.repo.full_name == github.repository is the standard fork-detection expression; both sides are lowercase for this repo, so equality is reliable. The comment step's isFork uses the same comparison (inverted), so publish-skip and comment-skip can never diverge.
  • The comment step still works on fork PRs: GITHUB_TOKEN permissions come from the workflow file's permissions: block (pull-requests: write), which applies to fork-triggered runs — only secrets are withheld. Artifact upload needs no special permission. So fork authors keep tarball download + install comment, as intended.
  • Heads-up (no action needed): fork PRs run the base branch's workflow version (GitHub security behavior), so the gate takes effect for fork PRs immediately after this merges to master — no follow-up needed.
  • Minor, non-blocking observations: (1) on fork PRs the tarball carries the base version string (the publish step's npm version is skipped) rather than -pr.N.M — the copy-to-billion-context-prN.tgz step is unconditional and npm install -g package works regardless, so instructions stay valid; cosmetic only. (2) The ternary's non-fork array keeps its original indentation while the fork array is indented deeper — I deliberately did not touch it, since "fixing" would churn ~9 lines that don't otherwise change and bloat the diff.

No functional issues found; nothing to patch on the branch.

中文摘要:逐行审查了 diff(单文件 +10/−1,干净),独立重跑了全部验证(YAML 解析、内嵌 JS 语法检查、fork/非-fork 两种评论模拟——非-fork 输出与旧格式字节级一致、fork 输出无幽灵 npm tag),并确认这是唯一需要该门控的 pull_request 触发工作流;无功能问题,可以合并。

@ranxianglei
ranxianglei merged commit 785dbed into master Sep 10, 2026
6 checks passed
@ranxianglei ranxianglei mentioned this pull request Sep 10, 2026
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.

fix(ci): PR Build Artifact publish step fails with ENEEDAUTH on fork PRs

1 participant