fix(lr-0d45): robust tarball-filename capture in install:local-test

package.json

@@ -13,7 +13,7 @@
     "postinstall": "node scripts/postinstall.js",
     "prepack": "node scripts/write-build-sha.js",
     "test": "node --test --test-force-exit test/*.test.js",
-    "install:local-test": "sh -c 'f=$(npm pack) && npm install -g \"$f\" && rm -f \"$f\"'",
+    "install:local-test": "node scripts/install-local-test.js",
     "verify:installed-build": "node scripts/verify-installed-build.js",
     "semantic-release": "semantic-release"
   },

scripts/install-local-test.js

@@ -0,0 +1,97 @@
+#!/usr/bin/env node
+'use strict';
+
+// install-local-test.js — pack the current tree and install it globally (lr-0d45).
+//
+// Replaces the old `sh -c 'f=$(npm pack) && npm install -g "$f" && rm -f "$f"'`
+// one-liner. That form broke because `$(npm pack)` captures npm's multi-line
+// human-readable notices AND any output written to stdout by the `prepack`
+// lifecycle hook (scripts/write-build-sha.js's own console.log) — not just the
+// tarball filename. The resulting `$f` was a garbage multi-line string, so
+// `npm install -g "$f"` failed with EINVALIDPACKAGENAME.
+//
+// `npm pack --json` also mixes prepack-hook stdout into the same stream, so
+// this reads npm's output and extracts the tarball filename by parsing the
+// trailing JSON array specifically (the only valid JSON substring `npm pack
+// --json` ever emits), rather than trusting the whole stdout to be clean.
+
+const { execFileSync } = require('child_process');
+
+// Extracts the packed tarball filename from `npm pack --json`'s raw stdout.
+// Exported (not just used internally) so tests can exercise the parsing
+// logic against captured/synthetic npm output without shelling out to a
+// real `npm pack` — see test/install-local-test-lr-0d45.test.js.
+function extractTarballFilename(raw) {
+  // npm pack --json emits a JSON array `[ { ... } ]` (pretty-printed,
+  // multi-line, in practice), but the prepack lifecycle hook
+  // (scripts/write-build-sha.js) writes its own log line to the same stdout
+  // stream first, and that line itself starts with a literal '[' (its
+  // "[write-build-sha] wrote ..." prefix) — so a naive raw.indexOf('[') finds
+  // that '[' instead of the JSON array's. Rather than assume a specific
+  // whitespace shape around the array's opening bracket, try every '[' that
+  // starts a line as a candidate array start and parse from there to the end
+  // of the string, taking the first candidate that parses as a non-empty
+  // array of objects with a "filename" field. This tolerates both
+  // pretty-printed and single-line npm output across npm versions.
+  const lines = raw.split('\n');
+  let offset = 0;
+  const candidates = [];
+  for (const line of lines) {
+    if (line.startsWith('[')) {
+      candidates.push(offset);
+    }
+    offset += line.length + 1;
+  }
+
+  for (const start of candidates) {
+    let parsed;
+    try {
+      parsed = JSON.parse(raw.slice(start));
+    } catch {
+      continue;
+    }
+    if (Array.isArray(parsed) && parsed.length > 0 && typeof parsed[0].filename === 'string') {
+      // npm pack --json's "filename" field uses the @scope/name@version form
+      // with a literal '/' in place of the tarball's on-disk '-' separator;
+      // the actual file on disk uses '-'. Normalize to get the real path.
+      return parsed[0].filename.replace('/', '-');
+    }
+  }
+
+  throw new Error(`could not locate a valid \`npm pack --json\` array in output:\n${raw}`);
+}
+
+function packAndGetFilename() {
+  const raw = execFileSync('npm', ['pack', '--json'], { encoding: 'utf8' });
+  return extractTarballFilename(raw);
+}
+
+function main() {
+  let tarball;
+  try {
+    tarball = packAndGetFilename();
+  } catch (err) {
+    console.error(`[install-local-test] ERROR: ${err.message}`);
+    process.exit(1);
+  }
+
+  console.log(`[install-local-test] packed ${tarball}`);
+
+  try {
+    execFileSync('npm', ['install', '-g', tarball], { stdio: 'inherit' });
+  } finally {
+    try {
+      execFileSync('rm', ['-f', tarball]);
+    } catch (cleanupErr) {
+      console.error(`[install-local-test] WARN: could not remove ${tarball}: ${cleanupErr.message}`);
+    }
+  }
+
+  console.log('[install-local-test] ok: installed');
+}
+
+module.exports = { extractTarballFilename };
+
+if (require.main === module) {
+  main();
+}

test/install-local-test-lr-0d45.test.js

