Skip to content

fix(vision): normalize CgBI image replay payloads - #38060

Open
stefanpieter wants to merge 3 commits into
NousResearch:mainfrom
stefanpieter:fix/vision-cgbi-replay-normalization
Open

stefanpieter wants to merge 3 commits into
NousResearch:mainfrom
stefanpieter:fix/vision-cgbi-replay-normalization

Conversation

@stefanpieter

@stefanpieter stefanpieter commented Jun 3, 2026

Copy link
Copy Markdown

Summary

  • Normalize Apple/iOS CgBI PNG bytes before embedding them as provider data URLs for vision payloads.
  • Repair stale inline data:image/png;base64,... image URLs on API-bound replay so older ACP/session history with CgBI PNGs does not keep poisoning future turns.
  • Add Codex Responses preflight coverage for replayed input_image items, including function_call_output.output[*].image_url.

Root cause

A VS Code ACP session used vision_analyze on an iOS app icon. The tool result stored a raw Apple-optimized PNG containing the private CgBI chunk as an inline data URL. On later turns, Codex Responses replayed that old function_call_output image payload and the provider rejected the whole request with:

HTTP 400: The image data you provided does not represent a valid image.

Normalizing only newly-created vision data URLs was not enough; already-stored multimodal tool outputs also need to be normalized on the API-bound copy/preflight path.

Verification

  • git diff --check
  • Static added-line scan for secrets/shell/eval/pickle/SQL patterns: no matches
  • ./venv/bin/python -m ruff check tools/vision_tools.py agent/agent_runtime_helpers.py agent/codex_responses_adapter.py tests/tools/test_vision_tools.py tests/run_agent/test_agent_guardrails.py tests/run_agent/test_run_agent_codex_responses.py
  • ./venv/bin/python -m pytest tests/tools/test_vision_tools.py tests/tools/test_vision_native_fast_path.py tests/run_agent/test_agent_guardrails.py tests/run_agent/test_run_agent_codex_responses.py -q203 passed, 1 warning
  • hermes acp --check
  • Replayed the captured failed request dump through Codex preflight and confirmed remaining CgBI image count was 0.
  • Independent review: no blockers.

2026-07-14 review remediation

Remediation head at that time: dce4d058cd6582289c87bad088c132cf86ae40f8.

  • Addressed the interactive-stdin blocker by passing stdin=subprocess.DEVNULL to both CgBI helper subprocesses: xcrun -find pngcrush and the pngcrush -revert-iphone-optimizations conversion.
  • Added an exact regression that executes both utility paths and asserts both calls use DEVNULL.
  • RED: the new regression failed because neither subprocess supplied stdin; GREEN: tests/tools/test_vision_tools.py is 91 passed.
  • Broader affected run: 232 passed; four unrelated guardrail assertions also fail unchanged on clean current upstream/main because the local runtime concurrency setting is 3 while those tests assume 10.
  • Full suite executed: 32 failures across unrelated files; the clean-target failing-file comparison reproduced the same environment/configuration failure classes. One unrelated process-cleanup timing failure did not reproduce in three bounded clean-target repeats (3/3 passed).
  • py_compile, Ruff on touched files, git diff --check, and added-line security scan passed.
  • A current-upstream/main rebase simulation applied cleanly and reran the 91-test vision suite successfully.
  • Fresh independent final-diff review returned PASS with no security or logic blockers.

No public API or replay contract changed beyond preventing the helper processes from consuming interactive input.

2026-07-28 current-main integration

Current head: 3541ea501811c761cbf64d7832015d66ae70ca29.

  • Merged current main at e4564586bc5dc9936d7a773eaabc42383bf7e4de with a normal fast-forward branch update; the only conflict was additive in agent/agent_runtime_helpers.py.
  • The resolution preserves both the PR's copy-on-write CgBI replay normalizer and current main's empty-non-final-message repair in sanitize_api_messages.
  • Focused affected suite: 246 passed; touched-file py_compile, Ruff, git diff --check, and added-line security scan passed.
  • Comprehensive local validation reached 2,782 passed before an unrelated Copilot HOME-state failure; nine encountered failures were reproduced on the exact clean target (ACP approval/edit environment, macOS Anthropic OAuth mocking, and Copilot HOME state). No baseline exclusion touches the six-file feature diff.
  • Fresh independent review of immutable merge snapshot 150c574dc0ae4888d008bcfba50e12515b071fc8: PASS with no security or logic blockers.
  • GitHub reports this exact head mergeable. The required All required checks pass gate has not started because workflow run 30344646227 is action_required; the exact head has zero check-runs pending repository-admin workflow approval.

