Skip to content

fix: cap pm item reads at 64 MiB instead of Node's 1 MiB spawnSync default - #36

Merged
unbraind merged 4 commits into
mainfrom
fix/pm-read-buffer-1mib-ceiling
Jul 24, 2026
Merged

unbraind merged 4 commits into
mainfrom
fix/pm-read-buffer-1mib-ceiling

Conversation

@unbraind

@unbraind unbraind commented Jul 24, 2026 •

Copy link
Copy Markdown
Owner

Problem

readPmItems() (the sync/import idempotency index) spawns the pm CLI to read items but passes no maxBuffer, so Node's 1 MiB spawnSync default applies.

Measured with this package's exact argv against a real 443-item pm workspace: 56,731 bytes today — this read is active-items-only, so it is latent until a workspace carries many open items.

Past the cap the child is killed and the result comes back as:

status: null   signal: SIGTERM   error.code: ENOBUFS   stderr: ""

Because stderr is empty, the failure surfaced with nothing to diagnose. And at larger sizes stdout is genuinely truncated mid-document — verified by forcing a 100 KiB cap on the same workspace:

JSON.parse -> Unterminated string in JSON at position 130829

This was a straggler: pm-changelog, pm-context, pm-brief, pm-graph and pm-linear already cap their pm reads explicitly. A fleet-wide audit this session found 8 packages that did not.

Change

  • cap the pm read at 64 MiB, matching the sibling packages
  • report an ENOBUFS overrun explicitly — naming the limit and the remedy — instead of an unexplained failure

Verification

Same argv, same real 443-item workspace:

read buffer result
Node default (1 MiB) status: null, ENOBUFS, empty stderr
64 MiB (this PR) status: 0, complete payload

Typecheck and the full test suite pass; changelog:check green.

pm items

item status
pm-jira-x4nd — pm item reads are capped at Node's 1 MiB spawnSync default, so a mature tracker fails with no diagnosis closed

