Skip to content

fix(ci): scope pnpm hook to package packing - #475

Merged
Charles Hudson (phobetron) merged 1 commit into
mainfrom
fix/pack-scoped-pnpmfile
Sep 3, 2026
Merged

fix(ci): scope pnpm hook to package packing#475
Charles Hudson (phobetron) merged 1 commit into
mainfrom
fix/pack-scoped-pnpmfile

Conversation

@phobetron

Copy link
Copy Markdown
Collaborator

Summary

  • move the publishing-only pnpm hook out of the repository root and load it explicitly during pnpm pack
  • remove the obsolete pnpmfileChecksum so Renovate lockfile updates remain compatible with frozen installs
  • pin pnpm 11.25.0 and allow the Node SDK implementation's required esbuild step
  • update code ownership and Slack change classification for the relocated hook

Why

Renovate updates pnpm lockfiles with the pnpmfile disabled for safety. pnpm then removes the root hook checksum, causing every frozen install to fail before build, lint, or tests run.

Scoping the hook to package packing removes that install-time coupling while preserving the published package manifests. This is intended to land before #474 is rebased or retried.

Validation

  • CI=true pnpm install --frozen-lockfile
  • pnpm format:check
  • pnpm lint
  • pnpm build:pkgs
  • compared all nine package tarballs with the pre-change baseline: identical entries, non-manifest content, normalized manifests, and local aliases
  • confirmed packed manifests still omit scripts, devDependencies, pnpm, and packageManager
  • pnpm run implementation:run -- --all -- implementation:install
  • push hook: Node SDK implementation install, typecheck, and unit-test command
  • commit message validated with commitlint

Four full tarball hashes differ from the pnpm 11.15.1 baseline only in archive metadata under pnpm 11.25.0; all unpacked content and manifests are identical.

Move the publishing-only pnpm hook out of the repository root so dependency updates no longer invalidate the frozen lockfile. Load it explicitly during package packing, update related repository automation, and use the validated pnpm configuration for downstream installs.
@bito-code-review

bito-code-review Bot commented Sep 3, 2026

Copy link
Copy Markdown

Code Review Agent Run #490137

Actionable Suggestions - 0
Review Details
  • Files reviewed - 5 · Commit Range: 5bb85ea..5bb85ea
    • .github/workflows/notify-slack.yml
    • CODEOWNERS
    • implementations/node-sdk/pnpm-workspace.yaml
    • package.json
    • scripts/pack-pkgs.sh
  • Files skipped - 1
    • pnpm-lock.yaml - Reason: Filter setting
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

  • /review - Manually triggers an incremental AI Review.

  • /review full - Manually triggers a full AI Review.

  • /pause - Pauses automatic reviews on this pull request.

  • /resume - Resumes automatic reviews.

  • /resolve - Marks all Bito-posted review comments as resolved.

  • /abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Default Agent You can customize the agent settings here or contact your Bito workspace admin at jared.jolton@contentful.com.

Documentation & Help

AI Code Review powered by Bito Logo

@bito-code-review

Copy link
Copy Markdown

Impact Analysis by Bito

