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