Skip to content

Fix hydrated image attachment previews#327300

Merged
justschen merged 1 commit into
mainfrom
ulugbekna/fix-agent-image-preview
Jul 24, 2026
Merged

Fix hydrated image attachment previews#327300
justschen merged 1 commit into
mainfrom
ulugbekna/fix-agent-image-preview

Conversation

@ulugbekna

@ulugbekna ulugbekna commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Fixes #327299

Summary

  • reject URI-backed image attachments when coercing serialized inline bytes
  • let the image carousel load hydrated attachments from their persisted file resource
  • preserve support for contiguous numeric byte maps restored from older serialized sessions

Why this is correct

Before send, pasted images carry inline Uint8Array data. Agent Host intentionally snapshots that data to disk, and history hydration restores the attachment as an image entry whose value is a URI and whose references point to the persisted file.

coerceImageBuffer is only responsible for recognizing inline bytes. Its previous object fallback accepted every object, so the seven enumerable fields of a URI became seven zero bytes. Because that result was truthy, Images Preview preferred the corrupt inline data and skipped its valid file-resource fallback.

The new validation accepts only the exact legacy serialized-Uint8Array shape: contiguous numeric keys starting at 0, each containing an integer byte from 0 through 255. Existing Uint8Array and ArrayBuffer paths are unchanged. URI and other structured objects now return undefined, which deliberately lets the existing resource path read the persisted image.

The regression test uses the hydrated shape (value: URI plus its resource reference), while the existing numeric-map tests ensure backward compatibility, including reordered numeric keys.

Validation

  • npm run typecheck-client
  • npm run valid-layers-check
  • ./scripts/test.sh --run src/vs/workbench/contrib/chat/test/common/chatImageExtraction.test.ts
  • focused hygiene checks
  • reopened a hydrated Agent Host session and confirmed the sent image renders in Images Preview

Fixes #327299

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 7f569181-94f9-4602-b654-95c75286953e
Copilot AI review requested due to automatic review settings July 24, 2026 14:10

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Fixes hydrated image previews by rejecting URI objects as inline bytes, enabling the carousel’s existing file-resource fallback.

Changes:

  • Strictly validate legacy numeric byte maps.
  • Add regression coverage for URI-backed attachments.
Show a summary per file
File Description
chatImageExtraction.ts Validates inline image byte representations.
chatImageExtraction.test.ts Covers URI rejection and legacy byte maps.

Review details

  • Files reviewed: 2/2 changed files
  • Comments generated: 0
  • Review effort level: Medium

@justschen
justschen merged commit c978d01 into main Jul 24, 2026
30 checks passed
@justschen
justschen deleted the ulugbekna/fix-agent-image-preview branch July 24, 2026 18:53
@vs-code-engineering vs-code-engineering Bot added this to the 1.131.0 milestone Jul 24, 2026
ulugbekna added a commit that referenced this pull request Jul 24, 2026
Hydrated (reload/resume/reconnect) image attachments arrive as uri-only
variable entries whose `value` is a URI, with the real file reference in
`references[0]` and no inline bytes. Since #327300, `coerceImageBuffer`
correctly returns undefined for such values instead of a bogus buffer, so
`ImageAttachmentWidget` no longer renders garbage - but it still falls back
to a generic file-media icon because nothing loads the real bytes.

Make `ImageAttachmentWidget` lazily read the bytes via `IFileService` when
inline bytes are absent but a resource is available (and the image is not
fully omitted / over the image limit), then re-render the preview with the
actual image. The icon fallback is kept if the read fails (e.g. the file no
longer exists). Works for `file:` and remote `vscode-agent-host:` resources.

Adds regression tests for the widget lazy byte-load (a hydrated uri-only
image triggers a resource read; an inline-bytes image does not).

Fixes #327329

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: e04660de-4e7b-4432-99dd-24e0ffc35d1a
ulugbekna added a commit that referenced this pull request Jul 24, 2026
Hydrated (reload/resume/reconnect) image attachments arrive as uri-only
variable entries whose `value` is a URI, with the real file reference in
`references[0]` and no inline bytes. Since #327300, `coerceImageBuffer`
correctly returns undefined for such values instead of a bogus buffer, so
`ImageAttachmentWidget` no longer renders garbage - but it still falls back
to a generic file-media icon because nothing loads the real bytes.

Make `ImageAttachmentWidget` lazily read the bytes via `IFileService` when
inline bytes are absent but a resource is available (and the image is not
fully omitted / over the image limit), then re-render the preview with the
actual image. The icon fallback is kept if the read fails (e.g. the file no
longer exists). Works for `file:` and remote `vscode-agent-host:` resources.

Adds regression tests for the widget lazy byte-load (a hydrated uri-only
image triggers a resource read; an inline-bytes image does not).

Fixes #327329

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: e04660de-4e7b-4432-99dd-24e0ffc35d1a
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.

Agents window: sent image preview breaks after session hydration

3 participants