Cross-Repository Impact Analysis
What Changed Impact of Change Suggested Review Actions
The repository-root pnpm hook was renamed and moved to scripts/pack-pnpmfile.cjs, and package packing now loads it explicitly. - No cross-repo consumers found for .pnpmfile.cjs: The organization-indexed search returned no references to the old hook path. The old root hook is nevertheless a local configuration contract for any automation invoking pnpm from this repository. - Verify that every package-packing path uses scripts/pack-pnpmfile.cjs explicitly.
- Run a clean root install with pnpm 11.25.0 and confirm the publishing hook is not applied during normal dependency resolution.
- Run pnpm build:pkgs and inspect generated tarballs to ensure devDependencies and scripts are still removed.
The pnpm-lock.yaml pnpmfileChecksum entry was removed because the hook is no longer loaded as a root-level lockfile hook. - No cross-repo consumers found for pnpmfileChecksum: The organization-indexed search found no other repository referencing this lockfile metadata field. - Run pnpm install --frozen-lockfile using the pinned pnpm 11.25.0 version.
- Confirm that ordinary installs no longer depend on the package-packing hook and that the lockfile remains reproducible.
The packageManager pin changed from pnpm 11.15.1 to pnpm 11.25.0, including a new integrity hash. - ⚠️ unverified — create-contentful-app/packages/contentful--create-contentful-app/src/index.ts:133 CONSUMES optimization package-manager metadata by the same packageManager convention: The indexed search found packageManager handling in another repository, but the available result does not establish that it consumes contentful/optimization's package.json specifically. Treat this as a convention-level verification item rather than a confirmed dependency.
- ⚠️ unverified — webhooks-stack/package.json:10 CONSUMES the same packageManager configuration convention: The repository uses a pinned pnpm packageManager field, but no direct dependency edge to optimization was returned and the consumer call-site was not opened.
- Run the complete unit, typecheck, package-build, and implementation-install workflows under pnpm 11.25.0.
- Validate Corepack activation and frozen-lockfile installs in CI and in the node-sdk implementation.
- Confirm the updated integrity hash corresponds to the exact pnpm 11.25.0 distribution used by CI.
The node-sdk workspace gained allowBuilds.esbuild: true. - intra-repo: implementations/node-sdk dependency installation and native/binary build setup: This permits esbuild's install/build script only for the node-sdk workspace, changing installation behavior for local package and end-to-end workflows. - Run pnpm --dir implementations/node-sdk install --frozen-lockfile and verify esbuild is installed successfully.
- Run the node-sdk build, typecheck, and E2E setup paths that depend on esbuild.
- Confirm that no unrelated dependency install scripts are enabled by this workspace configuration.
Code Paths Analyzed

Impact:
The change scopes a publishing-only pnpm hook to package packing, removes its root lockfile checksum, updates repository ownership and CI change classification, upgrades pnpm from 11.15.1 to 11.25.0, and explicitly permits esbuild build scripts in the node-sdk workspace.

Flow:
Normal repository installs use the standard pnpm configuration without the publishing hook. Package generation flows enter scripts/pack-pkgs.sh, enumerate npm targets, invoke pnpm pack with --config.pnpmfile=./scripts/pack-pnpmfile.cjs, and create local tarball aliases under pkgs/. The node-sdk implementation uses its own workspace configuration, including the esbuild build approval, when installing and consuming those tarballs.

Direct Changes (Diff Files):
• .github/workflows/notify-slack.yml [5-10] — Updates global-configuration detection so changes to scripts/pack-pnpmfile.cjs are treated like changes to the former root pnpm hook.
• CODEOWNERS [19-27] — Moves package-manager configuration ownership from /.pnpmfile.cjs to /scripts/pack-pnpmfile.cjs.
• implementations/node-sdk/pnpm-workspace.yaml [31-39] — Adds allowBuilds.esbuild: true to permit esbuild's installation/build script in the node-sdk workspace.
• package.json [45-50] — Changes the packageManager pin from pnpm 11.15.1 to pnpm 11.25.0.
• pnpm-lock.yaml [57-65] — Removes pnpmfileChecksum metadata associated with the former root pnpm hook.
• scripts/pack-pkgs.sh [71-78] — Explicitly passes scripts/pack-pnpmfile.cjs to pnpm pack.
• .pnpmfile.cjs => scripts/pack-pnpmfile.cjs [1-82] — Renames and relocates the beforePacking hook that removes devDependencies and scripts from published package manifests.

Repository Impact:
Package publishing and local tarball generation: The packing hook is no longer globally active and is now applied only by scripts/pack-pkgs.sh.
Root dependency installation and lockfile validation: Removing pnpmfileChecksum and the root hook prevents publishing-only manifest mutation from affecting ordinary installs and lockfile updates.
Node SDK implementation setup: The node-sdk workspace now explicitly allows esbuild's build script, which can affect binary availability and build reproducibility.
CI notification and ownership automation: The Slack workflow and CODEOWNERS must recognize the relocated hook path.

Cross-Repository Dependencies:
Organization repositories using pinned pnpm packageManager fields: The version change is a repository-local runtime/tooling change, but the indexed organization search shows packageManager is a shared bootstrap convention.
webhooks-stack workspace configuration: The indexed repository also uses pnpm allowBuilds configuration, indicating related workspace installation policy patterns, but no direct dependency on optimization was confirmed.

Database/Caching Impact:
• None

API Contract Violations:
• No HTTP, gRPC, event, database, or public code API contracts are changed.
• The root pnpm hook path is a configuration contract for automation and is removed in favor of scripts/pack-pnpmfile.cjs.
• The packageManager field changes the required package-manager runtime from pnpm 11.15.1 to pnpm 11.25.0.

Infrastructure Dependencies:
• CI jobs and local automation must use pnpm 11.25.0 with the updated integrity hash.
• The Slack notification workflow must continue classifying scripts/pack-pnpmfile.cjs as global configuration.
• Package packing depends on the relative path ./scripts/pack-pnpmfile.cjs after scripts/pack-pkgs.sh changes directory to the repository root.
• The node-sdk workspace depends on pnpm support for allowBuilds.esbuild.

Additional Insights:
Behavioral isolation of the publishing hook: The hook deletes devDependencies and scripts from packed manifests; explicit loading reduces the risk that normal installs or lockfile updates are affected.
Reproducibility: The pnpm version and integrity hash are pinned, but the upgrade should be validated across CI, package packing, and implementation installation paths.

Testing Recommendations

Frontend Impact:
None.

Service Integration:
• Run pnpm build:pkgs and verify every generated package tarball can be installed by the node-sdk implementation.
• Run the node-sdk implementation install, build, typecheck, and relevant E2E setup commands after enabling allowBuilds.esbuild.
• Exercise the CI notification workflow with changes to scripts/pack-pnpmfile.cjs and confirm they are classified as global configuration changes.

Data Serialization:
• Inspect packed package.json files and verify devDependencies and scripts are removed by scripts/pack-pnpmfile.cjs.
• Run pnpm install --frozen-lockfile with pnpm 11.25.0 and confirm pnpm-lock.yaml is accepted without pnpmfileChecksum.

Privacy Compliance:
None.

Backward Compatibility:
• Verify normal root pnpm install behavior does not apply the publishing hook, while pnpm pack through scripts/pack-pkgs.sh still applies it.
• Check automation and documentation for stale references to .pnpmfile.cjs and update or fail fast on any remaining references.
• Validate Corepack and CI setup with the exact pnpm 11.25.0 integrity-pinned packageManager value.

OAuth Functionality:
• None

Reliability Testing:
• None

Additional Insights:
• Run the full unit and typecheck suites under pnpm 11.25.0.
• Test a clean checkout where node_modules and pkgs/ are absent, then run pnpm build:pkgs to validate the complete packing flow.
• Confirm esbuild is actually executable after node-sdk installation and that no unrelated pnpm build scripts are unintentionally permitted.
• Analysis based on known dependency patterns. Actual impact may vary.

Analysis based on known dependency patterns and edges. Actual impact may vary.

@bito-code-review

Copy link
Copy Markdown

✅ Review Settings Overridden

Status: Overridden Successfully

Guidelines:

  • Accepted:

    • General : Review Posture, Repo Truth And Boundaries, Domain Invariants

    • Language : typescript= TypeScript Review Evidence

Note: Extra guidelines beyond 3 general purpose guidelines and 1 language specific guideline per language are not processed. Guidelines are fetched from the source branch.

@phobetron
Charles Hudson (phobetron) merged commit e1cb410 into main Sep 3, 2026
42 checks passed
@phobetron
Charles Hudson (phobetron) deleted the fix/pack-scoped-pnpmfile branch September 3, 2026 11:11
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.

1 participant