@@ -0,0 +1,90 @@
+// Regression test for lr-0d45 — the install:local-test npm script used to be
+// `sh -c 'f=$(npm pack) && npm install -g "$f" && rm -f "$f"'`. $(npm pack)
+// captures npm's multi-line human-readable notices, so `$f` was a garbage
+// multi-line string and `npm install -g "$f"` failed EINVALIDPACKAGENAME.
+// This surfaced for real when NAOMI's post-merge hardening (this same task)
+// started running install:local-test with on_failure: fail instead of warn.
+//
+// The fix moved the logic into scripts/install-local-test.js, which shells
+// out to `npm pack --json` and parses the tarball filename out of npm's
+// output. `npm pack --json`'s own stdout is ALSO not clean — the prepack
+// lifecycle hook (scripts/write-build-sha.js) writes a log line ahead of the
+// JSON array, and that log line itself starts with a literal '[' (its
+// "[write-build-sha] wrote ..." prefix), which broke a naive
+// raw.indexOf('[') scan. These tests exercise extractTarballFilename against
+// captured/synthetic npm output shapes directly, without shelling out to a
+// real npm pack, so a regression in the parsing logic fails here instead of
+// only being caught by an interactive install run. The parser itself scans
+// every line-leading '[' as a candidate array start and takes the first one
+// that parses as a non-empty array of objects with a "filename" field —
+// tolerant of both pretty-printed and single-line npm output.
+
+"use strict";
+
+var test = require("node:test");
+var assert = require("node:assert/strict");
+
+var { extractTarballFilename } = require("../scripts/install-local-test");
+
+test("lr-0d45: extracts filename from clean npm pack --json output", function () {
+  var raw = JSON.stringify([{ filename: "@clagentic/console-1.7.0-beta.1.tgz" }]);
+  assert.equal(extractTarballFilename(raw), "@clagentic-console-1.7.0-beta.1.tgz");
+});
+
+test("lr-0d45: extracts filename when prepack hook stdout precedes the JSON array", function () {
+  // Reproduces the real failure mode: write-build-sha.js's own console.log
+  // line lands on stdout before npm's JSON array, and that line's own "["
+  // prefix must not be mistaken for the array's opening bracket.
+  var raw =
+    "[write-build-sha] wrote /repo/lib/build-sha.json (sha=abc123)\n" +
+    JSON.stringify([{ filename: "@clagentic/console-1.7.0-beta.1.tgz" }], null, 2) +
+    "\n";
+  assert.equal(extractTarballFilename(raw), "@clagentic-console-1.7.0-beta.1.tgz");
+});
+
+test("lr-0d45: extracts filename when npm warnings precede the JSON array", function () {
+  var raw =
+    "npm warn deprecated something@1.0.0: use something-else instead\n" +
+    JSON.stringify([{ filename: "@clagentic/console-1.7.0-beta.1.tgz" }], null, 2) +
+    "\n";
+  assert.equal(extractTarballFilename(raw), "@clagentic-console-1.7.0-beta.1.tgz");
+});
+
+test("lr-0d45: throws (does not silently return garbage) when no JSON array is present", function () {
+  // This is the shape of the ORIGINAL bug: npm's human-readable notice text
+  // with no machine-parseable filename anywhere in it. The old `$(npm pack)`
+  // form would have handed this whole string to `npm install -g` as a single
+  // corrupt argument; the fix must fail loudly instead.
+  var raw =
+    "npm notice \n" +
+    "npm notice package: @clagentic/console@1.7.0-beta.1\n" +
+    "npm notice Tarball Contents\n";
+  assert.throws(function () {
+    extractTarballFilename(raw);
+  }, /could not locate a valid `npm pack --json` array/);
+});
+
+test("lr-0d45: throws on malformed JSON that merely starts with '['", function () {
+  var raw = "[not actually json}";
+  assert.throws(function () {
+    extractTarballFilename(raw);
+  }, /could not locate a valid `npm pack --json` array/);
+});
+
+test("lr-0d45: throws when the only JSON array present has no filename field", function () {
+  var raw = JSON.stringify([{ name: "@clagentic/console" }]);
+  assert.throws(function () {
+    extractTarballFilename(raw);
+  }, /could not locate a valid `npm pack --json` array/);
+});
+
+test("lr-0d45: skips a non-array/no-filename '[' candidate and finds the real array later in the stream", function () {
+  // Regression guard for the line-scan approach itself: a spurious '['-led
+  // line earlier in the stream (e.g. from an unrelated hook or warning) must
+  // not cause a false failure when a valid array follows it.
+  var raw =
+    "[irrelevant-tool] some other message\n" +
+    JSON.stringify([{ filename: "@clagentic/console-1.7.0-beta.1.tgz" }], null, 2) +
+    "\n";
+  assert.equal(extractTarballFilename(raw), "@clagentic-console-1.7.0-beta.1.tgz");
+});