Skip to content

Comments

fix: ensure --input-format stream-json is added when sending images#314

Merged
reachraza merged 1 commit intomainfrom
code-refactor
Feb 6, 2026
Merged

fix: ensure --input-format stream-json is added when sending images#314
reachraza merged 1 commit intomainfrom
code-refactor

Conversation

@reachraza
Copy link
Collaborator

Summary

  • Fix regression where sending images to Claude Code triggered "Prompt is too long" error due to missing --input-format stream-json CLI flag
  • The Windows cmd.exe fix (2d227ed) made promptViaStdin match on --output-format stream-json (always present in Claude Code's default args), which prevented --input-format stream-json from being added when images were present
  • Without the flag, Claude Code treated the raw JSON+base64 stdin blob as a plain text prompt, massively inflating token count and blowing the context limit
  • Fix promptViaStdin detection to only match --input-format stream-json, not --output-format stream-json
  • Always add --input-format stream-json when images are present (idempotent, skips if already in args)
  • Add 10 regression tests covering image flag injection, promptViaStdin detection, and non-stream-json agent fallback

Root Cause

ChildProcessSpawner.ts line 80 used a broad check:

// Before (broken)
const argsHaveStreamJson = args.some((arg) => arg.includes('stream-json'));

This matched Claude Code's --output-format stream-json default arg, making promptViaStdin always true, which caused the image code path to skip adding --input-format stream-json.

Test plan

  • Verify --input-format stream-json is added when sending images with default Claude Code args
  • Verify flag is added even when sendPromptViaStdin is explicitly true
  • Verify no duplicate --input-format when already present in args
  • Verify stream-json message is written to stdin with single and multiple images
  • Verify --output-format stream-json does NOT trigger promptViaStdin
  • Verify --input-format stream-json DOES trigger promptViaStdin
  • Verify sendPromptViaStdin and sendPromptViaStdinRaw still work correctly
  • Verify file-based image args work for non-stream-json agents (Codex)
  • All 21 tests pass, lint clean

…o Claude Code

The Windows cmd.exe fix (2d227ed) made promptViaStdin match on any arg
containing "stream-json", which false-positived on --output-format stream-json
(always present in Claude Code default args). This prevented --input-format
stream-json from being added when sending images, so Claude Code interpreted
the raw JSON+base64 stdin blob as plain text, blowing the token limit.

- Fix promptViaStdin to only match --input-format stream-json, not --output-format
- Always add --input-format stream-json when images are present (idempotent)
- Add 10 regression tests covering image flag injection, promptViaStdin detection,
  and non-stream-json agent fallback
@claude
Copy link

claude bot commented Feb 6, 2026

Pull Request Review

This is an excellent bug fix with thorough testing and clear documentation. The fix addresses a critical regression where image input to Claude Code was breaking due to incorrect flag detection logic.

Strengths

1. Root Cause Analysis

  • The PR clearly identifies the exact commit (2d227ed) that introduced the regression
  • Explains the problem mechanism: --output-format stream-json (always present in Claude Code) was incorrectly triggering promptViaStdin detection
  • Shows how this prevented --input-format stream-json from being added, causing Claude to misinterpret base64 JSON as plain text

2. Precise Fix

The logic change in ChildProcessSpawner.ts:86-88 is surgically precise:

// Old (broken): matched any arg containing 'stream-json'
const argsHaveStreamJson = args.some((arg) => arg.includes('stream-json'));

// New (correct): only matches --input-format stream-json
const argsHaveInputStreamJson = args.some(
  (arg, i) => arg === 'stream-json' && i > 0 && args[i - 1] === '--input-format'
);

This correctly distinguishes between:

  • --output-format stream-json (output formatting, doesn't require stdin)
  • --input-format stream-json (input formatting, requires stdin)

3. Idempotent Flag Addition

Lines 100-103 ensure --input-format stream-json is added when needed, with proper idempotency:

const needsInputFormat = !args.includes('--input-format')
  ? ['--input-format', 'stream-json']
  : [];

4. Excellent Test Coverage

The 10 new tests (219 lines) comprehensively cover:

  • Flag injection with default Claude Code args
  • Flag injection when sendPromptViaStdin is explicitly true
  • No duplication when flag already present
  • Stream-json message written to stdin (single and multiple images)
  • promptViaStdin detection for all three trigger conditions
  • Non-stream-json agent fallback (file-based images for Codex)

All tests follow the existing codebase patterns and include clear explanatory comments.

5. Documentation Quality

  • Inline comments explain the "why" not just the "what"
  • Test suite includes a detailed regression comment linking back to the problematic commit
  • PR description provides clear summary, root cause, and test plan

🔍 Minor Observations

1. Potential Edge Case: Invalid Args Array

The flag detection logic assumes args[i - 1] is valid when i > 0:

const argsHaveInputStreamJson = args.some(
  (arg, i) => arg === 'stream-json' && i > 0 && args[i - 1] === '--input-format'
);

This is safe for well-formed arg arrays, but could theoretically fail if someone passes ['stream-json'] at index 0. However:

  • This is extremely unlikely in practice
  • The i > 0 check prevents the crash
  • The worst case is returning false (expected behavior)

Verdict: Not a blocker, but worth noting.

2. Test Mock Setup

The test suite imports and mocks buildStreamJsonMessage and saveImageToTempFile, but I don't see explicit mock setup in the diff. The tests appear to verify these functions are called (expect(buildStreamJsonMessage).toHaveBeenCalledWith(...)), which suggests they're mocked elsewhere in the test file.

Recommendation: Verify the mock setup is correct by running the tests locally. If they pass, this is fine.


🚀 Performance Considerations

No performance concerns. The changes are minimal:

  • One additional .some() iteration (same complexity as before)
  • One additional .includes() check for idempotency
  • No memory leaks or blocking operations

🔒 Security Considerations

No security concerns identified. The changes:

  • Don't introduce command injection vectors
  • Don't expose sensitive data
  • Don't modify trust boundaries
  • Properly handle user-provided base64 image data (already validated by parseDataUrl)

📊 Code Quality

  • Follows codebase conventions: Uses tabs for indentation ✅
  • TypeScript safety: All types are properly defined ✅
  • Error handling: Delegates to existing utilities (saveImageToTempFile, buildStreamJsonMessage) ✅
  • Readability: Clear variable names, comprehensive comments ✅

🎯 Recommendation

APPROVE with confidence. This is a well-researched, precisely implemented bug fix with excellent test coverage. The regression is critical (breaks image input entirely), and the fix is minimal and low-risk.

Suggested follow-up (non-blocking):

  • After merge, monitor Sentry for any image-related errors in production
  • Consider adding integration tests that verify end-to-end image flow with Claude Code

📝 Final Score

Criteria Rating Notes
Code Quality ⭐⭐⭐⭐⭐ Clean, well-documented, follows conventions
Test Coverage ⭐⭐⭐⭐⭐ Comprehensive regression tests
Bug Fix Precision ⭐⭐⭐⭐⭐ Surgical fix, addresses root cause
Security ⭐⭐⭐⭐⭐ No concerns
Performance ⭐⭐⭐⭐⭐ No impact

Overall: ⭐⭐⭐⭐⭐

Excellent work! This fixes a critical bug and sets a great example for regression testing.

@reachraza reachraza merged commit 995c711 into main Feb 6, 2026
2 checks passed
@pedramamini pedramamini deleted the code-refactor branch February 7, 2026 01:44
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