fix: restore download functionality for text-based attachments (#1244)

BYK · web-flow · commit 66f6b90777b9 · 2026-01-08T21:41:52.000Z
## Problem The refactoring in #1237 accidentally broke download links for text-based attachments (txt, csv, json, code files) by returning `undefined` instead of creating a blob URL. ## Solution Restore the original behavior where all content types get a blob URL for downloading. The `else` branch now creates a blob from the raw attachment string instead of returning `undefined`. ## Testing All 172 tests pass.
diff --git a/packages/spotlight/src/ui/telemetry/components/insights/envelopes/Attachment.test.tsx b/packages/spotlight/src/ui/telemetry/components/insights/envelopes/Attachment.test.tsx
@@ -0,0 +1,221 @@
+import { render, screen } from "@testing-library/react";
+import { beforeEach, describe, expect, it, vi } from "vitest";
+import Attachment from "./Attachment";
+
+// Mock SVG imports
+vi.mock("@spotlight/ui/assets/download.svg", () => ({
+  ReactComponent: () => null,
+}));
+
+// Mock URL.createObjectURL and URL.revokeObjectURL
+const mockCreateObjectURL = vi.fn(() => "blob:mock-url");
+const mockRevokeObjectURL = vi.fn();
+
+beforeEach(() => {
+  vi.clearAllMocks();
+  global.URL.createObjectURL = mockCreateObjectURL;
+  global.URL.revokeObjectURL = mockRevokeObjectURL;
+});
+
+describe("Attachment", () => {
+  describe("download URL generation", () => {
+    it("should create download URL for text/plain attachments", () => {
+      render(
+        <Attachment
+          header={{ type: "attachment", content_type: "text/plain", filename: "test.txt" }}
+          attachment="Hello, World!"
+          expanded={true}
+        />,
+      );
+
+      expect(mockCreateObjectURL).toHaveBeenCalledTimes(1);
+      const blob = mockCreateObjectURL.mock.calls[0][0] as Blob;
+      expect(blob).toBeInstanceOf(Blob);
+      expect(blob.type).toBe("text/plain");
+    });
+
+    it("should create download URL for text/csv attachments", () => {
+      render(
+        <Attachment
+          header={{ type: "attachment", content_type: "text/csv", filename: "data.csv" }}
+          attachment="name,value\nfoo,bar"
+          expanded={true}
+        />,
+      );
+
+      expect(mockCreateObjectURL).toHaveBeenCalledTimes(1);
+      const blob = mockCreateObjectURL.mock.calls[0][0] as Blob;
+      expect(blob.type).toBe("text/csv");
+    });
+
+    it("should create download URL for application/json attachments", () => {
+      render(
+        <Attachment
+          header={{ type: "attachment", content_type: "application/json", filename: "config.json" }}
+          attachment='{"key": "value"}'
+          expanded={true}
+        />,
+      );
+
+      expect(mockCreateObjectURL).toHaveBeenCalledTimes(1);
+      const blob = mockCreateObjectURL.mock.calls[0][0] as Blob;
+      expect(blob.type).toBe("application/json");
+    });
+
+    it("should create download URL for text/javascript attachments", () => {
+      render(
+        <Attachment
+          header={{ type: "attachment", content_type: "text/javascript", filename: "script.js" }}
+          attachment="console.log('hello');"
+          expanded={true}
+        />,
+      );
+
+      expect(mockCreateObjectURL).toHaveBeenCalledTimes(1);
+      const blob = mockCreateObjectURL.mock.calls[0][0] as Blob;
+      expect(blob.type).toBe("text/javascript");
+    });
+
+    it("should create download URL for image/png attachments with base64 data", () => {
+      // Valid base64 for a tiny 1x1 PNG
+      const base64Png =
+        "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mNk+M9QDwADhgGAWjR9awAAAABJRU5ErkJggg==";
+
+      render(
+        <Attachment
+          header={{ type: "attachment", content_type: "image/png", filename: "screenshot.png" }}
+          attachment={base64Png}
+          expanded={true}
+        />,
+      );
+
+      expect(mockCreateObjectURL).toHaveBeenCalledTimes(1);
+      const blob = mockCreateObjectURL.mock.calls[0][0] as Blob;
+      expect(blob.type).toBe("image/png");
+    });
+
+    it("should not create download URL when not expanded", () => {
+      render(
+        <Attachment
+          header={{ type: "attachment", content_type: "text/plain", filename: "test.txt" }}
+          attachment="Hello, World!"
+          expanded={false}
+        />,
+      );
+
+      expect(mockCreateObjectURL).not.toHaveBeenCalled();
+    });
+  });
+
+  describe("download link", () => {
+    it("should render download link with correct filename", () => {
+      render(
+        <Attachment
+          header={{ type: "attachment", content_type: "text/plain", filename: "my-file.txt" }}
+          attachment="content"
+          expanded={true}
+        />,
+      );
+
+      const link = screen.getByRole("link");
+      expect(link.getAttribute("download")).toBe("my-file.txt");
+      expect(link.getAttribute("href")).toBe("blob:mock-url");
+    });
+
+    it("should use inferred filename when not provided", () => {
+      render(
+        <Attachment
+          header={{ type: "attachment", content_type: "application/json" }}
+          attachment='{"key": "value"}'
+          expanded={true}
+        />,
+      );
+
+      const link = screen.getByRole("link");
+      expect(link.getAttribute("download")).toBe("untitled.json");
+    });
+  });
+
+  describe("error handling", () => {
+    it("should show error message for invalid base64 image data", () => {
+      const { container } = render(
+        <Attachment
+          header={{ type: "attachment", content_type: "image/png", filename: "bad.png" }}
+          attachment="this is not valid base64!!!"
+          expanded={true}
+        />,
+      );
+
+      expect(container.textContent).toContain("Failed to decode attachment data");
+    });
+  });
+
+  describe("content preview", () => {
+    it("should render text content for text/plain", () => {
+      const { container } = render(
+        <Attachment
+          header={{ type: "attachment", content_type: "text/plain", filename: "test.txt" }}
+          attachment="Hello, World!"
+          expanded={true}
+        />,
+      );
+
+      expect(container.textContent).toContain("Hello, World!");
+    });
+
+    it("should render JSON viewer for application/json", () => {
+      const { container } = render(
+        <Attachment
+          header={{ type: "attachment", content_type: "application/json", filename: "data.json" }}
+          attachment='{"name": "test"}'
+          expanded={true}
+        />,
+      );
+
+      // JSON viewer should parse and display the content
+      expect(container.textContent).toContain("name");
+    });
+
+    it("should show parse error for invalid JSON", () => {
+      const { container } = render(
+        <Attachment
+          header={{ type: "attachment", content_type: "application/json", filename: "bad.json" }}
+          attachment="not valid json"
+          expanded={true}
+        />,
+      );
+
+      expect(container.textContent).toContain("Failed to parse JSON attachment");
+    });
+
+    it("should show expand prompt when not expanded", () => {
+      const { container } = render(
+        <Attachment
+          header={{ type: "attachment", content_type: "text/plain", filename: "test.txt" }}
+          attachment="Hello"
+          expanded={false}
+        />,
+      );
+
+      expect(container.textContent).toContain("Expand to preview attachment");
+    });
+  });
+
+  describe("cleanup", () => {
+    it("should revoke blob URL on unmount", () => {
+      const { unmount } = render(
+        <Attachment
+          header={{ type: "attachment", content_type: "text/plain", filename: "test.txt" }}
+          attachment="content"
+          expanded={true}
+        />,
+      );
+
+      expect(mockCreateObjectURL).toHaveBeenCalledTimes(1);
+
+      unmount();
+
+      expect(mockRevokeObjectURL).toHaveBeenCalledWith("blob:mock-url");
+    });
+  });
+});
diff --git a/packages/spotlight/src/ui/telemetry/components/insights/envelopes/Attachment.tsx b/packages/spotlight/src/ui/telemetry/components/insights/envelopes/Attachment.tsx
@@ -23,8 +23,8 @@ export default function Attachment({
   const extension = inferExtension(header.content_type as string | null, header.type as string | null);
   const name = (header.filename as string) || `untitled.${extension}`;
 
-  // Create download URL for binary content types
-  // Returns: string (success), null (decode error)
+  // Create download URL for attachment
+  // Returns: string (success), null (decode error), undefined (not expanded yet)
   const downloadUrl = useMemo(() => {
     if (!expanded) {
       return undefined; // Not needed yet
@@ -34,19 +34,22 @@ export default function Attachment({
     let blobData: BlobPart;
 
     if (IMAGE_CONTENT_TYPES.has(contentType) || VIDEO_CONTENT_TYPES.has(contentType)) {
+      // Binary content types are base64-encoded
       const decoded = base64Decode(attachment);
       if (!decoded) {
         return null; // Decode error
       }
       blobData = decoded.buffer as BlobPart;
     } else if (extension === "bin") {
+      // Unknown binary files are base64-encoded
       const decoded = safeAtob(attachment);
       if (decoded === null) {
         return null; // Decode error
       }
       blobData = decoded;
     } else {
-      return undefined; // Not a binary type, no blob URL needed
+      // Text-based content types (json, txt, csv, code, etc.) - use raw attachment
+      blobData = attachment;
     }
 
     const blob = new Blob([blobData], { type: contentType || "application/octet-stream" });
