Automate dual-package npm publishing with trusted OIDC - #802
Conversation
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 35 minutes Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughAdds tag-triggered dual-package npm publishing through Trusted Publishing. The release script creates and verifies parity-checked artifacts, workflow jobs validate release prerequisites and registry slots, and documentation and tests cover operation and safeguards. ChangesDual npm release automation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant GitTag
participant ValidateJob
participant ReleaseScript
participant RegistryCheck
participant PublishJob
participant npm
GitTag->>ValidateJob: trigger workflow for v* tag
ValidateJob->>ReleaseScript: prepare dual release
ReleaseScript-->>ValidateJob: tarballs and manifest
ValidateJob->>RegistryCheck: check unused registry slots
RegistryCheck->>npm: query package versions and integrity
ValidateJob-->>PublishJob: upload validated artifacts
PublishJob->>RegistryCheck: recheck recoverable slots
PublishJob->>npm: publish canonical then compatibility packages
npm-->>PublishJob: return published identities
Possibly related issues
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Warning Billing warning: we have not been able to collect payment for this subscription for more than 72 hours. Please update the payment method or pay any pending invoices in Billing to avoid service interruption. Comment |
coderabbitai
Bot
added
documentation
evaOS review status: completedPR: #802 - Automate dual-package npm publishing with trusted OIDC evaOS review completed for this PR head. Automation note: agents should wait for this comment to reach PR URL: #802 Review URL: #802 (review) |
![evaos-code-review-bot[bot]](https://avatars.githubusercontent.com/in/4184532?s=60&v=4){kind=small}
There was a problem hiding this comment.
Walkthrough
PR: #802 - Automate dual-package npm publishing with trusted OIDC
Head: 6a402b89e3df1e1f3f47a0b76c2eaa21d191c126 into main. Review event: COMMENT.
Provider: Unregistered provider id (builtin:zai-coding-plan, zcode (registry miss), model unknown).
Estimated review effort: 5/5 (~60 min)
Changed Files
| File | Status | Churn | Purpose | Risk |
|---|---|---|---|---|
.github/workflows/publish-npm.yml |
added | +231/-0 | Changed file | Elevated: validated P1 finding |
docs/BETA_RELEASE_RUNBOOK.md |
modified | +64/-2 | Documentation | Low |
docs/RELEASE_CHECKLIST.md |
modified | +24/-0 | Documentation | Low |
scripts/prepare-dual-npm-release.mjs |
added | +364/-0 | Changed file | Moderate: validated P2 finding |
tests/npm-trusted-publishing.test.ts |
added | +110/-0 | Test coverage | Moderate: validated P3 finding |
Review Signal
Validated inline findings: 6 (P0: 0, P1: 2, P2: 3, P3: 1).
Dropped findings before posting: 0. High-severity findings: 2.
Risk Taxonomy
- Auth: 2
- CI/build: 1
- Release regression: 1
- Runtime correctness: 2
Validation and Proof
2 required validation/proof recommendation(s) selected from changed files.
- required: TypeScript/web build or CI proof - Runtime TypeScript/web files or package/config files changed. Proof: npm run build; typecheck; focused Vitest; green GitHub check.
- required: CI/release smoke proof - CI, release, launchd, or package metadata changed. Proof: green GitHub check; release-status; coverage-audit; rollback note.
Proof status: missing - 1 required validation/proof recommendation(s) missing from PR metadata.
Profile validation hints: Call out evidence leakage, replay/collision risks, duplicate side effects, and brittle sanitizer logic.
Profile proof expectations: Look for focused sanitizer, signature, orchestration, or fixture proof.
Related Context
Related issues/PRs: #801.
Suggested labels: bug, docs, tests.
Suggested reviewers: none from current metadata.
Review Settings Preview
- Profile: assertive
- Enabled sections: Review summary (inline_review); Walkthrough (inline_review); Changed-files table (walkthrough); Effort estimate (walkthrough); Related issues/PRs (walkthrough); Suggested labels (suggestion_only); Review status comment (sticky_status)
- Path instructions: none
- Label suggestions: orchestration, security, regression-hardening
- Reviewer suggestions: none
- Suggestion behavior: suggestions only; labels and reviewers are not auto-applied.
- Roadmap-only settings: auto-apply labels; auto-request reviewers; required status checks
Pre-merge checklist
- Inline comments target current RIGHT-side diff lines.
- No secret-like content survived into posted inline comments.
- REQUEST_CHANGES is only used when eligible P0/P1 findings survive validation.
- Required behavior proof is present or not applicable.
- Labels and reviewers are suggestions only; the bot did not auto-apply them.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/BETA_RELEASE_RUNBOOK.md (1)
524-531: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winDocument the deterministic dual-package manual fallback.
This only gives dist-tag guidance; it does not identify the two validated tarballs, require publishing both identities in canonical-then-compatibility order, or require verifying both registry entries. An OIDC outage could therefore leave the compatibility package unpublished. Add the exact artifact/manifest-driven procedure and retain the partial-release rollback path.
🤖 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 `@docs/BETA_RELEASE_RUNBOOK.md` around lines 524 - 531, Update the manual npm fallback procedure in the release runbook to define the two validated tarballs from the generated artifacts/manifests, publish the canonical package first and the compatibility package second using the appropriate dist tag, and require verifying both registry entries afterward. Preserve and explicitly retain the existing partial-release rollback path if either publication or verification fails.
🤖 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-npm.yml:
- Around line 29-32: Set persist-credentials to false in the with configuration
for both checkout actions in the publish workflow, including the checkout step
identified by “Checkout exact tag” and the other checkout referenced in the
comment. Leave fetch-depth and the remaining checkout behavior unchanged.
- Around line 123-137: Update .github/workflows/publish-npm.yml lines 123-137 to
redirect npm view stdout and stderr into the same diagnostic file, then extract
the registry-availability loop into a shared script. Replace the duplicate check
at lines 198-212 with a call to that script so both gates use the same
implementation and cannot drift.
- Line 30: Pin every GitHub Action reference in the publish workflow, including
the checkout and all additionally identified `uses` entries, to immutable
40-character commit SHAs. Preserve each action’s current version through an
inline version comment, and ensure no mutable tags or branch references remain.
- Line 127: Update the package iteration loops in the workflow, including the
loops around the existing package_name references, to derive package names from
dual-package-manifest.json using its .packages[].name values instead of
hardcoded names. Preserve the current loop behavior and ensure all three loops
use the manifest-derived list so package renames remain covered.
- Line 98: Move all GitHub Actions expression interpolations referenced inside
run scripts to step-level env variables, including the values used by
canonical_tarball and the additional sites identified in the comment. Update
each shell command to reference the corresponding environment variable instead
of embedding ${{ ... }} directly, while preserving the existing release and
verification behavior.
- Around line 214-222: Update the “Recheck both registry slots” validation and
publish flow in the publish canonical then compatibility package step so
recovery after a successful canonical publish is possible. Treat an
already-published canonical package as satisfied only when its registry artifact
checksum matches the expected canonical tarball, allowing the compatibility
package to publish without requiring a version bump; preserve rejection for
missing or checksum-mismatched artifacts.
- Around line 53-64: Update the workflow loop over CI and CodeQL so the gate
counts completed runs whose conclusion is not "success" and asserts that count
is zero, rather than counting successful runs and requiring at least one.
Preserve the existing commit, workflow, and completed-run filters.
In `@scripts/prepare-dual-npm-release.mjs`:
- Around line 227-237: Update run to include result.stderr in the thrown failure
error, preserving the command and exit status context. Handle signal termination
as well by reporting result.signal when no numeric status is available, so npm
pack/tar diagnostics are visible in CI logs.
- Around line 184-198: Update assertSafeOutputRoot to reject output directories
that are ancestors of sourceRoot as well as descendants or the same directory.
Treat path.relative(sourceRoot, outputRoot) === ".." and other parent-relative
results as unsafe, while preserving the existing rejection of the source
directory and allowing only unrelated output paths.
- Around line 99-120: Update the packaging flow around runNpmPack and
packageManifestEntry to compare the npm pack metadata for canonicalPack and
compatibilityPack before constructing the packages array. Validate that both
artifacts have matching entryCount and unpackedSize, and only then allow the
manifest to assert payloadParity: true.
In `@tests/npm-trusted-publishing.test.ts`:
- Around line 75-97: Update the npm trusted-publishing test around
prepareDualNpmRelease to read the repository version from package.json,
construct the release tag as v${version}, and derive the expected dist-tag using
distTagForVersion. Replace the hardcoded 1.7.0 and latest assertions with these
derived values while preserving the remaining manifest and verification checks.
- Around line 15-55: The workflow test currently checks OIDC permissions and
publish ordering only globally, so it can miss incorrect job scoping and absent
artifact verification. Update the test around the existing workflow assertions
to isolate or parse the relevant jobs, assert id-token: write is scoped to the
publishing job, and require checksum verification after artifact download and
before both canonical and compatibility publishes.
---
Outside diff comments:
In `@docs/BETA_RELEASE_RUNBOOK.md`:
- Around line 524-531: Update the manual npm fallback procedure in the release
runbook to define the two validated tarballs from the generated
artifacts/manifests, publish the canonical package first and the compatibility
package second using the appropriate dist tag, and require verifying both
registry entries afterward. Preserve and explicitly retain the existing
partial-release rollback path if either publication or verification fails.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: debccb0c-7e5a-42b7-89e6-427a64178534
📒 Files selected for processing (5)
.github/workflows/publish-npm.ymldocs/BETA_RELEASE_RUNBOOK.mddocs/RELEASE_CHECKLIST.mdscripts/prepare-dual-npm-release.mjstests/npm-trusted-publishing.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: CodeQL (javascript-typescript)
- GitHub Check: test
🧰 Additional context used
🪛 LanguageTool
docs/RELEASE_CHECKLIST.md
[uncategorized] ~172-~172: The official name of this software platform is spelled with a capital “H”.
Context: ...The normal dual-package publish path is .github/workflows/publish-npm.yml with npm Tru...
(GITHUB)
docs/BETA_RELEASE_RUNBOOK.md
[uncategorized] ~459-~459: The official name of this software platform is spelled with a capital “H”.
Context: ... Routine dual-package publication uses .github/workflows/publish-npm.yml and npm Trus...
(GITHUB)
🪛 zizmor (1.28.0)
.github/workflows/publish-npm.yml
[warning] 29-32: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[warning] 173-174: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[info] 98-98: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[info] 103-103: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[info] 110-110: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[info] 117-117: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[info] 126-126: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[info] 143-143: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[info] 144-144: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[info] 147-147: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[info] 148-148: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[info] 201-201: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[info] 218-218: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[info] 219-219: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[info] 220-220: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[info] 227-227: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[error] 30-30: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 35-35: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 153-153: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 174-174: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 177-177: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 187-187: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[warning] 21-21: permissions without explanatory comments (undocumented-permissions): needs an explanatory comment
(undocumented-permissions)
[warning] 171-171: permissions without explanatory comments (undocumented-permissions): needs an explanatory comment
(undocumented-permissions)
[warning] 42-42: ad-hoc installation of packages (adhoc-packages): installs a package outside of a lockfile
(adhoc-packages)
[warning] 99-99: ad-hoc installation of packages (adhoc-packages): installs a package outside of a lockfile
(adhoc-packages)
[warning] 184-184: ad-hoc installation of packages (adhoc-packages): installs a package outside of a lockfile
(adhoc-packages)
🔇 Additional comments (5)
docs/BETA_RELEASE_RUNBOOK.md (1)
457-513: LGTM!docs/RELEASE_CHECKLIST.md (1)
170-192: LGTM!tests/npm-trusted-publishing.test.ts (1)
1-7: LGTM!scripts/prepare-dual-npm-release.mjs (1)
139-182: LGTM!.github/workflows/publish-npm.yml (1)
163-171: 🔒 Security & PrivacyRemove this warning.
lossless-openclaw-orchestratoralready has published npm versions, so a manual bootstrap isn’t needed here.> Likely an incorrect or invalid review comment.
evaOS review status: stale headPR: #802 - Automate dual-package npm publishing with trusted OIDC evaOS review stopped because this queued head is no longer the live PR head. Automation note: agents should wait for this comment to reach PR URL: #802 |
coderabbitai
Bot
removed
the
orchestrator
|
Review delta at |
evaOS review status: completedPR: #802 - Automate dual-package npm publishing with trusted OIDC evaOS review completed for this PR head. Automation note: agents should wait for this comment to reach PR URL: #802 Review URL: #802 (review) |
There was a problem hiding this comment.
Walkthrough
PR: #802 - Automate dual-package npm publishing with trusted OIDC
Head: c44c222f01c907404a07c4b15b11ee70139e1971 into main. Review event: COMMENT.
Provider: GLM/Z.ai through ZCode (zcode-glm, zcode, model GLM-5.2).
Estimated review effort: 5/5 (~64 min)
Changed Files
| File | Status | Churn | Purpose | Risk |
|---|---|---|---|---|
.github/workflows/ci.yml |
modified | +3/-0 | Changed file | Low |
.github/workflows/publish-npm.yml |
added | +250/-0 | Changed file | Elevated: large change |
docs/BETA_RELEASE_RUNBOOK.md |
modified | +78/-7 | Documentation | Low |
docs/RELEASE_CHECKLIST.md |
modified | +27/-0 | Documentation | Low |
scripts/check-npm-release-slots.mjs |
added | +154/-0 | Changed file | Low |
scripts/prepare-dual-npm-release.mjs |
added | +378/-0 | Changed file | Elevated: large change |
tests/npm-trusted-publishing.test.ts |
added | +208/-0 | Test coverage | Elevated: large change |
Review Signal
No validated inline findings.
Dropped findings before posting: 3. High-severity findings: 0.
Risk Taxonomy
No finding categories.
Validation and Proof
2 required validation/proof recommendation(s) selected from changed files.
- required: TypeScript/web build or CI proof - Runtime TypeScript/web files or package/config files changed. Proof: npm run build; typecheck; focused Vitest; green GitHub check.
- required: CI/release smoke proof - CI, release, launchd, or package metadata changed. Proof: green GitHub check; release-status; coverage-audit; rollback note.
Proof status: missing - 1 required validation/proof recommendation(s) missing from PR metadata.
Profile validation hints: Call out evidence leakage, replay/collision risks, duplicate side effects, and brittle sanitizer logic.
Profile proof expectations: Look for focused sanitizer, signature, orchestration, or fixture proof.
Related Context
Related issues/PRs: #801.
Suggested labels: docs, tests.
Suggested reviewers: none from current metadata.
Review Settings Preview
- Profile: assertive
- Enabled sections: Review summary (inline_review); Walkthrough (inline_review); Changed-files table (walkthrough); Effort estimate (walkthrough); Related issues/PRs (walkthrough); Suggested labels (suggestion_only); Review status comment (sticky_status)
- Path instructions: none
- Label suggestions: orchestration, security, regression-hardening
- Reviewer suggestions: none
- Suggestion behavior: suggestions only; labels and reviewers are not auto-applied.
- Roadmap-only settings: auto-apply labels; auto-request reviewers; required status checks
Pre-merge checklist
- Inline comments target current RIGHT-side diff lines.
- No secret-like content survived into posted inline comments.
- REQUEST_CHANGES is only used when eligible P0/P1 findings survive validation.
- Required behavior proof is present or not applicable.
- Labels and reviewers are suggestions only; the bot did not auto-apply them.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c44c222f01
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Superseded by fixes through c44c222, terminal dispositions on every thread, green exact-head CI/CodeQL, and an independent current-head delta review with no validated findings.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/BETA_RELEASE_RUNBOOK.md (1)
467-480: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDocument the npm version floor for
npm trust.
npm trustneeds npm 11.15.0+, but this runbook only shows the trust commands. Add an explicitnpm --versioncheck or the samenpm install --global npm@11.17.0setup used in CI before either command.🤖 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 `@docs/BETA_RELEASE_RUNBOOK.md` around lines 467 - 480, Add an npm version prerequisite before the two npm trust commands in the runbook, using either an explicit npm --version check or the CI-standard global installation of npm@11.17.0, and ensure it establishes npm 11.15.0 or newer before invoking npm trust.
🤖 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-npm.yml:
- Around line 218-241: Add the npm provenance flag to both publish invocations
in the “Publish canonical then compatibility package” step, covering the
canonical and compatibility artifacts while preserving their existing tag and
access options.
- Around line 243-250: Update the “Verify both published identities” workflow
step to inspect the JSON written by check-npm-release-slots.mjs and fail unless
every packages[].state equals "matching". Preserve the existing recoverable-mode
invocation and output path, and ensure missing or otherwise non-matching states
produce a nonzero exit status.
In `@scripts/check-npm-release-slots.mjs`:
- Around line 108-113: Update the registry lookup error in the relevant helper
to include the existing diagnostic value, containing stdout and stderr,
alongside the package name. Match the error-reporting behavior of
prepare-dual-npm-release.mjs's run() helper while preserving the existing E404
handling.
---
Outside diff comments:
In `@docs/BETA_RELEASE_RUNBOOK.md`:
- Around line 467-480: Add an npm version prerequisite before the two npm trust
commands in the runbook, using either an explicit npm --version check or the
CI-standard global installation of npm@11.17.0, and ensure it establishes npm
11.15.0 or newer before invoking npm trust.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4bf321f7-6795-43a5-a794-a79039fd84f9
📒 Files selected for processing (7)
.github/workflows/ci.yml.github/workflows/publish-npm.ymldocs/BETA_RELEASE_RUNBOOK.mddocs/RELEASE_CHECKLIST.mdscripts/check-npm-release-slots.mjsscripts/prepare-dual-npm-release.mjstests/npm-trusted-publishing.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: test
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-07-30T12:11:30.162Z
Learnt from: 100yenadmin
Repo: 100yenadmin/Lossless-Codex-Orchestrator-LCO PR: 802
File: .github/workflows/publish-npm.yml:30-30
Timestamp: 2026-07-30T12:11:30.162Z
Learning: When reviewing GitHub Actions workflow files in this repository, treat GitHub-maintained major action tags (e.g., actions/checkoutv4) as the established convention. Do not treat immutable 40-character SHA pinning for GitHub Actions (e.g., actions/checkout@<40-char-SHA>) as a blocking policy violation unless the repo explicitly adds an enforcement rule and/or a required CI check that requires SHA pinning.
Applied to files:
.github/workflows/ci.yml.github/workflows/publish-npm.yml
🪛 zizmor (1.28.0)
.github/workflows/ci.yml
[warning] 18-18: ad-hoc installation of packages (adhoc-packages): installs a package outside of a lockfile
(adhoc-packages)
.github/workflows/publish-npm.yml
[warning] 44-44: ad-hoc installation of packages (adhoc-packages): installs a package outside of a lockfile
(adhoc-packages)
[warning] 183-183: ad-hoc installation of packages (adhoc-packages): installs a package outside of a lockfile
(adhoc-packages)
🔇 Additional comments (17)
tests/npm-trusted-publishing.test.ts (3)
27-34: Delimit thepublishjob before checking OIDC.
publishJobextends to EOF, so a later job’sid-token: writecould satisfy this assertion if the publish job loses it. Bound the slice at the next top-level job (or parse YAML).
10-25: LGTM!Also applies to: 44-79
82-115: LGTM!Also applies to: 142-208
.github/workflows/ci.yml (1)
17-19: LGTM!docs/BETA_RELEASE_RUNBOOK.md (1)
505-520: LGTM!Also applies to: 533-540
docs/RELEASE_CHECKLIST.md (1)
170-195: LGTM!scripts/prepare-dual-npm-release.mjs (2)
184-207: LGTM!
236-251: LGTM!scripts/check-npm-release-slots.mjs (4)
10-32: LGTM! Good design — allowing "matching" to short-circuit inrecoverablemode is exactly what's needed to make a partial dual-publish resumable without a version bump.
34-78: LGTM!
115-154: LGTM!
80-107: 🗄️ Data Integrity & IntegrationNo issue:
npm view <pkg>@<version> version dist.integrity --jsonreturns flat top-level keys, soparsed["dist.integrity"]is the correct lookup.> Likely an incorrect or invalid review comment..github/workflows/publish-npm.yml (5)
33-33: LGTM!Also applies to: 171-172
43-45: LGTM! The post-install version assertion closes the drift gap flagged previously.Also applies to: 182-184
56-67: LGTM!
126-134: LGTM! Delegating both the validate-time and pre-publish-time registry checks tocheck-npm-release-slots.mjseliminates the prior duplicated/hardcoded inline bash logic.Also applies to: 198-217
126-134: 🩺 Stability & AvailabilityConfirm the recovery playbook uses "re-run failed jobs," not "re-run all jobs."
validate'sunused-mode check hard-fails if either slot is already published — even an exact match from a prior partial publish. Recovery only works viapublish'srecoverablerecheck (Lines 198-217), which is skipped ifvalidatereruns from scratch. If an operator (or automation) re-runs the whole workflow instead of only the failedpublishjob after a partial-publish incident,validatewould now hard-fail on the already-matching slot. Please confirm the runbook explicitly calls out "re-run failed jobs only" as the supported recovery path for this scenario.
evaOS review status: completedPR: #802 - Automate dual-package npm publishing with trusted OIDC evaOS review completed for this PR head. Automation note: agents should wait for this comment to reach PR URL: #802 Review URL: #802 (review) |
There was a problem hiding this comment.
Walkthrough
PR: #802 - Automate dual-package npm publishing with trusted OIDC
Head: 578991b495032ded8ed8f9683f13f5cdc8fe1cf8 into main. Review event: COMMENT.
Provider: GLM/Z.ai through ZCode (zcode-glm, zcode, model GLM-5.2).
Estimated review effort: 5/5 (~64 min)
Changed Files
| File | Status | Churn | Purpose | Risk |
|---|---|---|---|---|
.github/workflows/ci.yml |
modified | +3/-0 | Changed file | Elevated: validated P1 finding |
.github/workflows/publish-npm.yml |
added | +250/-0 | Changed file | Moderate: validated P2 finding |
docs/BETA_RELEASE_RUNBOOK.md |
modified | +81/-7 | Documentation | Low |
docs/RELEASE_CHECKLIST.md |
modified | +27/-0 | Documentation | Low |
scripts/check-npm-release-slots.mjs |
added | +190/-0 | Changed file | Elevated: validated P1 finding |
scripts/prepare-dual-npm-release.mjs |
added | +378/-0 | Changed file | Moderate: validated P2 finding |
tests/npm-trusted-publishing.test.ts |
added | +235/-0 | Test coverage | Moderate: validated P3 finding |
Review Signal
Validated inline findings: 8 (P0: 0, P1: 3, P2: 4, P3: 1).
Dropped findings before posting: 0. High-severity findings: 3.
Risk Taxonomy
- CI/build: 2
- Flaky test risk: 1
- Release regression: 1
- Runtime correctness: 1
- Security boundary: 3
Validation and Proof
2 required validation/proof recommendation(s) selected from changed files.
- required: TypeScript/web build or CI proof - Runtime TypeScript/web files or package/config files changed. Proof: npm run build; typecheck; focused Vitest; green GitHub check.
- required: CI/release smoke proof - CI, release, launchd, or package metadata changed. Proof: green GitHub check; release-status; coverage-audit; rollback note.
Proof status: missing - 1 required validation/proof recommendation(s) missing from PR metadata.
Profile validation hints: Call out evidence leakage, replay/collision risks, duplicate side effects, and brittle sanitizer logic.
Profile proof expectations: Look for focused sanitizer, signature, orchestration, or fixture proof.
Related Context
Related issues/PRs: #801.
Suggested labels: bug, docs, tests.
Suggested reviewers: none from current metadata.
Review Settings Preview
- Profile: assertive
- Enabled sections: Review summary (inline_review); Walkthrough (inline_review); Changed-files table (walkthrough); Effort estimate (walkthrough); Related issues/PRs (walkthrough); Suggested labels (suggestion_only); Review status comment (sticky_status)
- Path instructions: none
- Label suggestions: orchestration, security, regression-hardening
- Reviewer suggestions: none
- Suggestion behavior: suggestions only; labels and reviewers are not auto-applied.
- Roadmap-only settings: auto-apply labels; auto-request reviewers; required status checks
Pre-merge checklist
- Inline comments target current RIGHT-side diff lines.
- No secret-like content survived into posted inline comments.
- REQUEST_CHANGES is only used when eligible P0/P1 findings survive validation.
- Required behavior proof is present or not applicable.
- Labels and reviewers are suggestions only; the bot did not auto-apply them.
Superseded by targeted fixes at 578991b, terminal dispositions on all review threads, green exact-head CI/CodeQL, and completed current-head semantic review.
1 participant
Outcome
Routine LCO releases can publish both npm package identities through npm Trusted Publishing without storing a write token or asking for two physical npm authentications.
Related: #801
Changes
publish-npm.ymlworkflownpm-releasepublish jobmain, exact-SHA CI and CodeQL success, the full repository check, package/Hermes smoke, unused registry slots, and dry-run publication before OIDC is requestedValidation
node --test --import tsx tests/npm-trusted-publishing.test.ts— 2/2 passactionlint .github/workflows/publish-npm.yml— passnode --check scripts/prepare-dual-npm-release.mjs— passnpm run check— 1,287/1,287 passThe integration test creates both real 1.7 tarballs, verifies payload parity and checksums, and proves a modified artifact is rejected.
Safety and proof boundary
npm-releaseenvironment before the automation is ready.Agent authorship
This PR was authored by Codex under the user-approved LCO release-maintainer scope.
Summary by CodeRabbit
New Features
Documentation
Tests
CI / Chores