Skip to content

fix(release): parse npm config output - #280

Merged
kattsushi merged 1 commit into
masterfrom
fix/stable-npm-config-output
Aug 31, 2026
Merged

kattsushi merged 1 commit into
masterfrom
fix/stable-npm-config-output

Conversation

@kattsushi

@kattsushi kattsushi commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Linked issue

Closes #268

Type

  • Bug fix

Summary

  • Parse npm 11 npm config get output as bounded plain text instead of assuming --json returns JSON.
  • Preserve exact npmjs registry pinning and fail-closed user/global npm auth inspection.
  • Add realistic hosted npm output and malformed-output regression coverage.

Failure evidence

Stable FINALIZE run 33446156507 stopped before publication with effective npm registry returned malformed JSON. No stable npm package was published by that run.

Test plan

  • 123 release tests passed
  • oxfmt check passed
  • oxlint: 0 errors
  • git diff check passed

Summary by CodeRabbit

  • Bug Fixes

    • Improved validation of npm configuration values during release finalization.
    • Added safeguards against malformed, ambiguous, oversized, multiline, control-character, and invalid text values.
    • Strengthened trusted publishing checks to reject unsafe configuration output.
  • Tests

    • Expanded coverage for raw npm configuration responses, including invalid UTF-8 and whitespace-padded values.
    • Verified configuration checks run without JSON formatting.

@kattsushi kattsushi added the type:bug Bug fix label Aug 31, 2026
@coderabbitai

coderabbitai Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Team

Run ID: ce5d2460-e159-4d1b-9015-cbb1dcd80e29

📥 Commits

Reviewing files that changed from the base of the PR and between 2e67507 and 8755582.

📒 Files selected for processing (2)
  • scripts/release-finalize-stable.mjs
  • scripts/release-finalize-stable.test.mjs

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The release finalizer now reads npm configuration as bounded raw UTF-8 instead of JSON. It validates registry and configuration paths, rejects malformed values, and preserves fail-closed behavior without mutations.

Changes

npm configuration validation

Layer / File(s) Summary
Raw configuration parser
scripts/release-finalize-stable.mjs
The command runner exposes raw stdout bytes. parseNpmConfigValue validates UTF-8, size, whitespace, control characters, and line structure.
Boundary integration and regression coverage
scripts/release-finalize-stable.mjs, scripts/release-finalize-stable.test.mjs
Configuration and registry lookups no longer use --json. Tests cover malformed values and verify that failures perform no mutations.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to 87555

This change makes the release finalizer correctly handle npm 11 configuration output while preserving strict registry and authentication safeguards; no actionable merge-blocking risk remains after normal checks and review.

Poem

A rabbit checks the npm stream,
Bytes stay bounded, clean, and keen.
Bad paths stop before they run,
No mutation mars the bun.
Plain-text guards the release gate.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The changes address npm configuration parsing, registry pinning, fail-closed authentication checks, and regression coverage. They do not implement the linked issue's required dynamic package and versi… Implement dynamic derivation of reviewed package and version targets, then verify that protected-master, tag, GitHub Release, npm publication, and fail-closed safeguards remain enforced. Add or reference regression coverage for these requir…
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: parsing npm configuration output in the release workflow.
Out of Scope Changes check ✅ Passed The production and test changes are limited to stable release npm configuration parsing and related fail-closed regression coverage. No unrelated changes are evident.
Full details: Linked Issues check

Explanation

The changes address npm configuration parsing, registry pinning, fail-closed authentication checks, and regression coverage. They do not implement the linked issue's required dynamic package and version targeting for stable releases, so the release objective remains incomplete [#268].

Resolution

Implement dynamic derivation of reviewed package and version targets, then verify that protected-master, tag, GitHub Release, npm publication, and fail-closed safeguards remain enforced. Add or reference regression coverage for these requirements [#268].

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/stable-npm-config-output

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@kattsushi
kattsushi merged commit 55cb6e5 into master Aug 31, 2026
7 checks passed
@kattsushi
kattsushi deleted the fix/stable-npm-config-output branch August 31, 2026 22:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type:bug Bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix: complete Effect v4 cleanup and restore stable releases

1 participant