Skip to content

Commit 69765af

Browse files
committed
Name the existing attachment in the duplicate-paste flash and prove the hash source
The rejection flash was reporting the rejected paste's own name, which the operator never saw -- every clipboard read gets a fresh clipboard-<timestamp>.png, so the message pointed at a filename that didn't match the chip already in their prompt. Report the attachment already in the pending set instead. Also add a producer-level test against imageAttachmentFromPath with an oversized fixture, so the pre-cap hashing is pinned by more than the comment above it; make the hash helper module-private since nothing outside this file needs it.
1 parent 52ca3c2 commit 69765af

4 files changed

Lines changed: 45 additions & 8 deletions

File tree

src/tui-opentui/prompt-features.test.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,11 @@ describe("image attachments", () => {
171171
expect(await attachClipboardImage(shell)).toBe(false)
172172
expect(shell.pendingAttachments).toHaveLength(1)
173173
expect(shell.pendingAttachments[0]?.id).toBe("clip-1")
174-
expect(shell.statusFlash).toContain("already attached")
174+
// Names the attachment already sitting in the pending set (CLIP), not
175+
// the rejected paste (CLIP_SAME_CONTENT) -- the operator never saw the
176+
// rejected paste's filename, so naming it would read as a bug.
177+
expect(shell.statusFlash).toContain(`${CLIP.name} is already attached`)
178+
expect(shell.statusFlash).not.toContain(CLIP_SAME_CONTENT.name)
175179
})
176180
})
177181

src/tui-opentui/shell.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -932,11 +932,11 @@ export async function attachClipboardImage(shell: AppShell): Promise<boolean> {
932932
setStatusFlash(shell, `image attach failed: ${result.reason}`)
933933
return false
934934
}
935-
const isDuplicate = shell.pendingAttachments.some(
935+
const duplicate = shell.pendingAttachments.find(
936936
(attachment) => attachment.contentHash === result.attachment.contentHash,
937937
)
938-
if (isDuplicate) {
939-
setStatusFlash(shell, `${result.attachment.name} is already attached`)
938+
if (duplicate !== undefined) {
939+
setStatusFlash(shell, `${duplicate.name} is already attached`)
940940
return false
941941
}
942942
addPendingAttachment(shell, result.attachment)

src/tui/image-attachments.test.ts

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,14 @@
11
import { describe, expect, test } from "bun:test";
22
import { deflateSync } from "node:zlib";
33
import { unlink } from "node:fs/promises";
4+
import { tmpdir } from "node:os";
5+
import { join } from "node:path";
46
import {
57
extractPastedImagePaths,
68
findImagePathMentions,
79
imageMimeTypeForPath,
810
capImageForIngestion,
11+
imageAttachmentFromPath,
912
MAX_IMAGE_DIMENSION,
1013
} from "./image-attachments.js";
1114

@@ -150,4 +153,31 @@ describe("image attachment helpers", () => {
150153
expect(result.data).toBe(large);
151154
expect(result.contentType).toBe("image/png");
152155
});
156+
157+
// The dedupe fix (see shell.ts attachClipboardImage) rests entirely on this:
158+
// two ingests of identical source bytes must hash identically even though
159+
// capImageForIngestion re-encodes oversized images through `sips`, whose
160+
// JPEG output is not byte-stable across runs. Sizing the fixture above
161+
// DOWNSCALE_THRESHOLD_BYTES (300 KB) exercises that re-encode path -- a
162+
// small fixture would pass even if the hash were taken after capping.
163+
test("hashes identical source bytes the same regardless of filename, even through the sips recompression path", async () => {
164+
const bytes = buildTestPng(1200, 1200);
165+
expect(bytes.byteLength).toBeGreaterThan(300 * 1024);
166+
167+
const pathA = join(tmpdir(), `corbits-hash-test-a-${process.pid}-${Date.now()}.png`);
168+
const pathB = join(tmpdir(), `corbits-hash-test-b-${process.pid}-${Date.now()}.png`);
169+
await Bun.write(pathA, bytes);
170+
await Bun.write(pathB, bytes);
171+
try {
172+
const resultA = await imageAttachmentFromPath(pathA);
173+
const resultB = await imageAttachmentFromPath(pathB);
174+
if (!resultA.ok || !resultB.ok) throw new Error("expected both ingests to succeed");
175+
176+
expect(resultA.attachment.contentHash).toBe(resultB.attachment.contentHash);
177+
expect(resultA.attachment.id).not.toBe(resultB.attachment.id);
178+
} finally {
179+
await unlink(pathA).catch(() => undefined);
180+
await unlink(pathB).catch(() => undefined);
181+
}
182+
});
153183
});

src/tui/image-attachments.ts

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ const IMAGE_MIME_BY_EXT: Readonly<Record<string, string>> = {
2727
export type PendingImageAttachment = MessageAttachment & {
2828
id: string;
2929
path?: string;
30-
/** SHA-256 of the decoded image bytes, used to dedupe repeat pastes. */
30+
/** SHA-256 of the source image file's bytes, used to dedupe repeat pastes. */
3131
contentHash: string;
3232
};
3333

@@ -110,9 +110,12 @@ export async function imageAttachmentFromPath(path: string): Promise<AttachImage
110110
};
111111
}
112112

113-
/** SHA-256 of decoded image bytes, used to identify identical pastes regardless of filename or timing. */
114-
export async function hashImageBytes(bytes: Uint8Array): Promise<string> {
115-
const digest = await crypto.subtle.digest("SHA-256", new Uint8Array(bytes));
113+
/** SHA-256 of the source image file's bytes, used to identify identical pastes regardless of filename or timing. */
114+
async function hashImageBytes(bytes: Buffer): Promise<string> {
115+
// Buffer's type parameter is the looser ArrayBufferLike (it may back onto a
116+
// pooled allocation), but readFile never actually hands back a
117+
// SharedArrayBuffer-backed view, so this is a type-only cast, not a copy.
118+
const digest = await crypto.subtle.digest("SHA-256", bytes as BufferSource);
116119
return [...new Uint8Array(digest)].map((b) => b.toString(16).padStart(2, "0")).join("");
117120
}
118121

0 commit comments

Comments
 (0)