Skip to content

ci(sdk): correct the publish workflow's account of why auth failed - #36

Merged
bytesbrains merged 1 commit into
mainfrom
ci/sdk-publish-followups
Jul 28, 2026
Merged

ci(sdk): correct the publish workflow's account of why auth failed#36
bytesbrains merged 1 commit into
mainfrom
ci/sdk-publish-followups

Conversation

@nandal

@nandal nandal commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

Problem

0.1.0-dev.4 published successfully (run 30398802432), but the workflow's own comments now tell a wrong story about how it got there. The setup-node block asserts that registry-url's placeholder token was the cause of the failure. It was a cause, not the cause — and leaving that uncorrected sends the next person down one branch of a two-part problem.

Change summary

Corrects the record. What actually happened across four runs:

Run Result Cause
30397586602 E404 registry-url's placeholder _authToken shadowed OIDC
30398261989 ENEEDAUTH credential path clear; npmjs trusted-publisher entry didn't match
30398531344 ENEEDAUTH same
30398802432 published after the npmjs entry was corrected

Both had to be right. Fixing either alone still failed, with a different error each time — and neither error names its real cause: E404 rather than E401 because npm won't leak which private packages exist, and ENEEDAUTH with no preceding log line at all because npm doesn't announce that it's skipping OIDC.

  • The setup-node comment now states that dropping registry-url is necessary but not sufficient, and records both failure signatures.
  • The OIDC gate gains a note that green there proves only the runner side — the trusted-publisher entry lives on npmjs.com and is invisible from the workflow.
  • The exact npmjs trusted-publisher values are pinned in the header. They're reconstructible only by reading the workflow carefully (basename not path, empty environment, GitHub org not npm scope), and getting any one wrong reproduces the silent ENEEDAUTH.

Bumps setup-node 4.4.0 → 7.0.0 (SHA-pinned), clearing the Node 20 deprecation warning this workflow emitted on every run.

Header de-staled: it described the carrier drift in the present tense, but 0.1.0-dev.4 shipped it. The latest0.1.0-dev.2 pointer is called out instead, since that one is still true.

Tests

Comments and a pinned action bump — no logic change. YAML re-validated: 13 steps, permissions unchanged (contents: read, id-token: write), both uses: lines SHA-pinned.

The bumped setup-node is exercised on the next dispatch. Its inputs (node-version, cache, cache-dependency-path) are unchanged and stable across v4→v7; the majors dropped Node 20 support, which is the point.

Not scoped in: ci.yml's eight floating setup-node@v4 uses emit the same deprecation. Left to Dependabot, which already handles that class of bump (#31 did exactly this for checkout) and would conflict with a manual edit here.

Risk

Very low. The only executable change is the action version; everything else is comments. The workflow is inert until dispatched, and every publish gate still runs ahead of the irreversible step.

Backwards-compat

None affected. No source, no published artifact, no other workflow touched. 0.1.0-dev.4 on npm is unaffected.

Privacy copy

No user-facing privacy copy.

🤖 Generated with Claude Code

The setup-node comment claimed registry-url's placeholder token was the cause of
the publish failure. It was *a* cause, not *the* cause, and leaving that
uncorrected would send the next person down one branch of a two-part problem.

What actually happened across four runs:
  E404       30397586602 — registry-url's placeholder _authToken shadowed OIDC
  ENEEDAUTH  30398261989 — credential path clear; npm still never attempted the
  ENEEDAUTH  30398531344   exchange, because the npmjs trusted-publisher entry
                           did not match this repo + workflow filename
  published  30398802432 — after the npmjs entry was corrected

Both had to be right. Fixing either alone still failed, with a different error
each time, and neither error names the real cause: E404 (not E401) because npm
will not leak which private packages exist, and ENEEDAUTH with no preceding log
line at all because npm does not announce that it is skipping OIDC.

So this records the failure signatures, states plainly that dropping registry-url
is necessary but not sufficient, and notes on the OIDC gate that a green result
proves only the runner side — the trusted-publisher entry lives on npmjs.com and
is invisible from here.

Also pins the exact npmjs trusted-publisher values in the header. They are
reconstructible only by reading the workflow carefully (basename not path, empty
environment, GitHub org not npm scope), and getting any of them wrong reproduces
the silent ENEEDAUTH.

Bumps setup-node 4.4.0 -> 7.0.0, clearing the Node 20 deprecation warning this
workflow emitted on every run. ci.yml's eight floating @v4 uses are left to
Dependabot, which already handles that class of bump (#31) and would otherwise
conflict.

Header no longer describes the carrier drift in the present tense: 0.1.0-dev.4
shipped it. The stale `latest` -> 0.1.0-dev.2 pointer is called out instead,
since that one is still true.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@qodo-code-review

Copy link
Copy Markdown

ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing

@wrokin

wrokin Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

🤖 wrokin code review

The diff upgrades the setup-node action reference to a non-existent v7.0.0 — the official action is at v4. This will cause the workflow to fail immediately.

  • .github/workflows/sdk-publish.yml: the uses: step pins a commit hash annotated # v7.0.0, but no such release exists for actions/setup-node. The workflow will abort with “unable to resolve action”. Revert to the previous v4.4.0 commit or use the semver tag actions/setup-node@v4.

No other logic, security, or style issues found.

🌾 wrokin contribution signal

  • The PR title promises only to "correct the publish workflow's account of why auth failed", but the diff also bumps setup-node from 4.4.0 to 7.0.0 (a major version leap). This functional change is unrelated to the stated intent, introduces untested risk to a publish path, and should be justified and reviewed separately.
  • The version bump appears at the uses: actions/setup-node@… line; it is not accompanied by any test run or verification that the workflow still behaves correctly with the newer action version. For a workflow that controls npm publishing, a silent breakage could block releases.

Advisory — signals worth a closer look, not a verdict or a score.

Model: deepseek/deepseek-v4-pro · your key, your model (BYOK)

@wrokin

wrokin Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

🛡️ wrokin security hunter

No vulnerabilities were proven in this pull request — the hunter found nothing it could exploit and prove by execution.

Advisory only — this check never blocks your PR.

@wrokin

wrokin Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

🤖 wrokin code review

No high-signal issues — the diff only updates comments, adds a note about trusted publisher failure modes, and bumps setup-node from v4 to v7 (a routine action update). No bug, security, or correctness defect introduced.

Model: deepseek/deepseek-v4-pro · your key, your model (BYOK)

@bytesbrains
bytesbrains merged commit 24c5614 into main Jul 28, 2026
10 checks passed
@wrokin

wrokin Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

🤖 wrokin security audit

No security-relevant findings in this diff. The changes are documentation updates and a routine version bump of actions/setup-node (pin to a known, publicly verifiable SHA). No code paths, credential handling, or input usage are altered.

Model: deepseek/deepseek-v4-pro · your key, your model (BYOK)

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.

2 participants