Add tag-triggered publish CI (npm, anywidget, VS Code Open VSX)#357
Merged
Conversation
…ation Publish pipeline (.github): - New publish.yml: on a v*.*.* tag, gate on lint + unit tests, then publish in parallel the matterviz npm package, the VS Code extension (Marketplace + Open VSX), and matterviz-anywidget. workflow_dispatch supports dry_run (default true) that builds/packages/validates without publishing. - Shared `setup` composite action (Node 24 + pnpm install, optional extension installs) reused by publish, test, and lint. - Fold the former publish-anywidget.yml into publish.yml. Also bundled: - package.json: expose plot subfolders via a ./plot/* wildcard export; pin dompurify to 3.4.7 - sanitize.ts: memoize sanitize_html (skips repeated double DOMPurify passes when a component re-sanitizes many cells per render, e.g. HeatmapTable) - extensions/vscode/package.json: regenerate contributes.configuration for the new Box/Sankey/Sunburst component settings
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a reusable composite GitHub Action for Node 24 + pnpm, refactors lint/test workflows to use it, introduces a gated multi-target publish workflow, inserts VS Code config schemas for box/sankey/sunburst, adds ChangesCI/CD Infrastructure Consolidation
Feature and Packaging Updates
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
.github/actions/setup/action.yml (1)
12-15: ⚡ Quick winConsider enabling pnpm caching for faster CI runs.
The
actions/setup-nodeaction supports built-in dependency caching viacache: 'pnpm', which would speed up all workflows using this composite action.⚡ Proposed enhancement to add pnpm caching
- uses: actions/setup-node@v6 with: node-version: 24 registry-url: https://registry.npmjs.org + cache: pnpm🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/actions/setup/action.yml around lines 12 - 15, Add pnpm caching to the actions/setup-node invocation by updating the "with" inputs for the existing uses: actions/setup-node@v6 step (the block that currently sets node-version: 24 and registry-url). Specifically, add cache: 'pnpm' and set cache-dependency-path to the repository lockfile (e.g., pnpm-lock.yaml) so the action will use built-in pnpm dependency caching during CI runs.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/publish.yml:
- Around line 77-92: Move the version check so it runs before any parallel
upload/publish jobs and ensure every package (root, VS Code extension, and
matterviz-anywidget) either depends on that check or performs the same
validation; reuse the existing verification snippet that reads EXPECTED from
needs.version.outputs.version and compares it to node -p
"require('./package.json').version" (the EXPECTED env and the actual=$(...)
comparison) by extracting it into a dedicated pre-publish job or adding the same
comparison step to the anywidget job so no upload runs unless the version/tag
match.
- Around line 15-24: The workflow currently allows manual publishes from any ref
via the workflow_dispatch inputs (version, dry_run); to block live publishes,
add a guard so publishing jobs only proceed when dry_run is true OR the ref is a
trusted release ref (e.g., main branch or tags). Concretely, in the publishing
jobs (e.g., the jobs that handle vscode and anywidget publishing) add an if
condition like: only run the live-publish steps when inputs.dry_run == 'false'
AND (github.ref == 'refs/heads/main' || startsWith(github.ref, 'refs/tags/')),
otherwise skip publishing; alternatively require inputs.dry_run to never be set
to false for arbitrary refs by forcing dry_run default true and preventing false
execution unless the ref matches the trusted refs above. Ensure you reference
the workflow_dispatch inputs (version, dry_run) and update the job-level if
conditions for the vscode and anywidget publish jobs accordingly.
- Line 52: The version regex in the resolver currently allows prerelease
versions (e.g., in the line "[[ \"$version\" =~
^[0-9]+\\.[0-9]+\\.[0-9]+([.-][0-9A-Za-z.-]+)?$ ]]") but the subsequent publish
steps (the `npm publish` and the `anywidget` publish commands) do not pass an
explicit tag; fix this by either updating that resolver regex to reject
prerelease identifiers (remove the "([.-][0-9A-Za-z.-]+)?" part) or, preferable,
keep prerelease support and add an explicit tag argument to the publish commands
(add `--tag` with a computed value such as "next" or the prerelease identifier)
and apply the same `--tag` usage to both the `npm publish` and the anywidget
publish invocation so prerelease publishes succeed and do not go to "latest".
In `@package.json`:
- Line 196: The package.json downgrade of dompurify from 3.4.8 to 3.4.7 is
unexplained; verify why "dompurify": "3.4.7" was pinned in package.json, check
changelog/release notes and npm/GitHub security advisories for 3.4.7 vs 3.4.8,
and either (a) revert the dependency to 3.4.8 if there is no
compatibility/security reason, updating the lockfile accordingly, or (b) add a
brief comment/commit message (and ideally a package.json rationale field or PR
description) documenting the specific compatibility or packaging issue that
required pinning to 3.4.7 so reviewers understand the intentional downgrade.
---
Nitpick comments:
In @.github/actions/setup/action.yml:
- Around line 12-15: Add pnpm caching to the actions/setup-node invocation by
updating the "with" inputs for the existing uses: actions/setup-node@v6 step
(the block that currently sets node-version: 24 and registry-url). Specifically,
add cache: 'pnpm' and set cache-dependency-path to the repository lockfile
(e.g., pnpm-lock.yaml) so the action will use built-in pnpm dependency caching
during CI runs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 706c0921-642d-4b62-a07c-8b53c2ab02ff
📒 Files selected for processing (8)
.github/actions/setup/action.yml.github/workflows/lint.yml.github/workflows/publish-anywidget.yml.github/workflows/publish.yml.github/workflows/test.ymlextensions/vscode/package.jsonpackage.jsonsrc/lib/sanitize.ts
💤 Files with no reviewable changes (1)
- .github/workflows/publish-anywidget.yml
matterviz + matterviz-anywidget now publish via OIDC trusted publishing instead of a long-lived token: drop NODE_AUTH_TOKEN/NPM_TOKEN and the registry-url .npmrc scaffolding (an empty _authToken would block OIDC), ensure npm >= 11.5.1, and drop --provenance (emitted automatically under trusted publishing). VS Code Marketplace + Open VSX keep PATs (Open VSX has no OIDC yet; Marketplace OIDC needs Azure federated identity).
workflow_dispatch now resolves the version to its `vX.Y.Z` tag and every job checks out that exact commit, so a manual run can (re)publish an already-pushed tag that never reached a registry -- instead of building whatever ref the workflow ran from. The version job verifies the tag exists (falls back to the triggering ref for a dry run of a not-yet-tagged version, and errors on a real run when the tag is missing). The version-match guard is skipped on dry runs since a not-yet-tagged ref won't match.
- Add a `verify` job (checked out at the release tag) that all publish jobs depend on, so a version/tag mismatch blocks every upload instead of letting parallel jobs publish partially; drop the now-duplicated inline checks in the npm/vscode jobs. - Tighten the version regex to plain X.Y.Z: a prerelease would land on npm 'latest' (no --tag is passed) and the repo ships no prereleases. - Revert dompurify 3.4.7 -> 3.4.8: 3.4.8 includes 3.4.7's CVE-2026-49978 fix plus template-scrubbing / Trusted-Types fixes with no regressions, so the downgrade was unwarranted.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/publish.yml:
- Around line 32-35: The concurrency group key currently mixes manual input and
raw refs, causing different groups for the same release; update the group
expression to normalize to the tag name (prefer github.ref_name or strip
refs/tags/) so manual workflow_dispatch input and tag pushes resolve to the same
identifier. Replace the current group: publish-${{ github.event.inputs.version
|| github.ref }} with a normalized expression that uses
github.event.inputs.version || github.ref_name ||
github.ref.replace('refs/tags/','') (ensuring a fallback) so the group always
becomes publish-<tag> and prevents duplicate publishes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 02ad8014-e0bd-45b7-a2c2-ceacf87ca813
📒 Files selected for processing (1)
.github/workflows/publish.yml
There was a problem hiding this comment.
♻️ Duplicate comments (1)
.github/workflows/publish.yml (1)
32-35:⚠️ Potential issue | 🟠 Major | ⚡ Quick winNormalize the concurrency key to the release tag.
Line 34 still keys
workflow_dispatchruns by raw input but tag pushes by the fullrefs/tags/...ref. The same release can therefore run under different groups (publish-0.4.0,publish-v0.4.0,publish-refs/tags/v0.4.0) and race duplicate npm / Marketplace publishes.Suggested fix
concurrency: - group: publish-${{ github.event.inputs.version || github.ref }} + group: publish-${{ github.event.inputs.version && (startsWith(github.event.inputs.version, 'v') && github.event.inputs.version || format('v{0}', github.event.inputs.version)) || github.ref_name || github.ref }} cancel-in-progress: false🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/publish.yml around lines 32 - 35, Normalize the concurrency group to use the tag name (without refs/) by replacing the current expression with one that prefers github.ref_name (which yields a bare tag) and falls back to the workflow_dispatch input; update the group value currently using "publish-${{ github.event.inputs.version || github.ref }}" to "publish-${{ github.ref_name || github.event.inputs.version }}" so both tag pushes and manual dispatches produce the same normalized key.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In @.github/workflows/publish.yml:
- Around line 32-35: Normalize the concurrency group to use the tag name
(without refs/) by replacing the current expression with one that prefers
github.ref_name (which yields a bare tag) and falls back to the
workflow_dispatch input; update the group value currently using "publish-${{
github.event.inputs.version || github.ref }}" to "publish-${{ github.ref_name ||
github.event.inputs.version }}" so both tag pushes and manual dispatches produce
the same normalized key.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9dc0d0c8-1b1a-44a2-b08c-e8da047b3a0b
📒 Files selected for processing (2)
.github/workflows/publish.ymlpackage.json
The Marketplace job now uses azure/login (OIDC federated identity) + `vsce publish --azure-credential` instead of VSCE_PAT, since Azure DevOps blocks PAT creation for the publisher's org. Adds id-token: write to the vscode job and swaps the required secrets to AZURE_CLIENT_ID + AZURE_TENANT_ID. Open VSX still uses OVSX_PAT.
…d-credential subject
…* wildcard - dompurify 3.4.8's template-scrubbing change over-strips matterviz's two-pass sanitize_html (allowed tags like <strong>/<sub> were dropped), failing sanitize.test.ts. Pin back to 3.4.7 -- this reverts an earlier review change that removed the deliberate pin. - package-exports.test.ts now expands `*` subpath patterns (e.g. ./dist/plot/*/index.js) against the real subdirectories instead of checking a literal `*` path, so the new ./plot/* wildcard export validates.
…release share a group
…rtifact The publisher's Azure org can't issue a publish PAT or register an OIDC app, so drop the Marketplace publish (and its azure/login + release environment + id-token). The vscode job now uploads a `matterviz-vsix` artifact for manual Marketplace upload and keeps auto-publishing to Open VSX. Also format package-exports.test.ts so prek passes.
janosh
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
On a
v*.*.*tag, gate on lint + unit tests + a version-match check, then publish in parallel:matterviz→ npm via OIDC trusted publishing (no token; provenance automatic)matterviz-anywidget→ npm via OIDC trusted publishingmatterviz-vsixartifact for manual upload to the VS Code MarketplaceThe VS Code Marketplace is not auto-published. The publisher's Azure DevOps org can't issue a publish PAT or register an Entra OIDC app, so the
vscodejob builds + uploads the.vsixartifact for a manual drag-drop at https://marketplace.visualstudio.com/manage. Open VSX (separate registry, token-based) is still auto-published.Other details:
workflow_dispatchwith adry_runinput (defaulttrue) builds/packages/validates everything but uploads nothing. A real manual run builds the requested version's tag, so you can (re)publish an already-pushed tag that never reached a registry.setupcomposite action (Node 24 + pnpm install, optional extension installs) is reused bypublish,test, andlint.publish-anywidget.yml(folded intopublish.yml).Also bundled in this branch:
package.json:./plot/*wildcard subpath export (e.g.import { ScatterPlot } from 'matterviz/plot/scatter') and dompurify pinned to3.4.7(3.4.8's template-scrubbing change over-strips the two-passsanitize_html).sanitize.ts: memoizesanitize_html(avoids repeated double-DOMPurify passes, e.g.HeatmapTable).extensions/vscode/package.json: regeneratedcontributes.configurationfor the new Box/Sankey/Sunburst component settings.Required repo secret
OVSX_PAT— Open VSX token (one-time namespace:npx ovsx create-namespace janosh -p <OVSX_PAT>).npm needs no secret: configure a Trusted Publisher for both
mattervizandmatterviz-anywidgeton npmjs.com pointing at this repo +publish.yml.Test plan
gh workflow run publish.yml -f version=0.5.0 -f dry_run=trueis greenvX.Y.Ztag publishes npm + anywidget + Open VSX and uploads thematterviz-vsixartifactsetupaction