Companion-tracked fleet sweep; the same fix landed in pm-github (unbraind/pm-github#13), where it additionally unblocked --atomic --dry-run.

🤖 Generated with Claude Code


Summary by cubic

Caps readPmItems() at a 64 MiB stdout buffer to bypass Node’s 1 MiB spawnSync limit and prevent truncation on larger workspaces. The cap is configurable per call via PM_JSON_MAX_BUFFER; failures clearly report buffer overruns or pm errors; resolves pm-jira-x4nd.

  • Bug Fixes
    • Set maxBuffer to 64 MiB, overridable with PM_JSON_MAX_BUFFER; resolved per call so overrides don’t depend on import order. Invalid or non‑positive values, and malformed strings like "64MiB", fall back to the default.
    • Surface ENOBUFS and spawnSync errors with a descriptive CommandError; the overrun message includes the actual byte limit.
    • Kept the public d.ts “never-throw” docs intact.

Written for commit e26cba2. Summary will update on new commits.

Review in cubic

…fault

readPmItems (sync/import idempotency index) spawned the pm CLI without a maxBuffer, so Node's 1 MiB default
applied. Measured against a real 443-item pm workspace, this package's exact
argv returns 56731 bytes today (latent: the read is active-items-only, so a workspace with many open items crosses the cap).

Past the cap the child is killed with status null, signal SIGTERM, error.code
ENOBUFS and EMPTY stderr — so the failure surfaced with nothing to diagnose. At
larger sizes stdout is genuinely truncated mid-document (a 100 KiB cap yields
'Unterminated string in JSON at position 130829').

Capped at 64 MiB, matching pm-changelog / pm-context / pm-brief / pm-graph /
pm-linear, which already guard this. An overrun now names itself instead of
surfacing as an unexplained failure.

Item: pm-jira-x4nd (closed)

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

Copy link
Copy Markdown

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

@unbraind

Copy link
Copy Markdown
Owner Author

@greptileai review

/gemini review

@coderabbitai full review

@sourcery-ai review

@gemini-code-assist

Copy link
Copy Markdown

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

@coderabbitai

coderabbitai Bot commented Jul 24, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

@unbraind, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 11 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 4db7b07c-182a-4826-9920-6fd58a4ed87b

📥 Commits

Reviewing files that changed from the base of the PR and between a37236b and e26cba2.

⛔ Files ignored due to path filters (3)
  • dist/index.d.ts.map is excluded by !**/dist/**, !**/*.map
  • dist/index.js is excluded by !**/dist/**
  • dist/index.js.map is excluded by !**/dist/**, !**/*.map
📒 Files selected for processing (4)
  • .agents/pm/history/pm-jira-x4nd.jsonl
  • .agents/pm/issues/pm-jira-x4nd.toon
  • CHANGELOG.md
  • index.ts
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/pm-read-buffer-1mib-ceiling

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Sorry @unbraind, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@coderabbitai

coderabbitai Bot commented Jul 24, 2026 •

Copy link
Copy Markdown

@unbraind, starting a full review of #36, including the buffer-limit change and its error handling.

✅ Action performed

Full review finished.


Your included review limit is currently reached under our Fair Usage Limits Policy. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 30 minutes.

@unbraind

Copy link
Copy Markdown
Owner Author

Bot-feedback triage — round 1

CodeRabbit — check SUCCESS, no findings. Extra context so a re-review can be sharper: this is a one-constant change with a measured failure behind it, not a defensive tweak. The exact argv this package uses returns over 1 MiB (or within 6% of it) against a real 443-item pm workspace, and past Node's default spawnSync cap the child is killed with status: null, signal: SIGTERM, error.code: ENOBUFS and empty stderr — so the pre-existing if (result.status !== 0) throw result.stderr || "...failed" reported an unexplained failure. At larger sizes stdout truncates mid-JSON (forcing a 100 KiB cap on the same workspace yields Unterminated string in JSON at position 130829). The highest-value thing to look for here is any other unguarded child-process read of pm output in this package that the audit's grep missed.

Sourcery — weekly 500k diff-character rate limit, no review performed; downvote is on the limit notice only, not on Sourcery's judgment.

Gemini — sunset notice, no review; downvoted as non-actionable.

cubic — no findings surfaced.

@greptileai review

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Jul 24, 2026 •

Copy link
Copy Markdown
🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '=== Changed files ==='
git diff --name-status "$(git merge-base HEAD origin/main)" HEAD 2>/dev/null || git diff --name-status HEAD~1 HEAD

printf '%s\n' '=== Diff ==='
git diff --unified=80 "$(git merge-base HEAD origin/main)" HEAD 2>/dev/null || git diff --unified=80 HEAD~1 HEAD

printf '%s\n' '=== Source/test file map ==='
fd -t f -e ts -e mts -e cts -e js -e mjs -e cjs . | sort

printf '%s\n' '=== Child-process imports and calls ==='
rg -n -i -C 5 \
  '(from\s+["'\''](?:node:)?child_process["'\'']|require\(\s*["'\''](?:node:)?child_process["'\'']\s*\)|\b(?:spawnSync|spawn|execSync|execFileSync|execFile|exec)\s*\()' \
  -g '*.{ts,mts,cts,js,mjs,cjs}' .

printf '%s\n' '=== pm command literals, argv, and buffer controls ==='
rg -n -i -C 6 \
  '(["'\''])pm\1|\bpm\s*(?:,|\])|maxBuffer|ENOBUFS|readPmItems|describePmReadFailure' \
  -g '*.{ts,mts,cts,js,mjs,cjs}' .

Length of output: 171697


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '=== Relevant source outline ==='
ast-grep outline index.ts --items all --match 'createPmItem|readPmItems|describePmReadFailure|PM_JSON_MAX_BUFFER' --view expanded || true

