fix(vision): distinct clipboard stage errors, oversized-paste downscale, furl/PDF cascade - #353
Merged
Merged
Conversation
…le, furl/PDF cascade (#349, #350) A Ctrl-V paste could fail four different ways and say the same thing about all of them — or say nothing at all — and a clipboard that held a *file* or a PDF instead of raster pixels was dropped silently. StageResult is now a tagged union (ok / no_vision / too_large / not_found / read_error) instead of `enum { ok, no_vision, read_fail }`. `too_large` carries the real byte count and the limit, so the line the user reads is "the clipboard image is 7.3 MB — over the 3.5 MB limit; downscale it or use a smaller crop" rather than the old guess "missing, or larger than 5MB". The staging path stats before it reads, so that size is a number we looked up, never something inferred from error.StreamTooLong. Both call sites (readline.zig Ctrl-V and commands_model.zig /image + /paste) print the new lines; so does the drag-and-drop stager. The 5 MiB read cap was wrong in both directions: 5 MiB of raw bytes base64-expands to 6.99 MB, which is already over the ~5 MB per-image provider cap, while refusing perfectly sendable 4 MB screenshots. Raising it would make the wire failure worse. Instead there is now ONE named budget — vision_clipboard.max_staged_image_bytes = 3,700,000, documented with the 4/3 base64 math (it encodes to 4,933,336 bytes) — and an over-budget image is re-encoded with `sips -Z` stepping 2048 → 1568 → 1024 until it fits. `too_large` is only surfaced when even the smallest step is still too big. A 7,682,253-byte PNG that used to be dropped now attaches at 2,963,762 bytes. grabClipboardImage asks for one pasteboard flavor per osascript invocation and stops at the first that yields a real file: «class PNGf» exactly as before (macOS synthesizes it from TIFF/JPEG/HEIC and from live data-provider promises, so it still covers almost everything), then «class furl», then «class PDF ». The furl arm cannot trust the coercion: on a plain-text clipboard `the clipboard as «class furl»` SUCCEEDS and hands back /hello for the text "hello". So every furl path is validated — it must exist, be a regular non-empty file, and either carry a stageable extension or survive a `sips -g pixelWidth` probe (sips exits 13 on a non-image). A file the user owns is staged in place and never deleted; anything else is normalized to PNG into our own temp file. The osascript export is statted afterwards, because exit 0 is not proof the bytes landed. /tmp/.harness-clip.png is gone. It was hardcoded twice — a Zig constant and, separately, a string inside the AppleScript source — with no mechanism keeping them in sync, and being fixed and world-writable it was both a symlink-planting target and a live race: a concurrent process deleting the file between osascript writing it and the stager reading it produced a spurious read_fail (reproduced during the investigation). Paths are now built once per invocation with std.fmt.allocPrint from the pid plus 64 bits of io.random, interpolated into the script's argv so the two cannot drift, and restricted to characters needing no shell or AppleScript quoting. Grab owns its temp file and releases it from a defer at both call sites, success or failure; the intermediate .pdf never outlives the call. Every paste attempt now emits a `clipboard_paste` trace record (result, cascade flavor, byte count, mime — never the pixels). Before this a dropped paste left ZERO evidence: failing and passing runs produced byte-identical .graff/traces JSONL, which is why #350 was invisible. Removed the dead `.no_vision` prong at readline.zig:272. It was unreachable: clipboardPasteSource checks vision capability BEFORE it touches the clipboard (#258), so a text-only model returns .no_vision without ever reaching the grabber, and stageImagePath could never answer .no_vision from inside that arm. The MCP image ceiling points at the same constant — the base64 math is identical whether the pixels came from a tool or from disk, so an oversized screenshot is refused at our boundary with a note the model reads instead of becoming a provider 400 one call later. The pasteboard cascade, temp paths, sips gate/downscaler and the budget live in a new src/vision_clipboard.zig (600-line-per-file convention); vision.zig re-exports what readline/commands_model need. Tests cover the budget's base64 math, temp-path uniqueness and charset, the /hello furl trap plus non-image and non-regular-file rejection, planStage's not_found vs too_large-with-the-real-byte-count split, and the downscale ladder including cleanup of rejected steps. Nothing requiring a live pasteboard is in the default suite. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QbsV84fdmf39Bh2RF8LdPs
….png `sips -Z <dim> in --out out.png` preserves the SOURCE encoding regardless of the output filename: verified on macOS 15, `sips -Z 2048 photo.jpg --out out.png` exits 0 and `file out.png` reports `JPEG image data ... 2048x1152`. Every downscale temp is named `.png` and `stageImagePath` derives the provider `media_type` from that extension, so `/image photo.jpg` on a 4 MB phone photo (or the #350 furl paste of the same file) shipped JPEG bytes declared as `image/png`. Anthropic validates the magic bytes against `source.media_type` and 400s with "image does not match the provided media type"; the OpenAI/responses arms embed the same wrong mime in the `data:` URL. That is a regression from the #349 budget fix — under the old 5 MiB raw cap the same photo went out untouched as `image/jpeg` and worked. Two changes, belt and braces: - `sipsResize` now passes `-s format png`, which makes the output a real PNG (verified: 2048px re-encode of the same photo is a genuine PNG). - `fitToBudget` accepts a step only if the file starts with the PNG signature, so the invariant holds for any resizer, not just the one we ship. The PNG re-encode is much larger than its JPEG source — 2,795,024 bytes vs 468,171 for that photo at 2048px — but still under the 3,700,000 budget, and the 1568/1024 rungs cover anything noisier. Also corrects the comment in vision.zig that asserted "a downscale step is always PNG", which was exactly the false premise behind the bug. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QbsV84fdmf39Bh2RF8LdPs
… the 600-line cap
Two things kept this branch red.
1. CI pins codegraff-zig-0.17.0-dev.813+2153f8143, which rejects the spaced
repeat operator outright:
error: binary operator '*' has whitespace on one side, but not the other
`"0123456789" ** 4` in the planStage test is that form, so the whole tree
failed to compile on CI while building fine on 0.16. Replaced with an
explicit `++` concat, which both compilers accept — same convention already
documented at src/shapes.zig:174 (where the array case uses `@splat`).
2. Running canonical `zig fmt` over the file splits the paired
`"-e", "script"` osascript argv literals onto separate lines, which pushed
vision_clipboard.zig to 618 lines and tripped the hard
scripts/check-zig-lines.sh gate (600 max). Rather than fight fmt or
restructure the argv builders, the test block moves to
src/vision_clipboard_tests.zig, mirroring how imagegen.zig keeps its tests
in imagegen_tests.zig. vision_clipboard.zig references it from a `test {}`
block, because an unreferenced module's tests never run.
407 lines of implementation + 238 lines of tests, both well under the cap.
Every one of the 9 tests moved verbatim; no coverage was dropped.
Verified against both compilers with GRAFF_TRAJECTORY_ENDPOINT set:
0.17.0-dev.813+2153f8143 (CI): zig build, zig fmt --check src build.zig,
scripts/check-zig-lines.sh, zig build test (684/684), and
tests/learn_e2e.py — all pass.
0.16.0 (local toolchain): zig build and zig build test (684/684) pass.
Test count went 683 -> 684: the delta is the new anonymous `test {}` import
block itself, not a changed assertion.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QbsV84fdmf39Bh2RF8LdPs
The Windows CI job fails two of these tests, both because the clipboard
staging path is POSIX-shaped by design, not because the logic is wrong.
- "furlLooksStageable: the plain-text /hello coercion trap": the input to
this function is always AppleScript's `POSIX path of …` output, so it
validates with a leading-`/` test. On Windows a tmpDir realPath is `C:\...`
(fails the check that should pass) and "/hello" is not an absolute path at
all (passes a check for the wrong reason), so every case is decided by the
wrong branch.
- "fitToBudget: downscales instead of dropping": `fitToBudget` writes each
downscale step to `tempPath`, which hardcodes /tmp deliberately — the path
is interpolated into AppleScript and must need no quoting. With no /tmp on
Windows every step fails to write and the ladder reports "cannot shrink"
for a reason unrelated to the test.
Both are now `error.SkipZigTest` on Windows only, so Linux and macOS keep
running them. "fitToBudget: a step that is not really a PNG" is skipped on the
same grounds even though it was green: it only passed because every step
failed to write, which would keep it green if the magic-byte check were
deleted — a false signal is worse than an honest skip.
No production behaviour changes. `furlLooksStageable` does NOT need a comptime
OS gate — its only caller is `grabFurl`, reachable solely through
`grabClipboardImage`, which already returns null off macOS; that reasoning is
now recorded in its doc comment so the next reader does not re-derive it.
Verified with GRAFF_TRAJECTORY_ENDPOINT set:
0.17.0-dev.813 (CI pin): zig build, zig fmt --check src build.zig,
scripts/check-zig-lines.sh, zig build test (684/684), tests/learn_e2e.py.
0.16.0: zig build and zig build test (684/684).
x86_64-windows-gnu: the test binary cross-compiles clean (it can only be
run on a Windows host, but the gated code type-checks for that target).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QbsV84fdmf39Bh2RF8LdPs
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.
Fixes #349. Fixes #350.
What changed
StageResultis now a tagged union (ok{bytes,media_type}/no_vision/too_large{bytes,limit}/not_found/read_error) instead of collapsing every failure into.read_fail; both paste call sites (Ctrl-V inreadline.zig,/paste+/imageincommands_model.zig) print actionable messages with real sizes (e.g. "clipboard image is 7.6 MB — over the 3.5 MB limit").sips -s format png -Zsteps through 2048 → 1568 → 1024 until the file fits the budget, with a PNG-magic check on every step so a non-PNG can never ship mislabeled.grabClipboardImageis now a flavor cascade:«class PNGf»→«class furl»(validated: absolute, regular, non-empty, image-gated via extension orsipsprobe — the plain-text/hellocoercion trap is guarded) →«class PDF »viasipsconversion. File-URL clipboards (the Telegram case) and PDF clipboards now attach./tmp/.harness-clip.pngis replaced by a unique per-paste path (/tmp/graff-clip-<pid>-<16hex>.<ext>) built once and interpolated into the AppleScript argv, deleted after staging on every path.clipboard_pastetrace event (result, flavor, bytes, mime — never payload) is emitted for every paste outcome, so dropped attachments are finally visible in.graff/traces..no_visionprong removed from the Ctrl-V handler; cascade/budget/temp-path logic extracted to newsrc/vision_clipboard.zigwith 10 headless regression tests (683/683 pass). All files stay ≤600 lines.Why
sips, so oversized images fail with the honesttoo_largemessage rather than downscaling.Verification
Live PTY smoke against the rebuilt binary: 7.6 MB PNG now downscales at 2048 and attaches (previously dropped); 1.4 MB control unchanged; file-URL clipboard attaches via cascade (PNGf confirmed to fail on it first); PDF clipboard attaches; plain-text clipboard still correctly refused; oversized non-image reports its real size. Temp-file hygiene confirmed (
/tmp/graff-clip-*empty after runs). Adversarially reviewed; the one blocking finding (sips -Z preserves source encoding → JPEG bytes in a .png) was fixed in the follow-up commit with byte-levelfile/magic verification.🤖 Generated with Claude Code
https://claude.ai/code/session_01QbsV84fdmf39Bh2RF8LdPs