@alt-glitch alt-glitch added type/bug Something isn't working comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint tool/vision Vision analysis and image generation provider/openai OpenAI / Codex Responses API P2 Medium — degraded but workaround exists labels Jun 3, 2026
@stefanpieter
stefanpieter force-pushed the fix/vision-cgbi-replay-normalization branch from 58881b1 to c9d14c0 Compare July 8, 2026 18:28

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for tracing this through both new vision output and historical Codex replay. The premise remains live on current main: tools/vision_tools.py:529-539 wraps raw PNG bytes as data URLs, and agent/codex_responses_adapter.py:666-673 preserves replayed input_image URLs.

Problems

  • tools/vision_tools.py:625 and tools/vision_tools.py:641 spawn xcrun/pngcrush without stdin=subprocess.DEVNULL. The existing vision rasterizer uses that guard at tools/vision_tools.py:292-295; commit 6c068358e documents that a child image utility reading stdin can hang under prompt_toolkit.

Suggested changes

  • Add stdin=subprocess.DEVNULL to both new subprocess calls, particularly the pngcrush conversion call.

This is an automated hermes-sweeper review.

Comment thread tools/vision_tools.py
@teknium1 teknium1 added sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-caching Sweeper risk: may break/degrade prompt caching or cache-key stability (invariant) sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit labels Jul 14, 2026
@stefanpieter

Copy link
Copy Markdown
Author

Review blocker fixed on dce4d058cd6582289c87bad088c132cf86ae40f8.

  • Both CgBI utility subprocesses now pass stdin=subprocess.DEVNULL; capture, timeouts, and error handling are unchanged.
  • Added a regression that executes the xcrun -find pngcrush and conversion paths and asserts DEVNULL on both calls.
  • RED: 1 failed / 90 passed before the source fix. GREEN: 91 passed.
  • py_compile, touched-file Ruff, git diff --check, added-line security scan, and a current-main rebase simulation all passed.
  • Fresh independent final-diff review: PASS; GitHub reports this exact head mergeable with no checks configured.

@teknium1 please re-review the interactive-stdin finding on the current head.

@stefanpieter

stefanpieter commented Jul 28, 2026

Copy link
Copy Markdown
Author

@teknium1 fresh review requested for exact head 3541ea501811c761cbf64d7832015d66ae70ca29.

  • Interactive-stdin blocker: both the xcrun probe and pngcrush conversion still pass stdin=subprocess.DEVNULL; the exact regression remains in the current six-file feature diff.
  • Current-main conflict: merged main@e4564586bc5dc9936d7a773eaabc42383bf7e4de normally. The sole additive conflict preserves both CgBI copy-on-write replay normalization and main's empty-non-final-message repair in sanitize_api_messages.
  • Verification: focused affected suite 246 passed; touched-file py_compile, Ruff, git diff --check, and added-line security scan passed. Independent immutable-snapshot review: PASS with no security or logic blockers.
  • Baseline disclosure: comprehensive validation reached 2,782 passed before an unrelated Copilot HOME-state failure; all nine encountered failures were reproduced on the exact clean target and are outside the six-file feature diff.
  • Live state: remote/local/PR head match; GitHub reports MERGEABLE; the only inline thread is resolved. The required All required checks pass gate has not started because workflow run 30344646227 is action_required; the exact head has zero check-runs pending repository-admin workflow approval.

The PR body now records the exact head and current evidence. Reviewer-controlled clearance remains pending.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P2 Medium — degraded but workaround exists provider/openai OpenAI / Codex Responses API sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit sweeper:risk-caching Sweeper risk: may break/degrade prompt caching or cache-key stability (invariant) sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data tool/vision Vision analysis and image generation type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants