Skip to content

Commit c77a8fe

Browse files
fix(scripts): read the last JSON document on pnpm pack --json stdout (#22753)
Fixes #22752 Clause-②: no ## What changed `scripts/publish-smoke-pack.mjs` `packOne` parsed the WHOLE stdout of `pnpm pack --json --pack-destination DEST`. Since `@objectstack/spec` declared `prepare: pnpm gen:migration-registry`, pnpm prints that lifecycle banner and the generator's output above the JSON document. The parse therefore threw on every pack of the full population, so the scaffold smoke (`os-create-smoke.yml`) and the release smoke (`scripts/publish-smoke.sh`) both died at their first pack step. The parse moves into an exported pure helper, `parsePackStdout(stdout, name)`, and `packOne` now calls it. The helper returns the LAST JSON document on stdout: it walks the `{` offsets from the end and takes the first suffix that `JSON.parse` accepts whole. These stay unchanged: the `filename` check, the set of packages packed, the overrides map, `assertPinSetTotal`, and everything `publish-smoke.sh` / `create-scaffold-smoke.sh` judge. The spec's `prepare` is untouched. The diff is one file, +120 / -26. ## Measurements (pnpm 10.31.0, worktree at `bf515e724`) **1. The banner precedes the document (confirmed).** Here is the stdout head of `pnpm pack --json --pack-destination D` in `packages/spec`, verbatim except the worktree path, written WT: ```text > @objectstack/spec@17.7.0 prepare WT/packages/spec > pnpm gen:migration-registry > @objectstack/spec@17.7.0 gen:migration-registry WT/packages/spec > tsx scripts/build-migration-registry.ts build-migration-registry --self-test: ok ✓ src/migrations/registry.ts is current (420 semantic, 258 retired-key, 222 retired-def) { "name": "@objectstack/spec", "version": "17.7.0", ``` - The pack exits 0 with empty stderr and 974 stdout lines. The document starts on line 11. - Parsing that stdout whole throws `Unexpected token` at the first banner character, which is the base behaviour. - `parsePackStdout` on the same bytes returns the tarball filename. **2. The document is always last.** - pnpm 10.31.0's own pack handler (`dist/pnpm.cjs`) runs `prepack` and `prepare` before the tarball and `postpack` right after it. All three run inside the pack API with inherited stdio. The CLI writes the returned JSON string only after the handler returns, and its update notifier runs for `install`/`add` only. - I measured this on a throwaway package that declares all three hooks. Every banner and echo line (PREPACK, PREPARE, POSTPACK) lands above the document, and nothing lands after it. - Census of all 81 workspace members: no `prepack` or `postpack` anywhere. `prepare` appears only on `@objectstack/spec` (publishable) and on the private root. `prepublishOnly` appears on `@objectstack/console`, and pack does not run it. - A second measurement shaped the selector. A lifecycle script whose last write has no newline glues onto the document's opening brace. For example, `printf "{ not json"` as `postpack` leaves a line reading `{ not json{`. A line-start selector would refuse that valid pack, and the per-brace scan reads it. **3. No flag keeps lifecycle output off stdout.** - I tried `--silent`, `--reporter=silent`, `--loglevel=error`, `--loglevel=silent`, `--reporter=ndjson` and `--reporter=append-only`, each before and after `pack`. In every case the banner and the script output stay on stdout. Lifecycle stdio is inherited, so no reporter setting reaches it. - pnpm 10.31.0's pack refuses `--ignore-scripts` outright: `ERROR Unknown option: 'ignore-scripts'`, exit 1. - The env spelling `npm_config_ignore_scripts=true` is accepted, but it skips `prepare`, which `gen:migration-registry` needs (its output, `src/migrations/registry.ts`, is gitignored). It was not measured equal, so it is not used. ## Why the last document rather than the document carrying `filename` pnpm writes its document strictly last (measurement 2), so output after it is a shape nobody has observed. The last-document selector keeps that case loud: no suffix parses, and the named error prints stdout. A search keyed on `filename` would skip such output silently. The `filename` check still runs on the selected document. ## Self-test The battery `publish-smoke-pack self-test` grows from 4 to 10 cases, and its pin is raised to 10. | # | case | expected | |---|---|---| | 5 | banner (the measured spec head), then the document | filename read | | 6 | the document alone (the shape before `prepare` existed) | read | | 7 | script output glued to the opening brace | read | | 8 | output AFTER the document | throws, names the package, carries stdout | | 9 | stdout with no JSON document | throws, names the package, carries stdout | | 10 | `packOne` itself against a fake `pnpm` | the fake's document is read | The fake `pnpm` in case 10 is a `#!/bin/sh` script in a `mkdtemp` dir, prepended to PATH. It prints the banner, then a document whose `filename` echoes its `--pack-destination` argument, so the filename proves that the fake answered, with packOne's argv. `selfTest()` is now async, because case 10 awaits `packOne`. The handshake keeps its kind: the dispatch awaits the self-test and still refuses anything but `SELF_TEST_VERDICT`. **How case 10 fits the header's "pure fixtures, no workspace, no network".** It spawns one process: a fake pnpm that the self-test writes into an OS temp dir and removes in a `finally`, where PATH is restored too. It reads no workspace, no network and no tracked file, and it never runs the real pnpm. The `no-path-population` declaration therefore still holds. The header and the marker's reason now say "fixtures plus one fake pnpm in a temp dir" instead of "in-memory fixtures". ## Ablation (fix committed first, at `34fff6b49`) The mutation makes the helper parse stdout whole: ```text node scripts/ablation-replace.mjs --file REPO/scripts/publish-smoke-pack.mjs \ --anchor 'parsed = JSON.parse(stdout.slice(at));' --replacement 'parsed = JSON.parse(stdout);' \ -- node REPO/scripts/publish-smoke-pack.mjs --self-test ablation-replace: ok mutation landed: anchor 1 -> 0, blob 8ef4ed2 -> a4e5fc75a5b6 FAIL — a lifecycle banner above the document is skipped FAIL — script output glued to the opening brace is skipped FAIL — packOne reads the document past a fake pnpm that prints a banner first SELF-TEST FAILED ablation-replace: command exited 1 ablation-replace: ok restored: blob == HEAD (8ef4ed2) and `git diff HEAD` is empty ``` The other seven cases stay ok under the mutation. The re-run after the restore prints `SELF-TEST PASSED (10 cases)`. ## Real pnpm - `node scripts/publish-smoke-pack.mjs D` is the scaffold smoke's step 1. I ran it with real pnpm 10.31.0 over the full population, under the shared verify lock. It printed `Packing 69 publishable package(s)`, then `packed 69/69`, then `Wrote 69 override(s)`, with `VERDICT command-exit 0`. At base this step throws on `@objectstack/spec` (the card's CI run). - `bash scripts/create-scaffold-smoke.sh` was **not run locally**. It refuses without `packages/cli/dist`, and it needs a full monorepo build plus registry installs of every template. That does not fit the container's ~10-minute foreground cap on a shared box. The end-to-end proof is this PR's own `os-create-smoke` run, whose path filter names this file. ## Gates (all at `34fff6b49`) - `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` took the change set from git (1 path) and derived 31 commands. All 31 ran and exited 0. `--ran` reconciles them: `✓ dispatch-gates --ran: 31 derived famil(ies) accounted for — 31 run, 0 NOT-MEASURED`. - `pnpm check:publish-smoke-pin` prints `SELF-TEST PASSED (10 cases)`. - `check-self-test-wired` and `check-self-test-workflow-commands` exit 0, and so do their self-tests. - `pnpm check:lockstep-package-count` prints `all 3 occurrence(s) agree with the true count (69)`. The docblock count line is untouched. - `pnpm check:pm-dispatch-gates` prints `1828 cases pass`. - **ESLint, narrowed to this file, with the three proofs:** - `eslint --print-config` puts this file in scope with 2 rules: `no-restricted-imports` and `comment-swallow/no-code-inside-block-comment`. - `eslint --no-inline-config --format json scripts/publish-smoke-pack.mjs` reports 1 file, 0 errors, 0 warnings. - Invariance: the config enables no type-aware linting (no `parserOptions.project` / `projectService`, as `eslint.config.mjs` states near line 327), and both rules are per-file. This diff cannot move any untouched file's verdict. - Repo-wide `pnpm lint` belongs to CI. No changeset: root `scripts/` ships in no package's `files[]`. The PR carries the `skip-changeset` label. ## Acceptance notes - The refusal text changed from "returned non-JSON" to "printed no JSON document ending its stdout". No script or workflow parses it: grep finds zero hits. - `listPublicPackages` also parses `pnpm -r list --depth -1 --json` whole. pnpm list runs no lifecycle script, and its stdout parsed cleanly in this worktree (81 members, 69 publishable). It is not touched. - PR #22691 edits this file's docblock count line (69 → 70), which this diff does not touch. Whichever PR lands later merges main. Its red `os-create-smoke` is this defect. --- _Generated by [Claude Code](https://claude.ai/code/session_01VQftDB6niKg91LeQANrS7d)_ Co-authored-by: Claude <noreply@anthropic.com>
1 parent 9859e4e commit c77a8fe

1 file changed

Lines changed: 120 additions & 26 deletions

File tree

‎scripts/publish-smoke-pack.mjs‎

Lines changed: 120 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -45,16 +45,18 @@
4545
// `main()` shells out to `pnpm -r list` and `pnpm pack`; it opens no path this
4646
// file names, and it runs only from `scripts/publish-smoke.sh` during a release.
4747
// The family CI schedules on a pull request is `check:publish-smoke-pin`, which
48-
// is this file's `--self-test`: pure fixtures, no workspace, no network, no
49-
// tracked file. There is no path population for a card to implicate, so the
48+
// is this file's `--self-test`: pure fixtures plus one fake `pnpm` it writes to
49+
// a temp dir -- no workspace, no network, no tracked file, no real pnpm. There
50+
// is no path population for a card to implicate, so the
5051
// marker is the honest declaration -- and `packages/**` would be the costly
5152
// error, naming this family on thousands of cards for reads it never performs.
5253
//
53-
// dispatch-gates: no-path-population -- check:publish-smoke-pin runs this packer's --self-test against in-memory fixtures; the workspace enumeration and the pack itself belong to the release smoke, which no pull request schedules
54+
// dispatch-gates: no-path-population -- check:publish-smoke-pin runs this packer's --self-test against in-memory fixtures and a temp-dir fake pnpm; the workspace enumeration and the pack itself belong to the release smoke, which no pull request schedules
5455

5556
import { execFile } from 'node:child_process';
56-
import { mkdirSync, writeFileSync } from 'node:fs';
57-
import { resolve } from 'node:path';
57+
import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs';
58+
import { tmpdir } from 'node:os';
59+
import { delimiter, join, resolve } from 'node:path';
5860
import { promisify } from 'node:util';
5961

6062
import { isEntrypoint } from './invoked-as.mjs';
@@ -118,22 +120,47 @@ async function listPublicPackages(repoRoot) {
118120
return selectPublishable(JSON.parse(stdout));
119121
}
120122

123+
/**
124+
* The pack result is the LAST JSON document on `pnpm pack --json` stdout, never
125+
* the whole stdout. pnpm (10.31.0, measured) runs `prepack` and `prepare` before
126+
* the tarball and `postpack` after it, all with inherited stdio, and writes its
127+
* document only once the command returns, so every lifecycle banner and line of
128+
* script output lands ABOVE the document (`@objectstack/spec`'s `prepare` does,
129+
* on every pack). The scan is per brace, not per line, because a script whose
130+
* last write has no newline glues to the document's opening brace. Output AFTER
131+
* the document leaves no suffix that parses, so it reddens below with stdout
132+
* attached rather than being skipped: absence stays loud.
133+
*
134+
* @param {string} stdout everything `pnpm pack --json` printed on stdout
135+
* @param {string} name the package being packed, for the diagnostic
136+
* @returns {{name: string, filename: string}}
137+
*/
138+
export function parsePackStdout(stdout, name) {
139+
let parsed;
140+
for (let at = stdout.lastIndexOf('{'); at !== -1; at = at === 0 ? -1 : stdout.lastIndexOf('{', at - 1)) {
141+
try {
142+
parsed = JSON.parse(stdout.slice(at));
143+
break;
144+
} catch {
145+
// Not a document that runs to the end of stdout; try the brace before it.
146+
}
147+
}
148+
if (parsed === undefined) {
149+
throw new Error(`pnpm pack --json printed no JSON document ending its stdout for ${name}:\n${stdout}`);
150+
}
151+
if (!parsed.filename) {
152+
throw new Error(`pnpm pack reported no tarball filename for ${name}`);
153+
}
154+
return { name, filename: parsed.filename };
155+
}
156+
121157
async function packOne(pkg, destDir) {
122158
const { stdout } = await execFileP(
123159
'pnpm',
124160
['pack', '--json', '--pack-destination', destDir],
125161
{ cwd: pkg.path, maxBuffer: 64 * 1024 * 1024 },
126162
);
127-
let parsed;
128-
try {
129-
parsed = JSON.parse(stdout);
130-
} catch {
131-
throw new Error(`pnpm pack --json returned non-JSON for ${pkg.name}:\n${stdout}`);
132-
}
133-
if (!parsed.filename) {
134-
throw new Error(`pnpm pack reported no tarball filename for ${pkg.name}`);
135-
}
136-
return { name: pkg.name, filename: parsed.filename };
163+
return parsePackStdout(stdout, pkg.name);
137164
}
138165

139166
async function main() {
@@ -180,8 +207,11 @@ async function main() {
180207
}
181208

182209
/**
183-
* Self-test — runs without pnpm, a workspace, or a network. It pins the two
184-
* properties the release smoke depends on, and both are ABLATION-CHECKED
210+
* Self-test — runs without the real pnpm, a workspace, or a network. Cases 5-10
211+
* pin that the pack result is read from the LAST JSON document on stdout, the
212+
* last of them through `packOne` itself against a fake `pnpm` on PATH that
213+
* prints a lifecycle banner first; parsing stdout whole turns 5, 7 and 10 red.
214+
* The first four pin the two properties below, both ABLATION-CHECKED
185215
* (2026-08-23): restoring `EXCLUDE = new Set(['create-objectstack'])` and
186216
* filtering it out of `selectPublishable` turns case 1 red by name; deleting
187217
* the `missing`/`surplus` branch of `assertPinSetTotal` turns cases 2/3 red.
@@ -215,7 +245,7 @@ async function main() {
215245
// not red. A battery BELOW its floor means cases stopped running; the remedy is
216246
// to find what stopped registering.
217247
const SELF_TEST_BATTERIES = Object.freeze({
218-
'publish-smoke-pack self-test': 4,
248+
'publish-smoke-pack self-test': 10,
219249
});
220250

221251
// DELETING an entry silences that battery's floor exactly as effectively as
@@ -233,7 +263,7 @@ const UNATTRIBUTED_BATTERY = '(no battery open)';
233263
// as one that passed (#13798).
234264
const SELF_TEST_VERDICT = 'publish-smoke-pack self-test reached its verdict';
235265

236-
function selfTest() {
266+
async function selfTest() {
237267
// The battery ledger this self-test's floor is evaluated against (#13489).
238268
// `battery()` opens a battery; every assertion below is attributed to the one
239269
// most recently opened, so a section that stops running stops registering and
@@ -249,10 +279,10 @@ function selfTest() {
249279
};
250280
battery('publish-smoke-pack self-test');
251281
const cases = [];
252-
const check = (name, fn) => {
282+
const check = async (name, fn) => {
253283
registerCase();
254284
try {
255-
fn();
285+
await fn();
256286
cases.push(` ok — ${name}`);
257287
} catch (err) {
258288
cases.push(` FAIL — ${name}\n ${(err.message ?? String(err)).split('\n').join('\n ')}`);
@@ -263,7 +293,7 @@ function selfTest() {
263293
if (!cond) throw new Error(msg);
264294
};
265295

266-
check('an unscoped public package is in the derived set', () => {
296+
await check('an unscoped public package is in the derived set', () => {
267297
const picked = selectPublishable([
268298
{ name: '@objectstack/cli', private: false },
269299
{ name: 'create-objectstack' }, // no `private` key at all — the real manifest
@@ -281,11 +311,11 @@ function selfTest() {
281311
assert(picked.length === 2, `expected 2 publishable, got ${picked.length}`);
282312
});
283313

284-
check('set == publishable set is accepted', () => {
314+
await check('set == publishable set is accepted', () => {
285315
assertPinSetTotal(['create-objectstack', '@objectstack/cli'], ['@objectstack/cli', 'create-objectstack']);
286316
});
287317

288-
check('a MISSING member reddens, by name', () => {
318+
await check('a MISSING member reddens, by name', () => {
289319
let msg = '';
290320
try {
291321
assertPinSetTotal(['@objectstack/cli'], ['@objectstack/cli', 'create-objectstack']);
@@ -299,7 +329,7 @@ function selfTest() {
299329
);
300330
});
301331

302-
check('a SURPLUS member reddens, by name', () => {
332+
await check('a SURPLUS member reddens, by name', () => {
303333
let msg = '';
304334
try {
305335
assertPinSetTotal(['@objectstack/cli', '@objectstack/gone'], ['@objectstack/cli']);
@@ -310,6 +340,70 @@ function selfTest() {
310340
assert(msg.includes('@objectstack/gone'), `the diagnostic does not name the surplus package: ${msg}`);
311341
});
312342

343+
// pnpm 10.31.0's stdout head when packing `@objectstack/spec`, verbatim except the path.
344+
const BANNER = [
345+
'', '> @objectstack/spec@17.7.0 prepare /repo/packages/spec', '> pnpm gen:migration-registry', '', '',
346+
'> @objectstack/spec@17.7.0 gen:migration-registry /repo/packages/spec',
347+
'> tsx scripts/build-migration-registry.ts', '', 'build-migration-registry --self-test: ok',
348+
'✓ src/migrations/registry.ts is current (420 semantic, 258 retired-key, 222 retired-def)', '',
349+
].join('\n');
350+
const doc = (filename) =>
351+
`${JSON.stringify({ name: 'p', version: '1.0.0', filename, files: [{ path: 'package.json' }] }, null, 2)}\n`;
352+
const refusal = (stdout) => {
353+
try {
354+
parsePackStdout(stdout, '@objectstack/probe');
355+
} catch (err) {
356+
return err.message;
357+
}
358+
return '';
359+
};
360+
361+
await check('a lifecycle banner above the document is skipped', () => {
362+
const got = parsePackStdout(BANNER + doc('/d/p.tgz'), 'p').filename;
363+
assert(got === '/d/p.tgz', `banner + document read as filename ${JSON.stringify(got)}`);
364+
});
365+
366+
await check('the document alone still parses', () => {
367+
assert(parsePackStdout(doc('/d/p.tgz'), 'p').filename === '/d/p.tgz', 'a bare document was not read');
368+
});
369+
370+
await check('script output glued to the opening brace is skipped', () => {
371+
// Measured: a `postpack` of `printf "{ not json"` leaves stdout reading `{ not json{` then the document.
372+
const got = parsePackStdout(`${BANNER}{ not json${doc('/d/p.tgz')}`, 'p').filename;
373+
assert(got === '/d/p.tgz', `glued output read as filename ${JSON.stringify(got)}`);
374+
});
375+
376+
await check('output AFTER the document reddens, by name, with stdout attached', () => {
377+
const msg = refusal(`${doc('/d/p.tgz')}trailing line\n`);
378+
assert(msg !== '', 'output after the document was skipped silently');
379+
assert(msg.includes('@objectstack/probe') && msg.includes('trailing line'), `diagnostic: ${msg}`);
380+
});
381+
382+
await check('stdout with no JSON document reddens, by name, with stdout attached', () => {
383+
const msg = refusal(`${BANNER}{ not json\n`);
384+
assert(msg !== '', 'stdout with no document was accepted');
385+
assert(msg.includes('@objectstack/probe') && msg.includes('gen:migration-registry'), `diagnostic: ${msg}`);
386+
});
387+
388+
await check('packOne reads the document past a fake pnpm that prints a banner first', async () => {
389+
const dir = mkdtempSync(join(tmpdir(), 'publish-smoke-pack-'));
390+
const savedPath = process.env.PATH;
391+
try {
392+
writeFileSync(join(dir, 'banner.txt'), BANNER);
393+
const fake = ['#!/bin/sh', 'cat "$(dirname "$0")/banner.txt"', 'printf \'{\\n "filename": "%s/p.tgz"\\n}\\n\' "$4"'];
394+
writeFileSync(join(dir, 'pnpm'), `${fake.join('\n')}\n`, { mode: 0o755 });
395+
process.env.PATH = `${dir}${delimiter}${savedPath ?? ''}`;
396+
const dest = join(dir, 'dest');
397+
const got = await packOne({ name: 'p', path: dir }, dest);
398+
// `$4` is --pack-destination's value: the filename proves the FAKE answered, with packOne's argv.
399+
assert(got.filename === `${dest}/p.tgz`, `packOne read filename ${JSON.stringify(got.filename)}`);
400+
} finally {
401+
if (savedPath === undefined) delete process.env.PATH;
402+
else process.env.PATH = savedPath;
403+
rmSync(dir, { recursive: true, force: true });
404+
}
405+
});
406+
313407
// ── The floor: every declared battery RAN, and ran its cases (#13489) ────
314408
//
315409
// Evaluated after every battery has had its chance and BEFORE the verdict, so
@@ -369,7 +463,7 @@ function selfTest() {
369463

370464
if (isEntrypoint(import.meta.url)) {
371465
if (process.argv.includes('--self-test')) {
372-
if (selfTest() !== SELF_TEST_VERDICT) {
466+
if ((await selfTest()) !== SELF_TEST_VERDICT) {
373467
console.error(
374468
'\n✗ publish-smoke-pack self-test: selfTest() returned without reaching its verdict,\n'
375469
+ 'so no success line was printed. Exiting 0 here would report a self-test\n'

0 commit comments

Comments
 (0)