Compare commits

...

1 Commits

Author SHA1 Message Date
Kyle Carberry 603e68cc80 feat(site): render image diffs in the files changed panel
When binary image files (png, jpg, gif, svg, webp, etc.) are added,
deleted, or modified, the diff panel now renders them visually instead
of silently dropping them.

- Added ImageDiffView component with support for added (green border),
  deleted (red border), and modified (side-by-side Before/After) images
- Added parseBinaryImageDiffs() to extract binary image entries from
  raw unified diff text that @pierre/diffs skips
- Integrated into FilesChangedPanel rendering loop with a DiffEntry
  union type that branches between FileDiff and ImageDiffView
- File tree sidebar shows A/D/M badges for image files
- Checkerboard transparency background, error handling with fallback
- Images fetched from raw.githubusercontent.com using repo/branch info
- 7 unit tests for the parser covering all edge cases
- Storybook stories for dark/light with mixed text+image diffs
2026-03-06 00:10:57 +00:00
10 changed files with 1264 additions and 30 deletions
+192
View File
@@ -861,6 +861,198 @@ func (api *API) getChatDiffContents(rw http.ResponseWriter, r *http.Request) {
httpapi.Write(ctx, rw, http.StatusOK, diff)
}
// @Summary Get file content from a chat's linked GitHub repository
// @ID get-chat-diff-file-content
// @Security CoderSessionToken
// @Produce application/octet-stream
// @Tags Chats
// @Param chat path string true "Chat ID" format(uuid)
// @Param path query string true "Repo-relative file path"
// @Param ref query string true "Git ref (SHA or branch name)"
// @Success 200
// @Router /chats/{chat}/diff/file-content [get]
//
// getChatDiffFileContent proxies a single file's raw content from
// the chat's linked GitHub repository. The frontend uses this to
// render image diffs for binary files that cannot be shown as text.
// It resolves the repository owner/name from the chat's cached diff
// reference and streams the raw bytes back from the GitHub Contents
// API with a 10 MiB body limit.
//
//nolint:revive // HTTP handler writes to ResponseWriter.
func (api *API) getChatDiffFileContent(rw http.ResponseWriter, r *http.Request) {
ctx := r.Context()
chat := httpmw.ChatParam(r)
filePath := r.URL.Query().Get("path")
if filePath == "" {
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
Message: "Missing required query parameter: path.",
})
return
}
// Reject absolute paths and path traversal attempts. The path
// must be a clean relative path within the repository tree.
if strings.HasPrefix(filePath, "/") {
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
Message: "Query parameter 'path' must be a relative file path.",
})
return
}
if strings.Contains(filePath, "..") {
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
Message: "Query parameter 'path' must not contain path traversal segments.",
})
return
}
ref := r.URL.Query().Get("ref")
if ref == "" {
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
Message: "Missing required query parameter: ref.",
})
return
}
owner, repo, token, err := api.resolveGitHubRepoForChat(ctx, chat)
if err != nil {
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
Message: "Failed to resolve repository for chat.",
})
return
}
if owner == "" || repo == "" {
httpapi.Write(ctx, rw, http.StatusNotFound, codersdk.Response{
Message: "Chat does not have a GitHub repository reference.",
})
return
}
contentType, body, err := api.fetchGitHubFileContent(ctx, owner, repo, filePath, ref, token)
if err != nil {
httpapi.Write(ctx, rw, http.StatusBadGateway, codersdk.Response{
Message: "Failed to fetch file content from GitHub.",
})
return
}
rw.Header().Set("Content-Type", contentType)
rw.Header().Set("Cache-Control", "private, max-age=300")
rw.WriteHeader(http.StatusOK)
_, _ = rw.Write(body)
}
// resolveGitHubRepoForChat resolves the GitHub owner, repo, and
// access token for a chat by looking up its cached diff reference.
// Returns empty owner/repo when the chat is not linked to a GitHub
// repository.
func (api *API) resolveGitHubRepoForChat(
ctx context.Context,
chat database.Chat,
) (owner, repo, token string, err error) {
status, found, err := api.getCachedChatDiffStatus(ctx, chat.ID)
if err != nil {
return "", "", "", xerrors.Errorf("get cached diff status: %w", err)
}
reference, err := api.resolveChatDiffReference(ctx, chat, found, status)
if err != nil {
return "", "", "", xerrors.Errorf("resolve diff reference: %w", err)
}
if reference.RepositoryRef == nil ||
!strings.EqualFold(reference.RepositoryRef.Provider, string(codersdk.EnhancedExternalAuthProviderGitHub)) {
return "", "", "", nil
}
token = api.resolveChatGitHubAccessToken(ctx, chat.OwnerID)
owner = reference.RepositoryRef.Owner
repo = reference.RepositoryRef.Repo
if owner == "" || repo == "" {
if reference.PullRequestURL != "" {
prRef, ok := parseGitHubPullRequestURL(reference.PullRequestURL)
if ok {
owner = prRef.Owner
repo = prRef.Repo
}
}
}
return owner, repo, token, nil
}
// fetchGitHubFileContent fetches raw file content from the GitHub
// Contents API. Each path segment is individually URL-escaped to
// prevent path traversal. The response body is limited to 10 MiB.
func (api *API) fetchGitHubFileContent(
ctx context.Context,
owner, repo, filePath, ref, token string,
) (contentType string, body []byte, err error) {
// Escape each path segment individually so directory separators
// are preserved but special characters cannot break out.
segments := strings.Split(filePath, "/")
for i, seg := range segments {
segments[i] = url.PathEscape(seg)
}
safePath := strings.Join(segments, "/")
requestURL := fmt.Sprintf(
"%s/repos/%s/%s/contents/%s?ref=%s",
githubAPIBaseURL, url.PathEscape(owner), url.PathEscape(repo), safePath, url.QueryEscape(ref),
)
req, err := http.NewRequestWithContext(ctx, http.MethodGet, requestURL, nil)
if err != nil {
return "", nil, xerrors.Errorf("create github file content request: %w", err)
}
req.Header.Set("Accept", "application/vnd.github.raw+json")
req.Header.Set("X-GitHub-Api-Version", "2022-11-28")
req.Header.Set("User-Agent", "coder-chat-diff")
if token != "" {
req.Header.Set("Authorization", "Bearer "+token)
}
httpClient := api.HTTPClient
if httpClient == nil {
httpClient = http.DefaultClient
}
resp, err := httpClient.Do(req)
if err != nil {
return "", nil, xerrors.Errorf("execute github file content request: %w", err)
}
defer resp.Body.Close()
if resp.StatusCode != http.StatusOK {
errBody, readErr := io.ReadAll(io.LimitReader(resp.Body, 8192))
if readErr != nil {
return "", nil, xerrors.Errorf(
"github file content request failed with status %d",
resp.StatusCode,
)
}
return "", nil, xerrors.Errorf(
"github file content request failed with status %d: %s",
resp.StatusCode,
strings.TrimSpace(string(errBody)),
)
}
contentType = resp.Header.Get("Content-Type")
if contentType == "" {
contentType = "application/octet-stream"
}
const maxFileSize = 10 << 20 // 10 MiB
body, err = io.ReadAll(io.LimitReader(resp.Body, maxFileSize))
if err != nil {
return "", nil, xerrors.Errorf("read github file content response: %w", err)
}
return contentType, body, nil
}
// chatCreateWorkspace provides workspace creation for the chat
// processor. RBAC authorization uses context-based checks via
// dbauthz.As rather than fake *http.Request objects.
+269
View File
@@ -5,13 +5,16 @@ import (
"encoding/json"
"fmt"
"net/http"
"net/http/httptest"
"regexp"
"strings"
"testing"
"time"
"github.com/google/uuid"
"github.com/stretchr/testify/require"
"github.com/coder/coder/v2/coderd"
"github.com/coder/coder/v2/coderd/coderdtest"
"github.com/coder/coder/v2/coderd/database"
"github.com/coder/coder/v2/coderd/database/dbauthz"
@@ -2032,6 +2035,272 @@ func TestGetChatDiffContents(t *testing.T) {
})
}
func TestGetChatDiffFileContent(t *testing.T) {
t.Parallel()
// setupGitHubChat creates a test server with a GitHub external auth
// config, inserts a chat with a cached diff reference pointing at a
// GitHub repository, and returns the SDK client, API handle, and
// chat ID. The caller can then set api.HTTPClient to intercept
// outbound GitHub API calls made by the handler.
setupGitHubChat := func(t *testing.T) (*codersdk.Client, *coderd.API, uuid.UUID) {
t.Helper()
ctx := testutil.Context(t, testutil.WaitLong)
client, _, api := coderdtest.NewWithAPI(t, &coderdtest.Options{
DeploymentValues: chatDeploymentValues(t),
ExternalAuthConfigs: []*externalauth.Config{
{
ID: "github-test",
Type: string(codersdk.EnhancedExternalAuthProviderGitHub),
Regex: regexp.MustCompile(`github\.com`),
},
},
})
db := api.Database
user := coderdtest.CreateFirstUser(t, client)
modelConfig := createChatModelConfig(t, client)
chat, err := db.InsertChat(dbauthz.AsSystemRestricted(ctx), database.InsertChatParams{
OwnerID: user.UserID,
LastModelConfigID: modelConfig.ID,
Title: "file content test",
})
require.NoError(t, err)
_, err = db.UpsertChatDiffStatusReference(
dbauthz.AsSystemRestricted(ctx),
database.UpsertChatDiffStatusReferenceParams{
ChatID: chat.ID,
Url: sql.NullString{},
GitBranch: "feat/images",
GitRemoteOrigin: "https://github.com/acme/repo.git",
StaleAt: time.Now().UTC().Add(time.Hour),
},
)
require.NoError(t, err)
return client, api, chat.ID
}
t.Run("MissingPath", func(t *testing.T) {
t.Parallel()
ctx := testutil.Context(t, testutil.WaitLong)
client, _, chatID := setupGitHubChat(t)
// Call with ref but no path.
_, _, err := client.GetChatDiffFileContent(ctx, chatID, "", "abc123")
requireSDKError(t, err, http.StatusBadRequest)
})
t.Run("MissingRef", func(t *testing.T) {
t.Parallel()
ctx := testutil.Context(t, testutil.WaitLong)
client, _, chatID := setupGitHubChat(t)
// Call with path but no ref.
_, _, err := client.GetChatDiffFileContent(ctx, chatID, "img/logo.png", "")
requireSDKError(t, err, http.StatusBadRequest)
})
t.Run("PathTraversal", func(t *testing.T) {
t.Parallel()
ctx := testutil.Context(t, testutil.WaitLong)
client, _, chatID := setupGitHubChat(t)
// Paths containing ".." must be rejected to prevent
// traversal outside the repository tree.
_, _, err := client.GetChatDiffFileContent(ctx, chatID, "../../../etc/passwd", "abc123")
requireSDKError(t, err, http.StatusBadRequest)
})
t.Run("AbsolutePath", func(t *testing.T) {
t.Parallel()
ctx := testutil.Context(t, testutil.WaitLong)
client, _, chatID := setupGitHubChat(t)
// Absolute paths must be rejected — only relative
// repository paths are valid.
_, _, err := client.GetChatDiffFileContent(ctx, chatID, "/etc/passwd", "abc123")
requireSDKError(t, err, http.StatusBadRequest)
})
t.Run("NoGitHubReference", func(t *testing.T) {
t.Parallel()
ctx := testutil.Context(t, testutil.WaitLong)
client := newChatClient(t)
_ = coderdtest.CreateFirstUser(t, client)
_ = createChatModelConfig(t, client)
// Create a chat with no diff reference at all.
chat, err := client.CreateChat(ctx, codersdk.CreateChatRequest{
Content: []codersdk.ChatInputPart{
{
Type: codersdk.ChatInputPartTypeText,
Text: "no ref",
},
},
})
require.NoError(t, err)
_, _, err = client.GetChatDiffFileContent(ctx, chat.ID, "img/logo.png", "abc123")
requireSDKError(t, err, http.StatusNotFound)
})
t.Run("NotFoundForDifferentUser", func(t *testing.T) {
t.Parallel()
ctx := testutil.Context(t, testutil.WaitLong)
client, _, api := coderdtest.NewWithAPI(t, &coderdtest.Options{
DeploymentValues: chatDeploymentValues(t),
})
firstUser := coderdtest.CreateFirstUser(t, client)
_ = createChatModelConfig(t, client)
chat, err := client.CreateChat(ctx, codersdk.CreateChatRequest{
Content: []codersdk.ChatInputPart{
{
Type: codersdk.ChatInputPartTypeText,
Text: "private file content",
},
},
})
require.NoError(t, err)
_ = api
otherClient, _ := coderdtest.CreateAnotherUser(t, client, firstUser.OrganizationID)
_, _, err = otherClient.GetChatDiffFileContent(ctx, chat.ID, "img/logo.png", "abc123")
requireSDKError(t, err, http.StatusNotFound)
})
t.Run("Success", func(t *testing.T) {
t.Parallel()
ctx := testutil.Context(t, testutil.WaitLong)
client, api, chatID := setupGitHubChat(t)
// Fake GitHub API server that validates the request and
// returns a small PNG-like payload.
wantBody := []byte("\x89PNG fake image data")
github := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
// The reference resolver also hits /pulls to look
// for open PRs — return an empty list for that.
if strings.HasPrefix(r.URL.Path, "/repos/acme/repo/pulls") {
w.Header().Set("Content-Type", "application/json")
w.WriteHeader(http.StatusOK)
_, _ = w.Write([]byte("[]"))
return
}
// Verify the file content request.
require.Equal(t, "/repos/acme/repo/contents/img/logo.png", r.URL.Path)
require.Equal(t, "abc123", r.URL.Query().Get("ref"))
require.Equal(t, "application/vnd.github.raw+json", r.Header.Get("Accept"))
w.Header().Set("Content-Type", "image/png")
w.WriteHeader(http.StatusOK)
_, _ = w.Write(wantBody)
}))
t.Cleanup(github.Close)
// Route outbound HTTP through the fake server by
// rewriting the URL in a custom RoundTripper.
api.HTTPClient = &http.Client{
Transport: roundTripperFunc(func(req *http.Request) (*http.Response, error) {
req.URL.Scheme = "http"
req.URL.Host = strings.TrimPrefix(github.URL, "http://")
return http.DefaultTransport.RoundTrip(req)
}),
}
data, ct, err := client.GetChatDiffFileContent(ctx, chatID, "img/logo.png", "abc123")
require.NoError(t, err)
require.Equal(t, "image/png", ct)
require.Equal(t, wantBody, data)
})
t.Run("GitHubReturnsError", func(t *testing.T) {
t.Parallel()
ctx := testutil.Context(t, testutil.WaitLong)
client, api, chatID := setupGitHubChat(t)
github := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if strings.HasPrefix(r.URL.Path, "/repos/acme/repo/pulls") {
w.Header().Set("Content-Type", "application/json")
w.WriteHeader(http.StatusOK)
_, _ = w.Write([]byte("[]"))
return
}
w.WriteHeader(http.StatusNotFound)
_, _ = w.Write([]byte(`{"message": "Not Found"}`))
}))
t.Cleanup(github.Close)
api.HTTPClient = &http.Client{
Transport: roundTripperFunc(func(req *http.Request) (*http.Response, error) {
req.URL.Scheme = "http"
req.URL.Host = strings.TrimPrefix(github.URL, "http://")
return http.DefaultTransport.RoundTrip(req)
}),
}
_, _, err := client.GetChatDiffFileContent(ctx, chatID, "img/missing.png", "abc123")
requireSDKError(t, err, http.StatusBadGateway)
})
t.Run("PathSegmentsAreEscaped", func(t *testing.T) {
t.Parallel()
ctx := testutil.Context(t, testutil.WaitLong)
client, api, chatID := setupGitHubChat(t)
var capturedPath string
github := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if strings.HasPrefix(r.URL.Path, "/repos/acme/repo/pulls") {
w.Header().Set("Content-Type", "application/json")
w.WriteHeader(http.StatusOK)
_, _ = w.Write([]byte("[]"))
return
}
capturedPath = r.URL.EscapedPath()
w.Header().Set("Content-Type", "image/png")
w.WriteHeader(http.StatusOK)
_, _ = w.Write([]byte("ok"))
}))
t.Cleanup(github.Close)
api.HTTPClient = &http.Client{
Transport: roundTripperFunc(func(req *http.Request) (*http.Response, error) {
req.URL.Scheme = "http"
req.URL.Host = strings.TrimPrefix(github.URL, "http://")
return http.DefaultTransport.RoundTrip(req)
}),
}
// A path with characters that need escaping.
_, _, err := client.GetChatDiffFileContent(ctx, chatID, "dir with spaces/image (1).png", "main")
require.NoError(t, err)
// r.URL.EscapedPath() preserves percent-encoding, while
// r.URL.Path decodes it. Verify the raw request was escaped.
require.Equal(t, "/repos/acme/repo/contents/dir%20with%20spaces/image%20%281%29.png", capturedPath)
})
}
// roundTripperFunc adapts a plain function to the http.RoundTripper
// interface, making it easy to intercept outbound HTTP requests in
// tests.
type roundTripperFunc func(*http.Request) (*http.Response, error)
func (f roundTripperFunc) RoundTrip(req *http.Request) (*http.Response, error) {
return f(req)
}
func TestDeleteChatQueuedMessage(t *testing.T) {
t.Parallel()
+1
View File
@@ -1138,6 +1138,7 @@ func New(options *Options) *API {
r.Post("/interrupt", api.interruptChat)
r.Get("/diff-status", api.getChatDiffStatus)
r.Get("/diff", api.getChatDiffContents)
r.Get("/diff/file-content", api.getChatDiffFileContent)
r.Route("/queue/{queuedMessage}", func(r chi.Router) {
r.Delete("/", api.deleteChatQueuedMessage)
r.Post("/promote", api.promoteChatQueuedMessage)
+29
View File
@@ -11,6 +11,7 @@ import (
"time"
"github.com/google/uuid"
"golang.org/x/xerrors"
"github.com/coder/websocket"
"github.com/coder/websocket/wsjson"
@@ -938,6 +939,34 @@ func (c *Client) GetChatDiffContents(ctx context.Context, chatID uuid.UUID) (Cha
return diff, json.NewDecoder(res.Body).Decode(&diff)
}
// GetChatDiffFileContent proxies a file's raw content from the
// chat's GitHub repository at a given git ref. It returns the raw
// bytes and the Content-Type reported by the server.
func (c *Client) GetChatDiffFileContent(ctx context.Context, chatID uuid.UUID, filePath, ref string) ([]byte, string, error) {
res, err := c.Request(ctx, http.MethodGet,
fmt.Sprintf("/api/experimental/chats/%s/diff/file-content", chatID),
nil,
func(r *http.Request) {
q := r.URL.Query()
q.Set("path", filePath)
q.Set("ref", ref)
r.URL.RawQuery = q.Encode()
},
)
if err != nil {
return nil, "", err
}
defer res.Body.Close()
if res.StatusCode != http.StatusOK {
return nil, "", ReadBodyAsError(res)
}
data, err := io.ReadAll(res.Body)
if err != nil {
return nil, "", xerrors.Errorf("read file content response: %w", err)
}
return data, res.Header.Get("Content-Type"), nil
}
func formatChatStreamResponseError(response Response) string {
message := strings.TrimSpace(response.Message)
detail := strings.TrimSpace(response.Detail)
+43
View File
@@ -3024,6 +3024,26 @@ class ApiMethods {
return response.data;
};
getChatDiffFileContent = async (
chatId: string,
path: string,
ref: string,
): Promise<string> => {
const response = await this.axios.get<Blob>(
`/api/experimental/chats/${chatId}/diff/file-content`,
{
params: { path, ref },
responseType: "blob",
},
);
// Convert to a self-contained data URL instead of a blob URL.
// Blob URLs created via URL.createObjectURL leak memory unless
// explicitly revoked, which is hard to coordinate with React
// Query's cache lifecycle. Data URLs are garbage-collected
// normally when no longer referenced.
return await blobToDataURL(response.data);
};
getChatModels = async (): Promise<TypesGen.ChatModelsResponse> => {
const response = await this.axios.get<TypesGen.ChatModelsResponse>(
"/api/experimental/chats/models",
@@ -3227,3 +3247,26 @@ export class Api extends ApiMethods implements ClientApi {
}
export const API = new Api();
/**
* Convert a Blob to a data URL string. Unlike blob URLs created by
* URL.createObjectURL, data URLs are self-contained and
* garbage-collected normally when no longer referenced — no manual
* revocation step required.
*/
function blobToDataURL(blob: Blob): Promise<string> {
return new Promise((resolve, reject) => {
const reader = new FileReader();
reader.onload = () => {
if (typeof reader.result === "string") {
resolve(reader.result);
} else {
reject(new Error("FileReader did not produce a string result"));
}
};
reader.onerror = () => {
reject(reader.error ?? new Error("FileReader failed"));
};
reader.readAsDataURL(blob);
});
}
+15
View File
@@ -196,6 +196,21 @@ export const chatDiffContents = (chatId: string) => ({
queryFn: () => API.getChatDiffContents(chatId),
});
export const chatDiffFileContentKey = (
chatId: string,
path: string,
ref: string,
) => ["chats", chatId, "diff-file-content", path, ref] as const;
export const chatDiffFileContent = (
chatId: string,
path: string,
ref: string,
) => ({
queryKey: chatDiffFileContentKey(chatId, path, ref),
queryFn: () => API.getChatDiffFileContent(chatId, path, ref),
});
export const chatModelsKey = ["chat-models"] as const;
export const chatModels = () => ({
@@ -426,3 +426,127 @@ export const LargeDiffLight: Story = {
});
},
};
/**
* Shows a diff containing binary image files (added, deleted, modified)
* alongside normal text diffs. Verifies that the ImageDiffView
* component renders properly within the panel.
*/
export const WithImageDiffs: Story = {
beforeEach: () => {
const diff = [
// Normal text file change.
"diff --git a/src/main.ts b/src/main.ts",
"index abc1234..def5678 100644",
"--- a/src/main.ts",
"+++ b/src/main.ts",
"@@ -1,3 +1,4 @@",
" import { foo } from './foo';",
"+import { bar } from './bar';",
" ",
" console.log(foo);",
// Added image.
"diff --git a/assets/logo.png b/assets/logo.png",
"new file mode 100644",
"index 0000000..abcdef1",
"Binary files /dev/null and b/assets/logo.png differ",
// Deleted image.
"diff --git a/images/old-banner.jpg b/images/old-banner.jpg",
"deleted file mode 100644",
"index abcdef1..0000000",
"Binary files a/images/old-banner.jpg and /dev/null differ",
// Modified image.
"diff --git a/icons/app.svg b/icons/app.svg",
"index 1111111..2222222 100644",
"Binary files a/icons/app.svg and b/icons/app.svg differ",
].join("\n");
spyOn(API, "getChatDiffStatus").mockResolvedValue({
...defaultDiffStatus,
url: "https://github.com/coder/coder/pull/999",
additions: 1,
deletions: 0,
changed_files: 4,
});
spyOn(API, "getChatDiffContents").mockResolvedValue({
...defaultDiffContents,
diff,
branch: "feat/add-images",
remote_origin: "https://github.com/coder/coder.git",
});
},
};
/**
* Same as WithImageDiffs but in light mode. Includes all three
* change types (added, deleted, modified) so both themes have
* full visual coverage.
*/
export const WithImageDiffsLight: Story = {
globals: {
theme: "light",
},
beforeEach: () => {
const diff = [
// Added image.
"diff --git a/assets/logo.png b/assets/logo.png",
"new file mode 100644",
"index 0000000..abcdef1",
"Binary files /dev/null and b/assets/logo.png differ",
// Deleted image.
"diff --git a/images/old-banner.jpg b/images/old-banner.jpg",
"deleted file mode 100644",
"index abcdef1..0000000",
"Binary files a/images/old-banner.jpg and /dev/null differ",
// Modified image.
"diff --git a/icons/app.svg b/icons/app.svg",
"index 1111111..2222222 100644",
"Binary files a/icons/app.svg and b/icons/app.svg differ",
].join("\n");
spyOn(API, "getChatDiffStatus").mockResolvedValue({
...defaultDiffStatus,
url: "https://github.com/coder/coder/pull/999",
changed_files: 3,
});
spyOn(API, "getChatDiffContents").mockResolvedValue({
...defaultDiffContents,
diff,
branch: "feat/add-images",
remote_origin: "https://github.com/coder/coder.git",
});
},
};
/**
* Exercises the NoBranchPlaceholder state by omitting the branch
* field from the diff contents response. The added and modified
* images should show the "preview unavailable" placeholder.
*/
export const WithImageDiffsNoBranch: Story = {
beforeEach: () => {
const diff = [
// Added image — will show placeholder.
"diff --git a/assets/logo.png b/assets/logo.png",
"new file mode 100644",
"index 0000000..abcdef1",
"Binary files /dev/null and b/assets/logo.png differ",
// Modified image — "After" will show placeholder.
"diff --git a/icons/app.svg b/icons/app.svg",
"index 1111111..2222222 100644",
"Binary files a/icons/app.svg and b/icons/app.svg differ",
].join("\n");
spyOn(API, "getChatDiffStatus").mockResolvedValue({
...defaultDiffStatus,
url: "https://github.com/coder/coder/pull/999",
changed_files: 2,
});
spyOn(API, "getChatDiffContents").mockResolvedValue({
...defaultDiffContents,
diff,
// No branch field — triggers NoBranchPlaceholder.
remote_origin: "https://github.com/coder/coder.git",
});
},
};
+88 -30
View File
@@ -31,6 +31,13 @@ import {
} from "react";
import { useQuery } from "react-query";
import { cn } from "utils/cn";
import {
type BinaryImageDiff,
badgeColor,
badgeLetter,
ImageDiffView,
parseBinaryImageDiffs,
} from "./ImageDiffView";
interface FilesChangedPanelProps {
chatId: string;
@@ -103,46 +110,69 @@ function parsePullRequestUrl(url: string): {
// File tree data model
// -------------------------------------------------------------------
/** Maps a diff change type to a Tailwind text-color class. */
/**
* Maps a diff change type to a Tailwind text-color class.
* Handles both `ChangeTypes` from @pierre/diffs and
* `BinaryImageDiff["changeType"]` values since the shared
* `badgeColor` helper covers the common subset.
*/
function changeColor(type?: ChangeTypes): string | undefined {
switch (type) {
case "new":
return "text-green-700 dark:text-green-300";
case "deleted":
return "text-red-700 dark:text-red-300";
case "change":
return badgeColor(type);
case "rename-pure":
case "rename-changed":
return "text-orange-700 dark:text-orange-300";
case "change":
return "text-orange-700 dark:text-orange-300";
return badgeColor("change");
default:
return undefined;
}
}
/** Short letter shown after the filename, matching VS Code style. */
/**
* Short letter shown after the filename, matching VS Code style.
* Delegates to the shared `badgeLetter` for common change types
* and handles rename variants that are specific to text diffs.
*/
function changeLabel(type: ChangeTypes): string {
switch (type) {
case "new":
return "A";
case "deleted":
return "D";
case "change":
return badgeLetter(type);
case "rename-pure":
case "rename-changed":
return "R";
case "change":
return "M";
default:
return "";
}
}
/**
* Union type representing either a text file diff (from @pierre/diffs)
* or a binary image diff entry. The `isBinaryImage` discriminator
* field is used to distinguish between the two at render time.
*/
type DiffEntry = FileDiffMetadata | BinaryImageDiff;
function isDiffEntryBinaryImage(entry: DiffEntry): entry is BinaryImageDiff {
return "isBinaryImage" in entry && entry.isBinaryImage === true;
}
interface FileTreeNode {
name: string;
fullPath: string;
type: "file" | "directory";
children: FileTreeNode[];
fileDiff?: FileDiffMetadata;
imageDiff?: BinaryImageDiff;
/**
* Unified change type derived from either `fileDiff.type` or
* `imageDiff.changeType` so tree rendering doesn't need to
* inspect both optional fields.
*/
unifiedChangeType?: ChangeTypes;
}
/**
@@ -152,7 +182,7 @@ interface FileTreeNode {
* Single-child directory chains are collapsed so that e.g.
* `src/pages/AgentsPage` renders as one row.
*/
function buildFileTree(files: FileDiffMetadata[]): FileTreeNode[] {
function buildFileTree(files: DiffEntry[]): FileTreeNode[] {
const root: FileTreeNode[] = [];
for (const file of files) {
@@ -177,12 +207,15 @@ function buildFileTree(files: FileDiffMetadata[]): FileTreeNode[] {
// Leaf file node.
const fileName = segments[segments.length - 1];
const isImage = isDiffEntryBinaryImage(file);
children.push({
name: fileName,
fullPath: file.name,
type: "file",
children: [],
fileDiff: file,
fileDiff: isImage ? undefined : file,
imageDiff: isImage ? file : undefined,
unifiedChangeType: isImage ? file.changeType : file.type,
});
}
@@ -287,20 +320,20 @@ const FileTreeNodeView: FC<{
<span
className={cn(
"truncate",
changeColor(node.fileDiff?.type) ??
changeColor(node.unifiedChangeType) ??
(isActive ? "text-content-primary" : "text-content-secondary"),
)}
>
{node.name}
</span>
{node.fileDiff?.type && (
{node.unifiedChangeType && (
<span
className={cn(
"ml-auto shrink-0 pr-2 text-xs",
changeColor(node.fileDiff.type),
changeColor(node.unifiedChangeType),
)}
>
{changeLabel(node.fileDiff.type)}
{changeLabel(node.unifiedChangeType)}
</span>
)}
</button>
@@ -353,7 +386,7 @@ export const FilesChangedPanel: FC<FilesChangedPanelProps> = ({
enabled: Boolean(diffStatusQuery.data?.url),
});
const parsedFiles = useMemo(() => {
const parsedFiles = useMemo((): DiffEntry[] => {
const diff = diffContentsQuery.data?.diff;
if (!diff) {
return [];
@@ -363,8 +396,19 @@ export const FilesChangedPanel: FC<FilesChangedPanelProps> = ({
// so highlighted ASTs are reused across re-renders instead
// of being re-computed on every render cycle.
const patches = parsePatchFiles(diff, `chat-${chatId}`);
return patches.flatMap((p) => p.files);
} catch {
const textFiles: DiffEntry[] = patches.flatMap((p) => p.files);
// Extract binary image diffs that parsePatchFiles skips.
const imageDiffs: DiffEntry[] = parseBinaryImageDiffs(diff);
// Deduplicate: if parsePatchFiles already captured a file
// (rare for binary, but possible), prefer the text entry.
const textNames = new Set(textFiles.map((f) => f.name));
const uniqueImages = imageDiffs.filter((img) => !textNames.has(img.name));
return [...textFiles, ...uniqueImages];
} catch (ex) {
console.error("[FilesChangedPanel] failed to parse diff:", ex);
return [];
}
}, [diffContentsQuery.data?.diff, chatId]);
@@ -395,6 +439,8 @@ export const FilesChangedPanel: FC<FilesChangedPanelProps> = ({
const pullRequestUrl = diffStatusQuery.data?.url;
const parsedPr = pullRequestUrl ? parsePullRequestUrl(pullRequestUrl) : null;
const diffBranch = diffContentsQuery.data?.branch;
// ---------------------------------------------------------------
// Container width measurement via ResizeObserver so we can decide
// whether to show the file tree sidebar without a prop from the
@@ -418,9 +464,9 @@ export const FilesChangedPanel: FC<FilesChangedPanelProps> = ({
roRef.current = ro;
}, []);
const hasFiles = sortedFiles.length > 0;
const showTree =
(isExpanded || containerWidth >= FILE_TREE_THRESHOLD) &&
sortedFiles.length > 0;
(isExpanded || containerWidth >= FILE_TREE_THRESHOLD) && hasFiles;
// ---------------------------------------------------------------
// Refs for each file diff wrapper so we can scroll-to and track
@@ -610,7 +656,7 @@ export const FilesChangedPanel: FC<FilesChangedPanelProps> = ({
</div>
</div>
{/* Diff contents */}
{sortedFiles.length === 0 ? (
{!hasFiles ? (
<div className="flex flex-1 items-center justify-center p-6 text-center text-xs text-content-secondary">
No file changes to display.
</div>
@@ -653,14 +699,26 @@ export const FilesChangedPanel: FC<FilesChangedPanelProps> = ({
}}
>
<div className="min-w-0 text-xs">
{sortedFiles.map((fileDiff) => (
<div
key={fileDiff.name}
ref={(el) => setFileRef(fileDiff.name, el)}
>
<LazyFileDiff fileDiff={fileDiff} options={fileOptions} />
{sortedFiles.map((entry) => (
<div key={entry.name} ref={(el) => setFileRef(entry.name, el)}>
{isDiffEntryBinaryImage(entry) ? (
<ImageDiffView
diff={entry}
chatId={chatId}
branch={diffBranch}
// TODO: Pass the actual base branch
// from the API response instead of
// letting ImageDiffView fall back to
// "main". Repos with a different
// default branch will show broken
// "before" images for modified or
// deleted files.
/>
) : (
<LazyFileDiff fileDiff={entry} options={fileOptions} />
)}
</div>
))}
))}{" "}
{/* Spacer so the last file can scroll fully to the top. */}
<div className="h-[calc(100vh-100px)]" />
</div>
@@ -0,0 +1,153 @@
import { describe, expect, it } from "vitest";
import { parseBinaryImageDiffs } from "./ImageDiffView";
describe("parseBinaryImageDiffs", () => {
it("returns empty array for text-only diffs", () => {
const textDiff = [
"diff --git a/src/main.ts b/src/main.ts",
"index abc1234..def5678 100644",
"--- a/src/main.ts",
"+++ b/src/main.ts",
"@@ -1,3 +1,4 @@",
" import { foo } from './foo';",
"+import { bar } from './bar';",
" ",
" console.log(foo);",
].join("\n");
expect(parseBinaryImageDiffs(textDiff)).toEqual([]);
});
it("detects added image", () => {
const diff = [
"diff --git a/images/logo.png b/images/logo.png",
"new file mode 100644",
"index 0000000..abcdef1",
"Binary files /dev/null and b/images/logo.png differ",
].join("\n");
expect(parseBinaryImageDiffs(diff)).toEqual([
{ name: "images/logo.png", changeType: "new", isBinaryImage: true },
]);
});
it("detects deleted image", () => {
const diff = [
"diff --git a/old/banner.jpg b/old/banner.jpg",
"deleted file mode 100644",
"index abcdef1..0000000",
"Binary files a/old/banner.jpg and /dev/null differ",
].join("\n");
expect(parseBinaryImageDiffs(diff)).toEqual([
{ name: "old/banner.jpg", changeType: "deleted", isBinaryImage: true },
]);
});
it("detects modified image", () => {
const diff = [
"diff --git a/assets/icon.svg b/assets/icon.svg",
"index abc1234..def5678 100644",
"Binary files a/assets/icon.svg and b/assets/icon.svg differ",
].join("\n");
expect(parseBinaryImageDiffs(diff)).toEqual([
{ name: "assets/icon.svg", changeType: "change", isBinaryImage: true },
]);
});
it("ignores non-image binary files", () => {
const diff = [
"diff --git a/lib/module.wasm b/lib/module.wasm",
"new file mode 100644",
"index 0000000..abcdef1",
"Binary files /dev/null and b/lib/module.wasm differ",
].join("\n");
expect(parseBinaryImageDiffs(diff)).toEqual([]);
});
it("handles multiple images mixed with text diffs", () => {
const diff = [
"diff --git a/README.md b/README.md",
"index aaa1111..bbb2222 100644",
"--- a/README.md",
"+++ b/README.md",
"@@ -1,2 +1,3 @@",
" # Project",
"+Some new docs.",
" ",
"diff --git a/assets/screenshot.png b/assets/screenshot.png",
"new file mode 100644",
"index 0000000..ccc3333",
"Binary files /dev/null and b/assets/screenshot.png differ",
"diff --git a/src/index.ts b/src/index.ts",
"index ddd4444..eee5555 100644",
"--- a/src/index.ts",
"+++ b/src/index.ts",
"@@ -1 +1,2 @@",
" export {};",
"+export { App } from './App';",
].join("\n");
expect(parseBinaryImageDiffs(diff)).toEqual([
{
name: "assets/screenshot.png",
changeType: "new",
isBinaryImage: true,
},
]);
});
it("handles various image extensions", () => {
const extensions = [
"png",
"jpg",
"jpeg",
"gif",
"webp",
"svg",
"avif",
"bmp",
"ico",
];
const diff = extensions
.map((ext) =>
[
`diff --git a/img/photo.${ext} b/img/photo.${ext}`,
"new file mode 100644",
"index 0000000..abcdef1",
`Binary files /dev/null and b/img/photo.${ext} differ`,
].join("\n"),
)
.join("\n");
const result = parseBinaryImageDiffs(diff);
expect(result).toHaveLength(extensions.length);
for (const ext of extensions) {
expect(result).toContainEqual({
name: `img/photo.${ext}`,
changeType: "new",
isBinaryImage: true,
});
}
});
it("returns empty array for empty string input", () => {
expect(parseBinaryImageDiffs("")).toEqual([]);
});
it("matches extensions case-insensitively", () => {
const diff = [
"diff --git a/img/photo.PNG b/img/photo.PNG",
"new file mode 100644",
"index 0000000..abcdef1",
"Binary files /dev/null and b/img/photo.PNG differ",
].join("\n");
expect(parseBinaryImageDiffs(diff)).toEqual([
{ name: "img/photo.PNG", changeType: "new", isBinaryImage: true },
]);
});
});
+350
View File
@@ -0,0 +1,350 @@
import { chatDiffFileContent } from "api/queries/chats";
import { FileIcon } from "components/FileIcon/FileIcon";
import { ImageOff, Loader2Icon } from "lucide-react";
import { type FC, useEffect } from "react";
import { useQuery } from "react-query";
import { cn } from "utils/cn";
// -------------------------------------------------------------------
// Types
// -------------------------------------------------------------------
export interface BinaryImageDiff {
name: string;
changeType: "new" | "deleted" | "change";
/** Literal discriminator for identifying binary image diffs. */
isBinaryImage: true;
}
// -------------------------------------------------------------------
// Constants
// -------------------------------------------------------------------
/** Matches common image file extensions at the end of a path. */
const IMAGE_EXTENSIONS = /\.(png|jpe?g|gif|svg|webp|bmp|ico|avif)$/i;
/** Maps a change type to a human-readable label for accessibility. */
const CHANGE_TYPE_LABELS: Record<BinaryImageDiff["changeType"], string> = {
new: "Added",
deleted: "Deleted",
change: "Modified",
};
// -------------------------------------------------------------------
// Parsing
// -------------------------------------------------------------------
/**
* Parses a raw unified diff string to extract entries for binary
* image files. Git represents binary files differently from text
* files — instead of line-by-line hunks, they appear as:
*
* Binary files /dev/null and b/path/to/image.png differ
*
* This function splits on `diff --git ` boundaries, identifies
* blocks that contain "Binary files", extracts the file path from
* the header, and filters to only image extensions.
*/
export function parseBinaryImageDiffs(rawDiff: string): BinaryImageDiff[] {
// Split on diff boundaries. The first element is empty or
// preamble text before the first diff block, so we skip it.
const blocks = rawDiff.split("diff --git ").slice(1);
const results: BinaryImageDiff[] = [];
for (const block of blocks) {
if (!block.includes("Binary files")) {
continue;
}
// Extract file name from the `a/path b/path` header line.
// The header is the first line of the block, formatted as:
// a/some/file.png b/some/file.png
const headerMatch = block.match(/^a\/(.+?)\s+b\/(.+)/m);
if (!headerMatch) {
continue;
}
const filePath = headerMatch[2];
if (!IMAGE_EXTENSIONS.test(filePath)) {
continue;
}
// Determine the change type from mode markers.
let changeType: BinaryImageDiff["changeType"] = "change";
if (block.includes("new file mode")) {
changeType = "new";
} else if (block.includes("deleted file mode")) {
changeType = "deleted";
}
results.push({
name: filePath,
changeType,
isBinaryImage: true,
});
}
return results;
}
// -------------------------------------------------------------------
// Checkerboard background styles
// -------------------------------------------------------------------
/**
* Tailwind classes for a checkerboard pattern backdrop. Uses CSS
* custom properties so that `dark:` variants handle the theme
* switch automatically without needing an `isDark` prop.
*/
const CHECKERBOARD_CLASSES = [
// Light-mode checkerboard color.
"[--checker-color:#f0f0f0]",
// Dark-mode override via Tailwind's dark variant.
"dark:[--checker-color:#2a2a2a]",
// The four-gradient checkerboard pattern.
"[background-image:linear-gradient(45deg,var(--checker-color)_25%,transparent_25%),linear-gradient(-45deg,var(--checker-color)_25%,transparent_25%),linear-gradient(45deg,transparent_75%,var(--checker-color)_75%),linear-gradient(-45deg,transparent_75%,var(--checker-color)_75%)]",
"[background-size:16px_16px]",
"[background-position:0_0,0_8px,8px_-8px,-8px_0px]",
].join(" ");
// -------------------------------------------------------------------
// Change type styling helpers
// -------------------------------------------------------------------
/** Maps a change type to a Tailwind text-color class for badges. */
export function badgeColor(type: BinaryImageDiff["changeType"]): string {
switch (type) {
case "new":
return "text-green-700 dark:text-green-300";
case "deleted":
return "text-red-700 dark:text-red-300";
case "change":
return "text-orange-700 dark:text-orange-300";
}
}
/** Short letter shown after the filename, matching VS Code style. */
export function badgeLetter(type: BinaryImageDiff["changeType"]): string {
switch (type) {
case "new":
return "A";
case "deleted":
return "D";
case "change":
return "M";
}
}
// -------------------------------------------------------------------
// Internal sub-components
// -------------------------------------------------------------------
/**
* Fetches image content through the backend proxy and renders it.
* The proxy handles GitHub authentication so this works for both
* public and private repositories.
*/
const ProxiedDiffImage: FC<{
chatId: string;
filePath: string;
gitRef: string;
alt: string;
}> = ({ chatId, filePath, gitRef, alt }) => {
const {
data: imageUrl,
isLoading,
isError,
} = useQuery({
...chatDiffFileContent(chatId, filePath, gitRef),
// Keep the image URL cached for the session to avoid
// re-fetching images the user has already seen.
staleTime: Number.POSITIVE_INFINITY,
});
// Revoke the object URL when the component unmounts or the
// URL changes to prevent browser memory leaks.
useEffect(() => {
return () => {
if (imageUrl) {
URL.revokeObjectURL(imageUrl);
}
};
}, [imageUrl]);
if (isLoading) {
return (
<div
className="flex items-center justify-center p-8 text-content-secondary"
role="status"
>
<Loader2Icon
className="size-6 animate-spin opacity-50"
aria-hidden="true"
/>
<span className="sr-only">Loading image</span>
</div>
);
}
if (isError || !imageUrl) {
return (
<div
className="flex flex-col items-center justify-center gap-2 p-8 text-content-secondary"
role="alert"
>
<ImageOff className="size-8 opacity-50" aria-hidden="true" />
<span className="text-xs">Failed to load image</span>
</div>
);
}
return (
<div
className={cn(
"flex items-center justify-center rounded p-2",
CHECKERBOARD_CLASSES,
)}
>
<img
src={imageUrl}
alt={alt}
className="max-h-[400px] max-w-full object-contain"
/>
</div>
);
};
/**
* Placeholder shown when a branch ref is not available and
* the image cannot be fetched.
*/
const NoBranchPlaceholder: FC<{ action: string }> = ({ action }) => (
<div className="flex items-center justify-center p-8 text-xs text-content-secondary">
Image {action} preview unavailable without branch info.
</div>
);
// -------------------------------------------------------------------
// Main component
// -------------------------------------------------------------------
interface ImageDiffViewProps {
diff: BinaryImageDiff;
chatId: string;
/** The current feature branch (head). */
branch?: string;
/** The base branch to compare against for deletions/modifications. */
baseBranch?: string;
}
/**
* Renders a visual diff for binary image files, similar to GitHub's
* image diff UI. Supports added, deleted, and modified images with
* appropriate visual treatment for each change type.
*
* Images are fetched through the backend proxy endpoint which
* handles GitHub authentication, so this works for both public
* and private repositories.
*/
export const ImageDiffView: FC<ImageDiffViewProps> = ({
diff,
chatId,
branch,
baseBranch,
}) => {
const fileName = diff.name.split("/").pop() ?? diff.name;
const baseRef = baseBranch ?? "main";
return (
<div className="border-b border-solid border-border-default">
{/* Sticky file header — matches the existing diff panel style. */}
<div
className="sticky top-0 z-10 flex items-center gap-2 border-b border-solid border-border-default bg-surface-secondary px-3 py-1.5"
style={{ fontSize: 13 }}
>
<FileIcon fileName={fileName} className="shrink-0" />
<span className="min-w-0 truncate text-content-primary">
{diff.name}
</span>
<span
role="img"
className={cn(
"ml-auto shrink-0 text-xs font-medium",
badgeColor(diff.changeType),
)}
title={CHANGE_TYPE_LABELS[diff.changeType]}
aria-label={CHANGE_TYPE_LABELS[diff.changeType]}
>
{" "}
{badgeLetter(diff.changeType)}
</span>
</div>
{/* Image content area */}
<div className="p-4">
{diff.changeType === "new" && (
<div className="rounded border-l-4 border-solid border-green-600 pl-4">
<span className="mb-2 inline-block rounded bg-green-100 px-2 py-0.5 text-xs font-medium text-green-800 dark:bg-green-950 dark:text-green-300">
Added
</span>
{branch ? (
<ProxiedDiffImage
chatId={chatId}
filePath={diff.name}
gitRef={branch}
alt={`Added: ${diff.name}`}
/>
) : (
<NoBranchPlaceholder action="added" />
)}
</div>
)}
{diff.changeType === "deleted" && (
<div className="rounded border-l-4 border-solid border-red-600 pl-4">
<span className="mb-2 inline-block rounded bg-red-100 px-2 py-0.5 text-xs font-medium text-red-800 dark:bg-red-950 dark:text-red-300">
Removed
</span>
<ProxiedDiffImage
chatId={chatId}
filePath={diff.name}
gitRef={baseRef}
alt={`Removed: ${diff.name}`}
/>
</div>
)}
{diff.changeType === "change" && (
<div className="grid grid-cols-1 gap-4 sm:grid-cols-2">
<div>
<span className="mb-2 block text-xs font-medium text-content-secondary">
Before
</span>
<ProxiedDiffImage
chatId={chatId}
filePath={diff.name}
gitRef={baseRef}
alt={`Before: ${diff.name}`}
/>
</div>
<div>
<span className="mb-2 block text-xs font-medium text-content-secondary">
After
</span>
{branch ? (
<ProxiedDiffImage
chatId={chatId}
filePath={diff.name}
gitRef={branch}
alt={`After: ${diff.name}`}
/>
) : (
<NoBranchPlaceholder action="modified" />
)}
</div>
</div>
)}
</div>
</div>
);
};