fix(slop-diff): redirect slop-scan stdout to a file — piped capture truncates on large reports#2324
Open
Andy-Haigh wants to merge 1 commit into
Open
Conversation
…uncates at 8KB slop-scan@0.3.0 emits its JSON report via async stdout writes and then calls process.exit(), so when stdout is a PIPE (spawnSync capture — this wrapper), the report is truncated at the first ~8KB chunk. A full-repo report is multi-MB, so both scans always failed JSON.parse and the /review and /ship integration silently reported 'slop-scan returned invalid JSON' (or, for the base scan, diffed against an empty baseline). File writes are synchronous, so redirecting the child's stdout to a temp file yields the complete report regardless of size. Repro: npx slop-scan scan . --json | wc -c → 8192 on any large repo, while a shell file-redirect of the same command yields the full report. Both scan sites now go through one runSlopScan() helper (fd-backed stdout, temp file cleaned up in finally). Behavior otherwise unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Merging to
After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
scripts/slop-diff.tscapturesnpx slop-scan scan … --jsonviaspawnSyncpipes. slop-scan (0.3.0) emits its JSON report with async stdout writes and then exits without flushing, so whether the report survives depends entirely on the consumer's pipe-draining behavior:bunspawnSyncnodespawnSync(anymaxBuffer)… --json | wc -c)Under
bun run slop:diffthe wrapper works today — by luck of bun's pipe implementation. Any node-based execution (running the script withtsx/node, CI runners, or ports of this wrapper into other repos) silently truncates:JSON.parsefails and the script reports "slop-scan returned invalid JSON" — or worse, the base scan silently parses as nothing and the diff runs against an empty baseline, marking every pre-existing finding as "new". I hit this porting the wrapper into a monorepo whose report is ~6.9 MB, where it fails 100% of the time under node.Fix
Both scan sites go through one
runSlopScan()helper that redirects the child's stdout to a temp file (stdio: ["ignore", fd, "pipe"]) and reads it back. File writes are synchronous, so the full report survives regardless of size, runtime, or slop-scan's exit behavior. Temp file is removed infinally. No behavior change otherwise.Verification
no new findings(this fix adds no slop).Note
The root cause is arguably in slop-scan itself (exit before flush — worth an upstream issue there too), but this wrapper-level fix makes slop-diff robust regardless of the CLI's version or the runtime it's executed with.
🤖 Generated with Claude Code