fix(import): raise the pm read buffer past 1 MiB and preview updates in non-atomic --dry-run

.agents/pm/history/pm-github-tn92.jsonl

@@ -0,0 +1,2 @@
+{"ts":"2026-07-24T20:12:27.784Z","author":"claude-opus-5","op":"create","patch":[{"op":"add","path":"/metadata/id","value":"pm-github-tn92"},{"op":"add","path":"/metadata/title","value":"Import dies with an unexplained pm list-all failed on any tracker over 1 MiB, and non-atomic --dry-run previews every issue as a create"},{"op":"add","path":"/metadata/description","value":"Two defects found by running the importer against a real 443-item workspace instead of fixtures. (1) readPmItems() spawned pm list-all --full --include-body without maxBuffer, so Node's 1 MiB spawnSync default applied; that workspace dumps 1,052,859 bytes, so the child was killed with status null, error.code ENOBUFS and EMPTY stderr, and the code surfaced a bare 'pm list-all failed' with nothing to diagnose. Every import, atomic plan and sync against a mature tracker was dead with no actionable message. Sibling packages (pm-changelog, pm-context, pm-brief) already cap at 64 MiB. (2) The non-atomic --dry-run path skipped building the provenance index (shouldMatchExisting = !dryRun || atomic), so every issue previewed as a create: 'Would import 29, skip 0' where the real run performs updates for already-linked issues; --atomic --dry-run reported the split correctly, so the two paths disagreed about the same plan. A preview that overstates creates reads as 'this will duplicate my whole tracker', which is the one thing --dry-run exists to rule out. Fixing (2) makes dry runs read the tracker, so (1) had to be fixed in the same change or previews would newly hit ENOBUFS."},{"op":"add","path":"/metadata/type","value":"Issue"},{"op":"add","path":"/metadata/status","value":"open"},{"op":"add","path":"/metadata/priority","value":1},{"op":"add","path":"/metadata/tags","value":["dry-run","import","maxbuffer","production-readiness","real-data"]},{"op":"add","path":"/metadata/created_at","value":"2026-07-24T20:12:27.784Z"},{"op":"add","path":"/metadata/updated_at","value":"2026-07-24T20:12:27.784Z"},{"op":"add","path":"/metadata/author","value":"claude-opus-5"}],"before_hash":"3cc22dff72be7b14824654a7a64ea62b04799939b2fee54c1b5f52ca60bf6df0","after_hash":"2d24404cad418770990e0e6e9eff5d7fdd4212738068cc23ea229419bf85c553","message":"File both import defects found by real-data testing against a 443-item workspace"}
+{"ts":"2026-07-24T20:12:38.775Z","author":"claude-opus-5","op":"close","patch":[{"op":"replace","path":"/metadata/updated_at","value":"2026-07-24T20:12:38.775Z"},{"op":"replace","path":"/metadata/status","value":"closed"},{"op":"add","path":"/metadata/closed_at","value":"2026-07-24T20:12:38.774Z"},{"op":"add","path":"/metadata/resolution","value":"Capped the pm read buffer at 64 MiB across readPmItems(), the central pmRun() spawner and the afterCommand hook's pm show, with an explicit ENOBUFS diagnosis that names the overrun and suggests narrowing the import (--labels/--since). Built the provenance index for dry runs too, and made the non-atomic preview label each issue import/update and report 'Would import X, update Y, skip Z', returning wouldUpdate like the atomic path."},{"op":"add","path":"/metadata/expected_result","value":"Import/sync works on mature trackers instead of dying with an unexplained error, and --dry-run previews the same create/update split the real run will perform, identically in both the atomic and non-atomic paths."},{"op":"add","path":"/metadata/actual_result","value":"Against the real workspace: non-atomic dry-run reports 'Would import 24, update 5, skip 0' (was 'import 29, skip 0'); --atomic --dry-run, which previously died with ENOBUFS, completes and agrees exactly (24/5/0). 168/168 tests pass including two new tests covering the preview split and cross-path agreement."},{"op":"add","path":"/metadata/close_reason","value":"Both fixed and verified against the same real 443-item workspace that exposed them"}],"before_hash":"2d24404cad418770990e0e6e9eff5d7fdd4212738068cc23ea229419bf85c553","after_hash":"cb7fc5b5cd01ae113321594c116299b7300d5e9ef884338a1f740114dff36a3f","message":"Close after real-data verification"}

.agents/pm/issues/pm-github-tn92.toon

