mosaicstack/stack
1
0
Fork 1
Code Issues 179 Pull Requests 9 Actions Packages Projects Releases 15 Wiki Activity

codex review diff-builder corrupts stdin on untracked binary files (null-byte / 'Failed to read prompt from stdin') #840

New Issue
Open
opened 2026-07-18 08:58:37 +00:00 by jason.woltje · 0 comments
jason.woltje commented 2026-07-18 08:58:37 +00:00
Owner
Copy Link
Copy Source

Problem

~/.config/mosaic/tools/codex/common.sh build_diff_context() (the diff-context builder shared by codex-code-review.sh / codex-security-review.sh) emits a bash warning and can cause the codex review to fail outright when the change-set contains an untracked binary file.

Observed in a live fleet session (stack cr-wi1-broker, pi worker, 2026-07-18, 3× in one session):

/home/hermes/.config/mosaic/tools/codex/common.sh: line 38: warning: command substitution: ignored null byte in input
Running Codex code review...
  Diff size: 1075 lines
[codex] Failed to read prompt from stdin: input

Root cause

Line ~38, uncommitted mode:

diff_text=$(git diff HEAD 2>/dev/null; git diff --cached 2>/dev/null; \
  git ls-files --others --exclude-standard 2>/dev/null | while read -r f; do \
    echo "=== NEW FILE: $f ==="; cat "$f" 2>/dev/null; done)

cat "$f" on an untracked binary file streams NUL bytes into a command substitution. Bash strips them and warns (ignored null byte in input). More importantly, the assembled diff_text (later piped to codex ... < stdin) is now corrupt/truncated at the NUL boundary, which surfaces downstream as [codex] Failed to read prompt from stdin: input — the review does not run.

Blast radius

Every codex-code-review.sh / codex-security-review.sh --uncommitted invocation whose working tree has an untracked binary (build artifact, image, .pyc, sqlite db, coverage file, etc.). Silent-ish: the bash warning is easy to miss and the failure mode looks like a codex/stdin problem, not a diff-builder problem. This is a rule-27 due-diligence path (workers run review before PR), so it recurs across the fleet.

Proposed fix

In build_diff_context(), do not inline raw binary content. Options (any one):

  1. Skip binary/untracked files that git flags as binary — e.g. gate the cat on git diff --no-index --numstat /dev/null "$f" returning non-- (text), or grep -Iq . "$f" (text-only) before emitting content; print === NEW FILE (binary, omitted): $f === otherwise.
  2. Strip NULs defensively: cat "$f" | tr -d '\0' (keeps text, drops the warning + corruption).

Option 1 is preferred (binaries add no review value and bloat the diff toward the 1M input cap — cf. the existing -b origin/main stale-diff input_too_large gotcha).

Add a regression: a temp repo with an untracked binary file → build_diff_context uncommitted returns a clean, NUL-free context and prints the omission marker.

Effort

Small — one function, one guard, one test.

Filed by enhance nightly pass 2026-07-18. Evidence: fleet digest digest-20260718T084834Z, pi session stack-cr-wi1-broker. Shared-runtime change → coordinate via Mos (AGENTS.md rule 38); deployed copy lives at ~/.config/mosaic/tools/codex/common.sh, SSOT in this repo's framework tools.

## Problem `~/.config/mosaic/tools/codex/common.sh` `build_diff_context()` (the diff-context builder shared by `codex-code-review.sh` / `codex-security-review.sh`) emits a bash warning and can cause the codex review to **fail outright** when the change-set contains an untracked **binary** file. Observed in a live fleet session (stack cr-wi1-broker, pi worker, 2026-07-18, 3× in one session): ``` /home/hermes/.config/mosaic/tools/codex/common.sh: line 38: warning: command substitution: ignored null byte in input Running Codex code review... Diff size: 1075 lines [codex] Failed to read prompt from stdin: input ``` ## Root cause Line ~38, `uncommitted` mode: ```bash diff_text=$(git diff HEAD 2>/dev/null; git diff --cached 2>/dev/null; \ git ls-files --others --exclude-standard 2>/dev/null | while read -r f; do \ echo "=== NEW FILE: $f ==="; cat "$f" 2>/dev/null; done) ``` `cat "$f"` on an untracked **binary** file streams NUL bytes into a command substitution. Bash strips them and warns (`ignored null byte in input`). More importantly, the assembled `diff_text` (later piped to `codex ... < stdin`) is now corrupt/truncated at the NUL boundary, which surfaces downstream as `[codex] Failed to read prompt from stdin: input` — the review does not run. ## Blast radius Every `codex-code-review.sh` / `codex-security-review.sh --uncommitted` invocation whose working tree has an untracked binary (build artifact, image, `.pyc`, sqlite db, coverage file, etc.). Silent-ish: the bash warning is easy to miss and the failure mode looks like a codex/stdin problem, not a diff-builder problem. This is a rule-27 due-diligence path (workers run review before PR), so it recurs across the fleet. ## Proposed fix In `build_diff_context()`, do not inline raw binary content. Options (any one): 1. Skip binary/untracked files that git flags as binary — e.g. gate the `cat` on `git diff --no-index --numstat /dev/null "$f"` returning non-`-` (text), or `grep -Iq . "$f"` (text-only) before emitting content; print `=== NEW FILE (binary, omitted): $f ===` otherwise. 2. Strip NULs defensively: `cat "$f" | tr -d '\0'` (keeps text, drops the warning + corruption). Option 1 is preferred (binaries add no review value and bloat the diff toward the 1M input cap — cf. the existing `-b origin/main` stale-diff `input_too_large` gotcha). Add a regression: a temp repo with an untracked binary file → `build_diff_context uncommitted` returns a clean, NUL-free context and prints the omission marker. ## Effort Small — one function, one guard, one test. --- _Filed by `enhance` nightly pass 2026-07-18. Evidence: fleet digest `digest-20260718T084834Z`, pi session stack-cr-wi1-broker. Shared-runtime change → coordinate via Mos (AGENTS.md rule 38); deployed copy lives at `~/.config/mosaic/tools/codex/common.sh`, SSOT in this repo's framework tools._
jason.woltje added the fleet-enhancement label 2026-07-18 08:58:37 +00:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
bug
Defect in framework tooling or delivered code
 fleet-enhancement
Fleet-wide reliability/speed/cost improvements surfaced by the nightly enhance pass
No labels fleet-enhancement
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
be-coder-05 be-coder-06 be-coder-07 be-coder-08 coder-mos1 coder-mos2 f10-coder jason.woltje (Jason Woltje) merge-gate mos-dt-0 pepper rev-974 (Rev-974 (Mosaic reviewer seat, web1)) rev-security-02
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: mosaicstack/stack#840