Skip to content

chore: pnpm-only, one lockfile — and the drift test was right about gpt-5.6 - #34

Merged
pa-arth merged 2 commits into
mainfrom
chore/migrate-to-pnpm
Aug 24, 2026
Merged

chore: pnpm-only, one lockfile — and the drift test was right about gpt-5.6#34
pa-arth merged 2 commits into
mainfrom
chore/migrate-to-pnpm

Conversation

@pa-arth

@pa-arth pa-arth commented Aug 24, 2026

Copy link
Copy Markdown
Owner

Why

Two lockfiles were committed and they had drifted:

package package-lock.json (what CI installed) pnpm-lock.yaml (what nobody installed)
oxlint 1.71.0 1.78.0
@clack/prompts 1.6.0 1.7.0
vitest 4.1.9 4.1.10
@types/node 22.20.0 22.20.1

CI ran npm ci. A second lockfile is a second, untested dependency set.

What changed

  • package-lock.json deleted and gitignored with yarn.lock.
  • packageManager: pnpm@10.12.1 — corepack resolves it. Without this pin, a stale pnpm 8.14.0 earlier in this machine's PATH silently rewrote the 9.0 lockfile on a plain pnpm install. That happened while writing this PR.
  • CI + publish install with pnpm install --frozen-lockfile, cache: pnpm, pnpm/action-setup@v4.
  • preinstall: node scripts/only-pnpm.mjs rejects npm/yarn/bun. Local on purpose — npx --yes only-allow pnpm runs an unpinned registry package with installer privileges on every install.
  • build-npm.mjs / bundle.mjs re-enter the invoking package manager via npm_execpath instead of spawning a bare pnpm.

What deliberately stays npm

npm publish ./bundles/npm (setup-node wires registry-url + NODE_AUTH_TOKEN for npm) and the npx @promptster/cc-audit / npm i -g instructions in the README. Consumers are unaffected: build-npm.mjs writes a fresh package.json for the published artifact with no scripts key at all — verified on the generated output — so the pnpm guard cannot leak into anyone's install.

Verification (corepack pnpm@10.12.1)

Check Result
pnpm install --frozen-lockfile clean — the committed pnpm lockfile is in sync
pnpm run lint clean
pnpm run typecheck clean
pnpm run build clean
pnpm run build:npm + node bundles/npm/cc-audit.mjs --help clean
real npm install, clean tree rejected, exit 1
pnpm test 436 passed (42 files) — see below

The failing test was real, and the mirror was wrong — now fixed

pricingDrift.test.ts"OpenAI rates agree with LiteLLM" failed on the first push. It failed identically on main under npm, so it was not caused by the migration — but it was not noise either. Following the table's own instruction (find out who is right, do not update the expectation), checked against two independent sources:

source gpt-5.6 / gpt-5.6-sol
OpenAI, developers.openai.com/api/docs/pricing $4.00 / $0.40 / $20.00
LiteLLM (what the drift test reads) 4 / 0.4 / 20
this repo's vendored mirror 5 / 0.5 / 30

We were 25% over on input and 50% over on output. OpenAI repriced on 2026-08-22; promptster-backend corrected config-cost the same day (#780, d2357e89); this hand-copied mirror has no subscriber to that repo and carried the old tier for two days. Published 0.9.0 has the wrong number.

Fixed by hand, not by sync-pricing.mjs --force — the sync guard fired, because the mirror carries a deliberate local comment recording this exact failure mode, and a force would have deleted it plus dragged in a month of unrelated upstream evolution (Anthropic affix regex, subsetViolated, a cacheWrite field this file's 3-field type does not have).

Then checked every other row, because the drift test is blind to models LiteLLM lacks (gpt-5.5-codex, gpt-5.4-codex, gpt-5.3-codex-spark) — compared the whole table against upstream key by key. Before: 6 differences, all gpt-5.6/sol. After: 0 differences across all 29 OpenAI and all 16 Anthropic keys.

pricingPinned.test.ts gains the new rows. The existing pin named terra and luna by key, so it structurally could not see the same repricing land one model over — which is exactly what happened. The new case also asserts gpt-5.6 ≠ gpt-5.5's input, since "Sol matches the GPT-5.5 tier" is the sentence that produced the stale number.

Not addressed here: upstream carries a cacheWrite rate for OpenAI models and this mirror has no such field, so cc-audit does not price Codex cache writes at all. That is a feature port, not a number.

Not run locally

pnpm run bundle needs bun, which isn't installed here. It got the same npm_execpath edit as build-npm.mjs (which was run), and CI exercises it on tag push.

After merge

Existing checkouts need one rm -rf node_modules && pnpm install.

🤖 Generated with Claude Code

This repo tracked BOTH `package-lock.json` and `pnpm-lock.yaml`, and they had
drifted — oxlint 1.71.0 vs 1.78.0, @clack/prompts 1.6.0 vs 1.7.0, vitest 4.1.9
vs 4.1.10, @types/node 22.20.0 vs 22.20.1. CI ran `npm ci`, so the pnpm set was
never once exercised. A second lockfile is a second, untested dependency set.

pnpm becomes the only one. `package-lock.json` deleted and gitignored alongside
`yarn.lock`; CI and publish install with `pnpm install --frozen-lockfile`;
`packageManager: pnpm@10.12.1` pins the version so corepack resolves it.

Two things deliberately stay npm, because they are about the PUBLISHED artifact
rather than this repo's own installs:
  - `npm publish ./bundles/npm` — setup-node wires registry-url and
    NODE_AUTH_TOKEN for npm, and the artifact is generated, dependency-free.
  - `npx @promptster/cc-audit` / `npm i -g` in the README. Verified the guard
    cannot leak to consumers: `build-npm.mjs` writes a fresh package.json with
    no `scripts` key at all (checked the generated output).

`preinstall: node scripts/only-pnpm.mjs` rejects npm/yarn/bun. Local on purpose
— the usual `npx --yes only-allow pnpm` executes an unpinned registry package
with installer privileges on every install. Caveat stated plainly: npm extracts
before running the root preinstall, so the guard fails the install after
node_modules is written, not before. `only-allow` is identical in this respect.

`build-npm.mjs` and `bundle.mjs` re-enter the invoking package manager via
`npm_execpath` instead of spawning a bare `pnpm`. Not cosmetic — a bare `pnpm`
under corepack fails outright with "pnpm does not switch versions when running
under corepack", which is how `pnpm run build:npm` broke the first time.

Verified under corepack pnpm@10.12.1: `install --frozen-lockfile` clean (the
committed pnpm lockfile IS in sync), lint clean, typecheck clean, build clean,
`build:npm` + `node bundles/npm/cc-audit.mjs --help` both good, real `npm
install` rejected with exit 1.

NOT clean, and NOT caused by this change: `src/__tests__/pricingDrift.test.ts`
> "OpenAI rates agree with LiteLLM" fails. It fails identically on `main` under
npm — same assertion, same three gpt-5.x-codex models LiteLLM has no entry for.
It is a live-network drift check against a hand-copied pricing mirror, so it
went red on its own after the last green CI run (2026-08-18). Needs its own fix;
it is not a pnpm regression.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@greptile-apps

greptile-apps Bot commented Aug 24, 2026

Copy link
Copy Markdown

Greptile Summary

This PR standardizes repository development and release automation on pinned pnpm, removes the competing npm lockfile, and adds a local package-manager guard.

  • Updates CI and publishing workflows to install and run through pnpm.
  • Re-enters the invoking package manager when creating npm and standalone artifacts.
  • Updates contributor and maintainer documentation for the pnpm-only workflow.

Confidence Score: 4/5

The PR appears safe to merge after considering two non-blocking hardening issues in the release action pin and package-manager re-entry helper.

The pnpm workflow ordering and lockfile configuration are internally consistent, while immutable action pinning and support for native pnpm executables would make the release path more robust.

Files Needing Attention: .github/workflows/publish.yml, scripts/build-npm.mjs, scripts/bundle.mjs

Security Review

The newly added pnpm setup action uses a mutable major tag in the privileged publish job. Pinning it to a reviewed commit would prevent upstream tag movement from changing release-time code.

Important Files Changed

Filename Overview
.github/workflows/ci.yml Migrates the Node matrix checks and artifact smoke test from npm to a pinned pnpm installation; the added setup action remains mutable.
.github/workflows/publish.yml Migrates release validation and builds to pnpm, but introduces a mutable third-party action into the credentialed publishing job.
package.json Pins pnpm 10.12.1 and adds a preinstall guard while retaining the existing build command structure.
scripts/build-npm.mjs Re-enters the invoking package manager for compilation and esbuild, but assumes npm_execpath always names JavaScript executable through Node.
scripts/bundle.mjs Applies the same package-manager re-entry strategy to standalone builds and shares the native-executable compatibility concern.
scripts/only-pnpm.mjs Rejects known non-pnpm lifecycle invocations while deliberately allowing direct or unidentified execution.

Fix all with Greploop Fix All in Claude Code Fix All in Cursor

Reviews (1): Last reviewed commit: "chore(pnpm): two committed lockfiles, an..." | Re-trigger Greptile


- name: Setup pnpm
# Version pinned by package.json `packageManager`.
uses: pnpm/action-setup@v4

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 security Mutable action enters release path

pnpm/action-setup@v4 is a mutable third-party reference executed in the release job with contents: write before the job uses npm and GitHub credentials. Pinning the action to a reviewed commit prevents upstream tag movement from introducing unreviewed code into package and release publishing.

How this was verified: The added action runs in the same publish job that grants write permission and later receives NPM_TOKEN and GITHUB_TOKEN.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Fix in Claude Code Fix in Cursor

Comment thread scripts/build-npm.mjs
Comment on lines +41 to +44
function pm(args) {
const execpath = process.env.npm_execpath;
if (execpath) run(process.execPath, [execpath, ...args]);
else run('pnpm', args);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Re-entry assumes JavaScript executable

When pnpm supplies a standalone native executable through npm_execpath, pm() passes that binary to Node as though it were JavaScript, preventing build:npm from producing the npm artifact. Re-enter the executable in a way that supports both JavaScript entry points and native package-manager binaries; the same helper in scripts/bundle.mjs has this concern.

Knowledge Base Used: Release, packaging, and update maintenance

Fix in Claude Code Fix in Cursor

…rror did not

The red `pricingDrift` run on #34 was real, and the mirror was the wrong one.
Following this file's own instruction — find out who is right, do not update the
expectation — checked against BOTH independent sources:

  OpenAI, developers.openai.com/api/docs/pricing:  Sol $4.00 / $0.40 / $20.00
  LiteLLM, what pricingDrift reads:                     4    /  0.4  /  20
  this table:                                           5    /  0.5  /  30

So we were 25% over on input and 50% over on output for gpt-5.6 and
gpt-5.6-sol. OpenAI repriced them on 2026-08-22; promptster-backend corrected
config-cost the same day (#780, d2357e89); this hand-copied mirror has no
subscriber to that repo and carried the old tier for two days.

Fixed by hand rather than by `scripts/sync-pricing.mjs --force`, deliberately.
The sync guard fired — the mirror HAS been hand-edited since the last sync
(2026-07-24) and the local edit is the drift-provenance comment block that
records exactly this failure mode. A --force would have deleted it and dragged
in a month of unrelated upstream evolution (the Anthropic affix regex,
subsetViolated, a cacheWrite field this file's 3-field OpenAIModelPricing does
not have). Six values were wrong; six values changed.

Checked every OTHER row rather than trusting the drift test, which cannot see
models LiteLLM lacks (gpt-5.5-codex, gpt-5.4-codex, gpt-5.3-codex-spark):
compared this table against upstream config-cost key by key. Before: 6
differences, all gpt-5.6/sol. After: 0 differences across all 29 OpenAI and all
16 Anthropic keys.

`pricingPinned.test.ts` gets the new rows. The existing pin covered terra and
luna BY NAME, so it could not see the same repricing happen one model over —
which is what happened. The new case also asserts gpt-5.6 is not equal to
gpt-5.5's input, because "Sol matches the GPT-5.5 tier" is the sentence that
produced the stale number in the first place.

`pricingCoverage.test.ts` pinned 5 as a literal; now 4, plus an explicit
assertion that it does not equal gpt-5's rate, which is the non-inheritance
property that line actually exists to test.

Verified under corepack pnpm@10.12.1: test 436/436 (42 files), including
pricingDrift against live LiteLLM — the failure this PR documented as
pre-existing is now gone. lint, typecheck, build, build:npm and the bundle
smoke test all exit 0.

Not addressed here: upstream carries a `cacheWrite` rate for OpenAI models and
this mirror has no such field, so cc-audit does not price Codex cache writes at
all. That is a feature port, not a number.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@pa-arth pa-arth changed the title chore: pnpm-only — one lockfile, and CI stops testing a set nobody installs chore: pnpm-only, one lockfile — and the drift test was right about gpt-5.6 Aug 24, 2026
@pa-arth
pa-arth merged commit 2b17f57 into main Aug 24, 2026
2 checks passed
@pa-arth
pa-arth deleted the chore/migrate-to-pnpm branch August 27, 2026 23:44
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