Repository navigation
feat(chat): show the pictures a Copilot CLI session interned - #2479
Merged
Merged
Conversation
Copilot CLI 1.0.105 writes each picture once, as a root `session.binary_asset` record (base64, named `sha256:<digest of its bytes>`), and a prompt's attachments and a tool's `binaryResultsForLlm` name it by that id. Chat now names those pictures `copilot:<digest>` (prompt image parts, tool `images`), and GET /api/pane/conversation/image reads them back from the pane's own pinned session file: a root record only, a raster within MAX_IMAGE_BYTES, whose bytes hash to the digest. A prompt that is only a picture opens a turn.
…long to serve The image lookup held every whole line before filtering it. It now reads 1 MiB windows, drops a line longer than the largest picture record it could serve (MAX_IMAGE_BYTES as base64 plus 64 KiB) without holding it, and lets the event loop run between windows. A 64 MiB unrelated line went from 173 MB to 33 MB RSS.
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 34 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. |
This was referenced Oct 31, 2026
Owner
Author
|
Merging 946a6e6 on the maintainer's OK of the screenshots (brought into 0.4.22 at his request): Chat shows the pictures a Copilot CLI session interned (prompt attachments and view-tool results), served from the pane's own pinned session file. Strict file-serving review by the merge pass (APPROVE, own repros): only the pane's own session file, opened pinned without following links (traversal and symlink refused); the bytes' sha256 must equal the asset id; 8 MiB per image, rejected before decoding; only PNG, JPEG, GIF and WebP, declared and sniffed (an SVG is refused); the existing image response headers, nosniff and CSP; same auth as the other transcript images; other agents unchanged. LOW noted: each image request scans the session file from the start (a cold-load cost, bounded 1 MiB reads). |
devswha
added a commit
that referenced
this pull request
Oct 31, 2026
…window (#2483) ## Change Closes #2478. Copilot CLI interns every picture of a turn in its session file as base64 (`session.binary_asset`), so one turn with a few pictures can pass the 16 MiB page window (`TRANSCRIPT_WINDOW_BYTES`). With no turn start in that window, the newest page began mid-turn and showed only the answer. The older page was also capped at one window (`MAX_PAGE_BYTES`), so it held the prompt and its tool calls with **empty output**: their results sat on the other side. - A Copilot page now widens back to its turn's start, the newest page included, up to four windows (`COPILOT_PAGE_BYTES`, 64 MiB). Past that it starts mid-turn, as before. - Widening searches only each chunk it adds, plus the rest of the line that chunk cuts (read forward 1 MiB at a time, stopping on an empty read), then reads the page once from the start it finds. - Other agents' pages are unchanged: their cap is still one window, so they never widen. Own PR from `main`, not stacked on #2479. `git merge-tree` against #2479, #2481 and #2482 is clean in either order. ## Validation - Failing first: the new `server/copilot.test.ts` case uses the real recorded `copilot-recorded.jsonl` turn, after an earlier turn. Two interned pictures go between its tool call and the call's result (over one window together). On `main`, the newest page held only the answer. With the fix, the newest page (`prompts: 1`) equals the whole-file parse of the pictured turn, tool output included, and the older page holds the earlier turn. - `bun run check fast` on 893d55c: all steps ok, unit **9828 pass, 0 fail, 41 skip**. - Cost per poll (merge-pass finding, fixed in 9bfb005): the first fix re-widened the newest page on every append. The newest Copilot turn that has outgrown its window is now kept between polls: - It keeps the turn's start, the read point, the 64 bytes before it, and the complete lines. Each picture record is checked whole once, then kept only as its byte length behind a per-process mark, so later records keep their exact offsets. - A poll reads only the appended bytes. - It reads afresh on another history, a changed tail, a new prompt, or a turn grown a window past the cap. - It is released with the pane's other per-path caches, and bounded by bytes as well as count: least-recently-read entries are evicted while the kept lines pass one capped turn (`COPILOT_PAGE_BYTES` + a window). Picture records count only as their length. - Measured on a 55 MB file (5 pictures of 11 MiB of base64, 5 small appends): | Code | First read | Read per append | Time per append | Turns on newest page | |---|---|---|---|---| | `main` | 87.1 MiB | 16.1–16.1 MiB | 22–52 ms | 1 (turn split: answer only) | | 0b5ce2f (first fix) | 200.0 MiB | 129.0–129.0 MiB | 247–341 ms | 2 | | 9bfb005 (this round) | 200.0 MiB | 741 bytes | 0–1 ms | 2 | The new test counts bytes read with a `readSync` spy (no timing asserts): under 64 KiB per append. It read 45 MB against 0b5ce2f's code. - Between the cap and the cap plus one window, a warm cache may hold up to one window more of the same turn than a fresh read would. This hysteresis rescans once per window of growth instead of on every poll. - Known limit, like the existing `liveScans` cache: the cache trusts Copilot to append. An earlier line rewritten in place, with the 64 bytes before the read point unchanged and the file grown, is not noticed. ## Reviews - omo-native-code-reviewer (cache round, e59939ff): REQUEST_CHANGES on H1, M1, M2 and L1. All four are the same findings as Codex's and are fixed in 9bfb005 (the reviewer read that diff and found them addressed). It also verified that tool-output refs resolve across set-aside lines, that partial and UTF-8-split lines and held cursors match a fresh read, and that malformed asset lines still make the strict reader throw. - omo-native-code-reviewer (first fix): **PASS / APPROVE**. A page walk over 27, 40 and 72 MiB turns, and sub-agent pictures, matched the whole-file parse. Other agents' paging is unchanged. - MEDIUM (per-poll `Buffer.concat` cost): fixed. The page is now read once. - MEDIUM (the test never checked the older page): fixed. An earlier turn is first, and both pages are asserted. - Codex Terra 6.1 fast xhigh (cache round, 0064d2ac): HIGH (a new prompt plus more than a window appended between polls was missed) fixed; HIGH (unbounded, never-released cache) fixed with the cap rescan, 4 entries and pane release; LOW (forged marker) fixed with a per-process nonce; HIGH (rewrite-and-grow) kept as the documented known limit above. Final round on 9bfb005: **PASS**. - Codex Terra 6.1 fast xhigh (first fix): - Round 1: MEDIUM (a `session.binary_asset` skip in `copilotRecords` bypassed strict validation). The skip is removed; it is not needed for the cost. - Delta round: HIGH (the forward scan could loop on an empty read after truncation). Fixed. - Final round: **PASS**. - Known limit, unchanged: a single record longer than the cap still starts a page mid-line. Codex found this pre-existing at the old 16 MiB cap; A 64 MiB single record is far beyond a Copilot picture as seen here, since Copilot resizes images before interning them.
devswha
added a commit
that referenced
this pull request
Oct 31, 2026
## Change Part of #992 (pictures in Chat), after #2479 (Copilot) and #2481 (Grok). Hermes v0.21.5 keeps **no picture bytes** in its store (checked on a real recording: `state.db` `messages` hold no base64 or data URL, and `vision_analyze`'s result is text only). A prompt with a picture is stored as its caption, an `[Image attached at: <path>]` hint per picture, and one `[screenshot]` stand-in per picture (`agent/image_routing.py`, `agent/session_persistence.py`). What changes: the chat reads that block the way Hermes's own history does (`tui_gateway/session_history.py`) and shows each hint as a Markdown image of that local file. The chat's existing local Markdown image path then draws it: #2435's `markdownImagePreview`, which stats via `/api/fs/stat`, requires `kind: image`, caps at 20 MiB, fetches via `/api/fs/file`, and falls back to a link otherwise. No new route, no server file-serving change, no new UI. This is the same reach a user-typed `` in that bubble already has. - The pictures go **above** the words, as Chat shows other agents' prompt pictures, so nothing in the caption (an open code fence, for one) can take them in. - Only when every hint names an absolute POSIX path and each has its `[screenshot]`. Hermes writes that stand-in for each picture it actually attached, so typed look-alike text stays text. URL hints, relative or Windows paths, malformed blocks and mismatched counts stay as written. - Hints come off the stored text before skill parsing, so a skill invocation keeps its picture. - Paths are percent-encoded per segment, so `localImageTarget` reads back the exact path. ## Validation - Real fixture: `server/fixtures/hermes-pictures-0.21.5-rows.json`, the rows of a session recorded on this PC with `hermes chat -q ... --image red.png` (the vision tool then also read `blue.png`). Paths are redacted. - Failing first: the new `server/hermes.test.ts` case failed (the raw hint text was returned), then passed. It also covers URL, relative, Windows, trailing-text, placeholder-mismatch, no-placeholder, open-fence and `[`-in-name inputs, and a real skill-invocation fixture with a picture. - `bun run check fast` on ae5912f: all steps ok, unit **9828 pass, 0 fail, 41 skip**. - Real-app screenshots (demo build of the unmodified app, the recorded rows as the pane's conversation, `/api/fs/stat` and `/api/fs/file` answered for the recorded path): the picture decodes in the prompt bubble on desktop and phone, with 0 page errors. ## Reviews - omo-native-code-reviewer: **PASS / APPROVE**. - About 20 hostile paths each became exactly one image and decoded back exactly, and the change adds no fetch reach beyond user-typed Markdown. - LOW-1: the alt escape added a visible backslash. Removed. - LOW-2: the doc now says Hermes's default caption is shown too. - LOW-3: Copy copies the Markdown. A note, left as is. - Codex Terra 6.1 fast xhigh: - Round 1: MEDIUM (a skill prompt lost its picture), MEDIUM (zero placeholders let plain text trigger a fetch), LOW (an open fence swallowed the image). All fixed. - Delta round: P2 (fence state judged by counting) and P2 (fences in a skill body). Both fixed by placing pictures before the words and dropping the fence check.
devswha
added a commit
that referenced
this pull request
Oct 31, 2026
Merge main and fold the changelog fragments of every PR merged since the last refresh into 0.4.22: - #2490 (Fixed): Chat keeps messages mounted when the pane's skill names load, by @nocstah - #2483 (Fixed): a Copilot turn stays whole when its pictures pass the page window - #2479 (Added): Chat shows the pictures in a Copilot CLI session - #2482 (Added): Chat shows the picture attached to a Hermes prompt #2480 (site only) had no fragment.
devswha
added a commit
that referenced
this pull request
Oct 31, 2026
devswha
added a commit
that referenced
this pull request
Oct 31, 2026
## Change Part of #992 (pictures in Chat), after #2479 (Copilot). Grok CLI 1.0.67 logs a session's pictures inline in its own `updates.jsonl`: - a prompt's `user_message_chunk` with `content: { type: "image", data, mimeType }`, and - a tool update's `content: [{ type: "content", content: { type: "image", ... } }]` (here `read_file` reading a PNG). What changes: - **Parser** (`server/grok.ts`): prompt pictures become image parts and tool pictures become the tool's `images`, each named `grok-image:<sha256 of the bytes>`. Hidden prompt chunks are skipped, ACP's replace-the-whole-`content` rule is honoured, and only refs reach the page. - **Endpoint** (`GET /api/pane/conversation/image`, no new route): reads only the pane's own Grok update log (`resolveGrokStore`), over the history `grokProjection` keeps active. A prompt that was rewound away serves nothing, including one rewound while the lookup reads. - It checks that the file it opened is still the one the path names. - It reads 1 MiB windows with the event loop running between them. - It serves a raster within `MAX_IMAGE_BYTES` whose sha256 matches the ref, so the response is immutable. - `workImages` keeps `grok-image:` refs, like Claude's, pi's and Copilot's. Design: same image part, ref route and viewer as the other agents' pictures (#1935, #1945, #2479); no new UI. ## Validation - Real fixture: `server/fixtures/grok/pictures-1.0.67.jsonl`, recorded on this PC: `grok --prompt-json` with a red PNG, then `read_file ./blue.png`, through the owner's CLIProxy (`gpt-5.6-luna`, isolated `GROK_HOME`). Paths are redacted. - Failing first: the new `server/grok.test.ts` case failed on the parser (`[]` instead of the image part), then passed. Added cases cover a rewind before and during a lookup, a file swapped into the path during a lookup (mutation-checked: fails without the identity pin), an escaped `"im\u0061ge"`, and a missing digest. - `bun run check fast` on 3a9858c: all steps ok, unit **9828 pass, 0 fail, 41 skip**. - `bun scripts/grok-image-browser-qa.ts` (added to the CI browser lane): desktop + phone, both pictures decode at 48x48. - Lookup cost: a 6.6 MB text-only log with a missing digest takes 50 ms, and a 10 ms timer keeps firing. The first version took 4 s or more on 2.4-4.8 MB logs. ## Reviews - Codex Terra 6.1 fast xhigh: - Round 1: HIGH (quadratic read with no yield), MEDIUM (a prompt rewound during the lookup was still served), LOW (escaped type). All fixed in 3a9858c. - Delta round: MEDIUM (a file swapped into the path during the lookup). Fixed, with a test. MEDIUM (the final revalidation may catch up up to 32 MiB of new records synchronously): this is the same `SCAN_BYTES` bound every Grok page poll already pays, so it is not new here. - omo-native-code-reviewer: REQUEST_CHANGES on the same HIGH, now fixed. L2 (open, then check the path still names it) is fixed too. - L1 (a picture a later update replaced is still served from the active history) is left as is: it is the pane's own history, so nothing leaks. - L4: the parser hashes each picture on every parse; not measured.
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
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.
Change
Part of #992 (pictures in Chat). Chat showed transcript pictures for Claude, Codex, omp, pi and OpenCode, but not for GitHub Copilot CLI, though Copilot keeps them in its own session file.
Copilot CLI 1.0.105 interns each picture once, as a root
session.binary_assetrecord (base64,assetId: "sha256:<digest of the bytes>"). A prompt'sattachments[].assetIdand a tool'sresult.binaryResultsForLlm[].assetIdname it by that id (checked against the SDK'ssession-events.d.tsand a real recording).server/copilot.ts): a prompt's attached pictures become image parts and a tool's become itsimages, each namedcopilot:<digest>. Only refs reach the page. A picture-only prompt opens a turn, andtranscript-turns.tsagrees for paging.GET /api/pane/conversation/image, no new route): reads only the pane's own pinned Copilot session file (openCopilotFile). It serves a root record only (no sub-agent), a raster withinMAX_IMAGE_BYTES(inlineImageType+ magic bytes), and only bytes whose sha256 equals the ref. It reads in 1 MiB windows and skips any line longer than the largest picture record without holding it. Served immutable because the ref is the content digest;workImageskeepscopilot:refs like Claude's and pi's.Design: same image part, ref route and viewer as the pi/Claude pictures (#1935, #1945); no new UI.
Validation
server/fixtures/copilot-pictures-1.0.105-events.jsonl, recorded on this PC withcopilot -p ... --attachment red.pngplus theviewtool readingblue.png(redacted; reasoning blobs and usage records dropped).server/copilot.test.tscase failed on the parser ([]instead of the image part), then passed.bun run check fast: all steps ok, unit 9828 pass, 0 fail, 41 skip (on 946a6e6).bun scripts/copilot-image-browser-qa.ts(added to the CI browser lane): desktop + phone, both pictures decoded at 48x48.server/copilot.contract.test.ts(new end-to-end case: pane-scoped 404, cache header) could not run locally. ItsbeforeAllfails with "isolated Copilot stand-in not detected" on unchangedorigin/maintoo (herdr 0.9.5 here); CI runs it.Reviews
origin/main, so it is filed as Copilot chat page splits a turn when large records exceed the page window #2478. Delta round: MEDIUM, a final record without a newline was dropped. Fixed, with a test.