From 4c203f17cb6e47d284d452c9407e4003b504c5b2 Mon Sep 17 00:00:00 2001 From: HeavyGee <133152184+heavygee@users.noreply.github.com> Date: Wed, 29 Jul 2026 03:05:24 +0100 Subject: [PATCH] feat(web,hub): scratchlist v2.2 hub attachment storage (#921) (#1205) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(hub,shared): scratchlist v2.2 hub attachment storage foundation (#921) Hub stores scratchlist attachment bytes on filesystem; SQLite holds AttachmentMetadata[] JSON via session_scratchlist.attachments (v11→v12). Upstream ladder: v10→v11 text-only scratchlist table (#896), v11→v12 attachments column. Configurable limits via HAPI_SCRATCHLIST_* env vars. Upload, serve, and limits REST routes; delete entry cleans hub files. Web promote/rehydrate still TODO. Soup renumber branch follows. Co-authored-by: Cursor * feat(web): scratchlist v2.2 attachment UX (#921) Route scratchlist-mode composer submits with attachments to hub storage, show image thumbnails in the drawer, and rehydrate attachments on promote to composer or queue (hub fetch → CLI upload for send). Co-authored-by: Cursor * fix(web): scratchlist attach submit, float thumbs, copy tooltip (#921) Hub upload adapter now sets path on ready attachments so the composer send button unlocks in scratchlist mode; routing label matches attachments too. Entry thumbnails float left with text wrap; copy tooltip clarifies text-only. Co-authored-by: Cursor * fix(hub): adapt scratchlist update tests to patch API (#921) update() now takes { text?, attachments? }; v12 CRUD tests still passed a string. Co-authored-by: Cursor * fix(hub,web): harden scratchlist attachment ownership and orphan cleanup Resolve claimed hub paths against the current session before persist, count on-disk session bytes for upload caps, delete blobs dropped on entry update, and DELETE pending uploads when composer remove runs. Co-authored-by: Cursor * chore: drop accidental .cursor files from attachment PR Co-authored-by: Cursor * fix(web): exit scratchlist mode before rehydrate; delete raced uploads Promote-to-composer flushes mode exit so attachments use the chat adapter. Cancel-during-upload deletes the hub blob once upload returns. Co-authored-by: Cursor * fix(hub,web): exact UUID delete match; stage hub paths on chat send Reject partial attachment ids on disk delete, and restage scratchlist hub attachments through uploadFile when sending after leaving scratchlist mode. Co-authored-by: Cursor * fix(hub): skip text-only PUT resolve; cleanup session attachment dirs Text-only edits keep existing attachment metadata after session-id transfer. Require full UUID on resolve. Delete scratchlist attachment files when a session is deleted. Co-authored-by: Cursor * fix(hub,web): scratchlist attach route, PUT bytes, orphan deletes Park only hub-resident attachments; subtract removed blobs from the PUT session cap; delete attachment files only when no other entry still references them. Co-authored-by: Cursor * fix(hub): canonicalize scratchlist attachment filenames Resolve stores the on-disk sanitized name (not claimed.filename) and hardens Content-Disposition against CR/LF/quote injection. Co-authored-by: Cursor * test(hub): cover toxic filename canonicalize on resolve Co-authored-by: Cursor * fix(hub,web): serialize scratchlist uploads; drop hub blobs after chat stage Per-session upload lock keeps disk byte caps honest under concurrency. After a successful toggle-off chat send, delete the staged hub copies so they no longer count against the session attachment budget. Co-authored-by: Cursor * fix(shared,web): allow clearing scratchlist attachments; cleanup staged uploads PUT may send attachments:[] without a text change. Staging to chat rolls back partial normal-upload copies on failure. Co-authored-by: Cursor * fix(hub): re-key scratchlist attachment files on session merge Move hub blobs when scratchlist rows transfer between session ids so quota and path ownership stay correct. Reject PUT that would leave an empty textless entry after clearing attachments. Co-authored-by: Cursor * fix(web): reuse restored scratchlist hub attachments without re-upload Composer draft remount was re-uploading blobs that already had a hapi-hub:scratchlist path, orphaning the originals against session quota. Co-authored-by: Cursor --------- Co-authored-by: Cursor --- hub/src/config/scratchlistAttachmentLimits.ts | 46 +++ .../scratchlistAttachments/storage.test.ts | 151 ++++++++ hub/src/scratchlistAttachments/storage.ts | 334 ++++++++++++++++++ .../scratchlistAttachments/validate.test.ts | 85 +++++ hub/src/scratchlistAttachments/validate.ts | 73 ++++ hub/src/store/index.ts | 20 +- hub/src/store/migration-v12.test.ts | 4 +- hub/src/store/migration-v13.test.ts | 6 +- hub/src/store/migration-v15.test.ts | 180 ++++++++++ hub/src/store/scratchlist.ts | 59 +++- hub/src/store/scratchlistStore.ts | 18 +- hub/src/store/types.ts | 1 + .../sessionCache-merge-scratchlist.test.ts | 50 +++ hub/src/sync/sessionCache.ts | 43 +++ hub/src/sync/syncEngine-scratchlist.test.ts | 4 +- hub/src/sync/syncEngine.ts | 191 +++++++++- .../web/routes/sessions-scratchlist.test.ts | 78 +++- hub/src/web/routes/sessions.ts | 206 ++++++++++- shared/src/apiTypes.ts | 33 +- shared/src/index.ts | 1 + shared/src/schemas.ts | 9 +- shared/src/scratchlistAttachments.ts | 89 +++++ web/src/api/client.ts | 77 +++- .../AssistantChat/ComposerButtons.test.tsx | 7 +- .../AssistantChat/ComposerButtons.tsx | 20 +- .../AssistantChat/ScratchlistPanel.test.tsx | 6 +- .../AssistantChat/ScratchlistPanel.tsx | 110 +++++- .../components/SessionChat.exit-mode.test.tsx | 21 +- web/src/components/SessionChat.test.ts | 28 +- web/src/components/SessionChat.tsx | 120 ++++--- web/src/lib/locales/en.ts | 3 +- web/src/lib/locales/zh-CN.ts | 3 +- web/src/lib/scratchlist.ts | 3 + .../lib/scratchlistAttachmentAdapter.test.ts | 138 ++++++++ web/src/lib/scratchlistAttachmentAdapter.ts | 234 ++++++++++++ web/src/lib/scratchlistAttachmentFlow.ts | 70 ++++ web/src/lib/use-hub-scratchlist.ts | 30 +- 37 files changed, 2391 insertions(+), 160 deletions(-) create mode 100644 hub/src/config/scratchlistAttachmentLimits.ts create mode 100644 hub/src/scratchlistAttachments/storage.test.ts create mode 100644 hub/src/scratchlistAttachments/storage.ts create mode 100644 hub/src/scratchlistAttachments/validate.test.ts create mode 100644 hub/src/scratchlistAttachments/validate.ts create mode 100644 hub/src/store/migration-v15.test.ts create mode 100644 shared/src/scratchlistAttachments.ts create mode 100644 web/src/lib/scratchlistAttachmentAdapter.test.ts create mode 100644 web/src/lib/scratchlistAttachmentAdapter.ts create mode 100644 web/src/lib/scratchlistAttachmentFlow.ts diff --git a/hub/src/config/scratchlistAttachmentLimits.ts b/hub/src/config/scratchlistAttachmentLimits.ts new file mode 100644 index 00000000..a019da47 --- /dev/null +++ b/hub/src/config/scratchlistAttachmentLimits.ts @@ -0,0 +1,46 @@ +import { + SCRATCHLIST_ATTACHMENT_DEFAULT_LIMITS, + ScratchlistAttachmentLimitsSchema, + type ScratchlistAttachmentLimits, +} from '@hapi/protocol' + +function parsePositiveInt(raw: string | undefined, fallback: number): number { + if (!raw) return fallback + const parsed = Number.parseInt(raw, 10) + return Number.isFinite(parsed) && parsed > 0 ? parsed : fallback +} + +function parseMimeList(raw: string | undefined): string[] { + if (!raw) return SCRATCHLIST_ATTACHMENT_DEFAULT_LIMITS.allowedMimeTypes + const entries = raw.split(',').map((s) => s.trim()).filter(Boolean) + return entries.length > 0 ? entries : SCRATCHLIST_ATTACHMENT_DEFAULT_LIMITS.allowedMimeTypes +} + +export function loadScratchlistAttachmentLimitsFromEnv(): ScratchlistAttachmentLimits { + const candidate = { + maxBytesPerFile: parsePositiveInt( + process.env.HAPI_SCRATCHLIST_MAX_ATTACHMENT_BYTES_PER_FILE, + SCRATCHLIST_ATTACHMENT_DEFAULT_LIMITS.maxBytesPerFile + ), + maxAttachmentsPerEntry: parsePositiveInt( + process.env.HAPI_SCRATCHLIST_MAX_ATTACHMENTS_PER_ENTRY, + SCRATCHLIST_ATTACHMENT_DEFAULT_LIMITS.maxAttachmentsPerEntry + ), + maxBytesPerEntry: parsePositiveInt( + process.env.HAPI_SCRATCHLIST_MAX_ATTACHMENT_BYTES_PER_ENTRY, + SCRATCHLIST_ATTACHMENT_DEFAULT_LIMITS.maxBytesPerEntry + ), + maxBytesPerSession: parsePositiveInt( + process.env.HAPI_SCRATCHLIST_MAX_ATTACHMENT_BYTES_PER_SESSION, + SCRATCHLIST_ATTACHMENT_DEFAULT_LIMITS.maxBytesPerSession + ), + allowedMimeTypes: parseMimeList(process.env.HAPI_SCRATCHLIST_ALLOWED_ATTACHMENT_MIMES), + } + const parsed = ScratchlistAttachmentLimitsSchema.safeParse(candidate) + return parsed.success ? parsed.data : SCRATCHLIST_ATTACHMENT_DEFAULT_LIMITS +} + +export function isAllowedScratchlistMime(mimeType: string, limits: ScratchlistAttachmentLimits): boolean { + const normalized = mimeType.trim().toLowerCase() + return limits.allowedMimeTypes.some((allowed) => allowed.toLowerCase() === normalized) +} diff --git a/hub/src/scratchlistAttachments/storage.test.ts b/hub/src/scratchlistAttachments/storage.test.ts new file mode 100644 index 00000000..a20d05e5 --- /dev/null +++ b/hub/src/scratchlistAttachments/storage.test.ts @@ -0,0 +1,151 @@ +import { describe, expect, it } from 'bun:test' +import { mkdtempSync, rmSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' + +import { + deleteScratchlistAttachmentById, + moveScratchlistAttachmentFilesForSession, + resolveScratchlistAttachmentsForSession, + sumScratchlistAttachmentBytesOnDisk, + writeScratchlistAttachmentFile, +} from './storage' + +describe('scratchlistAttachments storage security', () => { + it('resolves only attachments owned by the session namespace path', async () => { + const hapiHome = mkdtempSync(join(tmpdir(), 'hapi-scratchlist-storage-')) + try { + const owned = await writeScratchlistAttachmentFile( + hapiHome, + 'default', + 'session-a', + 'a.png', + 'image/png', + Buffer.from('aaa') + ) + const other = await writeScratchlistAttachmentFile( + hapiHome, + 'default', + 'session-b', + 'b.png', + 'image/png', + Buffer.from('bbbb') + ) + + const ok = await resolveScratchlistAttachmentsForSession( + hapiHome, + 'default', + 'session-a', + [owned] + ) + expect(ok.ok).toBe(true) + if (ok.ok) { + expect(ok.attachments[0]?.size).toBe(3) + expect(ok.attachments[0]?.path).toBe(owned.path) + } + + const forged = await resolveScratchlistAttachmentsForSession( + hapiHome, + 'default', + 'session-a', + [{ ...other, id: other.id }] + ) + expect(forged.ok).toBe(false) + + const disk = await sumScratchlistAttachmentBytesOnDisk(hapiHome, 'default', 'session-a') + expect(disk).toBe(3) + + const deleted = await deleteScratchlistAttachmentById( + hapiHome, + 'default', + 'session-a', + owned.id + ) + expect(deleted).toBe(true) + expect(await sumScratchlistAttachmentBytesOnDisk(hapiHome, 'default', 'session-a')).toBe(0) + + const otherAgain = await writeScratchlistAttachmentFile( + hapiHome, + 'default', + 'session-a', + 'c.png', + 'image/png', + Buffer.from('ccc') + ) + const partialId = otherAgain.id.split('-')[0]! + expect(partialId.length).toBe(8) + expect( + await deleteScratchlistAttachmentById(hapiHome, 'default', 'session-a', partialId) + ).toBe(false) + expect(await sumScratchlistAttachmentBytesOnDisk(hapiHome, 'default', 'session-a')).toBe(3) + + const aliased = await resolveScratchlistAttachmentsForSession( + hapiHome, + 'default', + 'session-a', + [{ ...otherAgain, id: partialId, path: otherAgain.path }] + ) + expect(aliased.ok).toBe(false) + + const toxic = await writeScratchlistAttachmentFile( + hapiHome, + 'default', + 'session-a', + 'ok.png', + 'image/png', + Buffer.from('dd') + ) + const resolvedToxic = await resolveScratchlistAttachmentsForSession( + hapiHome, + 'default', + 'session-a', + [{ + ...toxic, + filename: 'evil\r\nContent-Type: text/html".png', + }] + ) + expect(resolvedToxic.ok).toBe(true) + if (resolvedToxic.ok) { + expect(resolvedToxic.attachments[0]?.filename).toBe('ok.png') + expect(resolvedToxic.attachments[0]?.filename).not.toMatch(/[\r\n"]/) + } + } finally { + rmSync(hapiHome, { recursive: true, force: true }) + } + }) + + it('re-keys attachment files when a session id is transferred', async () => { + const hapiHome = mkdtempSync(join(tmpdir(), 'hapi-scratchlist-move-')) + try { + const written = await writeScratchlistAttachmentFile( + hapiHome, + 'default', + 'session-old', + 'pic.png', + 'image/png', + Buffer.from('payload') + ) + const moved = await moveScratchlistAttachmentFilesForSession( + hapiHome, + 'default', + 'session-old', + 'session-new', + [written] + ) + expect(moved[0]?.path).toContain('/session-new/') + expect(moved[0]?.path).not.toContain('/session-old/') + expect(await sumScratchlistAttachmentBytesOnDisk(hapiHome, 'default', 'session-old')).toBe(0) + expect(await sumScratchlistAttachmentBytesOnDisk(hapiHome, 'default', 'session-new')).toBe(7) + + const resolved = await resolveScratchlistAttachmentsForSession( + hapiHome, + 'default', + 'session-new', + moved + ) + expect(resolved.ok).toBe(true) + } finally { + rmSync(hapiHome, { recursive: true, force: true }) + } + }) +}) diff --git a/hub/src/scratchlistAttachments/storage.ts b/hub/src/scratchlistAttachments/storage.ts new file mode 100644 index 00000000..689aa5c5 --- /dev/null +++ b/hub/src/scratchlistAttachments/storage.ts @@ -0,0 +1,334 @@ +import { mkdir, readdir, readFile, rename, rm, stat, writeFile } from 'node:fs/promises' +import { homedir } from 'node:os' +import { join, resolve, sep } from 'node:path' +import { randomUUID } from 'node:crypto' + +import type { ScratchlistAttachmentMetadata } from '@hapi/protocol' +import { + toHubScratchlistAttachmentPath, + parseHubScratchlistAttachmentPath, +} from '@hapi/protocol' + +function sanitizeFilename(filename: string): string { + // Strip path tricks and Content-Disposition hazards (CR/LF/quotes/controls). + const sanitized = filename + .replace(/[/\\]/g, '_') + .replace(/\.\./g, '_') + .replace(/[\r\n\0"\\]/g, '_') + .replace(/[\u0000-\u001f\u007f]/g, '_') + .replace(/\s+/g, '_') + .slice(0, 255) + return sanitized || 'upload' +} + +function sanitizeSegment(segment: string): string { + return segment.replace(/[/\\]/g, '_').replace(/\.\./g, '_').slice(0, 128) +} + +export function getHapiHomeDir(): string { + return process.env.HAPI_HOME || join(homedir(), '.hapi') +} + +export function getScratchlistAttachmentsRoot(hapiHome: string = getHapiHomeDir()): string { + return join(hapiHome, 'scratchlist-attachments') +} + +export function buildScratchlistStorageKey( + namespace: string, + sessionId: string, + attachmentId: string, + filename: string +): string { + return `${sanitizeSegment(namespace)}/${sanitizeSegment(sessionId)}/${attachmentId}-${sanitizeFilename(filename)}` +} + +export function resolveScratchlistStoragePath(hapiHome: string, storageKey: string): string { + const root = resolve(getScratchlistAttachmentsRoot(hapiHome)) + const resolved = resolve(root, storageKey) + const prefix = root.endsWith(sep) ? root : `${root}${sep}` + if (!resolved.startsWith(prefix)) { + throw new Error('Invalid scratchlist attachment path') + } + return resolved +} + +export async function writeScratchlistAttachmentFile( + hapiHome: string, + namespace: string, + sessionId: string, + filename: string, + mimeType: string, + buffer: Buffer, + attachmentId: string = randomUUID() +): Promise { + const storageKey = buildScratchlistStorageKey(namespace, sessionId, attachmentId, filename) + const filePath = resolveScratchlistStoragePath(hapiHome, storageKey) + await mkdir(join(filePath, '..'), { recursive: true }) + await writeFile(filePath, buffer) + return { + id: attachmentId, + filename: sanitizeFilename(filename), + mimeType, + size: buffer.length, + path: toHubScratchlistAttachmentPath(storageKey), + } +} + +export async function readScratchlistAttachmentFile( + hapiHome: string, + hubPath: string +): Promise<{ buffer: Buffer; metadataPath: string } | null> { + const storageKey = parseHubScratchlistAttachmentPath(hubPath) + if (!storageKey) return null + const filePath = resolveScratchlistStoragePath(hapiHome, storageKey) + try { + const buffer = await readFile(filePath) + return { buffer, metadataPath: filePath } + } catch { + return null + } +} + +export async function deleteScratchlistAttachmentFile( + hapiHome: string, + hubPath: string +): Promise { + const storageKey = parseHubScratchlistAttachmentPath(hubPath) + if (!storageKey) return false + const filePath = resolveScratchlistStoragePath(hapiHome, storageKey) + try { + await rm(filePath, { force: true }) + return true + } catch { + return false + } +} + +export async function deleteScratchlistAttachmentFiles( + hapiHome: string, + attachments: ScratchlistAttachmentMetadata[] +): Promise { + await Promise.all(attachments.map((att) => deleteScratchlistAttachmentFile(hapiHome, att.path))) +} + +/** + * Re-key hub attachment files when a scratchlist row is transferred to a new + * session id (merge/dedup). Paths embed the session id; without this step + * quota sums miss the bytes and resolve rejects the path as out-of-session. + * If the destination file already exists, keep it and delete the source + * (same "target wins" rule as SQL transfer collisions). + */ +export async function moveScratchlistAttachmentFilesForSession( + hapiHome: string, + namespace: string, + oldSessionId: string, + newSessionId: string, + attachments: ScratchlistAttachmentMetadata[] +): Promise { + if (oldSessionId === newSessionId || attachments.length === 0) { + return attachments + } + const oldPrefix = sessionStoragePrefix(namespace, oldSessionId) + const newPrefix = sessionStoragePrefix(namespace, newSessionId) + const moved: ScratchlistAttachmentMetadata[] = [] + for (const att of attachments) { + const storageKey = parseHubScratchlistAttachmentPath(att.path) + if (!storageKey || !storageKey.startsWith(oldPrefix)) { + moved.push(att) + continue + } + const fileName = storageKey.slice(oldPrefix.length) + const newKey = `${newPrefix}${fileName}` + let oldPath: string + let newPath: string + try { + oldPath = resolveScratchlistStoragePath(hapiHome, storageKey) + newPath = resolveScratchlistStoragePath(hapiHome, newKey) + } catch { + moved.push(att) + continue + } + await mkdir(join(newPath, '..'), { recursive: true }) + try { + const destExists = await stat(newPath).then((info) => info.isFile()).catch(() => false) + if (destExists) { + await rm(oldPath, { force: true }) + } else { + await rename(oldPath, newPath) + } + } catch { + // best effort — still rewrite metadata so quota/resolve use the new id + } + moved.push({ + ...att, + path: toHubScratchlistAttachmentPath(newKey), + }) + } + return moved +} + +export async function deleteScratchlistSessionAttachmentDir( + hapiHome: string, + namespace: string, + sessionId: string +): Promise { + const dir = resolveScratchlistStoragePath( + hapiHome, + `${sanitizeSegment(namespace)}/${sanitizeSegment(sessionId)}` + ) + try { + await rm(dir, { recursive: true, force: true }) + } catch { + // best effort + } +} + +export function estimateBase64Bytes(base64: string): number { + const len = base64.length + if (len === 0) return 0 + const padding = base64.endsWith('==') ? 2 : base64.endsWith('=') ? 1 : 0 + return Math.floor((len * 3) / 4) - padding +} + +function sessionStoragePrefix(namespace: string, sessionId: string): string { + return `${sanitizeSegment(namespace)}/${sanitizeSegment(sessionId)}/` +} + +/** + * Sum bytes of all files already written under the session's scratchlist + * attachment directory (includes pending uploads not yet referenced by an entry). + */ +export async function sumScratchlistAttachmentBytesOnDisk( + hapiHome: string, + namespace: string, + sessionId: string +): Promise { + const dir = resolveScratchlistStoragePath( + hapiHome, + `${sanitizeSegment(namespace)}/${sanitizeSegment(sessionId)}` + ) + let total = 0 + try { + const names = await readdir(dir) + for (const name of names) { + try { + const info = await stat(join(dir, name)) + if (info.isFile()) { + total += info.size + } + } catch { + // skip unreadable entries + } + } + } catch { + return 0 + } + return total +} + +export const SCRATCHLIST_ATTACHMENT_ID_RE = + /^[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i + +/** + * Verify claimed metadata points at a hub file owned by this namespace/session, + * then return server-authoritative metadata (size from disk). + */ +export async function resolveScratchlistAttachmentForSession( + hapiHome: string, + namespace: string, + sessionId: string, + claimed: ScratchlistAttachmentMetadata +): Promise< + | { ok: true; attachment: ScratchlistAttachmentMetadata } + | { ok: false; error: string } +> { + if (!SCRATCHLIST_ATTACHMENT_ID_RE.test(claimed.id)) { + return { ok: false, error: 'Invalid scratchlist attachment id' } + } + const storageKey = parseHubScratchlistAttachmentPath(claimed.path) + if (!storageKey) { + return { ok: false, error: 'Invalid scratchlist attachment path' } + } + const expectedPrefix = sessionStoragePrefix(namespace, sessionId) + if (!storageKey.startsWith(expectedPrefix)) { + return { ok: false, error: 'Attachment path is outside this session' } + } + const fileName = storageKey.slice(expectedPrefix.length) + if (!fileName.startsWith(`${claimed.id}-`)) { + return { ok: false, error: 'Attachment id does not match stored file' } + } + let filePath: string + try { + filePath = resolveScratchlistStoragePath(hapiHome, storageKey) + } catch { + return { ok: false, error: 'Invalid scratchlist attachment path' } + } + try { + const info = await stat(filePath) + if (!info.isFile()) { + return { ok: false, error: 'Attachment file missing' } + } + return { + ok: true, + attachment: { + id: claimed.id, + // Canonicalize from the on-disk key (already sanitized at write). + // Never trust claimed.filename for Content-Disposition later. + filename: sanitizeFilename(fileName.slice(`${claimed.id}-`.length)), + mimeType: claimed.mimeType, + size: info.size, + path: toHubScratchlistAttachmentPath(storageKey), + }, + } + } catch { + return { ok: false, error: 'Attachment file missing' } + } +} + +export async function resolveScratchlistAttachmentsForSession( + hapiHome: string, + namespace: string, + sessionId: string, + claimed: ScratchlistAttachmentMetadata[] +): Promise< + | { ok: true; attachments: ScratchlistAttachmentMetadata[] } + | { ok: false; error: string } +> { + const resolved: ScratchlistAttachmentMetadata[] = [] + for (const item of claimed) { + const result = await resolveScratchlistAttachmentForSession(hapiHome, namespace, sessionId, item) + if (!result.ok) { + return result + } + resolved.push(result.attachment) + } + return { ok: true, attachments: resolved } +} + +/** Delete a pending/orphan upload by attachment id from the session directory. */ +export async function deleteScratchlistAttachmentById( + hapiHome: string, + namespace: string, + sessionId: string, + attachmentId: string +): Promise { + // Require a full UUID so a partial first-segment like "a1b2c3d4" cannot + // startsWith-match `${uuid}-${filename}` and delete a still-referenced file. + if (!SCRATCHLIST_ATTACHMENT_ID_RE.test(attachmentId)) { + return false + } + const dir = resolveScratchlistStoragePath( + hapiHome, + `${sanitizeSegment(namespace)}/${sanitizeSegment(sessionId)}` + ) + try { + const names = await readdir(dir) + const prefix = `${attachmentId}-` + const match = names.find((name) => name.startsWith(prefix)) + if (!match) return false + await rm(join(dir, match), { force: true }) + return true + } catch { + return false + } +} diff --git a/hub/src/scratchlistAttachments/validate.test.ts b/hub/src/scratchlistAttachments/validate.test.ts new file mode 100644 index 00000000..38132d64 --- /dev/null +++ b/hub/src/scratchlistAttachments/validate.test.ts @@ -0,0 +1,85 @@ +import { describe, expect, it } from 'bun:test' +import { + scratchlistSessionBytesBeforeForPut, + validateScratchlistAttachmentsForWrite, +} from './validate' +import { + SCRATCHLIST_ATTACHMENT_DEFAULT_MAX_BYTES_PER_ENTRY, + SCRATCHLIST_ATTACHMENT_DEFAULT_MAX_BYTES_PER_FILE, + SCRATCHLIST_ATTACHMENT_DEFAULT_MAX_BYTES_PER_SESSION, + SCRATCHLIST_ATTACHMENT_DEFAULT_MAX_PER_ENTRY, +} from '@hapi/protocol' + +const limits = { + maxAttachmentsPerEntry: SCRATCHLIST_ATTACHMENT_DEFAULT_MAX_PER_ENTRY, + maxBytesPerFile: SCRATCHLIST_ATTACHMENT_DEFAULT_MAX_BYTES_PER_FILE, + maxBytesPerEntry: SCRATCHLIST_ATTACHMENT_DEFAULT_MAX_BYTES_PER_ENTRY, + maxBytesPerSession: SCRATCHLIST_ATTACHMENT_DEFAULT_MAX_BYTES_PER_SESSION, + allowedMimeTypes: ['image/png'], +} + +const eightMb = 8 * 1024 * 1024 + +describe('scratchlistSessionBytesBeforeForPut', () => { + it('subtracts removed blobs so replace-in-place does not double-count disk', () => { + // Old 8MB + new 8MB still on disk; PUT drops the old id. + // Without subtracting removed bytes, sessionBytesBefore would be 8MB and + // a 10MB session cap would falsely reject the replace. + const sessionBytesBefore = scratchlistSessionBytesBeforeForPut( + 16 * 1024 * 1024, + [{ size: eightMb }], + [{ size: eightMb }], + ) + expect(sessionBytesBefore).toBe(0) + const validation = validateScratchlistAttachmentsForWrite( + [{ + id: '11111111-1111-4111-8111-111111111111', + filename: 'new.png', + mimeType: 'image/png', + size: eightMb, + path: 'hapi-hub:scratchlist/default/s/11111111-1111-4111-8111-111111111111-new.png', + }], + { ...limits, maxBytesPerSession: 10 * 1024 * 1024 }, + sessionBytesBefore, + ) + expect(validation.ok).toBe(true) + + const naiveBefore = Math.max(0, 16 * 1024 * 1024 - eightMb) + const naive = validateScratchlistAttachmentsForWrite( + [{ + id: '11111111-1111-4111-8111-111111111111', + filename: 'new.png', + mimeType: 'image/png', + size: eightMb, + path: 'hapi-hub:scratchlist/default/s/11111111-1111-4111-8111-111111111111-new.png', + }], + { ...limits, maxBytesPerSession: 10 * 1024 * 1024 }, + naiveBefore, + ) + expect(naive.ok).toBe(false) + }) + + it('still counts sibling entry bytes against the session cap', () => { + // 9MB sibling + 8MB new upload already on disk. + const sessionBytesBefore = scratchlistSessionBytesBeforeForPut( + 17 * 1024 * 1024, + [{ size: eightMb }], + [], + ) + expect(sessionBytesBefore).toBe(9 * 1024 * 1024) + const validation = validateScratchlistAttachmentsForWrite( + [{ + id: '22222222-2222-4222-8222-222222222222', + filename: 'big.png', + mimeType: 'image/png', + size: eightMb, + path: 'hapi-hub:scratchlist/default/s/22222222-2222-4222-8222-222222222222-big.png', + }], + { ...limits, maxBytesPerSession: 10 * 1024 * 1024 }, + sessionBytesBefore, + ) + expect(validation.ok).toBe(false) + if (validation.ok) throw new Error('expected failure') + expect(validation.code).toBe('scratchlist_attachments_session_bytes') + }) +}) diff --git a/hub/src/scratchlistAttachments/validate.ts b/hub/src/scratchlistAttachments/validate.ts new file mode 100644 index 00000000..5e105a66 --- /dev/null +++ b/hub/src/scratchlistAttachments/validate.ts @@ -0,0 +1,73 @@ +import type { ScratchlistAttachmentLimits, ScratchlistAttachmentMetadata } from '@hapi/protocol' + +export type ScratchlistAttachmentValidationResult = + | { ok: true } + | { ok: false; error: string; code: string } + +/** + * Session byte budget ahead of a PUT that may replace attachments. + * `diskBytes` still includes files this update is about to drop — subtract + * those first so replacing an 80MB blob with another 80MB blob does not + * falsely trip the session cap. + */ +export function scratchlistSessionBytesBeforeForPut( + diskBytes: number, + nextAttachments: Array<{ size: number }>, + removedAttachments: Array<{ size: number }>, +): number { + const entryBytes = nextAttachments.reduce((sum, att) => sum + att.size, 0) + const removedBytes = removedAttachments.reduce((sum, att) => sum + att.size, 0) + const diskBytesAfterRemoval = Math.max(0, diskBytes - removedBytes) + return Math.max(0, diskBytesAfterRemoval - entryBytes) +} + +export function validateScratchlistAttachmentsForWrite( + attachments: ScratchlistAttachmentMetadata[], + limits: ScratchlistAttachmentLimits, + sessionBytesBefore: number +): ScratchlistAttachmentValidationResult { + if (attachments.length > limits.maxAttachmentsPerEntry) { + return { + ok: false, + error: `At most ${limits.maxAttachmentsPerEntry} attachments per scratchlist entry`, + code: 'scratchlist_attachments_per_entry', + } + } + + let entryBytes = 0 + for (const att of attachments) { + if (att.size > limits.maxBytesPerFile) { + return { + ok: false, + error: `Attachment exceeds per-file limit (${limits.maxBytesPerFile} bytes)`, + code: 'scratchlist_attachment_too_large', + } + } + if (!limits.allowedMimeTypes.some((m) => m.toLowerCase() === att.mimeType.toLowerCase())) { + return { + ok: false, + error: `Mime type not allowed: ${att.mimeType}`, + code: 'scratchlist_attachment_mime', + } + } + entryBytes += att.size + } + + if (entryBytes > limits.maxBytesPerEntry) { + return { + ok: false, + error: `Attachments exceed per-entry byte limit (${limits.maxBytesPerEntry} bytes)`, + code: 'scratchlist_attachments_entry_bytes', + } + } + + if (sessionBytesBefore + entryBytes > limits.maxBytesPerSession) { + return { + ok: false, + error: `Scratchlist attachments would exceed per-session byte limit (${limits.maxBytesPerSession} bytes)`, + code: 'scratchlist_attachments_session_bytes', + } + } + + return { ok: true } +} diff --git a/hub/src/store/index.ts b/hub/src/store/index.ts index 0de49c95..878e8655 100644 --- a/hub/src/store/index.ts +++ b/hub/src/store/index.ts @@ -29,7 +29,7 @@ export { ScratchlistStore } from './scratchlistStore' export { SessionStore } from './sessionStore' export { UserStore } from './userStore' -const SCHEMA_VERSION: number = 14 +const SCHEMA_VERSION: number = 15 const REQUIRED_TABLES = [ 'sessions', 'machines', @@ -141,6 +141,7 @@ export class Store { 11: () => this.migrateFromV11ToV12(), 12: () => this.migrateFromV12ToV13(), 13: () => this.migrateFromV13ToV14(), + 14: () => this.migrateFromV14ToV15(), }) if (currentVersion === 0) { @@ -295,6 +296,7 @@ export class Store { text TEXT NOT NULL, created_at INTEGER NOT NULL, updated_at INTEGER NOT NULL, + attachments TEXT DEFAULT NULL, PRIMARY KEY (session_id, entry_id), FOREIGN KEY (session_id) REFERENCES sessions(id) ON DELETE CASCADE ); @@ -547,6 +549,22 @@ export class Store { this.migrateFromV12ToV13() } + /** + * tiann/hapi#921 (scratchlist v2.2): attachment metadata JSON column. + * Bytes live on hub filesystem under HAPI_HOME/scratchlist-attachments/. + * Upstream ladder: V11→V12 = session_scratchlist (#896); V12–V14 = + * message_epochs reconciliation; this step is V14→V15 for attachments. + * + * Rollback: `ALTER TABLE session_scratchlist DROP COLUMN attachments` is + * unsupported on older SQLite; rebuild DB or leave column unused. + */ + private migrateFromV14ToV15(): void { + const columns = this.db.prepare('PRAGMA table_info(session_scratchlist)').all() as Array<{ name: string }> + if (!columns.some((col) => col.name === 'attachments')) { + this.db.exec(`ALTER TABLE session_scratchlist ADD COLUMN attachments TEXT DEFAULT NULL`) + } + } + private getSessionColumnNames(): Set { const rows = this.db.prepare('PRAGMA table_info(sessions)').all() as Array<{ name: string }> return new Set(rows.map((row) => row.name)) diff --git a/hub/src/store/migration-v12.test.ts b/hub/src/store/migration-v12.test.ts index 058a6f75..ee0e0376 100644 --- a/hub/src/store/migration-v12.test.ts +++ b/hub/src/store/migration-v12.test.ts @@ -185,13 +185,13 @@ describe('ScratchlistStore: CRUD through the typed-table wrapper', () => { }) if (created.outcome !== 'created') throw new Error('Expected created') - const updated = store.scratchlist.update(sessionId, 'u1', 'after') + const updated = store.scratchlist.update(sessionId, 'u1', { text: 'after' }) expect(updated).not.toBeNull() expect(updated!.text).toBe('after') expect(updated!.createdAt).toBe(1000) expect(updated!.updatedAt).toBeGreaterThan(1000) - const missing = store.scratchlist.update(sessionId, 'does-not-exist', 'noop') + const missing = store.scratchlist.update(sessionId, 'does-not-exist', { text: 'noop' }) expect(missing).toBeNull() }) diff --git a/hub/src/store/migration-v13.test.ts b/hub/src/store/migration-v13.test.ts index 990b32bb..d392ab76 100644 --- a/hub/src/store/migration-v13.test.ts +++ b/hub/src/store/migration-v13.test.ts @@ -10,7 +10,7 @@ describe('Store V12/V13→V14 schema reconciliation', () => { const store = new Store(':memory:') expect(tableExists(store, 'message_epochs')).toBe(true) expect(tableExists(store, 'session_scratchlist')).toBe(true) - expect(getUserVersion(store)).toBe(14) + expect(getUserVersion(store)).toBe(15) store.close() }) @@ -34,7 +34,7 @@ describe('Store V12/V13→V14 schema reconciliation', () => { store = new Store(dbPath) expect(tableExists(store, 'message_epochs')).toBe(true) expect(tableExists(store, 'session_scratchlist')).toBe(true) - expect(getUserVersion(store)).toBe(14) + expect(getUserVersion(store)).toBe(15) expect(store.messages.getMessageEpoch('session-1')).toBe(0) expect(store.messages.getMessages('session-1')).toHaveLength(1) } finally { @@ -72,7 +72,7 @@ describe('Store V12/V13→V14 schema reconciliation', () => { store = new Store(dbPath) expect(tableExists(store, 'message_epochs')).toBe(true) expect(tableExists(store, 'session_scratchlist')).toBe(true) - expect(getUserVersion(store)).toBe(14) + expect(getUserVersion(store)).toBe(15) expect(store.messages.getMessages('session-1')).toHaveLength(1) } finally { store?.close() diff --git a/hub/src/store/migration-v15.test.ts b/hub/src/store/migration-v15.test.ts new file mode 100644 index 00000000..e6a706df --- /dev/null +++ b/hub/src/store/migration-v15.test.ts @@ -0,0 +1,180 @@ +import { describe, expect, it } from 'bun:test' +import { Database } from 'bun:sqlite' +import { mkdtempSync, rmSync } from 'node:fs' +import { join } from 'node:path' +import { tmpdir } from 'node:os' +import { Store } from './index' + +/** + * Tests for V14→V15 schema migration: adds `session_scratchlist.attachments` + * for tiann/hapi#921 (scratchlist v2.2 hub attachment storage). + * + * Ladder: V11→V12 = session_scratchlist (#896), V12–V14 = message_epochs + * reconciliation, V14→V15 = attachments column (#921). + */ +describe('Store V14→V15 migration: scratchlist attachments column', () => { + it('fresh DB has session_scratchlist.attachments', () => { + const store = new Store(':memory:') + const cols = getColumns(store, 'session_scratchlist') + expect(cols).toContain('attachments') + expect(getUserVersion(store)).toBe(15) + store.close() + }) + + it('V14 text-only scratchlist migrates to V15 and gains attachments column', () => { + const dir = mkdtempSync(join(tmpdir(), 'hapi-migration-v14-to-v15-')) + const dbPath = join(dir, 'test.db') + let store: Store | undefined + try { + const db = new Database(dbPath, { create: true, readwrite: true, strict: true }) + db.exec('PRAGMA journal_mode = WAL') + db.exec('PRAGMA foreign_keys = ON') + createV14Schema(db) + db.exec('PRAGMA user_version = 14') + db.close() + + store = new Store(dbPath) + const cols = getColumns(store, 'session_scratchlist') + expect(cols).toContain('attachments') + expect(getUserVersion(store)).toBe(15) + } finally { + store?.close() + rmSync(dir, { recursive: true, force: true }) + } + }) + + it('V15 DB reopen is idempotent: schema unchanged', () => { + const dir = mkdtempSync(join(tmpdir(), 'hapi-migration-v15-idempotent-')) + const dbPath = join(dir, 'test.db') + let store1: Store | undefined + let store2: Store | undefined + try { + store1 = new Store(dbPath) + const cols1 = getColumns(store1, 'session_scratchlist') + expect(cols1).toContain('attachments') + + store2 = new Store(dbPath) + const cols2 = getColumns(store2, 'session_scratchlist') + expect(cols2).toEqual(cols1) + expect(getUserVersion(store2)).toBe(15) + } finally { + store2?.close() + store1?.close() + rmSync(dir, { recursive: true, force: true }) + } + }) +}) + +function getColumns(store: Store, table: string): string[] { + const db: Database = (store as unknown as { db: Database }).db + const rows = db.prepare(`PRAGMA table_info(${table})`).all() as Array<{ name: string }> + return rows.map((r) => r.name) +} + +function getUserVersion(store: Store): number { + const db: Database = (store as unknown as { db: Database }).db + const row = db.prepare('PRAGMA user_version').get() as { user_version: number } + return row.user_version +} + +/** Post-V13→V14 shape: scratchlist + message_epochs, no attachments column yet. */ +function createV14Schema(db: Database): void { + db.exec(` + CREATE TABLE IF NOT EXISTS sessions ( + id TEXT PRIMARY KEY, + tag TEXT, + namespace TEXT NOT NULL DEFAULT 'default', + created_at INTEGER NOT NULL, + updated_at INTEGER NOT NULL, + metadata TEXT, + metadata_version INTEGER DEFAULT 1, + agent_state TEXT, + agent_state_version INTEGER DEFAULT 1, + data_encryption_key BLOB, + thinking INTEGER DEFAULT 0, + thinking_updated_at INTEGER, + todos TEXT, + todos_updated_at INTEGER, + team_state TEXT, + team_state_updated_at INTEGER, + active INTEGER DEFAULT 0, + active_at INTEGER, + seq INTEGER DEFAULT 0, + service_tier TEXT + ); + + CREATE TABLE IF NOT EXISTS machines ( + id TEXT PRIMARY KEY, + namespace TEXT NOT NULL DEFAULT 'default', + created_at INTEGER NOT NULL, + updated_at INTEGER NOT NULL, + metadata TEXT, + metadata_version INTEGER DEFAULT 1, + runner_state TEXT, + runner_state_version INTEGER DEFAULT 1, + active INTEGER DEFAULT 0, + active_at INTEGER, + seq INTEGER DEFAULT 0 + ); + + CREATE TABLE IF NOT EXISTS messages ( + id TEXT PRIMARY KEY, + session_id TEXT NOT NULL, + content TEXT NOT NULL, + created_at INTEGER NOT NULL, + seq INTEGER NOT NULL, + local_id TEXT, + invoked_at INTEGER, + scheduled_at INTEGER, + FOREIGN KEY (session_id) REFERENCES sessions(id) ON DELETE CASCADE + ); + + CREATE TABLE IF NOT EXISTS message_epochs ( + session_id TEXT PRIMARY KEY, + epoch INTEGER NOT NULL DEFAULT 0, + FOREIGN KEY (session_id) REFERENCES sessions(id) ON DELETE CASCADE + ); + + CREATE TABLE IF NOT EXISTS users ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + platform TEXT NOT NULL, + platform_user_id TEXT NOT NULL, + namespace TEXT NOT NULL DEFAULT 'default', + created_at INTEGER NOT NULL, + UNIQUE(platform, platform_user_id) + ); + + CREATE TABLE IF NOT EXISTS push_subscriptions ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + namespace TEXT NOT NULL, + endpoint TEXT NOT NULL, + p256dh TEXT NOT NULL, + auth TEXT NOT NULL, + created_at INTEGER NOT NULL, + UNIQUE(namespace, endpoint) + ); + + CREATE TABLE IF NOT EXISTS fcm_devices ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + namespace TEXT NOT NULL, + token TEXT NOT NULL, + platform TEXT NOT NULL, + device_id TEXT NOT NULL, + created_at INTEGER NOT NULL, + updated_at INTEGER NOT NULL, + UNIQUE(namespace, device_id, platform) + ); + + CREATE TABLE IF NOT EXISTS session_scratchlist ( + session_id TEXT NOT NULL, + entry_id TEXT NOT NULL, + text TEXT NOT NULL, + created_at INTEGER NOT NULL, + updated_at INTEGER NOT NULL, + PRIMARY KEY (session_id, entry_id), + FOREIGN KEY (session_id) REFERENCES sessions(id) ON DELETE CASCADE + ); + CREATE INDEX IF NOT EXISTS idx_session_scratchlist_session_created + ON session_scratchlist(session_id, created_at DESC); + `) +} diff --git a/hub/src/store/scratchlist.ts b/hub/src/store/scratchlist.ts index 8d286f35..ae3b1b61 100644 --- a/hub/src/store/scratchlist.ts +++ b/hub/src/store/scratchlist.ts @@ -1,6 +1,12 @@ import type { Database } from 'bun:sqlite' import { randomUUID } from 'node:crypto' +import { + parseScratchlistAttachmentsJson, + serializeScratchlistAttachments, + type ScratchlistAttachmentMetadata, +} from '@hapi/protocol' + import type { StoredScratchlistEntry } from './types' /** @@ -34,6 +40,7 @@ type DbScratchlistRow = { text: string created_at: number updated_at: number + attachments: string | null } function toStoredEntry(row: DbScratchlistRow): StoredScratchlistEntry { @@ -42,16 +49,19 @@ function toStoredEntry(row: DbScratchlistRow): StoredScratchlistEntry { entryId: row.entry_id, text: row.text, createdAt: row.created_at, - updatedAt: row.updated_at + updatedAt: row.updated_at, + attachments: parseScratchlistAttachmentsJson(row.attachments), } } +const SCRATCHLIST_ENTRY_COLUMNS = `session_id, entry_id, text, created_at, updated_at, attachments` + export function listScratchlistEntries( db: Database, sessionId: string ): StoredScratchlistEntry[] { const rows = db.prepare( - `SELECT session_id, entry_id, text, created_at, updated_at + `SELECT ${SCRATCHLIST_ENTRY_COLUMNS} FROM session_scratchlist WHERE session_id = ? ORDER BY created_at DESC, entry_id DESC` @@ -72,7 +82,7 @@ export function getScratchlistEntry( entryId: string ): StoredScratchlistEntry | null { const row = db.prepare( - `SELECT session_id, entry_id, text, created_at, updated_at + `SELECT ${SCRATCHLIST_ENTRY_COLUMNS} FROM session_scratchlist WHERE session_id = ? AND entry_id = ?` ).get(sessionId, entryId) as DbScratchlistRow | undefined @@ -92,11 +102,28 @@ export type CreateScratchlistResult = | { outcome: 'duplicate'; entry: StoredScratchlistEntry } | { outcome: 'session-not-found' } +export function sumScratchlistAttachmentBytesForSession(db: Database, sessionId: string): number { + const rows = db.prepare( + `SELECT attachments FROM session_scratchlist WHERE session_id = ?` + ).all(sessionId) as Array<{ attachments: string | null }> + let total = 0 + for (const row of rows) { + for (const att of parseScratchlistAttachmentsJson(row.attachments)) { + total += att.size + } + } + return total +} + export function createScratchlistEntry( db: Database, sessionId: string, text: string, - options?: { entryId?: string; createdAt?: number } + options?: { + entryId?: string + createdAt?: number + attachments?: ScratchlistAttachmentMetadata[] + } ): CreateScratchlistResult { const now = Date.now() const entryId = options?.entryId ?? randomUUID() @@ -118,16 +145,19 @@ export function createScratchlistEntry( return { outcome: 'duplicate', entry: existing } } + const attachmentsJson = serializeScratchlistAttachments(options?.attachments ?? []) + db.prepare( `INSERT INTO session_scratchlist - (session_id, entry_id, text, created_at, updated_at) - VALUES (@session_id, @entry_id, @text, @created_at, @updated_at)` + (session_id, entry_id, text, created_at, updated_at, attachments) + VALUES (@session_id, @entry_id, @text, @created_at, @updated_at, @attachments)` ).run({ session_id: sessionId, entry_id: entryId, text, created_at: createdAt, - updated_at: updatedAt + updated_at: updatedAt, + attachments: attachmentsJson, }) const created = getScratchlistEntry(db, sessionId, entryId) @@ -147,20 +177,29 @@ export function updateScratchlistEntry( db: Database, sessionId: string, entryId: string, - text: string + patch: { text?: string; attachments?: ScratchlistAttachmentMetadata[] } ): StoredScratchlistEntry | null { + const existing = getScratchlistEntry(db, sessionId, entryId) + if (!existing) return null + const now = Date.now() + const text = patch.text ?? existing.text + const attachments = patch.attachments ?? existing.attachments + const attachmentsJson = serializeScratchlistAttachments(attachments) + const result = db.prepare( `UPDATE session_scratchlist SET text = @text, - updated_at = @updated_at + updated_at = @updated_at, + attachments = @attachments WHERE session_id = @session_id AND entry_id = @entry_id` ).run({ session_id: sessionId, entry_id: entryId, text, - updated_at: now + updated_at: now, + attachments: attachmentsJson, }) if (result.changes === 0) { return null diff --git a/hub/src/store/scratchlistStore.ts b/hub/src/store/scratchlistStore.ts index c75bc6cd..d8c67aff 100644 --- a/hub/src/store/scratchlistStore.ts +++ b/hub/src/store/scratchlistStore.ts @@ -7,6 +7,7 @@ import { deleteScratchlistEntry, getScratchlistEntry, listScratchlistEntries, + sumScratchlistAttachmentBytesForSession, transferScratchlistEntries, updateScratchlistEntry, type CreateScratchlistResult @@ -34,7 +35,11 @@ export class ScratchlistStore { create( sessionId: string, text: string, - options?: { entryId?: string; createdAt?: number } + options?: { + entryId?: string + createdAt?: number + attachments?: import('@hapi/protocol').ScratchlistAttachmentMetadata[] + } ): CreateScratchlistResult { return createScratchlistEntry(this.db, sessionId, text, options) } @@ -42,9 +47,16 @@ export class ScratchlistStore { update( sessionId: string, entryId: string, - text: string + patch: { + text?: string + attachments?: import('@hapi/protocol').ScratchlistAttachmentMetadata[] + } ): StoredScratchlistEntry | null { - return updateScratchlistEntry(this.db, sessionId, entryId, text) + return updateScratchlistEntry(this.db, sessionId, entryId, patch) + } + + sumAttachmentBytes(sessionId: string): number { + return sumScratchlistAttachmentBytesForSession(this.db, sessionId) } delete(sessionId: string, entryId: string): boolean { diff --git a/hub/src/store/types.ts b/hub/src/store/types.ts index 28683c99..0bd63f7c 100644 --- a/hub/src/store/types.ts +++ b/hub/src/store/types.ts @@ -80,6 +80,7 @@ export type StoredScratchlistEntry = { text: string createdAt: number updatedAt: number + attachments: import('@hapi/protocol').ScratchlistAttachmentMetadata[] } export type VersionedUpdateResult = diff --git a/hub/src/sync/sessionCache-merge-scratchlist.test.ts b/hub/src/sync/sessionCache-merge-scratchlist.test.ts index d528aae0..7d1bae8d 100644 --- a/hub/src/sync/sessionCache-merge-scratchlist.test.ts +++ b/hub/src/sync/sessionCache-merge-scratchlist.test.ts @@ -74,6 +74,56 @@ describe('mergeSessions (deleteOldSession=true) - scratchlist transfer', () => { expect(store.scratchlist.list(oldSession.id)).toEqual([]) }) + it('re-keys hub attachment paths when rows move to the new session id', async () => { + const { mkdtempSync, rmSync } = await import('node:fs') + const { join } = await import('node:path') + const { tmpdir } = await import('node:os') + const hapiHome = mkdtempSync(join(tmpdir(), 'hapi-merge-att-')) + const prevHome = process.env.HAPI_HOME + process.env.HAPI_HOME = hapiHome + try { + const { store, cache } = setup() + const { oldSession, newSession } = makeSessions(cache) + const { writeScratchlistAttachmentFile } = await import('../scratchlistAttachments/storage') + const att = await writeScratchlistAttachmentFile( + hapiHome, + 'default', + oldSession.id, + 'note.png', + 'image/png', + Buffer.from('img') + ) + store.scratchlist.create(oldSession.id, 'with pic', { + entryId: 'e-att', + createdAt: 100, + attachments: [att], + }) + + await cache.mergeSessions(oldSession.id, newSession.id, 'default') + + const onNew = store.scratchlist.list(newSession.id) + expect(onNew).toHaveLength(1) + expect(onNew[0]!.attachments[0]!.path).toContain(`/${newSession.id}/`) + expect(onNew[0]!.attachments[0]!.path).not.toContain(`/${oldSession.id}/`) + + const { sumScratchlistAttachmentBytesOnDisk, resolveScratchlistAttachmentsForSession } = + await import('../scratchlistAttachments/storage') + expect(await sumScratchlistAttachmentBytesOnDisk(hapiHome, 'default', newSession.id)).toBe(3) + expect(await sumScratchlistAttachmentBytesOnDisk(hapiHome, 'default', oldSession.id)).toBe(0) + const resolved = await resolveScratchlistAttachmentsForSession( + hapiHome, + 'default', + newSession.id, + onNew[0]!.attachments + ) + expect(resolved.ok).toBe(true) + } finally { + if (prevHome === undefined) delete process.env.HAPI_HOME + else process.env.HAPI_HOME = prevHome + rmSync(hapiHome, { recursive: true, force: true }) + } + }) + it('handles entryId PK collision by keeping the dedup target row (operator-visible session wins)', async () => { const { store, cache } = setup() const { oldSession, newSession } = makeSessions(cache) diff --git a/hub/src/sync/sessionCache.ts b/hub/src/sync/sessionCache.ts index 7b189391..5f372c1c 100644 --- a/hub/src/sync/sessionCache.ts +++ b/hub/src/sync/sessionCache.ts @@ -855,6 +855,10 @@ export class SessionCache { throw new Error('Cannot delete active session') } + const scratchlistAttachments = this.store.scratchlist + .list(sessionId) + .flatMap((entry) => entry.attachments) + const deleted = this.store.sessions.deleteSession(sessionId, session.namespace) if (!deleted) { throw new Error('Failed to delete session') @@ -865,6 +869,16 @@ export class SessionCache { this.todoBackfillAttemptedSessionIds.delete(sessionId) this.pendingThinkingUntilBySessionId.delete(sessionId) + void import('../scratchlistAttachments/storage').then(async ({ + deleteScratchlistAttachmentFiles, + deleteScratchlistSessionAttachmentDir, + getHapiHomeDir, + }) => { + const hapiHome = getHapiHomeDir() + await deleteScratchlistAttachmentFiles(hapiHome, scratchlistAttachments) + await deleteScratchlistSessionAttachmentDir(hapiHome, session.namespace, sessionId) + }) + this.publisher.emit({ type: 'session-removed', sessionId, namespace: session.namespace }) } @@ -917,9 +931,38 @@ export class SessionCache { // promise that scratchlist survives reloads. const movedScratchlist = this.store.scratchlist.transfer(oldSessionId, newSessionId) if (movedScratchlist.moved > 0) { + // Attachment hub paths embed the old session id. Re-key files + + // metadata so quota/resolve stay correct on the consolidated id. + const { + getHapiHomeDir, + moveScratchlistAttachmentFilesForSession, + deleteScratchlistSessionAttachmentDir, + } = await import('../scratchlistAttachments/storage') + const hapiHome = getHapiHomeDir() + for (const entry of this.store.scratchlist.list(newSessionId)) { + if (entry.attachments.length === 0) continue + const attachments = await moveScratchlistAttachmentFilesForSession( + hapiHome, + namespace, + oldSessionId, + newSessionId, + entry.attachments, + ) + if (attachments.some((att, i) => att.path !== entry.attachments[i]?.path)) { + this.store.scratchlist.update(newSessionId, entry.entryId, { attachments }) + } + } + // Collided SQL losers + orphan uploads still under the old dir. + await deleteScratchlistSessionAttachmentDir(hapiHome, namespace, oldSessionId) // Rows landed on the consolidated session - invalidate so // any client on the new id refetches. this.emitScratchlistChanged(newSessionId) + } else if (movedScratchlist.collided > 0) { + // Every old entry lost the PK race — drop leftover hub blobs. + const { getHapiHomeDir, deleteScratchlistSessionAttachmentDir } = await import( + '../scratchlistAttachments/storage' + ) + await deleteScratchlistSessionAttachmentDir(getHapiHomeDir(), namespace, oldSessionId) } if (!options.deleteOldSession && (movedScratchlist.moved > 0 || movedScratchlist.collided > 0)) { // HAPI Bot PR #896: when every old entry collides (moved=0, diff --git a/hub/src/sync/syncEngine-scratchlist.test.ts b/hub/src/sync/syncEngine-scratchlist.test.ts index 6559cc34..30167250 100644 --- a/hub/src/sync/syncEngine-scratchlist.test.ts +++ b/hub/src/sync/syncEngine-scratchlist.test.ts @@ -125,7 +125,7 @@ describe('SyncEngine scratchlist mutations emit session-updated patches', () => engine.createScratchlistEntry(session.id, 'before', { entryId: 'e1' }) engineEvents.length = 0 - const updated = engine.updateScratchlistEntry(session.id, 'e1', 'after') + const updated = engine.updateScratchlistEntry(session.id, 'e1', { text: 'after' }) expect(updated).not.toBeNull() const matching = engineEvents.filter( (e) => e.type === 'session-updated' && (e.data as Record).scratchlistUpdatedAt !== undefined @@ -144,7 +144,7 @@ describe('SyncEngine scratchlist mutations emit session-updated patches', () => 'default' ) engineEvents.length = 0 - const updated = engine.updateScratchlistEntry(session.id, 'never-existed', 'whatever') + const updated = engine.updateScratchlistEntry(session.id, 'never-existed', { text: 'whatever' }) expect(updated).toBeNull() const matching = engineEvents.filter( (e) => e.type === 'session-updated' && (e.data as Record).scratchlistUpdatedAt !== undefined diff --git a/hub/src/sync/syncEngine.ts b/hub/src/sync/syncEngine.ts index 035f3264..ee4c65a0 100644 --- a/hub/src/sync/syncEngine.ts +++ b/hub/src/sync/syncEngine.ts @@ -149,6 +149,8 @@ export class SyncEngine { private inactivityTimer: NodeJS.Timeout | null = null /** Sessions that emitted `session-ready` (Cursor ACP load/newSession complete). */ private readonly sessionReadyIds = new Set() + /** Serialize scratchlist uploads per session so disk-byte caps cannot race. */ + private readonly scratchlistUploadTails = new Map>() constructor( private readonly store: Store, @@ -447,12 +449,14 @@ export class SyncEngine { text: string createdAt: number updatedAt: number + attachments: import('@hapi/protocol').ScratchlistAttachmentMetadata[] }> { return this.store.scratchlist.list(sessionId).map((row) => ({ entryId: row.entryId, text: row.text, createdAt: row.createdAt, - updatedAt: row.updatedAt + updatedAt: row.updatedAt, + attachments: row.attachments, })) } @@ -460,6 +464,10 @@ export class SyncEngine { return this.store.scratchlist.count(sessionId) } + sumScratchlistAttachmentBytes(sessionId: string): number { + return this.store.scratchlist.sumAttachmentBytes(sessionId) + } + /** * Read a single entry by id. The route layer uses this to short- * circuit duplicate POSTs (migration retry) BEFORE running the @@ -470,14 +478,21 @@ export class SyncEngine { getScratchlistEntry( sessionId: string, entryId: string - ): { entryId: string; text: string; createdAt: number; updatedAt: number } | null { + ): { + entryId: string + text: string + createdAt: number + updatedAt: number + attachments: import('@hapi/protocol').ScratchlistAttachmentMetadata[] + } | null { const row = this.store.scratchlist.get(sessionId, entryId) if (!row) return null return { entryId: row.entryId, text: row.text, createdAt: row.createdAt, - updatedAt: row.updatedAt + updatedAt: row.updatedAt, + attachments: row.attachments, } } @@ -496,10 +511,20 @@ export class SyncEngine { createScratchlistEntry( sessionId: string, text: string, - options?: { entryId?: string; createdAt?: number } + options?: { + entryId?: string + createdAt?: number + attachments?: import('@hapi/protocol').ScratchlistAttachmentMetadata[] + } ): { outcome: 'created' | 'duplicate' - entry: { entryId: string; text: string; createdAt: number; updatedAt: number } + entry: { + entryId: string + text: string + createdAt: number + updatedAt: number + attachments: import('@hapi/protocol').ScratchlistAttachmentMetadata[] + } } | { outcome: 'session-not-found' } { const result = this.store.scratchlist.create(sessionId, text, options) if (result.outcome === 'session-not-found') { @@ -514,7 +539,8 @@ export class SyncEngine { entryId: result.entry.entryId, text: result.entry.text, createdAt: result.entry.createdAt, - updatedAt: result.entry.updatedAt + updatedAt: result.entry.updatedAt, + attachments: result.entry.attachments, } } } @@ -522,27 +548,172 @@ export class SyncEngine { updateScratchlistEntry( sessionId: string, entryId: string, + patch: { + text?: string + attachments?: import('@hapi/protocol').ScratchlistAttachmentMetadata[] + } + ): { + entryId: string text: string - ): { entryId: string; text: string; createdAt: number; updatedAt: number } | null { - const updated = this.store.scratchlist.update(sessionId, entryId, text) + createdAt: number + updatedAt: number + attachments: import('@hapi/protocol').ScratchlistAttachmentMetadata[] + } | null { + const updated = this.store.scratchlist.update(sessionId, entryId, patch) if (!updated) return null this.sessionCache.emitScratchlistChanged(sessionId, updated.updatedAt) return { entryId: updated.entryId, text: updated.text, createdAt: updated.createdAt, - updatedAt: updated.updatedAt + updatedAt: updated.updatedAt, + attachments: updated.attachments, } } deleteScratchlistEntry(sessionId: string, entryId: string): boolean { + const existing = this.store.scratchlist.get(sessionId, entryId) const removed = this.store.scratchlist.delete(sessionId, entryId) - if (removed) { + if (removed && existing) { + // Attachment ids may be shared across entries (direct REST). + // Only delete blobs that no remaining entry still references. + const remainingIds = new Set( + this.store.scratchlist + .list(sessionId) + .flatMap((entry) => entry.attachments.map((att) => att.id)) + ) + const orphaned = existing.attachments.filter((att) => !remainingIds.has(att.id)) + if (orphaned.length > 0) { + void import('../scratchlistAttachments/storage').then(({ deleteScratchlistAttachmentFiles, getHapiHomeDir }) => + deleteScratchlistAttachmentFiles(getHapiHomeDir(), orphaned) + ) + } this.sessionCache.emitScratchlistChanged(sessionId, Date.now()) } return removed } + private async withScratchlistUploadLock( + namespace: string, + sessionId: string, + fn: () => Promise + ): Promise { + const key = `${namespace}:${sessionId}` + const previous = this.scratchlistUploadTails.get(key) ?? Promise.resolve() + let release!: () => void + const gate = new Promise((resolve) => { + release = resolve + }) + const tail = previous.catch(() => undefined).then(() => gate) + this.scratchlistUploadTails.set(key, tail) + await previous.catch(() => undefined) + try { + return await fn() + } finally { + release() + if (this.scratchlistUploadTails.get(key) === tail) { + this.scratchlistUploadTails.delete(key) + } + } + } + +async uploadScratchlistAttachment( + sessionId: string, + namespace: string, + filename: string, + contentBase64: string, + mimeType: string + ): Promise<{ success: true; attachment: import('@hapi/protocol').ScratchlistAttachmentMetadata } | { success: false; error: string; code?: string }> { + const { loadScratchlistAttachmentLimitsFromEnv, isAllowedScratchlistMime } = await import('../config/scratchlistAttachmentLimits') + const { + estimateBase64Bytes, + writeScratchlistAttachmentFile, + getHapiHomeDir, + sumScratchlistAttachmentBytesOnDisk, + } = await import('../scratchlistAttachments/storage') + const { validateScratchlistAttachmentsForWrite } = await import('../scratchlistAttachments/validate') + + const limits = loadScratchlistAttachmentLimitsFromEnv() + const estimated = estimateBase64Bytes(contentBase64) + if (estimated > limits.maxBytesPerFile) { + return { success: false, error: 'File too large', code: 'scratchlist_attachment_too_large' } + } + if (!isAllowedScratchlistMime(mimeType, limits)) { + return { success: false, error: 'Mime type not allowed', code: 'scratchlist_attachment_mime' } + } + + const hapiHome = getHapiHomeDir() + const buffer = Buffer.from(contentBase64, 'base64') + return await this.withScratchlistUploadLock(namespace, sessionId, async () => { + const sessionBytes = await sumScratchlistAttachmentBytesOnDisk(hapiHome, namespace, sessionId) + const provisional = { + id: 'pending', + filename, + mimeType, + size: buffer.length, + path: 'pending', + } + const validation = validateScratchlistAttachmentsForWrite([provisional], limits, sessionBytes) + if (!validation.ok) { + return { success: false, error: validation.error, code: validation.code } + } + + const attachment = await writeScratchlistAttachmentFile( + hapiHome, + namespace, + sessionId, + filename, + mimeType, + buffer + ) + return { success: true, attachment } + }) + } + + async resolveScratchlistAttachmentsForSession( + sessionId: string, + namespace: string, + claimed: import('@hapi/protocol').ScratchlistAttachmentMetadata[] + ): Promise< + | { ok: true; attachments: import('@hapi/protocol').ScratchlistAttachmentMetadata[] } + | { ok: false; error: string } + > { + const { + resolveScratchlistAttachmentsForSession: resolveAttachments, + getHapiHomeDir, + } = await import('../scratchlistAttachments/storage') + return resolveAttachments(getHapiHomeDir(), namespace, sessionId, claimed) + } + + async sumScratchlistAttachmentBytesOnDisk(sessionId: string, namespace: string): Promise { + const { + sumScratchlistAttachmentBytesOnDisk: sumOnDisk, + getHapiHomeDir, + } = await import('../scratchlistAttachments/storage') + return sumOnDisk(getHapiHomeDir(), namespace, sessionId) + } + + async deleteScratchlistAttachmentById( + sessionId: string, + namespace: string, + attachmentId: string + ): Promise { + const { + deleteScratchlistAttachmentById: deleteById, + getHapiHomeDir, + } = await import('../scratchlistAttachments/storage') + return deleteById(getHapiHomeDir(), namespace, sessionId, attachmentId) + } + + async readScratchlistAttachment( + hubPath: string + ): Promise<{ buffer: Buffer; mimeType: string; filename: string } | null> { + const { readScratchlistAttachmentFile, getHapiHomeDir } = await import('../scratchlistAttachments/storage') + const read = await readScratchlistAttachmentFile(getHapiHomeDir(), hubPath) + if (!read) return null + return { buffer: read.buffer, mimeType: 'application/octet-stream', filename: 'attachment' } + } + handleMachineAlive(payload: { machineId: string; time: number; health?: unknown }): void { this.machineCache.handleMachineAlive(payload) } diff --git a/hub/src/web/routes/sessions-scratchlist.test.ts b/hub/src/web/routes/sessions-scratchlist.test.ts index a9b5b972..05a11e7e 100644 --- a/hub/src/web/routes/sessions-scratchlist.test.ts +++ b/hub/src/web/routes/sessions-scratchlist.test.ts @@ -83,6 +83,7 @@ function createApp(session: Session, overrides: EngineOverrides = {}) { }, listScratchlistEntries: overrides.listScratchlistEntries ?? (() => []), countScratchlistEntries: overrides.countScratchlistEntries ?? (() => 0), + sumScratchlistAttachmentBytes: () => 0, getScratchlistEntry: overrides.getScratchlistEntry ?? (() => null), createScratchlistEntry: overrides.createScratchlistEntry ?? ((sessionId: string, text: string) => ({ @@ -91,17 +92,26 @@ function createApp(session: Session, overrides: EngineOverrides = {}) { entryId: `auto-${Date.now()}`, text, createdAt: 1000, - updatedAt: 1000 + updatedAt: 1000, + attachments: [], } })), updateScratchlistEntry: overrides.updateScratchlistEntry - ?? ((sessionId: string, entryId: string, text: string) => ({ + ?? ((sessionId: string, entryId: string, patch: { text?: string }) => ({ entryId, - text, + text: patch.text ?? '', createdAt: 1000, - updatedAt: 2000 + updatedAt: 2000, + attachments: [], })), - deleteScratchlistEntry: overrides.deleteScratchlistEntry ?? (() => true) + deleteScratchlistEntry: overrides.deleteScratchlistEntry ?? (() => true), + resolveScratchlistAttachmentsForSession: async ( + _sessionId: string, + _namespace: string, + claimed: Array<{ id: string; filename: string; mimeType: string; size: number; path: string }> + ) => ({ ok: true as const, attachments: claimed }), + sumScratchlistAttachmentBytesOnDisk: async () => 0, + deleteScratchlistAttachmentById: async () => true, } as unknown as SyncEngine const app = new Hono() @@ -118,8 +128,8 @@ describe('GET /api/sessions/:id/scratchlist', () => { const session = createSession() const app = createApp(session, { listScratchlistEntries: () => [ - { entryId: 'a', text: 'note A', createdAt: 1000, updatedAt: 1000 }, - { entryId: 'b', text: 'note B', createdAt: 2000, updatedAt: 2500 } + { entryId: 'a', text: 'note A', createdAt: 1000, updatedAt: 1000, attachments: [] }, + { entryId: 'b', text: 'note B', createdAt: 2000, updatedAt: 2500, attachments: [] } ] }) const res = await app.request('/api/sessions/session-1/scratchlist') @@ -156,7 +166,8 @@ describe('POST /api/sessions/:id/scratchlist', () => { entryId: options?.entryId ?? 'fresh-id', text, createdAt: options?.createdAt ?? 1000, - updatedAt: 1000 + updatedAt: 1000, + attachments: options?.attachments ?? [], } } } @@ -178,7 +189,7 @@ describe('POST /api/sessions/:id/scratchlist', () => { const app = createApp(session, { createScratchlistEntry: () => ({ outcome: 'duplicate' as const, - entry: { entryId: 'dup', text: 'pre-existing', createdAt: 100, updatedAt: 100 } + entry: { entryId: 'dup', text: 'pre-existing', createdAt: 100, updatedAt: 100, attachments: [] } }) }) const res = await app.request('/api/sessions/session-1/scratchlist', { @@ -244,7 +255,8 @@ describe('POST /api/sessions/:id/scratchlist', () => { entryId: 'pre-existing', text: 'already there', createdAt: 100, - updatedAt: 100 + updatedAt: 100, + attachments: [], } } return null @@ -253,7 +265,7 @@ describe('POST /api/sessions/:id/scratchlist', () => { createCalls.push(1) return { outcome: 'created' as const, - entry: { entryId: 'should-not-fire', text: 'noop', createdAt: 0, updatedAt: 0 } + entry: { entryId: 'should-not-fire', text: 'noop', createdAt: 0, updatedAt: 0, attachments: [] } } } }) @@ -335,11 +347,19 @@ describe('PUT /api/sessions/:id/scratchlist/:entryId', () => { it('returns the updated entry on success', async () => { const session = createSession() const app = createApp(session, { - updateScratchlistEntry: (_sessionId, entryId, text) => ({ - entryId, - text, + getScratchlistEntry: () => ({ + entryId: 'entry-1', + text: 'before', createdAt: 1000, - updatedAt: 5000 + updatedAt: 1000, + attachments: [], + }), + updateScratchlistEntry: (_sessionId, entryId, patch) => ({ + entryId, + text: patch.text ?? 'before', + createdAt: 1000, + updatedAt: 5000, + attachments: patch.attachments ?? [], }) }) const res = await app.request('/api/sessions/session-1/scratchlist/entry-1', { @@ -356,6 +376,7 @@ describe('PUT /api/sessions/:id/scratchlist/:entryId', () => { it('returns 404 when the entry does not exist', async () => { const session = createSession() const app = createApp(session, { + getScratchlistEntry: () => null, updateScratchlistEntry: () => null }) const res = await app.request('/api/sessions/session-1/scratchlist/missing-id', { @@ -377,6 +398,33 @@ describe('PUT /api/sessions/:id/scratchlist/:entryId', () => { expect(res.status).toBe(400) }) + it('rejects clearing attachments on a textless entry (would leave an empty row)', async () => { + const session = createSession() + const app = createApp(session, { + getScratchlistEntry: () => ({ + entryId: 'entry-1', + text: '', + createdAt: 1000, + updatedAt: 1000, + attachments: [{ + id: '11111111-1111-4111-8111-111111111111', + filename: 'a.png', + mimeType: 'image/png', + size: 3, + path: 'hapi-hub:scratchlist/default/session-1/11111111-1111-4111-8111-111111111111-a.png', + }], + }), + }) + const res = await app.request('/api/sessions/session-1/scratchlist/entry-1', { + method: 'PUT', + headers: { 'content-type': 'application/json' }, + body: JSON.stringify({ attachments: [] }) + }) + expect(res.status).toBe(400) + const body = await res.json() as { code?: string } + expect(body.code).toBe('scratchlist_entry_empty') + }) + it('returns 403 when the session is in another namespace', async () => { const session = createSession({ namespace: 'other' }) const app = createApp(session, { sessionAccess: 'wrong-namespace' }) diff --git a/hub/src/web/routes/sessions.ts b/hub/src/web/routes/sessions.ts index dfd2ef62..ac92b1fd 100644 --- a/hub/src/web/routes/sessions.ts +++ b/hub/src/web/routes/sessions.ts @@ -24,6 +24,8 @@ import type { SlashCommand } from '@hapi/protocol/apiTypes' import { Hono, type Context } from 'hono' import type { SyncEngine, Session } from '../../sync/syncEngine' import type { WebAppEnv } from '../middleware/auth' +import { loadScratchlistAttachmentLimitsFromEnv } from '../../config/scratchlistAttachmentLimits' +import { validateScratchlistAttachmentsForWrite, scratchlistSessionBytesBeforeForPut } from '../../scratchlistAttachments/validate' import { requireSessionFromParam, requireSyncEngine } from './guards' const MAX_UPLOAD_BYTES = 50 * 1024 * 1024 @@ -712,6 +714,84 @@ export function createSessionsRoutes(getSyncEngine: () => SyncEngine | null): Ho * client uses that as a cache-invalidation token to refetch GET. */ + app.get('/sessions/:id/scratchlist/limits', (c) => { + const engine = requireSyncEngine(c, getSyncEngine) + if (engine instanceof Response) { + return engine + } + const sessionResult = requireSessionFromParam(c, engine) + if (sessionResult instanceof Response) { + return sessionResult + } + return c.json({ limits: loadScratchlistAttachmentLimitsFromEnv() }) + }) + + app.post('/sessions/:id/scratchlist/upload', async (c) => { + const engine = requireSyncEngine(c, getSyncEngine) + if (engine instanceof Response) { + return engine + } + const sessionResult = requireSessionFromParam(c, engine) + if (sessionResult instanceof Response) { + return sessionResult + } + + const body = await c.req.json().catch(() => null) + const parsed = UploadFileRequestSchema.safeParse(body) + if (!parsed.success) { + return c.json({ error: 'Invalid body' }, 400) + } + + const namespace = c.get('namespace') + const result = await engine.uploadScratchlistAttachment( + sessionResult.sessionId, + namespace, + parsed.data.filename, + parsed.data.content, + parsed.data.mimeType + ) + if (!result.success) { + const status = result.code === 'scratchlist_attachment_too_large' ? 413 : 400 + return c.json({ success: false, error: result.error, code: result.code }, status) + } + return c.json({ success: true, attachment: result.attachment }) + }) + + app.get('/sessions/:id/scratchlist/attachments/:attachmentId', async (c) => { + const engine = requireSyncEngine(c, getSyncEngine) + if (engine instanceof Response) { + return engine + } + const sessionResult = requireSessionFromParam(c, engine) + if (sessionResult instanceof Response) { + return sessionResult + } + const attachmentId = c.req.param('attachmentId') + if (!attachmentId) { + return c.json({ error: 'Missing attachmentId' }, 400) + } + + const entries = engine.listScratchlistEntries(sessionResult.sessionId) + const match = entries + .flatMap((entry) => entry.attachments) + .find((att) => att.id === attachmentId) + if (!match) { + return c.json({ error: 'Attachment not found' }, 404) + } + + const file = await engine.readScratchlistAttachment(match.path) + if (!file) { + return c.json({ error: 'Attachment file missing' }, 404) + } + return new Response(file.buffer, { + headers: { + 'Content-Type': match.mimeType, + // Defense in depth: metadata may predate resolve-time canonicalize. + 'Content-Disposition': `inline; filename="${match.filename.replace(/[\r\n\0"\\]/g, '_')}"`, + }, + }) + }) + app.get('/sessions/:id/scratchlist', (c) => { const engine = requireSyncEngine(c, getSyncEngine) if (engine instanceof Response) { @@ -771,12 +851,36 @@ export function createSessionsRoutes(getSyncEngine: () => SyncEngine | null): Ho }, 409) } + const limits = loadScratchlistAttachmentLimitsFromEnv() + const namespace = c.get('namespace') + const checked = await engine.resolveScratchlistAttachmentsForSession( + sessionResult.sessionId, + namespace, + parsed.data.attachments + ) + if (!checked.ok) { + return c.json({ error: checked.error, code: 'scratchlist_attachment_invalid' }, 400) + } + const diskBytes = await engine.sumScratchlistAttachmentBytesOnDisk(sessionResult.sessionId, namespace) + const entryBytes = checked.attachments.reduce((sum, att) => sum + att.size, 0) + // Files are already on disk from upload; don't double-count them. + const sessionBytesBefore = Math.max(0, diskBytes - entryBytes) + const attachmentValidation = validateScratchlistAttachmentsForWrite( + checked.attachments, + limits, + sessionBytesBefore + ) + if (!attachmentValidation.ok) { + return c.json({ error: attachmentValidation.error, code: attachmentValidation.code }, 400) + } + const result = engine.createScratchlistEntry( sessionResult.sessionId, - parsed.data.text, + parsed.data.text.trim(), { entryId: parsed.data.entryId, - createdAt: parsed.data.createdAt + createdAt: parsed.data.createdAt, + attachments: checked.attachments, } ) if (result.outcome === 'session-not-found') { @@ -809,17 +913,113 @@ export function createSessionsRoutes(getSyncEngine: () => SyncEngine | null): Ho return c.json({ error: 'Invalid body', issues: parsed.error.issues }, 400) } + const existing = engine.getScratchlistEntry(sessionResult.sessionId, entryId) + if (!existing) { + return c.json({ error: 'Scratchlist entry not found' }, 404) + } + + const nextText = parsed.data.text !== undefined ? parsed.data.text.trim() : existing.text + const namespace = c.get('namespace') + let nextAttachments = existing.attachments + if (parsed.data.attachments !== undefined) { + const checked = await engine.resolveScratchlistAttachmentsForSession( + sessionResult.sessionId, + namespace, + parsed.data.attachments + ) + if (!checked.ok) { + return c.json({ error: checked.error, code: 'scratchlist_attachment_invalid' }, 400) + } + nextAttachments = checked.attachments + } + if (nextText.trim().length === 0 && nextAttachments.length === 0) { + return c.json({ + error: 'Scratchlist entry requires text or attachments', + code: 'scratchlist_entry_empty', + }, 400) + } + const limits = loadScratchlistAttachmentLimitsFromEnv() + const diskBytes = await engine.sumScratchlistAttachmentBytesOnDisk(sessionResult.sessionId, namespace) + const removedAttachments = existing.attachments.filter( + (old) => !nextAttachments.some((next) => next.id === old.id) + ) + const sessionBytesBefore = scratchlistSessionBytesBeforeForPut( + diskBytes, + nextAttachments, + removedAttachments, + ) + const attachmentValidation = validateScratchlistAttachmentsForWrite( + nextAttachments, + limits, + sessionBytesBefore + ) + if (!attachmentValidation.ok) { + return c.json({ error: attachmentValidation.error, code: attachmentValidation.code }, 400) + } + const updated = engine.updateScratchlistEntry( sessionResult.sessionId, entryId, - parsed.data.text + { + text: nextText, + attachments: nextAttachments, + } ) if (!updated) { return c.json({ error: 'Scratchlist entry not found' }, 404) } + if (removedAttachments.length > 0) { + const remainingIds = new Set( + engine + .listScratchlistEntries(sessionResult.sessionId) + .flatMap((entry) => entry.attachments.map((att) => att.id)) + ) + const orphaned = removedAttachments.filter((att) => !remainingIds.has(att.id)) + if (orphaned.length > 0) { + void import('../../scratchlistAttachments/storage').then(({ deleteScratchlistAttachmentFiles, getHapiHomeDir }) => + deleteScratchlistAttachmentFiles(getHapiHomeDir(), orphaned) + ) + } + } return c.json({ entry: updated }) }) + app.delete('/sessions/:id/scratchlist/attachments/:attachmentId', async (c) => { + const engine = requireSyncEngine(c, getSyncEngine) + if (engine instanceof Response) { + return engine + } + const sessionResult = requireSessionFromParam(c, engine) + if (sessionResult instanceof Response) { + return sessionResult + } + const attachmentId = c.req.param('attachmentId') + if (!attachmentId) { + return c.json({ error: 'Missing attachmentId' }, 400) + } + + const entries = engine.listScratchlistEntries(sessionResult.sessionId) + const stillReferenced = entries.some((entry) => + entry.attachments.some((att) => att.id === attachmentId) + ) + if (stillReferenced) { + return c.json({ + error: 'Attachment is still referenced by a scratchlist entry', + code: 'scratchlist_attachment_in_use', + }, 409) + } + + const removed = await engine.deleteScratchlistAttachmentById( + sessionResult.sessionId, + c.get('namespace'), + attachmentId + ) + if (!removed) { + return c.json({ error: 'Attachment not found' }, 404) + } + return c.json({ ok: true }) + }) + app.delete('/sessions/:id/scratchlist/:entryId', (c) => { const engine = requireSyncEngine(c, getSyncEngine) if (engine instanceof Response) { diff --git a/shared/src/apiTypes.ts b/shared/src/apiTypes.ts index d678e105..5c58a1b3 100644 --- a/shared/src/apiTypes.ts +++ b/shared/src/apiTypes.ts @@ -271,6 +271,8 @@ export const SCRATCHLIST_MAX_TEXT_LENGTH = 10_000 */ export const SCRATCHLIST_MAX_ENTRY_ID_LENGTH = 128 +import { ScratchlistAttachmentsArraySchema } from './scratchlistAttachments' + export const ScratchlistEntryCreateRequestSchema = z.object({ /** * Optional client-supplied entry id. Lets the web client preserve its @@ -280,20 +282,43 @@ export const ScratchlistEntryCreateRequestSchema = z.object({ * generate one. */ entryId: z.string().min(1).max(SCRATCHLIST_MAX_ENTRY_ID_LENGTH).optional(), - text: z.string().min(1).max(SCRATCHLIST_MAX_TEXT_LENGTH), + text: z.string().max(SCRATCHLIST_MAX_TEXT_LENGTH).default(''), + attachments: ScratchlistAttachmentsArraySchema.optional().default([]), /** * Optional client-supplied createdAt. Used by the migration path to * preserve the original timestamps from localStorage. New entries * omit this and let the hub stamp `Date.now()`. */ createdAt: z.number().int().nonnegative().optional() -}) +}).refine( + (data) => data.text.trim().length > 0 || data.attachments.length > 0, + { message: 'Scratchlist entry requires text or attachments', path: ['text'] } +) export type ScratchlistEntryCreateRequest = z.infer export const ScratchlistEntryUpdateRequestSchema = z.object({ - text: z.string().min(1).max(SCRATCHLIST_MAX_TEXT_LENGTH) -}) + text: z.string().max(SCRATCHLIST_MAX_TEXT_LENGTH).optional(), + attachments: ScratchlistAttachmentsArraySchema.optional(), +}).refine( + (data) => data.text !== undefined || data.attachments !== undefined, + { message: 'Update requires text and/or attachments', path: ['text'] } +).refine( + (data) => { + // Attachments-only patch may clear the list (`[]`) while keeping text. + if (data.text === undefined && data.attachments !== undefined) { + return true + } + if (data.text !== undefined && data.attachments === undefined) { + return data.text.trim().length > 0 + } + if (data.text !== undefined && data.attachments !== undefined) { + return data.text.trim().length > 0 || data.attachments.length > 0 + } + return true + }, + { message: 'Scratchlist entry requires non-empty text or attachments', path: ['text'] } +) export type ScratchlistEntryUpdateRequest = z.infer diff --git a/shared/src/index.ts b/shared/src/index.ts index 0aaf85f2..3ea41a83 100644 --- a/shared/src/index.ts +++ b/shared/src/index.ts @@ -1,3 +1,4 @@ +export * from './scratchlistAttachments' export * from './apiTypes' export * from './cursorCliSku' export * from './messages' diff --git a/shared/src/schemas.ts b/shared/src/schemas.ts index 7d8f104e..c7ff455f 100644 --- a/shared/src/schemas.ts +++ b/shared/src/schemas.ts @@ -268,7 +268,14 @@ export const ScratchlistEntrySchema = z.object({ entryId: z.string().min(1), text: z.string(), createdAt: z.number(), - updatedAt: z.number() + updatedAt: z.number(), + attachments: z.array(z.object({ + id: z.string(), + filename: z.string(), + mimeType: z.string(), + size: z.number(), + path: z.string(), + })).optional().default([]) }) export type ScratchlistEntry = z.infer diff --git a/shared/src/scratchlistAttachments.ts b/shared/src/scratchlistAttachments.ts new file mode 100644 index 00000000..02dd923d --- /dev/null +++ b/shared/src/scratchlistAttachments.ts @@ -0,0 +1,89 @@ +import { z } from 'zod' + +/** Hub-resident scratchlist files use this path prefix in AttachmentMetadata.path */ +export const HUB_SCRATCHLIST_ATTACHMENT_PATH_PREFIX = 'hapi-hub:scratchlist/' + +export function isHubScratchlistAttachmentPath(path: string): boolean { + return path.startsWith(HUB_SCRATCHLIST_ATTACHMENT_PATH_PREFIX) +} + +export function toHubScratchlistAttachmentPath(storageKey: string): string { + return `${HUB_SCRATCHLIST_ATTACHMENT_PATH_PREFIX}${storageKey}` +} + +export function parseHubScratchlistAttachmentPath(path: string): string | null { + if (!isHubScratchlistAttachmentPath(path)) { + return null + } + const key = path.slice(HUB_SCRATCHLIST_ATTACHMENT_PATH_PREFIX.length).trim() + return key.length > 0 ? key : null +} + +export const SCRATCHLIST_ATTACHMENT_DEFAULT_MAX_BYTES_PER_FILE = 10 * 1024 * 1024 +export const SCRATCHLIST_ATTACHMENT_DEFAULT_MAX_PER_ENTRY = 4 +export const SCRATCHLIST_ATTACHMENT_DEFAULT_MAX_BYTES_PER_ENTRY = 20 * 1024 * 1024 +export const SCRATCHLIST_ATTACHMENT_DEFAULT_MAX_BYTES_PER_SESSION = 100 * 1024 * 1024 + +export const SCRATCHLIST_ATTACHMENT_DEFAULT_ALLOWED_MIMES = [ + 'image/jpeg', + 'image/png', + 'image/gif', + 'image/webp', + 'image/svg+xml', + 'application/pdf', + 'text/plain', +] as const + +export const ScratchlistAttachmentLimitsSchema = z.object({ + maxBytesPerFile: z.number().int().positive(), + maxAttachmentsPerEntry: z.number().int().positive(), + maxBytesPerEntry: z.number().int().positive(), + maxBytesPerSession: z.number().int().positive(), + allowedMimeTypes: z.array(z.string().min(1)), +}) + +export type ScratchlistAttachmentLimits = z.infer + +export const SCRATCHLIST_ATTACHMENT_DEFAULT_LIMITS: ScratchlistAttachmentLimits = { + maxBytesPerFile: SCRATCHLIST_ATTACHMENT_DEFAULT_MAX_BYTES_PER_FILE, + maxAttachmentsPerEntry: SCRATCHLIST_ATTACHMENT_DEFAULT_MAX_PER_ENTRY, + maxBytesPerEntry: SCRATCHLIST_ATTACHMENT_DEFAULT_MAX_BYTES_PER_ENTRY, + maxBytesPerSession: SCRATCHLIST_ATTACHMENT_DEFAULT_MAX_BYTES_PER_SESSION, + allowedMimeTypes: [...SCRATCHLIST_ATTACHMENT_DEFAULT_ALLOWED_MIMES], +} + +/** Persisted attachment metadata — previewUrl is client-only and stripped at hub write */ +export const ScratchlistAttachmentMetadataSchema = z.object({ + id: z.string(), + filename: z.string(), + mimeType: z.string(), + size: z.number(), + path: z.string(), +}) + +export type ScratchlistAttachmentMetadata = z.infer + +export const ScratchlistAttachmentsArraySchema = z.array(ScratchlistAttachmentMetadataSchema) + +export function parseScratchlistAttachmentsJson(raw: string | null | undefined): ScratchlistAttachmentMetadata[] { + if (!raw) return [] + let parsed: unknown + try { + parsed = JSON.parse(raw) + } catch { + return [] + } + const result = ScratchlistAttachmentsArraySchema.safeParse(parsed) + return result.success ? result.data : [] +} + +export function serializeScratchlistAttachments(attachments: ScratchlistAttachmentMetadata[]): string | null { + if (attachments.length === 0) return null + return JSON.stringify(attachments) +} + +export function stripPreviewUrls( + attachments: Array<{ previewUrl?: string } & ScratchlistAttachmentMetadata> +): ScratchlistAttachmentMetadata[] { + return attachments.map(({ previewUrl: _preview, ...rest }) => rest) +} diff --git a/web/src/api/client.ts b/web/src/api/client.ts index 56459aa0..bef02a80 100644 --- a/web/src/api/client.ts +++ b/web/src/api/client.ts @@ -775,18 +775,81 @@ export class ApiClient { */ async getScratchlist(sessionId: string): Promise<{ - entries: Array<{ entryId: string; text: string; createdAt: number; updatedAt: number }> + entries: Array<{ + entryId: string + text: string + createdAt: number + updatedAt: number + attachments: import('@hapi/protocol').ScratchlistAttachmentMetadata[] + }> }> { return await this.request( `/api/sessions/${encodeURIComponent(sessionId)}/scratchlist` ) } + async uploadScratchlistAttachment( + sessionId: string, + filename: string, + content: string, + mimeType: string + ): Promise<{ + success: boolean + attachment?: import('@hapi/protocol').ScratchlistAttachmentMetadata + error?: string + code?: string + }> { + return await this.request( + `/api/sessions/${encodeURIComponent(sessionId)}/scratchlist/upload`, + { + method: 'POST', + body: JSON.stringify({ filename, content, mimeType }) + } + ) + } + + async fetchScratchlistAttachmentBlob(sessionId: string, attachmentId: string): Promise { + const headers = new Headers() + const liveToken = this.getToken ? this.getToken() : null + const authToken = liveToken ?? this.token + if (authToken) { + headers.set('authorization', `Bearer ${authToken}`) + } + const response = await fetch( + this.buildUrl( + `/api/sessions/${encodeURIComponent(sessionId)}/scratchlist/attachments/${encodeURIComponent(attachmentId)}` + ), + { headers } + ) + if (!response.ok) { + throw new ApiError(`Failed to fetch scratchlist attachment (${response.status})`, response.status) + } + return await response.blob() + } + + async deleteScratchlistAttachment(sessionId: string, attachmentId: string): Promise { + await this.request( + `/api/sessions/${encodeURIComponent(sessionId)}/scratchlist/attachments/${encodeURIComponent(attachmentId)}`, + { method: 'DELETE' } + ) + } + async createScratchlistEntry( sessionId: string, - body: { text: string; entryId?: string; createdAt?: number } + body: { + text: string + entryId?: string + createdAt?: number + attachments?: import('@hapi/protocol').ScratchlistAttachmentMetadata[] + } ): Promise<{ - entry: { entryId: string; text: string; createdAt: number; updatedAt: number } + entry: { + entryId: string + text: string + createdAt: number + updatedAt: number + attachments: import('@hapi/protocol').ScratchlistAttachmentMetadata[] + } }> { return await this.request( `/api/sessions/${encodeURIComponent(sessionId)}/scratchlist`, @@ -802,7 +865,13 @@ export class ApiClient { entryId: string, text: string ): Promise<{ - entry: { entryId: string; text: string; createdAt: number; updatedAt: number } + entry: { + entryId: string + text: string + createdAt: number + updatedAt: number + attachments: import('@hapi/protocol').ScratchlistAttachmentMetadata[] + } }> { return await this.request( `/api/sessions/${encodeURIComponent(sessionId)}/scratchlist/${encodeURIComponent(entryId)}`, diff --git a/web/src/components/AssistantChat/ComposerButtons.test.tsx b/web/src/components/AssistantChat/ComposerButtons.test.tsx index 90ed25be..c8fe33ca 100644 --- a/web/src/components/AssistantChat/ComposerButtons.test.tsx +++ b/web/src/components/AssistantChat/ComposerButtons.test.tsx @@ -48,10 +48,9 @@ describe('UnifiedButton — routesToScratchlist visual state', () => { expect(btn.className).toContain('bg-amber-500') }) - it('paints chat black + announces "Send" when routesToScratchlist=false even if scratchlist toggle conceptually on', () => { - // Caller computed routesToScratchlist=false because the payload - // would carry attachments or a pending schedule. The button must - // therefore look like a normal chat send. + it('paints chat black + announces "Send" when routesToScratchlist=false (e.g. pending schedule)', () => { + // Caller computed routesToScratchlist=false because a pending schedule + // forces chat fallback. The button must look like a normal chat send. renderInProviders( diff --git a/web/src/components/AssistantChat/ScratchlistPanel.test.tsx b/web/src/components/AssistantChat/ScratchlistPanel.test.tsx index 47768e7c..fe34d5fb 100644 --- a/web/src/components/AssistantChat/ScratchlistPanel.test.tsx +++ b/web/src/components/AssistantChat/ScratchlistPanel.test.tsx @@ -237,7 +237,7 @@ describe('ScratchlistPanel', () => { renderPanel() expandPanel() - const copyBtn = screen.getByRole('button', { name: 'Copy to clipboard' }) + const copyBtn = screen.getByRole('button', { name: 'Copy text to clipboard (not images)' }) fireEvent.click(copyBtn) await waitFor(() => expect(writeText).toHaveBeenCalledWith('copy me')) @@ -272,12 +272,12 @@ describe('ScratchlistPanel', () => { renderPanel() expandPanel() - fireEvent.click(screen.getByRole('button', { name: 'Copy to clipboard' })) + fireEvent.click(screen.getByRole('button', { name: 'Copy text to clipboard (not images)' })) await waitFor(() => expect(writeText).toHaveBeenCalled()) // Should NOT flip to "Copied!" because the copy failed. expect(screen.queryByRole('button', { name: 'Copied!' })).toBeNull() - expect(screen.getByRole('button', { name: 'Copy to clipboard' })).toBeTruthy() + expect(screen.getByRole('button', { name: 'Copy text to clipboard (not images)' })).toBeTruthy() }) it('persists collapse state across mounts for the same session', () => { diff --git a/web/src/components/AssistantChat/ScratchlistPanel.tsx b/web/src/components/AssistantChat/ScratchlistPanel.tsx index c9534f9a..0e3df3ba 100644 --- a/web/src/components/AssistantChat/ScratchlistPanel.tsx +++ b/web/src/components/AssistantChat/ScratchlistPanel.tsx @@ -18,6 +18,9 @@ import { shouldConfirmDelete, type ScratchlistEntry, } from '@/lib/scratchlist' +import type { ApiClient } from '@/api/client' +import type { ScratchlistAttachmentMetadata } from '@hapi/protocol' +import { isImageMimeType } from '@/lib/fileAttachments' import { safeCopyToClipboard } from '@/lib/clipboard' import { useTranslation } from '@/lib/use-translation' import { formatAbsoluteDateTime, formatRelativeTime } from '@/lib/relativeTime' @@ -243,6 +246,64 @@ function useCopiedFeedback(clearAfterMs: number = COPIED_FEEDBACK_MS) { * entries + callbacks. Used by both the always-visible ScratchlistPanel * and the composer-controlled drawer below. */ +function ScratchlistAttachmentThumbnails(props: { + sessionId: string + api: ApiClient + attachments: ScratchlistAttachmentMetadata[] +}) { + const [urls, setUrls] = useState>([]) + + useEffect(() => { + let cancelled = false + const created: string[] = [] + void (async () => { + const next: Array<{ id: string; url: string; filename: string }> = [] + for (const attachment of props.attachments) { + if (!isImageMimeType(attachment.mimeType)) continue + try { + const blob = await props.api.fetchScratchlistAttachmentBlob(props.sessionId, attachment.id) + const url = URL.createObjectURL(blob) + created.push(url) + next.push({ id: attachment.id, url, filename: attachment.filename }) + } catch { + // Non-fatal: entry still shows text/actions. + } + } + if (!cancelled) { + setUrls(next) + } else { + for (const url of created) URL.revokeObjectURL(url) + } + })() + return () => { + cancelled = true + setUrls((prev) => { + for (const item of prev) URL.revokeObjectURL(item.url) + return [] + }) + } + }, [props.api, props.sessionId, props.attachments]) + + if (urls.length === 0) return null + + return ( +
+ {urls.map((item) => ( + {item.filename} + ))} +
+ ) +} + function ScratchlistInventory({ entries, busyEntryId, @@ -250,13 +311,17 @@ function ScratchlistInventory({ onPromoteToQueue, onDelete, onMove, + sessionId, + api, }: { entries: ScratchlistEntry[] busyEntryId: string | null - onPromoteToComposer: (entry: ScratchlistEntry) => void - onPromoteToQueue: (entry: ScratchlistEntry) => void + onPromoteToComposer: (entry: ScratchlistEntry) => void | Promise + onPromoteToQueue: (entry: ScratchlistEntry) => void | Promise onDelete: (entry: ScratchlistEntry) => void onMove: (entry: ScratchlistEntry, direction: 'up' | 'down') => void + sessionId?: string + api?: ApiClient }) { const { t } = useTranslation() const { copiedEntryId, signalCopied } = useCopiedFeedback() @@ -289,13 +354,29 @@ function ScratchlistInventory({ return (
  • - - {entry.text} - -
    +
    +
    + {sessionId && api && entry.attachments && entry.attachments.length > 0 ? ( + + ) : null} +

    + {entry.text || (entry.attachments?.length ? t('scratchlist.attachmentOnly') : '')} +

    +
    +
    +
    • ) })} @@ -387,12 +469,16 @@ export function ScratchlistDrawer({ onDelete, onPromoteToComposer, onPromoteToQueue, + sessionId, + api, }: { entries: ScratchlistEntry[] onMove: (id: string, direction: 'up' | 'down') => void onDelete: (id: string) => void - onPromoteToComposer: (text: string) => void - onPromoteToQueue: (text: string) => Promise + onPromoteToComposer: (entry: ScratchlistEntry) => void | Promise + onPromoteToQueue: (entry: ScratchlistEntry) => Promise + sessionId: string + api: ApiClient }) { const { t } = useTranslation() const [busyEntryId, setBusyEntryId] = useState(null) @@ -418,14 +504,14 @@ export function ScratchlistDrawer({ }, [onMove]) const handlePromoteToComposer = useCallback((entry: ScratchlistEntry) => { - onPromoteToComposer(entry.text) + void onPromoteToComposer(entry) }, [onPromoteToComposer]) const handlePromoteToQueue = useCallback(async (entry: ScratchlistEntry) => { if (busyEntryId) return setBusyEntryId(entry.id) try { - const accepted = await onPromoteToQueue(entry.text) + const accepted = await onPromoteToQueue(entry) if (accepted) onDelete(entry.id) } finally { setBusyEntryId(null) @@ -461,6 +547,8 @@ export function ScratchlistDrawer({ { render( { render( { expect(queueButtons.length).toBeGreaterThan(0) fireEvent.click(queueButtons[0]!) - await waitFor(() => expect(onSend).toHaveBeenCalledWith('send-to-queue text')) + await waitFor(() => expect(onSend).toHaveBeenCalledWith('send-to-queue text', undefined)) expect(onExitScratchlistMode).toHaveBeenCalledTimes(1) expect(setText).not.toHaveBeenCalled() }) @@ -107,6 +118,8 @@ describe('ScratchlistDrawerHost.onPromoteToComposer', () => { render( { const queueButtons = screen.getAllByRole('button', { name: /queue|send/i }) fireEvent.click(queueButtons[0]!) - await waitFor(() => expect(onSend).toHaveBeenCalledWith('send-to-queue text')) + await waitFor(() => expect(onSend).toHaveBeenCalledWith('send-to-queue text', undefined)) expect(onExitScratchlistMode).not.toHaveBeenCalled() expect(setText).not.toHaveBeenCalled() }) @@ -143,6 +156,8 @@ describe('ScratchlistDrawer copy-to-clipboard action', () => { render( { , ) - fireEvent.click(screen.getByRole('button', { name: 'Copy to clipboard' })) + fireEvent.click(screen.getByRole('button', { name: 'Copy text to clipboard (not images)' })) await waitFor(() => expect(writeText).toHaveBeenCalledWith('copy this')) await waitFor(() => diff --git a/web/src/components/SessionChat.test.ts b/web/src/components/SessionChat.test.ts index f7e725a4..a16bd228 100644 --- a/web/src/components/SessionChat.test.ts +++ b/web/src/components/SessionChat.test.ts @@ -133,24 +133,25 @@ describe('shouldAutoClearPendingSchedule', () => { /** * Unit tests for shouldRouteToScratchlist. * - * Regression cover for upstream review on PR #798 (github-actions[bot] - * [Major]): scratchlist-mode submissions used to silently drop - * attachments and scheduledAt because the wrapper short-circuited to - * scratchlist.add(text) regardless of payload. The fix is to fall - * through to the regular chat send whenever the submission can't be - * represented as a pure-text scratchlist entry. + * Regression cover for upstream review on PR #798 / #1205: scratchlist-mode + * submissions must fall through to chat when the payload cannot be parked + * (schedule set, or any attachment still on a normal CLI upload path). */ describe('shouldRouteToScratchlist', () => { - function attachment(): AttachmentMetadata { + function attachment(path = '/tmp/attach-1.png'): AttachmentMetadata { return { id: 'attach-1', filename: 'attach-1.png', mimeType: 'image/png', size: 1024, - path: '/tmp/attach-1.png', + path, } } + function hubAttachment(): AttachmentMetadata { + return attachment('hapi-hub:scratchlist/default/session-1/aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee-a.png') + } + it('returns false when scratchlist mode is off, regardless of payload', () => { expect(shouldRouteToScratchlist(false, undefined, null)).toBe(false) expect(shouldRouteToScratchlist(false, [attachment()], null)).toBe(false) @@ -163,9 +164,14 @@ describe('shouldRouteToScratchlist', () => { expect(shouldRouteToScratchlist(true, [], null)).toBe(true) }) - it('returns false when scratchlist mode is on but attachments are present', () => { + it('returns true when every attachment is already hub-resident', () => { + expect(shouldRouteToScratchlist(true, [hubAttachment()], null)).toBe(true) + expect(shouldRouteToScratchlist(true, [hubAttachment(), hubAttachment()], null)).toBe(true) + }) + + it('returns false when any attachment still has a normal CLI path', () => { expect(shouldRouteToScratchlist(true, [attachment()], null)).toBe(false) - expect(shouldRouteToScratchlist(true, [attachment(), attachment()], null)).toBe(false) + expect(shouldRouteToScratchlist(true, [hubAttachment(), attachment()], null)).toBe(false) }) it('returns false when scratchlist mode is on but a scheduled-send is set', () => { @@ -174,7 +180,7 @@ describe('shouldRouteToScratchlist', () => { }) it('returns false when both attachments and scheduledAt are set', () => { - expect(shouldRouteToScratchlist(true, [attachment()], Date.now() + 60_000)).toBe(false) + expect(shouldRouteToScratchlist(true, [hubAttachment()], Date.now() + 60_000)).toBe(false) }) /** diff --git a/web/src/components/SessionChat.tsx b/web/src/components/SessionChat.tsx index 89426bd4..6afc8d92 100644 --- a/web/src/components/SessionChat.tsx +++ b/web/src/components/SessionChat.tsx @@ -1,4 +1,5 @@ import { useCallback, useEffect, useMemo, useRef, useState } from 'react' +import { flushSync } from 'react-dom' import { useNavigate } from '@tanstack/react-router' import { AssistantRuntimeProvider, useAssistantApi, useAssistantState } from '@assistant-ui/react' import { DragDropZone } from '@/components/AssistantChat/DragDropZone' @@ -36,6 +37,13 @@ import { useHubScratchlist } from '@/lib/use-hub-scratchlist' import { ScratchlistMigrationBanner } from '@/components/AssistantChat/ScratchlistMigrationBanner' import { useHappyRuntime } from '@/lib/assistant-runtime' import { createAttachmentAdapter } from '@/lib/attachmentAdapter' +import { createScratchlistAttachmentAdapter } from '@/lib/scratchlistAttachmentAdapter' +import { + rehydrateScratchlistAttachmentsToComposer, + stageScratchlistAttachmentsForComposeSend +} from '@/lib/scratchlistAttachmentFlow' +import type { ScratchlistEntry } from '@/lib/scratchlist' +import { isHubScratchlistAttachmentPath } from '@hapi/protocol' import { consumeSharePendingTransfer } from '@/lib/sharePendingState' import { deleteShareTransfer, getShareTransfer } from '@/lib/shareTransfer' import { getDraft } from '@/lib/composer-drafts' @@ -182,15 +190,8 @@ export function isScratchlistHotkeyBlockedTarget(target: EventTarget | null): bo /** * Decide whether a submit should be routed to the per-session scratchlist - * or to the regular chat send. Scratchlist entries are pure text - they - * don't carry attachments or schedules - so any submit that includes - * either of those MUST fall through to the normal chat path even if the - * scratchlist toggle is on. Otherwise the wrapper would silently drop - * attachments / scheduled-send metadata while telling the composer the - * submission succeeded (which then clears the composer state, losing - * the user's data). - * - * Per upstream review on PR #798 (github-actions[bot] [Major]). + * or to the regular chat send. Scratchlist entries support text and hub- + * stored attachments; scheduled sends still fall through to chat. * * Pure / exported so it can be unit tested without mounting SessionChat. */ @@ -200,9 +201,11 @@ export function shouldRouteToScratchlist( scheduledAt: number | null | undefined, ): boolean { if (!scratchlistMode) return false - if (attachments && attachments.length > 0) return false if (scheduledAt != null) return false - return true + // Only park when every attachment is already hub-resident. Composer + // uploads made before scratchlist mode was enabled still have normal + // CLI paths; the hub rejects those as scratchlist metadata. + return (attachments ?? []).every((att) => isHubScratchlistAttachmentPath(att.path)) } function isUninvokedScheduledMessage(message: DecryptedMessage): boolean { @@ -313,41 +316,51 @@ function ShareSeedConsumer(props: { sessionId: string; sessionActive: boolean }) * composer-toolbar counter and the drawer share one source of truth. */ export function ScratchlistDrawerHost(props: { + sessionId: string + api: ApiClient entries: ReturnType['entries'] onMove: ReturnType['move'] onDelete: ReturnType['remove'] onSend: (text: string, attachments?: AttachmentMetadata[], scheduledAt?: number | null) => Promise - /** - * Called when the operator promotes an entry to the composer. - * - * Promoting means "I want to send this for real now" - so the host - * MUST exit scratchlist mode, otherwise the next composer submit - * routes back to scratchlist (per the v1.1 modal-mode contract) and - * the user re-adds the same text instead of sending it to chat. - * Per upstream review on PR #798 (HAPI Bot, v6 follow-up). - */ onExitScratchlistMode: () => void }) { const assistantApi = useAssistantApi() - const handlePromoteToComposer = useCallback((text: string) => { - assistantApi.composer().setText(text) - props.onExitScratchlistMode() - }, [assistantApi, props.onExitScratchlistMode]) - const handlePromoteToQueue = useCallback(async (text: string) => { - // Promote-to-queue bypasses the scratchlist-mode wrapper by - // calling props.onSend directly (the chat send), so the queue - // entry lands in the conversation regardless of scratchlist - // mode. After a successful send, exit scratchlist mode so the - // operator can continue normal chat (issue #959). - const accepted = await props.onSend(text) + const handlePromoteToComposer = useCallback(async (entry: ScratchlistEntry) => { + assistantApi.composer().setText(entry.text) + // Exit scratchlist mode before rehydrating attachments so addAttachment + // uses the normal chat upload adapter (not the scratchlist hub adapter). + flushSync(() => { + props.onExitScratchlistMode() + }) + if (entry.attachments && entry.attachments.length > 0) { + await rehydrateScratchlistAttachmentsToComposer( + props.api, + props.sessionId, + entry.attachments, + assistantApi.composer() + ) + } + }, [assistantApi, props.api, props.onExitScratchlistMode, props.sessionId]) + const handlePromoteToQueue = useCallback(async (entry: ScratchlistEntry) => { + let attachments: AttachmentMetadata[] | undefined + if (entry.attachments && entry.attachments.length > 0) { + attachments = await stageScratchlistAttachmentsForComposeSend( + props.api, + props.sessionId, + entry.attachments + ) + } + const accepted = await props.onSend(entry.text, attachments) if (accepted) { props.onExitScratchlistMode() } return accepted - }, [props.onSend, props.onExitScratchlistMode]) + }, [props.api, props.onSend, props.onExitScratchlistMode, props.sessionId]) return ( => { if (shouldRouteToScratchlist(scratchlistMode, attachments, scheduledAt)) { - return scratchlist.add(text) + return scratchlist.add(text, attachments) + } + // If the user uploaded while scratchlist mode was on, then toggled + // it off before send, pending items still carry hub paths. Stage + // those through the normal CLI upload dir before chat send. + const list = attachments ?? [] + const hubItems = list.filter((att) => isHubScratchlistAttachmentPath(att.path)) + if (hubItems.length > 0) { + const normalItems = list.filter((att) => !isHubScratchlistAttachmentPath(att.path)) + const staged = await stageScratchlistAttachmentsForComposeSend( + props.api, + props.session.id, + hubItems, + ) + const accepted = await props.onSend(text, [...normalItems, ...staged], scheduledAt) + if (accepted) { + // Hub blobs were copied into the normal upload dir; drop the + // scratchlist copies so they stop counting against the session cap. + await Promise.allSettled( + hubItems.map((att) => props.api.deleteScratchlistAttachment(props.session.id, att.id)) + ) + } + return accepted } return props.onSend(text, attachments, scheduledAt) }, - [props.onSend, scratchlist, scratchlistMode], + [props.onSend, props.api, props.session.id, scratchlist, scratchlistMode], ) const agentFlavor = props.session.metadata?.flavor ?? null const controlledByUser = props.session.agentState?.controlledByUser === true @@ -1186,8 +1214,10 @@ function SessionChatInner(props: SessionChatProps) { if (!props.session.active) { return undefined } - return createAttachmentAdapter(props.api, props.session.id) - }, [props.api, props.session.id, props.session.active]) + return scratchlistMode + ? createScratchlistAttachmentAdapter(props.api, props.session.id) + : createAttachmentAdapter(props.api, props.session.id) + }, [props.api, props.session.id, props.session.active, scratchlistMode]) const runtime = useHappyRuntime({ session: props.session, @@ -1308,6 +1338,8 @@ function SessionChatInner(props: SessionChatProps) { */} {scratchlistMode ? ( { + it('reconstructs hub metadata from hapi-hub:scratchlist storage key', () => { + const file = new File([new Uint8Array([1, 2, 3])], 'proof.png', { type: 'image/png' }) + const path = 'hapi-hub:scratchlist/default/session-1/a1b2c3d4-e5f6-4789-a012-3456789abcde-proof.png' + expect(hubAttachmentFromRestoredDraft(path, file, 'image/png')).toEqual({ + id: 'a1b2c3d4-e5f6-4789-a012-3456789abcde', + filename: 'proof.png', + mimeType: 'image/png', + size: 3, + path, + }) + }) + + it('returns null for non-hub paths', () => { + const file = new File(['x'], 'x.txt', { type: 'text/plain' }) + expect(hubAttachmentFromRestoredDraft('/uploads/x.txt', file, 'text/plain')).toBeNull() + }) +}) + +describe('createScratchlistAttachmentAdapter', () => { + it('reuses restored hub path without re-uploading', async () => { + const drafts = await import('./composer-attachment-drafts') + const hubId = 'a1b2c3d4-e5f6-4789-a012-3456789abcde' + const path = `hapi-hub:scratchlist/default/session-1/${hubId}-proof.png` + const file = new File([new Uint8Array([137, 80, 78, 71])], 'proof.png', { type: 'image/png' }) + drafts.saveDraftAttachments('session-restore', [{ + id: 'composer-att-1', + file, + path, + previewUrl: 'data:image/png;base64,aW1hZ2U=', + }]) + const [restored] = await drafts.getDraftAttachments('session-restore') + expect(restored).toBeTruthy() + + const uploadScratchlistAttachment = vi.fn() + const api = { uploadScratchlistAttachment } as never + const adapter = createScratchlistAttachmentAdapter(api, 'session-1') + + const iter = adapter.add({ file: restored! }) as AsyncGenerator< + import('@assistant-ui/react').PendingAttachment + > + const states: import('@assistant-ui/react').PendingAttachment[] = [] + for await (const pending of iter) { + states.push(pending) + } + + expect(uploadScratchlistAttachment).not.toHaveBeenCalled() + expect(states).toHaveLength(1) + const ready = states[0] as { + id: string + status: unknown + path?: string + hubAttachment?: { id: string; path: string } + previewUrl?: string + } + expect(ready.id).toBe('composer-att-1') + expect(ready.status).toEqual({ type: 'requires-action', reason: 'composer-send' }) + expect(ready.path).toBe(path) + expect(ready.hubAttachment).toEqual({ + id: hubId, + filename: 'proof.png', + mimeType: 'image/png', + size: restored!.size, + path, + }) + expect(ready.previewUrl).toBe('data:image/png;base64,aW1hZ2U=') + }) + + it('sets path on requires-action yield so composer canSend unlocks after hub upload', async () => { + const uploadScratchlistAttachment = vi.fn().mockResolvedValue({ + success: true, + attachment: { + id: 'hub-1', + filename: 'proof.png', + mimeType: 'image/png', + size: 12, + path: '/scratchlist/sessions/s1/proof.png', + }, + }) + const api = { uploadScratchlistAttachment } as never + const adapter = createScratchlistAttachmentAdapter(api, 'session-1') + const file = new File([new Uint8Array([137, 80, 78, 71])], 'proof.png', { type: 'image/png' }) + + const iter = adapter.add({ file }) as AsyncGenerator + const states: import('@assistant-ui/react').PendingAttachment[] = [] + for await (const pending of iter) { + states.push(pending) + } + + const ready = states.at(-1) + expect(ready?.status).toEqual({ type: 'requires-action', reason: 'composer-send' }) + expect((ready as { path?: string }).path).toBe('/scratchlist/sessions/s1/proof.png') + }) + + it('deletes hub blob when cancel races the in-flight upload completion', async () => { + let pendingId = '' + let adapter: ReturnType + const deleteScratchlistAttachment = vi.fn().mockResolvedValue(undefined) + const uploadScratchlistAttachment = vi.fn().mockImplementation(async () => { + await adapter.remove({ + id: pendingId, + type: 'file', + name: 'proof.png', + contentType: 'image/png', + status: { type: 'running', reason: 'uploading', progress: 50 }, + } as never) + return { + success: true, + attachment: { + id: 'hub-race', + filename: 'proof.png', + mimeType: 'image/png', + size: 4, + path: 'hapi-hub:scratchlist/default/session-1/hub-race-proof.png', + }, + } + }) + const api = { uploadScratchlistAttachment, deleteScratchlistAttachment } as never + adapter = createScratchlistAttachmentAdapter(api, 'session-1') + const file = new File([new Uint8Array([137, 80, 78, 71])], 'proof.png', { type: 'image/png' }) + + const iter = adapter.add({ file }) as AsyncGenerator + const first = await iter.next() + pendingId = (first.value as { id: string }).id + + for await (const _pending of iter) { + // drain + } + + expect(deleteScratchlistAttachment).toHaveBeenCalledWith('session-1', 'hub-race') + }) +}) diff --git a/web/src/lib/scratchlistAttachmentAdapter.ts b/web/src/lib/scratchlistAttachmentAdapter.ts new file mode 100644 index 00000000..1ac16340 --- /dev/null +++ b/web/src/lib/scratchlistAttachmentAdapter.ts @@ -0,0 +1,234 @@ +import type { AttachmentAdapter, Attachment, CompleteAttachment, PendingAttachment } from '@assistant-ui/react' +import type { ScratchlistAttachmentMetadata } from '@hapi/protocol' +import { parseHubScratchlistAttachmentPath } from '@hapi/protocol' +import type { ApiClient } from '@/api/client' +import { getRestoredUploadMetadata } from '@/lib/composer-attachment-drafts' +import { isImageMimeType } from '@/lib/fileAttachments' +import { randomId } from '@/lib/randomId' + +const MAX_PREVIEW_BYTES = 5 * 1024 * 1024 + +/** Matches hub `SCRATCHLIST_ATTACHMENT_ID_RE` — file names are `${uuid}-${filename}`. */ +const HUB_ATTACHMENT_ID_PREFIX_RE = + /^([0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12})-/i + +type PendingScratchlistAttachment = PendingAttachment & { + /** Mirrors chat upload adapter — HappyComposer treats requires-action + path as ready. */ + path?: string + hubAttachment?: ScratchlistAttachmentMetadata + previewUrl?: string +} + +/** Rebuild hub metadata from a composer-draft path so remount does not re-upload. */ +export function hubAttachmentFromRestoredDraft( + path: string, + file: File, + contentType: string +): ScratchlistAttachmentMetadata | null { + const key = parseHubScratchlistAttachmentPath(path) + if (!key) return null + const storedName = key.split('/').pop() + if (!storedName) return null + const match = storedName.match(HUB_ATTACHMENT_ID_PREFIX_RE) + if (!match?.[1]) return null + return { + id: match[1], + filename: file.name, + mimeType: contentType, + size: file.size, + path, + } +} + +export function createScratchlistAttachmentAdapter(api: ApiClient, sessionId: string): AttachmentAdapter { + const cancelledAttachmentIds = new Set() + + return { + accept: '*/*', + + async *add({ file }): AsyncGenerator { + const contentType = file.type || 'application/octet-stream' + const restored = getRestoredUploadMetadata(file) + if (restored) { + const hubAttachment = hubAttachmentFromRestoredDraft(restored.path, file, contentType) + if (hubAttachment) { + yield { + id: restored.id, + type: 'file', + name: file.name, + contentType, + file, + status: { type: 'requires-action', reason: 'composer-send' }, + path: restored.path, + hubAttachment, + previewUrl: restored.previewUrl, + } as PendingScratchlistAttachment + return + } + } + + const id = randomId() + + yield { + id, + type: 'file', + name: file.name, + contentType, + file, + status: { type: 'running', reason: 'uploading', progress: 0 } + } + + try { + if (cancelledAttachmentIds.has(id)) { + return + } + + const content = await fileToBase64(file) + if (cancelledAttachmentIds.has(id)) { + return + } + + yield { + id, + type: 'file', + name: file.name, + contentType, + file, + status: { type: 'running', reason: 'uploading', progress: 50 } + } + + const result = await api.uploadScratchlistAttachment( + sessionId, + file.name, + content, + contentType + ) + if (cancelledAttachmentIds.has(id)) { + if (result.success && result.attachment) { + await api.deleteScratchlistAttachment(sessionId, result.attachment.id).catch(() => {}) + } + return + } + + if (!result.success || !result.attachment) { + yield { + id, + type: 'file', + name: file.name, + contentType, + file, + status: { type: 'incomplete', reason: 'error' } + } + return + } + + let previewUrl: string | undefined + if (isImageMimeType(contentType) && file.size <= MAX_PREVIEW_BYTES) { + previewUrl = await fileToDataUrl(file) + } + + yield { + id, + type: 'file', + name: file.name, + contentType, + file, + status: { type: 'requires-action', reason: 'composer-send' }, + path: result.attachment.path, + hubAttachment: result.attachment, + previewUrl + } as PendingScratchlistAttachment + } catch { + yield { + id, + type: 'file', + name: file.name, + contentType, + file, + status: { type: 'incomplete', reason: 'error' } + } + } + }, + + async remove(attachment: Attachment): Promise { + cancelledAttachmentIds.add(attachment.id) + const pending = attachment as PendingScratchlistAttachment + const hubId = pending.hubAttachment?.id + if (hubId) { + await api.deleteScratchlistAttachment(sessionId, hubId).catch(() => {}) + } + }, + + async send(attachment: PendingAttachment): Promise { + const pending = attachment as PendingScratchlistAttachment + const hubAttachment = pending.hubAttachment + + return { + id: attachment.id, + type: attachment.type, + name: attachment.name, + contentType: attachment.contentType, + status: { type: 'complete' }, + content: hubAttachment + ? [{ + type: 'text', + text: JSON.stringify({ + __attachmentMetadata: { + ...hubAttachment, + previewUrl: pending.previewUrl + } + }) + }] + : [] + } + } + } +} + +async function fileToBase64(file: File): Promise { + return new Promise((resolve, reject) => { + const reader = new FileReader() + reader.onload = () => { + const result = reader.result as string + const base64 = result.split(',')[1] + if (!base64) { + reject(new Error('Failed to read file')) + return + } + resolve(base64) + } + reader.onerror = reject + reader.readAsDataURL(file) + }) +} + +async function fileToDataUrl(file: File): Promise { + return new Promise((resolve, reject) => { + const reader = new FileReader() + reader.onload = () => { + resolve(reader.result as string) + } + reader.onerror = reject + reader.readAsDataURL(file) + }) +} + +export function extractScratchlistAttachmentMetadata( + attachments: import('@/types/api').AttachmentMetadata[] | undefined +): ScratchlistAttachmentMetadata[] { + if (!attachments || attachments.length === 0) return [] + const out: ScratchlistAttachmentMetadata[] = [] + for (const att of attachments) { + const rec = att as ScratchlistAttachmentMetadata & { previewUrl?: string } + if (rec.path && rec.id && rec.filename && rec.mimeType && typeof rec.size === 'number') { + out.push({ + id: rec.id, + filename: rec.filename, + mimeType: rec.mimeType, + size: rec.size, + path: rec.path + }) + } + } + return out +} diff --git a/web/src/lib/scratchlistAttachmentFlow.ts b/web/src/lib/scratchlistAttachmentFlow.ts new file mode 100644 index 00000000..1d5cb2f8 --- /dev/null +++ b/web/src/lib/scratchlistAttachmentFlow.ts @@ -0,0 +1,70 @@ +import type { ScratchlistAttachmentMetadata } from '@hapi/protocol' +import type { ApiClient } from '@/api/client' +import type { AttachmentMetadata } from '@/types/api' +import { isImageMimeType } from '@/lib/fileAttachments' + +async function blobToBase64(blob: Blob): Promise { + return new Promise((resolve, reject) => { + const reader = new FileReader() + reader.onload = () => { + const result = reader.result as string + const base64 = result.split(',')[1] + if (!base64) { + reject(new Error('Failed to read attachment')) + return + } + resolve(base64) + } + reader.onerror = reject + reader.readAsDataURL(blob) + }) +} + +export async function stageScratchlistAttachmentsForComposeSend( + api: ApiClient, + sessionId: string, + attachments: ScratchlistAttachmentMetadata[] +): Promise { + const staged: AttachmentMetadata[] = [] + try { + for (const attachment of attachments) { + const blob = await api.fetchScratchlistAttachmentBlob(sessionId, attachment.id) + const content = await blobToBase64(blob) + const upload = await api.uploadFile(sessionId, attachment.filename, content, attachment.mimeType) + if (!upload.success || !upload.path) { + throw new Error(`Failed to stage attachment ${attachment.filename}`) + } + let previewUrl: string | undefined + if (isImageMimeType(attachment.mimeType) && attachment.size <= 5 * 1024 * 1024) { + previewUrl = `data:${attachment.mimeType};base64,${content}` + } + staged.push({ + id: attachment.id, + filename: attachment.filename, + mimeType: attachment.mimeType, + size: attachment.size, + path: upload.path, + previewUrl + }) + } + return staged + } catch (error) { + await Promise.allSettled( + staged.map((att) => api.deleteUploadFile(sessionId, att.path)) + ) + throw error + } +} + +export async function rehydrateScratchlistAttachmentsToComposer( + api: ApiClient, + sessionId: string, + attachments: ScratchlistAttachmentMetadata[], + composer: { addAttachment: (file: File) => Promise } +): Promise { + for (const attachment of attachments) { + const blob = await api.fetchScratchlistAttachmentBlob(sessionId, attachment.id) + const file = new File([blob], attachment.filename, { type: attachment.mimeType }) + await composer.addAttachment(file) + } +} diff --git a/web/src/lib/use-hub-scratchlist.ts b/web/src/lib/use-hub-scratchlist.ts index 57b4811c..999a2080 100644 --- a/web/src/lib/use-hub-scratchlist.ts +++ b/web/src/lib/use-hub-scratchlist.ts @@ -1,8 +1,10 @@ import { useCallback, useEffect, useMemo, useRef, useState } from 'react' import { useMutation, useQuery, useQueryClient } from '@tanstack/react-query' +import type { ScratchlistAttachmentMetadata } from '@hapi/protocol' import type { ApiClient } from '@/api/client' import { ApiError } from '@/api/client' import { queryKeys } from '@/lib/query-keys' +import { extractScratchlistAttachmentMetadata } from '@/lib/scratchlistAttachmentAdapter' import { moveScratchlistEntry, persistScratchlist, @@ -71,6 +73,7 @@ type HubEntry = { text: string createdAt: number updatedAt: number + attachments: ScratchlistAttachmentMetadata[] } type ScratchlistResponse = { entries: HubEntry[] } @@ -126,7 +129,8 @@ function toLocalEntry(hub: HubEntry): ScratchlistEntry { id: hub.entryId, text: hub.text, createdAt: hub.createdAt, - updatedAt: hub.updatedAt + updatedAt: hub.updatedAt, + attachments: hub.attachments ?? [] } } @@ -142,7 +146,8 @@ function makeOptimisticHubEntry(text: string, now: number): HubEntry { entryId: fallbackId, text, createdAt: now, - updatedAt: now + updatedAt: now, + attachments: [] } } @@ -152,7 +157,7 @@ export function useHubScratchlist( ): { entries: ScratchlistEntry[] isLoading: boolean - add: (text: string) => Promise + add: (text: string, attachments?: import('@/types/api').AttachmentMetadata[]) => Promise remove: (id: string) => Promise update: (id: string, text: string) => Promise move: (id: string, direction: 'up' | 'down') => void @@ -325,17 +330,18 @@ export function useHubScratchlist( const addMutation = useMutation< { entry: HubEntry }, Error, - { text: string }, + { text: string; attachments: ScratchlistAttachmentMetadata[] }, { previousData: ScratchlistResponse | undefined; optimisticEntryId: string } >({ - mutationFn: async ({ text }) => { + mutationFn: async ({ text, attachments }) => { if (!api || !sessionId) throw new Error('Scratchlist unavailable') - return await api.createScratchlistEntry(sessionId, { text }) + return await api.createScratchlistEntry(sessionId, { text, attachments }) }, - onMutate: async ({ text }) => { + onMutate: async ({ text, attachments }) => { await queryClient.cancelQueries({ queryKey }) const previousData = queryClient.getQueryData(queryKey) const optimistic = makeOptimisticHubEntry(text, Date.now()) + optimistic.attachments = attachments queryClient.setQueryData(queryKey, (prev) => { const prior = prev?.entries ?? [] return { entries: [optimistic, ...prior] } @@ -445,9 +451,13 @@ export function useHubScratchlist( } }) - const add = useCallback(async (rawText: string): Promise => { + const add = useCallback(async ( + rawText: string, + composerAttachments?: import('@/types/api').AttachmentMetadata[] + ): Promise => { const text = rawText.trim() - if (text.length === 0) return false + const attachments = extractScratchlistAttachmentMetadata(composerAttachments) + if (text.length === 0 && attachments.length === 0) return false const truncated = text.length > SCRATCHLIST_MAX_TEXT_LENGTH ? text.slice(0, SCRATCHLIST_MAX_TEXT_LENGTH) : text @@ -456,7 +466,7 @@ export function useHubScratchlist( return false } try { - await addMutation.mutateAsync({ text: truncated }) + await addMutation.mutateAsync({ text: truncated, attachments }) return true } catch { return false