@@ -0,0 +1,16 @@
+id: pm-github-tn92
+title: "Import dies with an unexplained pm list-all failed on any tracker over 1 MiB, and non-atomic --dry-run previews every issue as a create"
+description: "Two defects found by running the importer against a real 443-item workspace instead of fixtures. (1) readPmItems() spawned pm list-all --full --include-body without maxBuffer, so Node's 1 MiB spawnSync default applied; that workspace dumps 1,052,859 bytes, so the child was killed with status null, error.code ENOBUFS and EMPTY stderr, and the code surfaced a bare 'pm list-all failed' with nothing to diagnose. Every import, atomic plan and sync against a mature tracker was dead with no actionable message. Sibling packages (pm-changelog, pm-context, pm-brief) already cap at 64 MiB. (2) The non-atomic --dry-run path skipped building the provenance index (shouldMatchExisting = !dryRun || atomic), so every issue previewed as a create: 'Would import 29, skip 0' where the real run performs updates for already-linked issues; --atomic --dry-run reported the split correctly, so the two paths disagreed about the same plan. A preview that overstates creates reads as 'this will duplicate my whole tracker', which is the one thing --dry-run exists to rule out. Fixing (2) makes dry runs read the tracker, so (1) had to be fixed in the same change or previews would newly hit ENOBUFS."
+type: Issue
+status: closed
+priority: 1
+tags[5]: dry-run,import,maxbuffer,production-readiness,real-data
+created_at: "2026-07-24T20:12:27.784Z"
+updated_at: "2026-07-24T20:12:38.775Z"
+closed_at: "2026-07-24T20:12:38.774Z"
+author: claude-opus-5
+resolution: "Capped the pm read buffer at 64 MiB across readPmItems(), the central pmRun() spawner and the afterCommand hook's pm show, with an explicit ENOBUFS diagnosis that names the overrun and suggests narrowing the import (--labels/--since). Built the provenance index for dry runs too, and made the non-atomic preview label each issue import/update and report 'Would import X, update Y, skip Z', returning wouldUpdate like the atomic path."
+expected_result: "Import/sync works on mature trackers instead of dying with an unexplained error, and --dry-run previews the same create/update split the real run will perform, identically in both the atomic and non-atomic paths."
+actual_result: "Against the real workspace: non-atomic dry-run reports 'Would import 24, update 5, skip 0' (was 'import 29, skip 0'); --atomic --dry-run, which previously died with ENOBUFS, completes and agrees exactly (24/5/0). 168/168 tests pass including two new tests covering the preview split and cross-path agreement."
+close_reason: Both fixed and verified against the same real 443-item workspace that exposed them
+body: ""

CHANGELOG.md

