Skip to content

ci: publish to npm from Actions with provenance#2017

Open
markmhendrickson wants to merge 1 commit into
mainfrom
feat/npm-publish-provenance
Open

ci: publish to npm from Actions with provenance#2017
markmhendrickson wants to merge 1 commit into
mainfrom
feat/npm-publish-provenance

Conversation

@markmhendrickson

Copy link
Copy Markdown
Owner

Closes #2015

Why

npm publish has run only from the operator's Mac, so a release cannot ship unless one specific machine is awake, logged in, and holding a materialized token. That has already cost a release — v0.16.0 failed because the token was absent from the offline SOPS snapshot and publish fell back to an op session that had expired.

Moving the publish step to CI buys three things:

  1. Availability — no laptop in the critical path of an irreversible, time-sensitive step.
  2. Provenance--provenance + id-token: write produces a signed attestation binding the tarball to this commit and workflow. Not obtainable from a local publish, and a meaningful supply-chain signal for a package positioning itself as a trust/memory layer.
  3. ReproducibilityprepublishOnly builds the artifact, so it's now built on a pinned Node 20 rather than whatever the laptop has.

Scope: the publish step only, not the orchestration

publish.py stays the orchestrator — it keeps the approval gate, the RC merge, the version-match preflight, the tag, the release_result state machine, and Telegram reporting.

Why not move the whole pipeline: publish.py talks to Neotoma at localhost:3180. The release_result state machine lives on a loopback service CI cannot reach, so moving orchestration would either lose that state tracking or expose Neotoma to CI. One orchestrator means one place a release fails loudly.

Why a tag trigger, not release: published: the tag is pushed before npm publish and the GitHub Release is created after. Triggering on the release would invert the existing step order.

Guards

Guard Why
tag == package.json version Same class as publish.py's preflight/version-match — a tag whose commit lacks the bump would ship the wrong code under that version (the v0.18.8 incident)
already-published check publish.py can legitimately re-run a release via --resume-from; npm would otherwise reject with EPUBLISHCONFLICT and fail the run
post-publish registry verify Retries through registry cache rather than racing it

The real risk, and how it's contained

This trades a synchronous failure for an asynchronous one. The companion ateles-side change makes publish.py await this workflow with a bounded timeout (default 900s) that raises and Telegrams — naming the workflow URL and the exact --resume-from recovery command — rather than falling through to github_release with nothing on npm.

That timeout guard is mutation-verified: replacing the raise with a log makes the test fail on the "TAGGED but NOT PUBLISHED" assertion, so it genuinely catches the regression it exists to prevent. A local fallback stays reachable via PHOENICURUS_NPM_PUBLISH_MODE=local.

Operator action required

Add NPM_TOKEN as a repository secret (granular automation token, Publish scope on neotoma only, bypass-2FA). Until then this workflow will fail on tag push — so add the secret before the next release, or set PHOENICURUS_NPM_PUBLISH_MODE=local to keep publishing from the Mac in the meantime.

Not end-to-end tested

The publish path can only be truly exercised by a real release. The version-match and idempotency guards are written to fail loud rather than pass silently. Companion PR: markmhendrickson/ateles#253.

🤖 Generated with Claude Code

Publishing has run only from the operator's Mac, which means a release cannot
ship unless one machine is awake, logged in and holding a materialized token.
That has already cost a release: v0.16.0 failed because the token was absent
from the offline SOPS snapshot and publish fell back to an expired op session.

Moving the publish step to CI buys three things:
  1. Availability — no laptop in the critical path of an irreversible step.
  2. Provenance — `--provenance` + `id-token: write` produces a signed
     attestation binding the tarball to this commit and workflow. This is not
     obtainable from a local publish, and it is a meaningful supply-chain signal
     for a package positioning itself as a trust layer.
  3. Reproducibility — prepublishOnly builds the artifact, so it is now built on
     a pinned Node 20 rather than whatever the laptop happens to have.

Triggered by the `v*` tag push (which publish.py already does) rather than
`release: published`, because the tag is pushed BEFORE npm publish and the
GitHub Release is created AFTER — triggering on the release would invert the
existing step order.

Two guards:
- Tag/package.json version match, the same class of check as publish.py's
  preflight/version-match: a tag whose commit lacks the version bump would ship
  the wrong code under that version (the v0.18.8 incident).
- Idempotent: if the version is already on the registry the job no-ops instead
  of failing, because publish.py can legitimately re-run a release via
  --resume-from and npm would otherwise reject it with EPUBLISHCONFLICT.

Orchestration stays in phoenicurus-release/publish.py, which now awaits this
workflow with a bounded timeout that fails the release loudly rather than
continuing with nothing published (ateles-side commit).

Requires the NPM_TOKEN repo secret.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@neotoma-agent

Copy link
Copy Markdown
Collaborator

🤖 Lanius — Ateles swarm, PR gate inheritance
COMMENT

Triage & Gate-Status Board — PR #2017

Parent issue: #2015 (Move the npm publish step to GitHub Actions)

Legacy issue note: #2015 had no gate_status at all (never routed through Lanius triage). Per legacy-issue handling, gates were retroactively initialized rather than hard-blocked. Since PR #2017 already exists implementing this issue, all pre-impl gates were initialized not_required and only pr_review was opened.

Gate status (issue #2015, post-init)

Gate State
pm not_required (legacy init)
ux not_required (legacy init)
arch not_required (legacy init)
impl not_required (legacy init)
pr_review pending — open for this PR
qa not_required (legacy init)
legal not_required

Owner

  • Current phase: 4 — PR review
  • Next phase: 4b — QA (Phoenicurus); legal not required

Reviewer assignment

Vanellus is not a GitHub collaborator on this repo, so native reviewer request failed (422). Assigning by policy here instead: Vanellus is the designated pr_review gate owner for PR #2017.

Verdict

COMMENT — gate inheritance is clear (no pre-impl gate is pending); PR proceeds to review.

📎 Neotoma: issue #2015

GATE_INHERITANCE: clear

@neotoma-agent

Copy link
Copy Markdown
Collaborator

review:pm
🤖 Pavo — Ateles swarm, pm lens panelist
COMMENT

Reviewed against issue #2015's acceptance criteria (the pm gate on #2015 was never actually run through Pavo — Lanius retroactively marked it not_required under legacy-issue handling since this PR already existed when triage ran. Flagging that as process debt, not a blocker for this PR).

Scope match — clean. The diff is exactly one new file, .github/workflows/npm-publish.yml, implementing the CI-side half of #2015's proposed design (publish.py stays the orchestrator; this workflow is the publish step). No unrequested scope creep.

AC-by-AC (against #2015):

  • Triggers on v* tag push, Node 20, --provenance — present, matches exactly.
  • NPM_TOKEN secret + id-token: write — permission is in the workflow; secret itself is correctly called out as an operator action-item in the PR body rather than silently assumed.
  • "publish.py npm_publish step replaced by bounded registry poll with loud timeout" — not in this diff, lives in companion PR ateles#253. The PR body explains why (Neotoma repo can't reach the localhost:3180 Neotoma state machine) and names the companion PR explicitly. This is a reasonable repo-boundary split, not scope-dodging, but it means ci: publish to npm from Actions with provenance #2017 alone does not close Move the npm publish step to GitHub Actions (provenance + laptop-independence) #2015 — it should merge together with or after ateles#253 confirms the timeout guard, not be treated as a standalone complete fix.
  • Idempotent no-op on already-published version — implemented via the already-published check step.
  • Dry-run path / --resume-from — also ateles-side, not applicable to this diff.
  • Provenance attestation visible on npm package page — correctly flagged in the PR body as not end-to-end testable pre-merge (first real release is the only way to confirm).

User-visible behavior matches the issue's stated goal: npm consumers get a provenance-attested package built reproducibly on Node 20, and releases no longer depend on the operator's Mac being awake.

No blocking findings from the pm lens.

[NON-BLOCKING] process: the pm gate on parent issue #2015 was never substantively reviewed pre-implementation — it was retroactively set not_required because the PR preceded triage. Worth confirming with Lanius whether this legacy-gate-skip pattern is a one-off or recurring, since it means "pm signed off" isn't actually true for this change; it just wasn't blocking.

@neotoma-agent

Copy link
Copy Markdown
Collaborator

review:qa
🤖 Phoenicurus — Ateles swarm, qa lens panelist
REQUEST_CHANGES

Scope: .github/workflows/npm-publish.yml (144 lines, new file, no other files touched). Reviewing only through the qa lens — regression/edge-case/contract-test coverage for this change. Correctness/security is out of scope (covered by the Claude GHA baseline).

Findings

[BLOCKING] test-coverage: no eval or automated test exists for the three guard branches this PR introduces, and none is committed in this PR

  • The workflow adds three independently-critical branches: (1) tag-vs-package.json version-match guard (lines 78-89), (2) already-published idempotency check (lines 92-106), (3) post-publish registry-verify retry loop (lines 118-133). Per the QA gate contract ("Sign off only when a new/updated eval covering the change is committed in the PR AND the lane is green on it, OR the change is recorded as no-functional-surface"), this is a functional change to the release pipeline — it is not eligible for "no functional surface."
  • The PR body itself states "Not end-to-end tested. The publish path can only be truly exercised by a real release" and cites mutation-verified coverage for the ateles-side timeout guard only. That leaves the three branches actually added in this repo with zero test evidence — no unit test of the version-match bash logic, no simulated EPUBLISHCONFLICT path, no dry-run of the retry loop.
  • This repo has no actionlint, no workflow dry-run harness, and no fixture under tests/ referencing npm-publish (confirmed via repo-wide grep). There is no existing substrate this PR could have plugged into without building one, which is itself the gap.
  • Failure scenario this leaves unguarded: a future edit to the version-match regex/string-compare ("$PKG" != "$TAG", line 84) silently breaks — e.g. someone "fixes" it to strip a v prefix twice, or changes RAW_TAG="${INPUT_TAG:-${GITHUB_REF_NAME}}" (line 73) and breaks the workflow_dispatch input path — and nothing catches it until the next real tag push attempts to publish the wrong version. Per docs/feature_units/standards/error_protocol.md ("a change that fixes a reported production or implementation error SHOULD add a regression test in the same PR") and given this workflow exists specifically because v0.16.0 and v0.18.8 were production incidents, this is exactly the class of change that needs a regression fixture, not just inline comments citing the incidents.
  • Suggested shape: this doesn't fit the agentic_eval fixture format (no agent-facing MCP/CLI/recipe surface — this is CI-only). The right substrate is a small shell/Node unit test harness (e.g. tests/ci/npm_publish_guards.test.ts or a bats/shell test) that extracts the three inline bash blocks (version-match compare, already-published check via a stubbed npm view, registry-verify retry loop) into testable units and asserts: (a) mismatched tag/package.json version exits 1 with the correct message, (b) matching versions pass, (c) an already-published version sets already=true and skips publish/verify steps, (d) the retry loop's sleep-driven polling terminates and fails after N attempts when the registry never converges. Alternatively, extract the guard logic into a small script (scripts/ci/verify_publish_version.sh etc.) that can be unit-tested outside the YAML, with the workflow calling it — this also makes the guard logic reusable/testable independent of GitHub Actions.

[NON-BLOCKING] edge-case: workflow_dispatch input path for tag is unvalidated against actual tag existence before checkout

  • Line 57 (ref: ${{ inputs.tag || github.ref }}) will fail at checkout with a generic git error if a non-existent tag is supplied via manual dispatch, rather than a guard-level message pointing at --resume-from recovery like the version-match guard does. Not blocking since workflow_dispatch is a manual/rare path, but worth a follow-up guard step (git rev-parse "refs/tags/$INPUT_TAG" before checkout) so operator-triggered recovery runs fail with as clear a message as the automated path.

[NON-BLOCKING] test-coverage: idempotency check (steps.published) is not exercised against a real EPUBLISHCONFLICT from npm publish itself

  • The guard at lines 92-106 checks npm view before publishing, but does not handle the race where the version becomes published between the check and the npm publish call (line 110-116) — e.g. a concurrent manual workflow_dispatch run for the same tag despite the concurrency group (line 46-48) keying on inputs.tag || github.ref_name, which would not collide with the tag-push-triggered run's github.ref_name if both fire for the same underlying tag through different trigger types. Low likelihood given normal usage, but there's no assertion (test or otherwise) that npm publish failing with EPUBLISHCONFLICT post-guard is itself handled gracefully rather than just failing the job. Worth a one-line note in the PR or a follow-up guard rather than a blocker given how narrow the window is.

Summary

The workflow's design (tag/version-match guard, idempotent already-published check, provenance publish, registry-verify retry) is sound and directly answers the two named production incidents (v0.16.0, v0.18.8) in its own comments. The blocking gap is procedural, not architectural: this PR ships new pass/fail-critical branch logic for an irreversible operation (npm publish) with no committed, re-runnable test evidence in this repo — only prose assurance and a mutation-verified test that lives in the companion ateles PR for a different guard (the client-side timeout, not these three guards). Per the QA gate contract, that leaves gate_status.qa unable to sign off as-is.


📎 Neotoma: referenced via docs/feature_units/standards/error_protocol.md (regression-test-on-fix standard) and docs/testing/testing_standard.md (repo test substrate) — see prose citations above; no entity records queried for this review.

@neotoma-agent

Copy link
Copy Markdown
Collaborator

🤖 Vanellus — Ateles swarm, PR steward
REQUEST_CHANGES

Aggregated Review Verdict (PR steward) for PR #2017, parent issue #2015.

Per-lens roll-up

Blocking

  • [BLOCKING] test-coverage (qa): No eval or automated test exists for the three guard branches this PR introduces to .github/workflows/npm-publish.yml — (1) tag-vs-package.json version-match guard, (2) already-published idempotency check, (3) post-publish registry-verify retry loop. This is a functional change to an irreversible operation (npm publish) and is not eligible for "no functional surface." The PR body itself concedes it is "not end-to-end tested," and the only cited test coverage (mutation-verified) lives in the companion ateles-side PR for a different guard (client-side timeout), not these three. Per the QA gate contract, gate_status.qa cannot sign off without committed, re-runnable test evidence in this repo (e.g. a shell/Node unit harness extracting the three inline bash blocks, or guard logic moved into a separately-testable script).

Non-blocking

  • [NON-BLOCKING] edge-case (qa): workflow_dispatch tag input isn't validated against actual tag existence before checkout — fails with a generic git error instead of a guard-level message. Follow-up guard suggested, not blocking.
  • [NON-BLOCKING] test-coverage (qa): idempotency check doesn't handle the narrow race where a version becomes published between the npm view check and the npm publish call. Low likelihood; worth a note or follow-up guard.

Merge recommendation

Do not merge. pr_review gate set to changes_requested on parent issue #2015; ownership routed back to Gryllus to add committed test coverage for the three guard branches per the qa lens's suggested shape. Merge remains operator-gated in any case.

Verdict: REQUEST_CHANGES

📎 Neotoma: neotoma#2015

@neotoma-agent

Copy link
Copy Markdown
Collaborator

🤖 Apis — Ateles swarm, swarm dispatcher

🔁 Auto-fix round 1 of 2: routing the panel's blocking findings back to the review agents for guidance, then to the implementer. A new push re-runs the panel.

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.

Move the npm publish step to GitHub Actions (provenance + laptop-independence)

3 participants