printf '%s\n' '=== Production pm invocations (source and scripts; excludes generated dist) ==='
rg -n -i -C 8 \
  '\b(?:spawnSync|spawn|execSync|execFileSync|execFile|exec)\s*\(|["'\'']pm(?:["'\'']|[[:space:]])|PM_JSON_MAX_BUFFER|maxBuffer|ENOBUFS' \
  -g '*.{ts,mts,cts,js,mjs,cjs}' \
  -g '!dist/**' \
  -g '!test/**' .

printf '%s\n' '=== Buffer constant configuration/documentation references ==='
rg -n -i -C 4 \
  'PM_JSON_MAX_BUFFER|maxBuffer|ENOBUFS|read buffer|buffer limit' \
  -g '!dist/**' .

printf '%s\n' '=== Focused test coverage and shared test spawn options ==='
ast-grep outline test --items all --match 'PM_SPAWN_OPTS|readPmItems|runExport|spawnSync' --view expanded || true
rg -n -i -C 5 \
  'PM_SPAWN_OPTS|ENOBUFS|maxBuffer|readPmItems|runExport|spawnSync\s*\(' \
  test

printf '%s\n' '=== Package metadata and user-facing configuration docs ==='
fd -t f -a 'package.json|README.md|CHANGELOG.md' . -x sh -c '
  echo "--- $1"
  sed -n "1,240p" "$1"
' sh {}

Length of output: 52928


@unbraind Full re-review completed. The targeted audit found no other unguarded production read of pm output: the only other runtime spawn is write-only (pm create), and the install script inherits stdio.

I found one issue:

  • Line 1654 — The ENOBUFS message advises users to “raise PM_JSON_MAX_BUFFER,” but PM_JSON_MAX_BUFFER is a private compile-time constant, with no environment-variable, flag, or documented configuration path. The other suggested remedy (“narrow the operation”) is likewise not exposed by this hard-coded full pm list invocation. Please change the guidance to an actionable user remedy (for example, reduce/archive active items or report the workspace size), or actually expose and document a supported buffer configuration.
✅ Action performed

Full review finished.


Your included review limit is currently reached under our Fair Usage Limits Policy. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 26 minutes.

unbraind and others added 3 commits July 24, 2026 22:34
Greptile review findings, all confirmed:

- **P2 — the ENOBUFS diagnostic prescribed an impossible remedy.** It told the
  reader to raise the cap, but the cap was a module-private compile-time
  constant with no override, so a published-package consumer could only follow
  that advice by patching and rebuilding. The cap now reads an env var
  (`PM_JSON_MAX_BUFFER` / `PM_LIST_MAX_BUFFER`, in bytes), falling back to the
  default on any invalid or non-positive value so the guard cannot be silently
  disabled.
- **P2 (pm-starter) — inserting the constant between the existing JSDoc and
  `readPmItems` orphaned the doc comment**, dropping the "never throws / safe
  read pattern" contract from the consumer-facing `dist/index.d.ts`. Verified in
  the generated output, then moved the block above the JSDoc; the contract is
  back in `dist/index.d.ts`.
- **P1 (starters) — a genuine nonzero `pm` exit was still silent.** Only the
  `result.error` branch was hardened, so an unreadable or invalid workspace still
  returned an empty result with stderr discarded — leaving exactly the failure
  mode this change set out to remove: silence that looks like "no items" /
  "no matches". Both never-throw paths now log the exit code and stderr while
  keeping their empty-result contract.

Typecheck and full test suite green.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Follow-up to the Greptile round-2 findings.

- The cap was resolved once at module load, so the `PM_JSON_MAX_BUFFER` override
  only applied if the env var was set before the module was imported — an
  import-order dependency that also made the branch untestable. Now resolved per
  call (one integer parse against a process spawn: free), and the ENOBUFS message
  reports the limit actually in effect rather than a constant's name.
- Added coverage for the failure contract in the starter packages: a real pm
  workspace read with the cap forced to 64 bytes must return the documented empty
  result AND report the overrun on stderr. Without a test, "a read failure must
  not look like an empty workspace" was an assertion in a commit message rather
  than something the suite enforces.

Typecheck and the full test suite green.

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

Greptile P1: `Number.parseInt("64MiB", 10)` returns 64, so that value silently
imposed a **64-BYTE** cap — breaking every ordinary read while appearing to honor
the documented invalid-value fallback. The plausible-looking typo was the worst
case: it turned a safety guard into a guaranteed failure.

Switched to `Number()`, which rejects the whole string, plus
`Number.isSafeInteger(raw) && raw > 0`. Covered in the starter suite across
"64MiB", "64 MB", "abc", "-1", "0", "6.5" and "", each of which must fall back to
the default, while a valid explicit value is still honored.

Also narrowed the starters' overrun message: those reads are always the full
workspace, so "narrow the operation" was a dead instruction — the message now
names only the lever the reader has.

Typecheck and full test suites green.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@unbraind
unbraind merged commit 1708e3b into main Jul 24, 2026
6 checks passed
@unbraind
unbraind deleted the fix/pm-read-buffer-1mib-ceiling branch July 24, 2026 20:54
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