@@ -6,6 +6,10 @@
 
 - CHANGELOG omits the shipped --link-deps feature and mislabels 2026.7.23 work as Unreleased ([pm-github-px45](https://github.com/unbraind/pm-github/blob/main/.agents/pm/issues/pm-github-px45.toon))
 
+### Fixed
+
+- Import dies with an unexplained pm list-all failed on any tracker over 1 MiB, and non-atomic --dry-run previews every issue as a create ([pm-github-tn92](https://github.com/unbraind/pm-github/blob/main/.agents/pm/issues/pm-github-tn92.toon))
+
 ## 2026.7.23 - 2026-07-23
 
 ### Added

index.ts

@@ -651,15 +651,29 @@ export function scopeItemsByIds<TItem extends { id?: string }>(
 // Read every pm item (active + closed) via `pm list-all --full --include-body`
 // so the idempotency index never misses closed issues and re-creates them.
 function readPmItems(pmRoot: string): PmItem[] {
+  const maxBuffer = pmJsonMaxBuffer();
   // `list-all` (NOT `list`) so CLOSED items are included: `pm list` returns only
   // active items, which would make the idempotency index miss every closed
   // issue and re-create it as a DUPLICATE on re-import. `--full --include-body`
   // so tags and body survive the read instead of the brief projection.
   const result = spawnSync(
     "pm",
     ["--path", pmRoot, "--json", "list-all", "--full", "--include-body", "--limit", "10000"],
-    { encoding: "utf-8" },
+    { encoding: "utf-8", maxBuffer },
   );
+  // A buffer overrun kills the child with status null and no stderr, so name the
+  // real cause instead of reporting an unexplained failure.
+  if (result.error) {
+    const code = (result.error as NodeJS.ErrnoException).code;
+    if (code === "ENOBUFS") {
+      throw new CommandError(
+        `pm list-all output exceeded the ${maxBuffer} byte read buffer. ` +
+          "The workspace is larger than this read limit; narrow the import " +
+          "(for example --labels or --since) or raise the PM_JSON_MAX_BUFFER env var.",
+      );
+    }
+    throw new CommandError(`pm list-all failed: ${result.error.message}`);
+  }
   if (result.status !== 0) {
     throw new CommandError(result.stderr || "pm list-all failed");
   }
@@ -689,6 +703,24 @@ export function indexByProvenance(items: PmItem[]): Map<string, PmItem> {
 // Atomic GitHub issue import (pm-cli >= 2026.7.20 commitItemMutations)
 // ---------------------------------------------------------------------------
 
+// Node's spawnSync defaults to a 1 MiB stdout cap. A mature tracker's full JSON
+// dump (`pm list-all --full --include-body`) passes that at a few hundred items,
+// and the child is then killed with ENOBUFS, status null and EMPTY stderr — which
+// surfaced as a bare "pm list-all failed" with nothing to diagnose. Reproduced on
+// a real 443-item workspace at 1,052,859 bytes. 64 MiB matches the cap the sibling
+// pm packages settled on (pm-changelog, pm-context, pm-brief).
+/** Read-buffer cap for `pm` output, in bytes. 64 MiB by default; override with the
+ * `PM_JSON_MAX_BUFFER` env var. Resolved per call so the override takes effect
+ * without an import-order dependency. Invalid or non-positive values fall back to
+ * the default rather than silently disabling the guard. */
+function pmJsonMaxBuffer(): number {
+  // Number(), not parseInt(): parseInt("64MiB") silently yields 64, which would
+  // impose a 64-BYTE cap and break every ordinary read while appearing to honor
+  // the documented invalid-value fallback. Number() rejects the whole string.
+  const raw = Number(process.env.PM_JSON_MAX_BUFFER);
+  return Number.isSafeInteger(raw) && raw > 0 ? raw : 64 * 1024 * 1024;
+}
+
 const ATOMIC_IMPORT_PREFIX = "github-import-";
 let cachedCommitItemMutations: CommitItemMutations | undefined;
 
@@ -1609,7 +1641,8 @@ export function parseImportOptions(options: Record<string, unknown>): ImportOpti
 // result (ok = exit code 0). Centralizes every pm mutation so callers share one
 // error-handling shape.
 function pmRun(args: string[]): { ok: boolean; stderr: string; stdout: string } {
-  const result = spawnSync("pm", args, { encoding: "utf-8" });
+  const maxBuffer = pmJsonMaxBuffer();
+  const result = spawnSync("pm", args, { encoding: "utf-8", maxBuffer });
   return { ok: result.status === 0, stderr: result.stderr || "", stdout: result.stdout || "" };
 }
 
@@ -2053,11 +2086,12 @@ export async function runImport(
 
   console.error(`Found ${filtered.length} issue(s).`);
 
-  // Build the idempotency index once up-front.
-  const shouldMatchExisting = !opts.dryRun || opts.atomic;
-  const existing = shouldMatchExisting
-    ? indexByProvenance((dependencies.readItems ?? readPmItems)(pmRoot))
-    : new Map<string, PmItem>();
+  // Build the idempotency index once up-front — including for a dry run. Skipping
+  // it on a non-atomic dry run made every issue look like a create, so the preview
+  // reported "would import N, skip 0" where the real run performs updates for
+  // already-linked issues. A preview that overstates creates reads as "this will
+  // duplicate my whole tracker" and is the one thing --dry-run exists to rule out.
+  const existing = indexByProvenance((dependencies.readItems ?? readPmItems)(pmRoot));
 
   let imported = 0;
   let updated = 0;
@@ -2193,8 +2227,10 @@ export async function runImport(
     } = prepared;
 
     if (opts.dryRun) {
-      console.error(`  [dry-run] #${issue.number} ${title} (${status}, ${labels.join(",")})`);
-      imported++;
+      const action = match?.id ? "update" : "import";
+      console.error(`  [dry-run] #${issue.number} ${action} ${title} (${status}, ${labels.join(",")})`);
+      if (match?.id) updated++;
+      else imported++;
       continue;
     }
 
@@ -2273,7 +2309,7 @@ export async function runImport(
   }
 
   if (opts.dryRun) {
-    console.error(`[dry-run] Would import ${imported}, skip ${skipped}.`);
+    console.error(`[dry-run] Would import ${imported}, update ${updated}, skip ${skipped}.`);
     if (opts.linkDeps) {
       console.error(
         `[dry-run] --link-deps: ${countDependencyRefCandidates(repo, filtered)} candidate reference(s) parsed; ` +
@@ -2283,6 +2319,7 @@ export async function runImport(
     return {
       dryRun: true,
       wouldImport: imported,
+      wouldUpdate: updated,
       wouldSkip: skipped,
       ...(opts.atomic ? { atomic: true } : {}),
       ...(opts.linkDeps ? { wouldLinkDependencyCandidates: countDependencyRefCandidates(repo, filtered) } : {}),
@@ -3914,7 +3951,7 @@ export default defineExtension({
       const res = spawnSync(
         "pm",
         ["--path", ctx.pm_root, "--json", "show", id],
-        { encoding: "utf-8" },
+        { encoding: "utf-8", maxBuffer: pmJsonMaxBuffer() },
       );
       if (res.status !== 0) return;
       let repo: string | undefined;

test/dryrun-preview.test.ts

@@ -0,0 +1,82 @@
+import assert from "node:assert/strict";
+import test from "node:test";
+
+import { parseImportOptions, runImport } from "../dist/index.js";
+import type { GhIssue } from "../dist/index.js";
+
+// A --dry-run preview exists to tell an agent what a real run will do. The
+// non-atomic path used to skip building the provenance index entirely, so every
+// already-linked issue was previewed as a fresh import: "would import N, skip 0"
+// where the real run performs updates. That reads as "this will duplicate my
+// whole tracker" and is exactly what --dry-run is supposed to rule out.
+
+const issue = (number: number, title: string): GhIssue => ({
+  number,
+  title,
+  body: "",
+  state: "open",
+  labels: [],
+  assignee: null,
+  milestone: null,
+  user: { login: "octocat" },
+  created_at: "2026-07-20T00:00:00Z",
+  updated_at: "2026-07-21T00:00:00Z",
+  html_url: `https://github.com/acme/widgets/issues/${number}`,
+});
+
+async function previewImport(atomic: boolean) {
+  const messages: string[] = [];
+  const originalError = console.error;
+  console.error = (...values: unknown[]) => messages.push(values.join(" "));
+  try {
+    const result = await runImport(
+      "acme/widgets",
+      "/unused-dry-run-workspace",
+      parseImportOptions({ dryRun: true, ...(atomic ? { atomic: true } : {}) }),
+      {
+        resolveToken: () => undefined,
+        fetchIssues: async () => [issue(1, "Brand new"), issue(2, "Already linked")],
+        // #2 is already in the workspace, carrying the provenance tag the import writes.
+        readItems: () => [{ id: "existing-id", tags: ["gh:acme/widgets#2"] }],
+      },
+    );
+    return { result, messages };
+  } finally {
+    console.error = originalError;
+  }
+}
+
+test("non-atomic --dry-run previews updates for already-linked issues instead of counting them as imports", async () => {
+  const { result, messages } = await previewImport(false);
+
+  assert.deepStrictEqual(result, {
+    dryRun: true,
+    wouldImport: 1,
+    wouldUpdate: 1,
+    wouldSkip: 0,
+  });
+  assert.ok(
+    messages.some((message) => /Would import 1, update 1, skip 0/.test(message)),
+    `summary line missing; saw: ${messages.join(" | ")}`
+  );
+  assert.ok(
+    messages.some((message) => /#2 update Already linked/.test(message)),
+    "per-issue line should label the already-linked issue as an update"
+  );
+  assert.ok(
+    messages.some((message) => /#1 import Brand new/.test(message)),
+    "per-issue line should label the unlinked issue as an import"
+  );
+});
+
+test("atomic and non-atomic --dry-run agree on the import/update split", async () => {
+  const plain = (await previewImport(false)).result as Record<string, unknown>;
+  const atomic = (await previewImport(true)).result as Record<string, unknown>;
+
+  assert.strictEqual(plain.wouldImport, atomic.wouldImport);
+  assert.strictEqual(plain.wouldUpdate, atomic.wouldUpdate);
+  assert.strictEqual(plain.wouldSkip, atomic.wouldSkip);
+  // The atomic variant additionally flags itself; that is the only difference.
+  assert.strictEqual(atomic.atomic, true);
+  assert.strictEqual(plain.atomic, undefined);
+});