diff --git a/config/scripts/vitest-sqlite-runtime-files.mjs b/config/scripts/vitest-sqlite-runtime-files.mjs index ed34837967d..c89d7a150bf 100644 --- a/config/scripts/vitest-sqlite-runtime-files.mjs +++ b/config/scripts/vitest-sqlite-runtime-files.mjs @@ -145,7 +145,6 @@ export const SQLITE_RUNTIME_INCLUDE = [ 'src/main/native-chat/agent-session-wire/structured-agent-session-recovery-exits.test.ts', 'src/main/native-chat/agent-session-wire/structured-agent-session-recovery-resolution.test.ts', 'src/main/native-chat/agent-session-wire/structured-agent-session-repeated-stop.test.ts', - 'src/main/native-chat/native-chat-visuals-sweep.test.ts', 'src/main/native-chat/agent-session-wire/structured-agent-session-resend-answer.test.ts', 'src/main/native-chat/agent-session-wire/structured-agent-session-replaced-server-turn.test.ts', 'src/main/native-chat/agent-session-wire/structured-agent-session-resend-ledger-refusal.test.ts', diff --git a/resources/native-chat-visuals/skills/orca-chat-visuals/SKILL.md b/resources/native-chat-visuals/skills/orca-chat-visuals/SKILL.md index 678b6936f94..2023d0a86c1 100644 --- a/resources/native-chat-visuals/skills/orca-chat-visuals/SKILL.md +++ b/resources/native-chat-visuals/skills/orca-chat-visuals/SKILL.md @@ -15,8 +15,8 @@ clearer. - Save it in the folder named by `ORCA_CHAT_SCRATCH_DIR` (`printenv ORCA_CHAT_SCRATCH_DIR`; in PowerShell `$env:ORCA_CHAT_SCRATCH_DIR`). If it's unset or you can't write there, skip the visual. - This folder is temporary: Orca may remove it after 30 days without chat activity or changes. - Save files meant to last in the workspace. + This folder belongs to this chat and is kept as long as the chat. Save files meant for the + project in the workspace. - One complete HTML file per visual, directly in that folder, under 512 KB, with a new file name each time (e.g. `latency-by-region-7c1e.html`): letters, digits, `.`, `_`, `-`, ending in `.html`. - Inline your styles, scripts and data. Libraries and fonts may load only from cdn.jsdelivr.net, diff --git a/src/main/ipc/filesystem.test.ts b/src/main/ipc/filesystem.test.ts index 6fd1d2e64da..bc9dc87af06 100644 --- a/src/main/ipc/filesystem.test.ts +++ b/src/main/ipc/filesystem.test.ts @@ -249,6 +249,13 @@ describe('registerFilesystemHandlers', () => { ) expect(statMock).toHaveBeenCalledWith(targetPath) + realpathMock.mockRejectedValue(Object.assign(new Error('missing'), { code: 'ENOENT' })) + await expect( + handlers.get('fs:pathExists')!(null, { + filePath: path.resolve('/state/agent-session-attachments/upload/gone.png'), + access: { kind: 'chat-image' } + }) + ).resolves.toBe(false) }) it('allows deletePath when a registered worktree parent resolves to a macOS canonical alias', async () => { diff --git a/src/main/ipc/filesystem/filesystem-write-handlers.ts b/src/main/ipc/filesystem/filesystem-write-handlers.ts index c99f6cd768e..f6ca923e686 100644 --- a/src/main/ipc/filesystem/filesystem-write-handlers.ts +++ b/src/main/ipc/filesystem/filesystem-write-handlers.ts @@ -14,8 +14,6 @@ import { registerFilesystemMutationHandlers } from '../filesystem-mutations' import { registerAgentSessionAttachmentUploadHandlers } from '../agent-session-attachment-upload-ipc' import type { FilesystemHandlerContext } from './filesystem-handler-context' import { assertLocalWriteTargetIsRegularFile } from './local-regular-file-read' -import { runNativeChatScratchSave } from '../../native-chat/native-chat-scratch-folder-lock' -import { getProfileUserDataPath } from '../../orca-profiles/profile-storage-paths' import type { FileWriteDiskPrecondition, FileWriteResult @@ -58,10 +56,7 @@ export function registerFilesystemWriteHandlers(context: FilesystemHandlerContex } } await assertLocalWriteTargetIsRegularFile(filePath) - const precondition = args.diskPrecondition - return runNativeChatScratchSave(getProfileUserDataPath(), filePath, () => - writeLocalTextFileWithDiskPrecondition(filePath, args.content, precondition) - ) + return writeLocalTextFileWithDiskPrecondition(filePath, args.content, args.diskPrecondition) } ) diff --git a/src/main/ipc/register-core-handlers/register-core-handlers.test.ts b/src/main/ipc/register-core-handlers/register-core-handlers.test.ts index 07e089c29ce..3bc9009221c 100644 --- a/src/main/ipc/register-core-handlers/register-core-handlers.test.ts +++ b/src/main/ipc/register-core-handlers/register-core-handlers.test.ts @@ -644,7 +644,9 @@ describe('registerCoreHandlers', () => { expect(registerCliHandlersMock).toHaveBeenCalled() expect(registerPreflightHandlersMock).toHaveBeenCalled() expect(registerShellHandlersMock).toHaveBeenCalledWith(store) - expect(registerClipboardHandlersMock).toHaveBeenCalledWith(store) + expect(registerClipboardHandlersMock).toHaveBeenCalledWith(store, { + localChatAttachmentStore: expect.any(Function) + }) expect(registerUpdaterHandlersMock).toHaveBeenCalled() expect(setTrustedBrowserRendererWebContentsIdMock).toHaveBeenCalledWith(null) expect(setTrustedClipboardRendererWebContentsIdMock).toHaveBeenCalledWith(null) diff --git a/src/main/ipc/register-core-handlers/register-core-handlers.ts b/src/main/ipc/register-core-handlers/register-core-handlers.ts index 508d177f084..8e2b479cfd7 100644 --- a/src/main/ipc/register-core-handlers/register-core-handlers.ts +++ b/src/main/ipc/register-core-handlers/register-core-handlers.ts @@ -102,6 +102,7 @@ import { } from '../../ai-vault/runtime-session-scanner' import { callRuntimeSessionSearch } from '../../ai-vault/runtime-session-search-call' import { ensureStructuredAgentSessionHostUnlessRefused } from '../../runtime/structured-agent-session-host-refusal' +import { getAgentSessionAttachmentStore } from '../../native-chat/agent-session-attachments/agent-session-attachment-store-registry' import type { PluginService } from '../../plugins/plugin-service' import type { PluginMarketplaceHandlerServices } from '../plugin-marketplaces' import { settleWithinMs } from '../../quit-teardown-deadline' @@ -280,7 +281,14 @@ export function registerCoreHandlers( prepareRuntimeAiVaultSessionResume(app.getPath('userData'), environmentId, args) }) registerNativeChatHandlers() - registerClipboardHandlers(store) + registerClipboardHandlers(store, { + localChatAttachmentStore: async () => { + await ensureStructuredAgentSessionHostUnlessRefused(() => + runtime.ensureStructuredAgentSessionHost() + ) + return getAgentSessionAttachmentStore() + } + }) registerUpdaterHandlers(store) registerSpeechHandlers(store) } diff --git a/src/main/native-chat/agent-session-attachments/agent-session-attachment-claims.ts b/src/main/native-chat/agent-session-attachments/agent-session-attachment-claims.ts index ac41b427e58..d2e6d58b2ac 100644 --- a/src/main/native-chat/agent-session-attachments/agent-session-attachment-claims.ts +++ b/src/main/native-chat/agent-session-attachments/agent-session-attachment-claims.ts @@ -2,8 +2,9 @@ // host transaction that makes it durable, and the sweep may delete only an upload nobody claims: // both are synchronous statements on the host's one journal connection, so exactly one wins. -import { existsSync, readdirSync } from 'node:fs' +import { lstatSync, readdirSync } from 'node:fs' import { join } from 'node:path' +import { isDefinitiveAbsence } from '../../../shared/definitive-filesystem-absence' import type Database from '../../sqlite/sync-database' import type { AgentJournalMessageItem } from '../../../shared/agent-session-journal-types' import { @@ -193,7 +194,18 @@ export function pruneAgentSessionAttachmentClaims( let removed = 0 for (const { uploadId, sessionId } of rows) { const chatGone = facts.recordedSessionIds !== null && !facts.recordedSessionIds.has(sessionId) - if (chatGone || !existsSync(join(facts.root, uploadId))) { + let uploadGone = chatGone + try { + if (!chatGone) { + lstatSync(join(facts.root, uploadId)) + } + } catch (error) { + if (!isDefinitiveAbsence(error)) { + throw error + } + uploadGone = true + } + if (uploadGone) { removed += Number(remove.run(uploadId, sessionId).changes) } } diff --git a/src/main/native-chat/agent-session-attachments/agent-session-attachment-store-registry.ts b/src/main/native-chat/agent-session-attachments/agent-session-attachment-store-registry.ts index e0f4201618a..bc24519b1f7 100644 --- a/src/main/native-chat/agent-session-attachments/agent-session-attachment-store-registry.ts +++ b/src/main/native-chat/agent-session-attachments/agent-session-attachment-store-registry.ts @@ -13,3 +13,18 @@ export function setAgentSessionAttachmentStore(store: AgentSessionAttachmentStor export function getAgentSessionAttachmentStore(): AgentSessionAttachmentStore | null { return installedStore } + +const uploadRemovalListeners = new Set<(uploadDir: string) => void>() + +/** Told after each sweep removal attempt, so a draft that checked its image before can check + * again; an attempt, not proof: a removal can fail. */ +export function subscribeAgentSessionUploadRemoval( + listener: (uploadDir: string) => void +): () => void { + uploadRemovalListeners.add(listener) + return () => uploadRemovalListeners.delete(listener) +} + +export function publishAgentSessionUploadRemoval(uploadDir: string): void { + uploadRemovalListeners.forEach((listener) => listener(uploadDir)) +} diff --git a/src/main/native-chat/agent-session-attachments/agent-session-attachment-store.ts b/src/main/native-chat/agent-session-attachments/agent-session-attachment-store.ts index bd6862e4c3c..91a7fd3881e 100644 --- a/src/main/native-chat/agent-session-attachments/agent-session-attachment-store.ts +++ b/src/main/native-chat/agent-session-attachments/agent-session-attachment-store.ts @@ -1,10 +1,12 @@ -// The server-owned store for files a client attaches to a structured chat. +// The host-owned store for files a client attaches to a structured chat: a paired server's for its +// clients, and this machine's for its own local chats' pastes. // // `//`: one directory per upload keeps the user's file name // intact (agents read meaning into it) without two uploads colliding. Bytes land in a hidden part // file and are renamed into place on commit, so a stored name is always complete. Which chats hold // an upload is recorded by the message that names it (`agent-session-attachment-claims.ts`). +import { randomUUID } from 'node:crypto' import { appendFile, mkdir, rm, writeFile } from 'node:fs/promises' import { extname, join } from 'node:path' import { @@ -154,6 +156,23 @@ export class AgentSessionAttachmentStore { } } + /** Stores bytes this process already holds as one committed upload; returns its path. Not in + * flight meanwhile: the sweep leaves a folder this new alone for an hour. */ + async storeFile(name: string, bytes: Buffer): Promise { + const uploadDir = join(this.rootDir, randomUUID()) + const finalPath = join(uploadDir, sanitizeAgentSessionAttachmentName(name)) + try { + await mkdir(uploadDir, { recursive: true }) + const partPath = join(uploadDir, AGENT_SESSION_ATTACHMENT_PART_FILE) + await writeFile(partPath, bytes, { flag: 'wx' }) + await renameFileWithWindowsRetryAsync(partPath, finalPath) + } catch (error) { + await removeQuietly(uploadDir) + throw error + } + return finalPath + } + async abortUpload(args: { callerKey: string; uploadId: string }): Promise<{ aborted: true }> { const upload = this.uploads.peek(args.uploadId) if (upload) { diff --git a/src/main/native-chat/agent-session-attachments/agent-session-attachment-sweep.test.ts b/src/main/native-chat/agent-session-attachments/agent-session-attachment-sweep.test.ts index e414633a66b..7da423dec72 100644 --- a/src/main/native-chat/agent-session-attachments/agent-session-attachment-sweep.test.ts +++ b/src/main/native-chat/agent-session-attachments/agent-session-attachment-sweep.test.ts @@ -1,7 +1,34 @@ -import { mkdir, mkdtemp, readdir, rm, utimes, writeFile } from 'node:fs/promises' +import { mkdir, mkdtemp, readdir, readFile, rm, utimes, writeFile } from 'node:fs/promises' import { tmpdir } from 'node:os' -import { join } from 'node:path' -import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import { dirname, join } from 'node:path' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type * as FsPromises from 'node:fs/promises' +import type * as Fs from 'node:fs' +const fault = vi.hoisted(() => ({ path: '', sync: false })) +vi.mock('node:fs/promises', async (original) => { + const fs = await original() + return { + ...fs, + readdir: (...args: Parameters) => { + if (!fault.sync && String(args[0]) === fault.path) { + throw Object.assign(new Error('listing failed'), { code: 'EIO' }) + } + return fs.readdir(...args) + } + } +}) +vi.mock('node:fs', async (original) => { + const fs = await original() + return { + ...fs, + lstatSync: (...args: Parameters) => { + if (fault.sync && String(args[0]).startsWith(fault.path)) { + throw Object.assign(new Error('probe failed'), { code: 'EACCES' }) + } + return fs.lstatSync(...args) + } + } +}) import type Database from '../../sqlite/sync-database' import type { AgentJournalMessageItem } from '../../../shared/agent-session-journal-types' import { @@ -20,12 +47,13 @@ import { import { AgentSessionAttachmentStore } from './agent-session-attachment-store' import { sweepAgentSessionAttachments, + UNCLAIMED_ATTACHMENT_MAX_AGE_MS, type AgentSessionAttachmentSweepFacts } from './agent-session-attachment-sweep' const HOUR = 60 * 60 * 1000 const NOW = Date.now() -const OLD = 24 * HOUR + 1 +const OLD = UNCLAIMED_ATTACHMENT_MAX_AGE_MS + 1 const SENT = '0b6f8a52-4a3e-4c4e-9a59-1d5d1f2b8c01' const UNSENT = '7c2d9e10-3f4a-4b5c-8d6e-2f1a0b9c8d02' const CARRIED = '3e1f0a9b-8c7d-4e6f-a5b4-c3d2e1f0a903' @@ -45,6 +73,8 @@ beforeEach(async () => { }) afterEach(async () => { + fault.path = '' + fault.sync = false store.clearInFlightForTests() closeTestJournalHostDatabases() await rm(stateDirectory, { recursive: true, force: true }) @@ -111,15 +141,35 @@ describe('sweepAgentSessionAttachments', () => { expect(await remaining()).toEqual([UNSENT]) }) - it('removes an upload nobody claimed after a day, and keeps a claimed one and a fresh one', async () => { + it('removes an upload nobody claimed after 30 days, and keeps a claimed one and a fresh one', async () => { claim('session-1', await seedUpload(SENT, OLD)) await seedUpload(UNSENT, OLD) await seedUpload(CARRIED, HOUR) - const { removed } = await sweepAgentSessionAttachments(store, facts(), NOW) + const onUploadRemoved = vi.fn() + const { removed } = await sweepAgentSessionAttachments(store, facts({ onUploadRemoved }), NOW) expect(removed).toEqual([join(store.rootDir, UNSENT)]) + expect(onUploadRemoved.mock.calls).toEqual([[join(store.rootDir, UNSENT)]]) expect(await remaining()).toEqual([SENT, CARRIED].sort()) }) + it.each(['contents', 'root', 'prune'] as const)( + 'keeps bytes and claims across an uncertain %s read and a healthy retry', + async (boundary) => { + const path = await store.storeFile('sent.png', Buffer.from('bytes')) + const old = new Date(NOW - OLD) + await utimes(dirname(path), old, old) + claim('session-1', path) + fault.path = boundary === 'contents' ? dirname(path) : store.rootDir + fault.sync = boundary === 'prune' + await expect(sweepAgentSessionAttachments(store, facts(), NOW)).rejects.toThrow() + fault.path = '' + fault.sync = false + await sweepAgentSessionAttachments(store, facts(), NOW) + expect(await readFile(path, 'utf8')).toBe('bytes') + expect(claimedSessions()).toEqual(['session-1']) + } + ) + it('keeps an upload any chat still claims: a /clear carry is a second holder', async () => { const path = await seedUpload(CARRIED, OLD) claim('session-1', path) @@ -155,7 +205,9 @@ describe('sweepAgentSessionAttachments', () => { await seedUpload(UNSENT, HOUR) // Marked, then the process died before the files went. expect(markUnclaimedUploadForSweep(db, UNSENT, NOW)).toBe(true) - await sweepAgentSessionAttachments(store, facts(), NOW) + const onUploadRemoved = vi.fn() + await sweepAgentSessionAttachments(store, facts({ onUploadRemoved }), NOW) + expect(onUploadRemoved).toHaveBeenCalledWith(join(store.rootDir, UNSENT)) expect(await remaining()).toEqual([]) expect(db.prepare('SELECT COUNT(*) AS n FROM agent_session_attachment_sweeps').get()).toEqual({ n: 0 diff --git a/src/main/native-chat/agent-session-attachments/agent-session-attachment-sweep.ts b/src/main/native-chat/agent-session-attachments/agent-session-attachment-sweep.ts index cb685a01893..b4726a368d1 100644 --- a/src/main/native-chat/agent-session-attachments/agent-session-attachment-sweep.ts +++ b/src/main/native-chat/agent-session-attachments/agent-session-attachment-sweep.ts @@ -1,8 +1,9 @@ // When a stored attachment may go, re-derived on every sweep from the disk and the claim table. // // - A part file nobody is writing (the uploader crashed or vanished) goes after an hour. -// - An upload no message ever claimed goes after a day: its chip was removed or its draft -// abandoned. The sweep first marks it in the same database the claims live in, and only if no +// - An upload no message ever claimed goes after 30 days: its chip was removed or its draft +// abandoned. A draft names its image by this path, so the window is how long an unsent draft +// keeps it. The sweep first marks it in the same database the claims live in, and only if no // claim exists; a send that names it afterwards is refused instead of losing its file. // - A claimed upload stays while any chat holding it has a record. The host never deletes a chat // today, so that is as long as its transcript. @@ -10,6 +11,7 @@ import { readdir, stat } from 'node:fs/promises' import { join } from 'node:path' +import { isDefinitiveAbsence } from '../../../shared/definitive-filesystem-absence' import type Database from '../../sqlite/sync-database' import { finishUploadSweep, @@ -21,7 +23,7 @@ import { AGENT_SESSION_ATTACHMENT_PART_FILE } from './agent-session-attachment-r import { removeQuietly, type AgentSessionAttachmentStore } from './agent-session-attachment-store' export const ABANDONED_PART_MAX_AGE_MS = 60 * 60 * 1000 -export const UNCLAIMED_ATTACHMENT_MAX_AGE_MS = 24 * 60 * 60 * 1000 +export const UNCLAIMED_ATTACHMENT_MAX_AGE_MS = 30 * 24 * 60 * 60 * 1000 export type AgentSessionAttachmentSweepFacts = { /** The host's journal connection, or null when it cannot take writes (closed, or written by a @@ -30,6 +32,8 @@ export type AgentSessionAttachmentSweepFacts = { /** Every chat this host holds a record for; null while that list is incomplete (records still * owed their import), when no chat's claims can be judged orphaned. */ recordedSessionIds: () => Pick, 'has'> | null + /** After each removal attempt, before the database work that may throw. */ + onUploadRemoved?: (uploadDir: string) => void } export type AgentSessionAttachmentSweepResult = { removed: string[] } @@ -42,8 +46,12 @@ export async function sweepAgentSessionAttachments( const removed: string[] = [] // Read again at every statement: the host may close the connection while the sweep awaits disk. const { database } = facts + const remove = async (uploadDir: string): Promise => { + await removeQuietly(uploadDir) + facts.onUploadRemoved?.(uploadDir) + } for (const uploadId of withDatabase(database, listUploadsBeingSwept) ?? []) { - await removeQuietly(join(store.rootDir, uploadId)) + await remove(join(store.rootDir, uploadId)) withDatabase(database, (db) => finishUploadSweep(db, uploadId)) } for (const uploadId of await listDirectories(store.rootDir)) { @@ -51,13 +59,13 @@ export async function sweepAgentSessionAttachments( continue } const uploadDir = join(store.rootDir, uploadId) - const stored = (await listEntries(uploadDir)).some( + const stored = (await readdir(uploadDir)).some( (entry) => entry !== AGENT_SESSION_ATTACHMENT_PART_FILE ) const ageMs = now - (await modifiedAt(uploadDir)) if (!stored) { if (ageMs > ABANDONED_PART_MAX_AGE_MS) { - await removeQuietly(uploadDir) + await remove(uploadDir) removed.push(uploadDir) } continue @@ -69,7 +77,7 @@ export async function sweepAgentSessionAttachments( ) { continue } - await removeQuietly(uploadDir) + await remove(uploadDir) withDatabase(database, (db) => finishUploadSweep(db, uploadId)) removed.push(uploadDir) } @@ -90,20 +98,15 @@ function withDatabase( return db ? run(db) : undefined } -async function listEntries(dir: string): Promise { - try { - return await readdir(dir) - } catch { - return [] - } -} - async function listDirectories(dir: string): Promise { try { const entries = await readdir(dir, { withFileTypes: true }) return entries.filter((entry) => entry.isDirectory()).map((entry) => entry.name) - } catch { - return [] + } catch (error) { + if (isDefinitiveAbsence(error)) { + return [] + } + throw error } } diff --git a/src/main/native-chat/agent-session-journal/journal-row-table.ts b/src/main/native-chat/agent-session-journal/journal-row-table.ts index 8a108c5f80f..7ff185857cf 100644 --- a/src/main/native-chat/agent-session-journal/journal-row-table.ts +++ b/src/main/native-chat/agent-session-journal/journal-row-table.ts @@ -21,23 +21,12 @@ const SELECT_ROWS_AFTER = `SELECT seq, ts, row_json FROM journal_rows WHERE session_id = ? AND epoch = ? AND seq > ? ORDER BY seq ASC` const SELECT_ROWS_AFTER_LIMITED = `${SELECT_ROWS_AFTER} LIMIT ?` const DELETE_EPOCH = 'DELETE FROM journal_rows WHERE session_id = ? AND epoch = ?' -const SELECT_LAST_ACTIVITY = 'SELECT MAX(ts) AS ts FROM journal_rows WHERE session_id = ?' export function readJournalSessionEpoch(db: Database.Database, sessionId: string): string | null { const epoch = db.prepare(SELECT_EPOCH).get(sessionId)?.epoch return typeof epoch === 'string' ? epoch : null } -/** When the chat's newest row was saved, across every epoch; null for a chat with no rows. One - * aggregate over the chat's key range, so no row body reaches this process. */ -export function readJournalSessionLastActivityAt( - db: Database.Database, - sessionId: string -): number | null { - const ts = db.prepare(SELECT_LAST_ACTIVITY).get(sessionId)?.ts - return typeof ts === 'number' ? ts : null -} - /** Points the chat at `epoch`. Only an epoch change writes this row. */ export function publishJournalSessionEpoch( db: Database.Database, diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-conversation-lifetime.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-conversation-lifetime.ts index d9bab20d37c..bca03eff7d8 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-conversation-lifetime.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-conversation-lifetime.ts @@ -268,17 +268,6 @@ export function createStructuredAgentSessionConversationLifetime(host: { host.context().runtimeState.startupAttempts.childEnded(sessionId, child) } }), - /** Runs `task` in the chat's lane only when execution is idle. */ - runWhileExecutionIdle: (sessionId: string, task: () => Promise): Promise => - serialize(sessionId, async () => - disposed || - sessions.get(sessionId)?.child || - host.deliveryActive(sessionId) || - providerHoldsDispatch(sessionId) || - hasOpenDispatch(sessionId) - ? null - : await task() - ), /** Quit has begun: nothing opens a conversation or sweeps one after this. */ dispose: (): void => { disposed = true diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-host.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-host.ts index b770dc4b6fc..11a56046f12 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-host.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-host.ts @@ -244,8 +244,6 @@ export class StructuredAgentSessionHost { showSessionTabs = this.tabs.showSessionTabs setSessionTabVisibility = this.tabs.setSessionTabVisibility notifySessionTabHidden = this.tabs.notifySessionTabHidden - runWhileExecutionIdle = (sessionId: string, task: () => Promise): Promise => - this.lifetime.runWhileExecutionIdle(sessionId, task) /** This runtime holds a chat record, readable or not. */ holdsSessions = (): boolean => this.deps.store.holdsRecords() onSessionsHeld = (listener: () => void): (() => void) => this.deps.store.onFirstRecord(listener) diff --git a/src/main/native-chat/native-chat-scratch-folder-lock.ts b/src/main/native-chat/native-chat-scratch-folder-lock.ts deleted file mode 100644 index a270f804151..00000000000 --- a/src/main/native-chat/native-chat-scratch-folder-lock.ts +++ /dev/null @@ -1,72 +0,0 @@ -// Orders an Orca save into a chat's scratch folder against the sweep expiring that folder. Expiry -// renames the folder aside under this lock, so a save either lands first (its candidate mark keeps -// the folder) or runs after and fails on the missing folder, leaving the draft unsaved. A save -// never takes the session lane, so it never waits for an agent start. - -import { relative } from 'node:path' -import { runKeyedSerializedOperation } from '../cli/keyed-promise-queue' -import { - isNativeChatScratchFolderName, - nativeChatScratchRootFor -} from './native-chat-scratch-folder' - -type ExpiryCandidate = { current: boolean } - -const folderQueues = new Map>() -// The age scan runs outside the lock, so a save that settles after it starts makes it stale. -// A set per folder: each runtime install runs its own sweep, so two can scan one folder at once. -const expiryCandidates = new Map>() - -/** The scratch folder `filePath` lies inside, or null for any other path. */ -function scratchFolderNameOf(stateDirectory: string, filePath: string): string | null { - const segments = relative(nativeChatScratchRootFor(stateDirectory), filePath).split(/[\\/]/) - return segments.length > 1 && isNativeChatScratchFolderName(segments[0]) ? segments[0] : null -} - -/** Runs an Orca save; one into a chat scratch folder holds that folder's lock. */ -export function runNativeChatScratchSave( - stateDirectory: string, - filePath: string, - save: () => Promise -): Promise { - const folderName = scratchFolderNameOf(stateDirectory, filePath) - if (!folderName) { - return save() - } - return runKeyedSerializedOperation(folderQueues, folderName, async () => { - try { - return await save() - } finally { - for (const candidate of expiryCandidates.get(folderName) ?? []) { - candidate.current = false - } - } - }) -} - -export function runUnderNativeChatScratchFolderLock( - folderName: string, - task: () => Promise -): Promise { - return runKeyedSerializedOperation(folderQueues, folderName, task) -} - -/** Registered before the sweep's age scan; the scan stays usable only while `isCurrent`. */ -export function trackNativeChatScratchExpiryCandidate(folderName: string): { - isCurrent: () => boolean - dispose: () => void -} { - const candidate: ExpiryCandidate = { current: true } - const candidates = expiryCandidates.get(folderName) ?? new Set() - candidates.add(candidate) - expiryCandidates.set(folderName, candidates) - return { - isCurrent: () => candidate.current, - dispose: () => { - candidates.delete(candidate) - if (candidates.size === 0) { - expiryCandidates.delete(folderName) - } - } - } -} diff --git a/src/main/native-chat/native-chat-scratch-folder-migration.test.ts b/src/main/native-chat/native-chat-scratch-folder-migration.test.ts index ac152a48384..336851fe7e8 100644 --- a/src/main/native-chat/native-chat-scratch-folder-migration.test.ts +++ b/src/main/native-chat/native-chat-scratch-folder-migration.test.ts @@ -4,10 +4,9 @@ import { mkdtempSync, readdirSync, readFileSync, + renameSync, rmSync, - statSync, symlinkSync, - utimesSync, writeFileSync } from 'node:fs' import { tmpdir } from 'node:os' @@ -15,9 +14,9 @@ import { basename, join } from 'node:path' import { afterEach, describe, expect, it, vi } from 'vitest' import type * as NodeFsPromises from 'node:fs/promises' -const fsFaults = vi.hoisted( - (): { operation: 'utimes' | 'copyFile' | 'unlink' | 'rename' | null } => ({ operation: null }) -) +const fsFaults = vi.hoisted((): { operation: 'copyFile' | 'unlink' | 'rename' | null } => ({ + operation: null +})) vi.mock('node:fs/promises', async (importOriginal) => { const fsp = await importOriginal() function throwFilesystemFault(operation: typeof fsFaults.operation): void { @@ -27,10 +26,6 @@ vi.mock('node:fs/promises', async (importOriginal) => { } return { ...fsp, - utimes: async (...args: Parameters) => { - throwFilesystemFault('utimes') - return fsp.utimes(...args) - }, copyFile: async (...args: Parameters) => { throwFilesystemFault('copyFile') return fsp.copyFile(...args) @@ -95,21 +90,22 @@ function logger(): StructuredAgentSessionLogger & { } describe('moving pre-rename chat visuals folders', () => { - it('moves each chat folder under the current root with a fresh window and removes the old root', async () => { + it('moves each chat folder under the current root and removes the old root', async () => { const state = stateDirectory() - const folder = legacyFor(state, 'chat-old', { 'chart.html': '

old chart

' }) - const then = new Date(Date.now() - 40 * 24 * 60 * 60_000) - utimesSync(join(folder, 'chart.html'), then, then) - utimesSync(folder, then, then) + legacyFor(state, 'chat-old', { 'chart.html': '

old chart

' }) writeFileSync(join(legacyRootOf(state), 'notes.txt'), 'not ours') - const before = Date.now() - 1_000 + const canonical = currentFor(state, 'chat-resume', { 'chart.html': '

saved

' }) + renameSync(canonical, `${canonical}.expired`) + mkdirSync(canonical) await migrateLegacyNativeChatVisualsFolders(state, logger()) const moved = nativeChatScratchFolderFor(state, 'chat-old') expect(readFileSync(join(moved, 'chart.html'), 'utf8')).toBe('

old chart

') - // The move is the first time the 30-day rule applies to this chat: it starts counting now. - expect(statSync(moved).mtimeMs).toBeGreaterThanOrEqual(before) + expect(await readNativeChatVisualFile(canonical, 'chart.html')).toMatchObject({ + ok: true, + html: '

saved

' + }) // Only what this host minted moves; the old root stays while anything else is in it. expect(readdirSync(legacyRootOf(state))).toEqual(['notes.txt']) rmSync(join(legacyRootOf(state), 'notes.txt')) @@ -172,11 +168,13 @@ describe('moving pre-rename chat visuals folders', () => { 'keeps failed work retryable after a %s failure', async (operation) => { const state = stateDirectory() - const legacy = legacyFor(state, 'chat-both', { + let legacy = legacyFor(state, 'chat-both', { 'chart.html': '

older

', 'other.html': '

saved visual

' }) const both = currentFor(state, 'chat-both', { 'chart.html': '

current

' }) + renameSync(legacy, `${both}.expired`) + legacy = `${both}.expired` fsFaults.operation = operation const log = logger() @@ -247,22 +245,6 @@ describe('moving pre-rename chat visuals folders', () => { ]) }) - it('leaves a folder in the old root when its fresh window cannot be set', async () => { - const state = stateDirectory() - const folder = legacyFor(state, 'chat-old', { 'chart.html': '

old chart

' }) - fsFaults.operation = 'utimes' - - const log = logger() - await migrateLegacyNativeChatVisualsFolders(state, log) - - // Still where the next install retries it, rather than moved with a time the sweep would act on. - expect(readFileSync(join(folder, 'chart.html'), 'utf8')).toBe('

old chart

') - expect(existsSync(nativeChatScratchFolderFor(state, 'chat-old'))).toBe(false) - expect(log.warn.mock.calls.map(([, meta]) => meta?.scope)).toEqual([ - 'nativeChatVisuals.migration' - ]) - }) - it('does nothing when there is no old root', async () => { const state = stateDirectory() const log = logger() diff --git a/src/main/native-chat/native-chat-scratch-folder-migration.ts b/src/main/native-chat/native-chat-scratch-folder-migration.ts index e990aa170ca..a93ce64c0a1 100644 --- a/src/main/native-chat/native-chat-scratch-folder-migration.ts +++ b/src/main/native-chat/native-chat-scratch-folder-migration.ts @@ -1,18 +1,8 @@ // Saved replies contain only filenames, so pre-rename visuals must move before the host serves reads. -// Conflicting files stay in a recovery subfolder governed by the chat's existing retention. +// Conflicting files stay in a recovery subfolder inside the chat's folder, so they live as long as it. import { constants } from 'node:fs' -import { - copyFile, - lstat, - mkdir, - mkdtemp, - readdir, - rename, - rmdir, - unlink, - utimes -} from 'node:fs/promises' +import { copyFile, lstat, mkdir, mkdtemp, readdir, rename, rmdir, unlink } from 'node:fs/promises' import { join } from 'node:path' import { isDefinitiveAbsence } from '../../shared/definitive-filesystem-absence' import type { StructuredAgentSessionLogger } from './agent-session-wire/structured-agent-session-logger' @@ -81,19 +71,35 @@ async function mergeFolder(from: string, to: string): Promise { } } -/** Best effort: failed work stays under the legacy root for the next install to retry. */ +/** Best effort: failed work stays at its source for the next install to retry. */ export async function migrateLegacyNativeChatVisualsFolders( stateDirectory: string, logger: StructuredAgentSessionLogger ): Promise { - const legacyRoot = join(stateDirectory, LEGACY_ROOT_NAME) const root = nativeChatScratchRootFor(stateDirectory) + for (const [sourceRoot, suffix] of [ + [join(stateDirectory, LEGACY_ROOT_NAME), ''], + [root, '.expired'] + ] as const) { + await migrateFolders(sourceRoot, root, suffix, logger) + } +} + +async function migrateFolders( + legacyRoot: string, + root: string, + suffix: string, + logger: StructuredAgentSessionLogger +): Promise { try { if ((await presenceOf(legacyRoot)) !== 'directory') { return } const folders = (await readdir(legacyRoot, { withFileTypes: true })).filter( - (entry) => entry.isDirectory() && isNativeChatScratchFolderName(entry.name) + (entry) => + entry.isDirectory() && + entry.name.endsWith(suffix) && + isNativeChatScratchFolderName(suffix ? entry.name.slice(0, -suffix.length) : entry.name) ) if (folders.length > 0) { if ((await presenceOf(root)) === 'other') { @@ -107,13 +113,10 @@ export async function migrateLegacyNativeChatVisualsFolders( } for (const folder of folders) { const from = join(legacyRoot, folder.name) - const to = join(root, folder.name) + const to = join(root, suffix ? folder.name.slice(0, -suffix.length) : folder.name) try { const destination = await presenceOf(to) if (destination === 'absent') { - // Set grace before moving so a timestamp failure leaves the source retryable. - const now = new Date() - await utimes(from, now, now) await rename(from, to) } else if (destination === 'directory') { await mergeFolder(from, to) @@ -126,7 +129,9 @@ export async function migrateLegacyNativeChatVisualsFolders( }) } } - await removeIfEmpty(legacyRoot) + if (legacyRoot !== root) { + await removeIfEmpty(legacyRoot) + } } catch (error) { logger.warn('pre-rename chat visuals folders could not be moved', { scope: SCOPE, error }) } diff --git a/src/main/native-chat/native-chat-visuals-sweep.test.ts b/src/main/native-chat/native-chat-visuals-sweep.test.ts index c7dd6036263..2a494a2861f 100644 --- a/src/main/native-chat/native-chat-visuals-sweep.test.ts +++ b/src/main/native-chat/native-chat-visuals-sweep.test.ts @@ -4,29 +4,18 @@ import { mkdtempSync, readdirSync, readFileSync, + renameSync, rmSync, statSync, symlinkSync, utimesSync, writeFileSync } from 'node:fs' -import { writeFile } from 'node:fs/promises' import { tmpdir } from 'node:os' import { basename, join } from 'node:path' import { afterEach, describe, expect, it, vi } from 'vitest' -import { runKeyedSerializedOperation } from '../cli/keyed-promise-queue' import type { StructuredAgentSessionLogger } from './agent-session-wire/structured-agent-session-logger' import { nativeChatScratchFolderFor, nativeChatScratchRootFor } from './native-chat-scratch-folder' -import { migrateLegacyNativeChatVisualsFolders } from './native-chat-scratch-folder-migration' -import { - runNativeChatScratchSave, - trackNativeChatScratchExpiryCandidate -} from './native-chat-scratch-folder-lock' -import { - attach, - hostTestState -} from './agent-session-wire/structured-agent-session-host-test-harness' -import { HOST_TEST_SESSION } from './agent-session-wire/structured-agent-session-host-test-data' import { scheduleNativeChatVisualsSweep, sweepNativeChatVisualsFolders, @@ -57,7 +46,7 @@ function visualsFor(state: string, sessionId: string): string { const DAY_MS = 24 * 60 * 60_000 /** Sets the mtime of `path` and everything under it to `daysAgo`, children before parents. */ -function age(path: string, daysAgo = 40): void { +function age(path: string, daysAgo: number): void { if (statSync(path).isDirectory()) { for (const name of readdirSync(path)) { age(join(path, name), daysAgo) @@ -69,21 +58,13 @@ function age(path: string, daysAgo = 40): void { function deps( state: string, - held: readonly string[] | null, - facts: { journal?: Record } = {} + held: readonly string[] | null ): NativeChatVisualsSweepDeps & { logger: { warn: ReturnType> } - lanes: Map> } { - const lanes = new Map>() return { stateDirectory: state, listHeldSessionIds: () => held, - lastJournalActivityAt: (sessionId) => facts.journal?.[sessionId] ?? null, - // Stands in for the host's lane: one task at a time per chat. - runWhileExecutionIdle: (sessionId: string, task: () => Promise) => - runKeyedSerializedOperation(lanes, sessionId, task), - lanes, logger: { warn: vi.fn(), error: vi.fn() } } } @@ -103,132 +84,21 @@ describe('the visuals folder sweep', () => { expect(existsSync(orphan)).toBe(false) }) - it('expires a chat unused for 30 days and keeps one with recent use', async () => { + it('keeps a held chat folder however long it went unused, and retains one set aside for startup recovery', async () => { const state = stateDirectory() - const unused = visualsFor(state, 'chat-unused') - const talkedTo = visualsFor(state, 'chat-journal') - const edited = visualsFor(state, 'chat-nested') - mkdirSync(join(edited, 'drafts')) - writeFileSync(join(edited, 'drafts', 'v2.html'), '

v2

') - age(nativeChatScratchRootFor(state)) - const fresh = new Date() - utimesSync(join(edited, 'drafts', 'v2.html'), fresh, fresh) + const idle = visualsFor(state, 'chat-idle') + const setAside = visualsFor(state, 'chat-set-aside') + renameSync(setAside, `${setAside}.expired`) + const orphan = visualsFor(state, 'chat-gone') + renameSync(orphan, `${orphan}.expired`) + age(nativeChatScratchRootFor(state), 400) - const result = await sweepNativeChatVisualsFolders( - deps(state, ['chat-unused', 'chat-journal', 'chat-nested'], { - journal: { 'chat-unused': Date.now() - 31 * DAY_MS, 'chat-journal': Date.now() - DAY_MS } - }) - ) + const result = await sweepNativeChatVisualsFolders(deps(state, ['chat-idle', 'chat-set-aside'])) expect(result).toEqual({ removed: 1, failed: 0 }) - expect(existsSync(unused)).toBe(false) - expect(existsSync(`${unused}.expired`)).toBe(false) - expect(existsSync(talkedTo)).toBe(true) - expect(existsSync(edited)).toBe(true) - }) - - it('gives migrated files one fresh window and expires recovered conflicts with the chat', async () => { - const state = stateDirectory() - const legacyRoot = join(state, 'native-chat-visuals') - const legacyFor = (sessionId: string, body: string): string => { - const folder = join(legacyRoot, basename(nativeChatScratchFolderFor(state, sessionId))) - mkdirSync(folder, { recursive: true }) - writeFileSync(join(folder, 'chart.html'), body) - return folder - } - // Idle since long before the update: moved, and the first sweep must not take it. - age(legacyFor('chat-old', '

old

')) - age(legacyFor('chat-both', '

older

')) - const both = visualsFor(state, 'chat-both') - age(both) - const { logger } = deps(state, []) - await migrateLegacyNativeChatVisualsFolders(state, logger) - - const result = await sweepNativeChatVisualsFolders( - deps(state, ['chat-old', 'chat-both'], { journal: { 'chat-old': Date.now() - 40 * DAY_MS } }) - ) - - expect(result).toEqual({ removed: 0, failed: 0 }) - expect( - readFileSync(join(nativeChatScratchFolderFor(state, 'chat-old'), 'chart.html'), 'utf8') - ).toBe('

old

') - expect(readFileSync(join(both, 'chart.html'), 'utf8')).toBe('

chart

') - const recovery = readdirSync(both, { withFileTypes: true }).filter((entry) => - entry.isDirectory() - ) - expect(recovery).toHaveLength(1) - expect(readFileSync(join(both, recovery[0].name, 'files', 'chart.html'), 'utf8')).toBe( - '

older

' - ) - expect(existsSync(legacyRoot)).toBe(false) - - // A later install does not renew retention; both copies expire and nothing comes back. - age(both) - await migrateLegacyNativeChatVisualsFolders(state, logger) - await expect( - sweepNativeChatVisualsFolders( - deps(state, ['chat-old', 'chat-both'], { - journal: { 'chat-both': Date.now() - 40 * DAY_MS } - }) - ) - ).resolves.toEqual({ removed: 1, failed: 0 }) - await migrateLegacyNativeChatVisualsFolders(state, logger) - expect(existsSync(both)).toBe(false) - }) - - it('keeps a chat past 30 days while the host runs its agent', async () => { - await attach() - const state = stateDirectory() - const folder = visualsFor(state, HOST_TEST_SESSION) - age(folder) - const result = await sweepNativeChatVisualsFolders({ - ...deps(state, [HOST_TEST_SESSION]), - runWhileExecutionIdle: hostTestState().host.runWhileExecutionIdle - }) - expect(result).toEqual({ removed: 0, failed: 0 }) - expect(existsSync(folder)).toBe(true) - }) - - it('gives a start queued behind expiry a fresh folder', async () => { - const state = stateDirectory() - const folder = visualsFor(state, 'chat-a') - age(folder) - const sweep = deps(state, ['chat-a']) - const result = await sweepNativeChatVisualsFolders({ - ...sweep, - runWhileExecutionIdle: (sessionId, task) => { - const expiry = sweep.runWhileExecutionIdle(sessionId, task) - // The start's own lane task, as the host's agent start runs it. - void runKeyedSerializedOperation(sweep.lanes, sessionId, async () => { - mkdirSync(folder, { recursive: true }) - writeFileSync(join(folder, 'new.html'), '

new

') - }) - return expiry - } - }) - expect(result).toEqual({ removed: 1, failed: 0 }) - expect(readdirSync(folder)).toEqual(['new.html']) - expect(existsSync(`${folder}.expired`)).toBe(false) - }) - - it('keeps a folder saved into after its age scan', async () => { - const state = stateDirectory() - const folder = visualsFor(state, 'chat-a') - age(folder) - const sweep = deps(state, ['chat-a']) - await sweepNativeChatVisualsFolders({ - ...sweep, - runWhileExecutionIdle: async (sessionId, task) => { - // An overlapping sweep of the same folder ends first; this sweep's candidate must survive. - trackNativeChatScratchExpiryCandidate(basename(folder)).dispose() - // Settles between the scan and the locked recheck, which does not scan again. - await runNativeChatScratchSave(state, join(folder, 'chart.html'), () => - writeFile(join(folder, 'chart.html'), '

saved

') - ) - return sweep.runWhileExecutionIdle(sessionId, task) - } - }) - expect(readFileSync(join(folder, 'chart.html'), 'utf8')).toBe('

saved

') + expect(readFileSync(join(idle, 'chart.html'), 'utf8')).toBe('

chart

') + expect(readFileSync(join(`${setAside}.expired`, 'chart.html'), 'utf8')).toBe('

chart

') + expect(existsSync(`${orphan}.expired`)).toBe(false) }) it('removes nothing when the held chats cannot be read', async () => { diff --git a/src/main/native-chat/native-chat-visuals-sweep.ts b/src/main/native-chat/native-chat-visuals-sweep.ts index f2a457b4f7a..d9f7b7b90b4 100644 --- a/src/main/native-chat/native-chat-visuals-sweep.ts +++ b/src/main/native-chat/native-chat-visuals-sweep.ts @@ -1,14 +1,11 @@ -// How a chat's scratch folder dies. The host never deletes a chat record, so no deletion event -// exists to hang cleanup on; instead each folder's life is re-derived every run: -// 1. a folder no chat record (readable or not) maps to is removed, so any future record -// deletion cleans up for free; -// 2. a folder whose chat went unused for the retention window is removed: its newest journal row -// and everything in the folder are older than that, and the host holds no execution for it. -// Rule 2 decides again under the chat's lane and then its folder lock, so a start or a save that -// lands after the first look keeps the folder. A failure is logged and never blocks anything else. +// How a chat's scratch folder dies: with its chat. The host never deletes a chat record, so no +// deletion event exists to hang cleanup on; instead every run removes each folder no chat record +// (readable or not) maps to, so any future record deletion cleans up for free. A folder is never +// removed for its age: a chat's visuals live as long as the chat, and one an older build set aside +// as unused goes back to its chat. A failure is logged and never blocks anything else. import type { Dirent } from 'node:fs' -import { lstat, readdir, rename, rm } from 'node:fs/promises' +import { lstat, readdir, rm } from 'node:fs/promises' import { join } from 'node:path' import { isDefinitiveAbsence } from '../../shared/definitive-filesystem-absence' import { yieldToEventLoop } from '../../shared/event-loop-yield' @@ -18,29 +15,18 @@ import { nativeChatScratchFolderName, nativeChatScratchRootFor } from './native-chat-scratch-folder' -import { - runUnderNativeChatScratchFolderLock, - trackNativeChatScratchExpiryCandidate -} from './native-chat-scratch-folder-lock' - -/** How long a chat's scratch files outlive its last use. */ -export const NATIVE_CHAT_SCRATCH_RETENTION_MS = 30 * 24 * 60 * 60_000 export type NativeChatVisualsSweepDeps = { stateDirectory: string /** Every chat this host holds a row for, readable or not; null when that can't be trusted. */ listHeldSessionIds: () => readonly string[] | null - /** When the chat's newest journal row was saved; null when it has none. */ - lastJournalActivityAt: (sessionId: string) => number | null - /** Runs `task` in the chat's lane unless the host holds execution for it; null when it does. */ - runWhileExecutionIdle: (sessionId: string, task: () => Promise) => Promise logger: StructuredAgentSessionLogger remove?: (path: string) => Promise } export type NativeChatVisualsSweepResult = { removed: number; failed: number } -// Renamed under the locks and deleted after them; a cut-short delete leaves it for a later run. +// What older builds renamed an unused chat's folder to before deleting it; a cut-short delete left it. const EXPIRED_SUFFIX = '.expired' // Bounds one run's disk work; whatever is left goes in a later run. const MAX_REMOVALS_PER_RUN = 200 @@ -74,11 +60,8 @@ export async function sweepNativeChatVisualsFolders( if (!held || entries.length === 0) { return result } - const sessionByFolder = new Map( - held.map((sessionId) => [nativeChatScratchFolderName(sessionId), sessionId]) - ) + const heldFolders = new Set(held.map(nativeChatScratchFolderName)) const remove = deps.remove ?? ((path: string) => rm(path, { recursive: true, force: true })) - const cutoff = Date.now() - NATIVE_CHAT_SCRATCH_RETENTION_MS let visited = 0 for (const entry of entries) { if (isStopped() || result.removed + result.failed >= MAX_REMOVALS_PER_RUN) { @@ -93,23 +76,17 @@ export async function sweepNativeChatVisualsFolders( if (!entry.isDirectory() || !isNativeChatScratchFolderName(folderName)) { continue } - const sessionId = expired ? undefined : sessionByFolder.get(entry.name) - const reason = expired ? 'expired' : sessionId === undefined ? 'no-record' : 'unused' try { - if ( - sessionId !== undefined && - !(await parkUnusedFolder(deps, root, folderName, sessionId, cutoff)) - ) { + if (heldFolders.has(folderName)) { continue } - await remove(join(root, sessionId === undefined ? entry.name : folderName + EXPIRED_SUFFIX)) + await remove(join(root, entry.name)) result.removed += 1 } catch (error) { result.failed += 1 deps.logger.warn('native-chat scratch folder could not be removed', { scope: 'nativeChatVisuals.sweep', - reason, - ...(sessionId ? { sessionId } : {}), + reason: expired ? 'set-aside' : 'no-record', error }) } @@ -117,67 +94,6 @@ export async function sweepNativeChatVisualsFolders( return result } -/** Renames a chat's folder to `.expired` under the chat's lane and then its folder lock; - * false when the chat was used since `cutoff`, holds live work, or a save landed after the scan. */ -async function parkUnusedFolder( - deps: NativeChatVisualsSweepDeps, - root: string, - name: string, - sessionId: string, - cutoff: number -): Promise { - const folder = join(root, name) - const usedSince = (): boolean => (deps.lastJournalActivityAt(sessionId) ?? 0) >= cutoff - // Before the scan, so a save already running when it starts still marks it stale. - const candidate = trackNativeChatScratchExpiryCandidate(name) - try { - if (usedSince() || (await changedSince(folder, cutoff))) { - return false - } - const parked = await deps.runWhileExecutionIdle(sessionId, () => - runUnderNativeChatScratchFolderLock(name, async () => { - if (!candidate.isCurrent() || usedSince()) { - return false - } - await rename(folder, folder + EXPIRED_SUFFIX) - return true - }) - ) - return parked === true - } catch (error) { - if (isDefinitiveAbsence(error)) { - return false - } - throw error - } finally { - candidate.dispose() - } -} - -/** Whether the folder or anything under it changed at or after `cutoff`. Every entry is read with - * lstat, so a link is judged as itself and never followed; the scan stops at the first change. */ -async function changedSince(folder: string, cutoff: number): Promise { - const pending = [folder] - let visited = 0 - for (let dir = pending.pop(); dir !== undefined; dir = pending.pop()) { - if ((await lstat(dir)).mtimeMs >= cutoff) { - return true - } - for (const entry of await readdir(dir, { withFileTypes: true })) { - if (++visited % YIELD_EVERY === 0) { - await yieldToEventLoop() - } - const path = join(dir, entry.name) - if (entry.isDirectory()) { - pending.push(path) - } else if ((await lstat(path)).mtimeMs >= cutoff) { - return true - } - } - } - return false -} - // Why minutes, not at boot: the first sweep must not compete with session restore; after that, a // few runs a day is enough for files nothing reads. const FIRST_SWEEP_DELAY_MS = 2 * 60_000 diff --git a/src/main/runtime/files/file-mutations.ts b/src/main/runtime/files/file-mutations.ts index 08071726c3d..dcf134e486d 100644 --- a/src/main/runtime/files/file-mutations.ts +++ b/src/main/runtime/files/file-mutations.ts @@ -6,8 +6,6 @@ import { resolveFileExplorerPaths } from './file-target' import type { RuntimeFileCommandHost } from './file-command-host' -import { runNativeChatScratchSave } from '../../native-chat/native-chat-scratch-folder-lock' -import { getProfileUserDataPath } from '../../orca-profiles/profile-storage-paths' import { resolveRuntimeAuthorizedPath } from './chat-scratch-file-root' import { constants, copyFile, mkdir, rm } from 'node:fs/promises' import { dirname } from 'node:path' @@ -40,12 +38,10 @@ export class RuntimeFileMutations { expectedSshTargetId, expectedSshConnectionGeneration ) - return runNativeChatScratchSave(getProfileUserDataPath(), target.path, () => - requireRuntimeFileProvider(target, this.host).writeFileWithDiskPrecondition( - target.path, - content, - diskPrecondition - ) + return requireRuntimeFileProvider(target, this.host).writeFileWithDiskPrecondition( + target.path, + content, + diskPrecondition ) } @@ -64,9 +60,7 @@ export class RuntimeFileMutations { expectedSshTargetId, expectedSshConnectionGeneration ) - await runNativeChatScratchSave(getProfileUserDataPath(), target.path, () => - requireRuntimeFileProvider(target, this.host).writeFileBase64(target.path, contentBase64) - ) + await requireRuntimeFileProvider(target, this.host).writeFileBase64(target.path, contentBase64) return { ok: true } } @@ -86,12 +80,10 @@ export class RuntimeFileMutations { expectedSshTargetId, expectedSshConnectionGeneration ) - await runNativeChatScratchSave(getProfileUserDataPath(), target.path, () => - requireRuntimeFileProvider(target, this.host).writeFileBase64Chunk( - target.path, - contentBase64, - append - ) + await requireRuntimeFileProvider(target, this.host).writeFileBase64Chunk( + target.path, + contentBase64, + append ) return { ok: true } } diff --git a/src/main/runtime/structured-agent-session-attachment-wiring.ts b/src/main/runtime/structured-agent-session-attachment-wiring.ts index 9ee32ec10f4..ee164195c7a 100644 --- a/src/main/runtime/structured-agent-session-attachment-wiring.ts +++ b/src/main/runtime/structured-agent-session-attachment-wiring.ts @@ -3,7 +3,10 @@ import { mkdirSync } from 'node:fs' import { AgentSessionAttachmentStore } from '../native-chat/agent-session-attachments/agent-session-attachment-store' -import { setAgentSessionAttachmentStore } from '../native-chat/agent-session-attachments/agent-session-attachment-store-registry' +import { + publishAgentSessionUploadRemoval, + setAgentSessionAttachmentStore +} from '../native-chat/agent-session-attachments/agent-session-attachment-store-registry' import { startAgentSessionAttachmentSweeps, type AgentSessionAttachmentSweeper @@ -50,7 +53,8 @@ export function installAgentSessionAttachments(deps: { // Readable or not: an unreadable chat's claims still protect its uploads. has: (sessionId) => deps.store.getRecord(sessionId) !== null || deps.store.isSessionUnreadable(sessionId) - }) + }), + onUploadRemoved: publishAgentSessionUploadRemoval }, { initialDelayMs: FIRST_SWEEP_DELAY_MS, diff --git a/src/main/runtime/structured-agent-session-runtime.ts b/src/main/runtime/structured-agent-session-runtime.ts index fa5a7afab6d..f54dbad65a7 100644 --- a/src/main/runtime/structured-agent-session-runtime.ts +++ b/src/main/runtime/structured-agent-session-runtime.ts @@ -63,7 +63,6 @@ import { import type { ClaudeCliFlagSupport } from '../claude/claude-cli-flag-support' import { migrateLegacyNativeChatVisualsFolders } from '../native-chat/native-chat-scratch-folder-migration' import { scheduleNativeChatVisualsSweep } from '../native-chat/native-chat-visuals-sweep' -import { readJournalSessionLastActivityAt } from '../native-chat/agent-session-journal/journal-row-table' /** Whether this profile holds a structured chat: a record or tab in the journal database. */ export function hasPersistedStructuredAgentSessionStore( @@ -96,8 +95,8 @@ export type StructuredAgentSessionRuntimeDeps = StructuredAgentSessionTransportO /** Which version-gated flags a Claude CLI takes; absent never passes one. */ claudeCliFlags?: ClaudeCliFlagSupport /** Gives each chat a scratch folder and the visuals skill that teaches it, and sweeps folders - * whose chat is gone or long unused. Wired by the real hosts only, so a test runtime never loads - * the bundled skill. */ + * whose chat is gone. Wired by the real hosts only, so a test runtime never loads the bundled + * skill. */ nativeChatVisuals?: { /** Read this host's preference when a chat starts a provider process. */ isEnabled: () => boolean @@ -356,9 +355,6 @@ async function installOnJournal( ? scheduleNativeChatVisualsSweep({ stateDirectory: deps.stateDirectory, listHeldSessionIds: () => (store.readOnly ? null : store.listHeldSessionIds()), - lastJournalActivityAt: (sessionId) => - readJournalSessionLastActivityAt(journalDatabase.db, sessionId), - runWhileExecutionIdle: host.runWhileExecutionIdle, logger: deps.logger }) : undefined diff --git a/src/main/startup/main-process-ipc-bootstrap.ts b/src/main/startup/main-process-ipc-bootstrap.ts index 306f1d1f394..b2073cf2aba 100644 --- a/src/main/startup/main-process-ipc-bootstrap.ts +++ b/src/main/startup/main-process-ipc-bootstrap.ts @@ -7,6 +7,7 @@ import { onStructuredAgentSessionsHeldChanged, structuredAgentSessionsHeld } from '../native-chat/agent-session-wire/structured-agent-session-registry' +import { subscribeAgentSessionUploadRemoval } from '../native-chat/agent-session-attachments/agent-session-attachment-store-registry' export function registerMainProcessIpcHandlers(): void { ipcMain.handle('app:awaitFirstWindowStartupServices', async () => { @@ -38,6 +39,13 @@ export function registerMainProcessIpcHandlers(): void { window.webContents.send('app:structuredAgentSessionsHeldChanged', held) } }) + // Why: drafts check their pastes once at restore; a sweep can remove one later in the run. + subscribeAgentSessionUploadRemoval((uploadDir) => { + const window = state.mainWindow + if (window && !window.isDestroyed() && !window.webContents.isDestroyed()) { + window.webContents.send('app:agentSessionUploadRemoved', uploadDir) + } + }) ipcMain.handle('app:recoverLegacyWorkerTerminalsForRendererStartup', () => recoverLegacyWorkerTerminalsForRendererStartup({ firstWindowStartupServicesReady: state.firstWindowStartupServicesReady, diff --git a/src/main/window/clipboard-agent-session-attachment-paste.test.ts b/src/main/window/clipboard-agent-session-attachment-paste.test.ts index 92af6461c3f..c05eeee1a70 100644 --- a/src/main/window/clipboard-agent-session-attachment-paste.test.ts +++ b/src/main/window/clipboard-agent-session-attachment-paste.test.ts @@ -1,6 +1,10 @@ -// A structured chat on a paired server keeps pasted images in that server's attachment store, not -// in its temp directory: a temp path has no owner and nothing ever removes it. +// A structured chat keeps pasted images in its host's attachment store, not in a temp directory: a +// temp path has no owner, so nothing can tell a sent paste from an abandoned one. +import { mkdtemp, readFile, rm } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' import { beforeEach, describe, expect, it, vi } from 'vitest' +import { AgentSessionAttachmentStore } from '../native-chat/agent-session-attachments/agent-session-attachment-store' const mocks = vi.hoisted(() => ({ handle: vi.fn(), @@ -121,3 +125,30 @@ describe('clipboard:saveImageAsTempFile for a structured chat on a paired server expect(mocks.callRuntimeEnvironment.mock.calls[0][2]).toBe('clipboard.startImageUpload') }) }) + +describe('clipboard:saveImageAsTempFile for a local structured chat', () => { + it("stores the image in this machine's attachment store, where a send can claim it", async () => { + const root = await mkdtemp(join(tmpdir(), 'orca-local-paste-store-')) + try { + const store = new AgentSessionAttachmentStore(root, { hasSession: () => false }) + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the image save path never reads the store. + registerClipboardHandlers({} as never, { localChatAttachmentStore: async () => store }) + + const saved = await saveHandler()(event, { + forNativeChatDraft: true, + forStructuredChat: true + }) + + expect(saved).toEqual( + expect.stringMatching( + /[\\/][0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}[\\/]orca-paste-.+\.png$/ + ) + ) + expect(String(saved).startsWith(root)).toBe(true) + expect(await readFile(String(saved), 'utf8')).toBe('png') + expect(mocks.callRuntimeEnvironment).not.toHaveBeenCalled() + } finally { + await rm(root, { recursive: true, force: true }) + } + }) +}) diff --git a/src/main/window/clipboard-image-temp-file.test.ts b/src/main/window/clipboard-image-temp-file.test.ts index fd4e5e0064d..eabdfa86829 100644 --- a/src/main/window/clipboard-image-temp-file.test.ts +++ b/src/main/window/clipboard-image-temp-file.test.ts @@ -32,7 +32,7 @@ describe('saveClipboardImageBufferAsTempFile', () => { expect(writeFileMock).toHaveBeenCalledWith(savedPath, Buffer.from([1, 2, 3])) }) - it('writes a native-chat composer paste into the paste folder, where its draft can find it', async () => { + it('writes a terminal-backed composer paste into the never-swept paste folder', async () => { const savedPath = await saveClipboardImageBufferAsTempFile(Buffer.from([1, 2, 3]), { forNativeChatDraft: true }) diff --git a/src/main/window/clipboard-image-temp-file.ts b/src/main/window/clipboard-image-temp-file.ts index 049f98bf03f..6845bc22b1e 100644 --- a/src/main/window/clipboard-image-temp-file.ts +++ b/src/main/window/clipboard-image-temp-file.ts @@ -15,6 +15,9 @@ export type SaveClipboardImageAsTempFileArgs = { agentSessionAttachment?: AgentSessionAttachmentClipboardTarget /** A native-chat composer paste: kept in Orca's paste folder so its draft can bring it back. */ forNativeChatDraft?: boolean + /** With `forNativeChatDraft`: a structured chat on this machine, whose sends claim its pastes + * from this machine's chat attachment store. */ + forStructuredChat?: boolean } export async function saveClipboardImageBufferAsTempFile( diff --git a/src/main/window/clipboard-ipc-handlers.ts b/src/main/window/clipboard-ipc-handlers.ts index 08347ff7fe2..60a5e4d7f34 100644 --- a/src/main/window/clipboard-ipc-handlers.ts +++ b/src/main/window/clipboard-ipc-handlers.ts @@ -6,6 +6,7 @@ import { type IpcMainInvokeEvent, type WebContents } from 'electron' +import { randomUUID } from 'node:crypto' import { open, stat } from 'node:fs/promises' import { runProcess } from '@orca/process-host' import type { Store } from '../persistence' @@ -46,7 +47,9 @@ import { buildClipboardImageThumbnail } from './clipboard-image-thumbnail' import { readClipboardNativeImage } from './clipboard-native-image' import { publishClipboardText } from './clipboard-text-publisher' import { isDashboardPopoutRenderer } from './dashboard-popout-window' -import { restoreNativeChatPastes, sweepExpiredNativeChatPastes } from './native-chat-paste-files' +import { restoreNativeChatPastes } from './native-chat-paste-files' +import { agentSessionPastedImageName } from '../../shared/agent-session-attachments' +import type { AgentSessionAttachmentStore } from '../native-chat/agent-session-attachments/agent-session-attachment-store' let trustedClipboardRendererWebContentsId: number | null = null @@ -55,9 +58,15 @@ type ClipboardWriteFileRequest = { connectionId?: string } +export type ClipboardHandlerOptions = { + /** This machine's chat attachment store, after starting its chat host; null when it can't. */ + localChatAttachmentStore?: () => Promise +} + async function saveClipboardImageBufferForTarget( buffer: Buffer, - args?: SaveClipboardImageAsTempFileArgs + args: SaveClipboardImageAsTempFileArgs | undefined, + options: ClipboardHandlerOptions ): Promise { assertClipboardImageByteLengthWithinLimit(buffer.byteLength) const runtimeEnvironmentId = args?.runtimeEnvironmentId?.trim() @@ -81,6 +90,14 @@ async function saveClipboardImageBufferForTarget( args?.connectionId ?? null ) } + // A local structured chat's paste goes in this machine's chat store: a send claims it for its + // chat, and only an unclaimed one is ever swept. Any other composer paste keeps the paste folder. + if (args?.forNativeChatDraft === true && args.forStructuredChat === true && !args.connectionId) { + const store = await options.localChatAttachmentStore?.().catch(() => null) + if (store) { + return store.storeFile(agentSessionPastedImageName(Date.now(), randomUUID()), buffer) + } + } return saveClipboardImageBufferAsTempFile(buffer, args) } @@ -88,7 +105,10 @@ export function setTrustedClipboardRendererWebContentsId(webContentsId: number | trustedClipboardRendererWebContentsId = webContentsId } -export function registerClipboardHandlers(store: Store): void { +export function registerClipboardHandlers( + store: Store, + handlerOptions: ClipboardHandlerOptions = {} +): void { ipcMain.removeHandler('clipboard:readText') ipcMain.removeHandler('clipboard:readSelectionText') ipcMain.removeHandler('clipboard:writeText') @@ -103,7 +123,6 @@ export function registerClipboardHandlers(store: Store): void { ipcMain.removeHandler('clipboard:restoreNativeChatPastes') void cleanupExpiredRemoteClipboardFiles() - void sweepExpiredNativeChatPastes() scheduleLegacyRemoteClipboardFileCleanup() ipcMain.handle('clipboard:readText', async (event, options?: ReadClipboardTextOptions) => { @@ -161,10 +180,12 @@ export function registerClipboardHandlers(store: Store): void { createImageFromBuffer: (buffer) => nativeImage.createFromBuffer(buffer), openFile: (filePath) => open(filePath, 'r') }) - return copiedFilePng ? saveClipboardImageBufferForTarget(copiedFilePng, args) : null + return copiedFilePng + ? saveClipboardImageBufferForTarget(copiedFilePng, args, handlerOptions) + : null } assertClipboardImageDimensionsWithinLimit(image.getSize()) - return saveClipboardImageBufferForTarget(image.toPNG(), args) + return saveClipboardImageBufferForTarget(image.toPNG(), args, handlerOptions) } ) // Why: copy the actual file to the OS clipboard so pasting in Finder/Explorer diff --git a/src/main/window/clipboard-macos-image-paste.test.ts b/src/main/window/clipboard-macos-image-paste.test.ts index 40d193571e2..f867301a3a3 100644 --- a/src/main/window/clipboard-macos-image-paste.test.ts +++ b/src/main/window/clipboard-macos-image-paste.test.ts @@ -42,7 +42,6 @@ vi.mock('./clipboard-remote-file-copy', () => ({ })) vi.mock('./native-chat-paste-files', () => ({ restoreNativeChatPastes: vi.fn(), - sweepExpiredNativeChatPastes: vi.fn(), nativeChatPasteFolder: () => '/tmp/native-chat-pastes' })) vi.mock('./dashboard-popout-window', () => ({ isDashboardPopoutRenderer: () => false })) diff --git a/src/main/window/native-chat-paste-files.test.ts b/src/main/window/native-chat-paste-files.test.ts index 05599675c7c..12c656fe12d 100644 --- a/src/main/window/native-chat-paste-files.test.ts +++ b/src/main/window/native-chat-paste-files.test.ts @@ -1,29 +1,13 @@ -import { - existsSync, - lutimesSync, - mkdirSync, - mkdtempSync, - realpathSync, - rmSync, - symlinkSync, - utimesSync, - writeFileSync -} from 'node:fs' +import { mkdirSync, mkdtempSync, realpathSync, rmSync, symlinkSync, writeFileSync } from 'node:fs' import { tmpdir } from 'node:os' import path from 'node:path' import { afterEach, beforeEach, describe, expect, it } from 'vitest' import { installFakeAppEnvironment } from '../../../config/scripts/vitest-host-ports-setup' -import { AGENT_SESSION_MAX_NEW_OPERATION_AGE_MS } from '../../shared/agent-session-host-authority' import type { Store } from '../persistence' import { resolveLocalFileRequestPath } from '../ipc/local-file-access-resolution' import { readLocalFileContent } from '../ipc/filesystem/filesystem-file-content-inspection' -import { - NATIVE_CHAT_PASTE_TTL_MS, - isInsideNativeChatPasteFolder, - restoreNativeChatPastes, - sweepExpiredNativeChatPastes -} from './native-chat-paste-files' +import { isInsideNativeChatPasteFolder, restoreNativeChatPastes } from './native-chat-paste-files' // A store with no projects: nothing but an access kind decides what a read may reach. const NO_PROJECTS: Store = Object.assign(Object.create(null), { @@ -212,79 +196,29 @@ describe('native-chat paste folder on disk', () => { expect(await chatImageReadable(path.join(folder, 'orca-paste-y.png'))).toBe(false) }) - it('neither restores from nor sweeps a paste folder that is itself a link', async () => { + it('does not restore from a paste folder that is itself a link', async () => { const outside = path.join(root, 'Documents') mkdirSync(outside) - const old = (Date.now() - NATIVE_CHAT_PASTE_TTL_MS - 60_000) / 1000 - for (const name of ['orca-paste-1.png', 'tax-return.pdf']) { - writeFileSync(path.join(outside, name), 'x') - utimesSync(path.join(outside, name), old, old) - } + writeFileSync(path.join(outside, 'orca-paste-1.png'), 'x') rmSync(folder, { recursive: true }) symlinkSync(outside, folder) await expect(restoreNativeChatPastes([path.join(folder, 'orca-paste-1.png')])).resolves.toEqual( [{ path: path.join(folder, 'orca-paste-1.png'), kept: false, exists: false }] ) - await sweepExpiredNativeChatPastes() - expect(existsSync(path.join(outside, 'orca-paste-1.png'))).toBe(true) - expect(existsSync(path.join(outside, 'tax-return.pdf'))).toBe(true) - }) - - it('expires only Orca paste files, whatever else is in the folder', async () => { - const old = (Date.now() - NATIVE_CHAT_PASTE_TTL_MS - 60_000) / 1000 - for (const name of ['orca-paste-old.png', 'notes.txt']) { - writeFileSync(path.join(folder, name), 'x') - utimesSync(path.join(folder, name), old, old) - } - - await sweepExpiredNativeChatPastes() - - expect(existsSync(path.join(folder, 'orca-paste-old.png'))).toBe(false) - expect(existsSync(path.join(folder, 'notes.txt'))).toBe(true) }) it('reports nothing kept when the folder does not exist yet', async () => { rmSync(folder, { recursive: true }) + await expect( + resolveLocalFileRequestPath( + path.join(root, 'agent-session-attachments', 'upload', 'gone.png'), + { kind: 'chat-image' }, + NO_PROJECTS + ) + ).rejects.toMatchObject({ code: 'ENOENT' }) await expect(restoreNativeChatPastes([path.join(folder, 'orca-paste-1.png')])).resolves.toEqual( [{ path: path.join(folder, 'orca-paste-1.png'), kept: false, exists: false }] ) }) - - it('expires old pastes only, and never follows a symlink or enters a folder', async () => { - const now = Date.now() - const old = (now - NATIVE_CHAT_PASTE_TTL_MS - 60_000) / 1000 - const oldPaste = path.join(folder, 'orca-paste-old.png') - const newPaste = path.join(folder, 'orca-paste-new.png') - writeFileSync(oldPaste, 'png') - writeFileSync(newPaste, 'png') - utimesSync(oldPaste, old, old) - const outsideOld = path.join(root, 'outside-old.png') - writeFileSync(outsideOld, 'png') - utimesSync(outsideOld, old, old) - symlinkSync(outsideOld, path.join(folder, 'orca-paste-link.png')) - lutimesSync(path.join(folder, 'orca-paste-link.png'), old, old) - const nested = path.join(folder, 'nested') - mkdirSync(nested) - const nestedOld = path.join(nested, 'orca-paste-nested.png') - writeFileSync(nestedOld, 'png') - utimesSync(nestedOld, old, old) - - await sweepExpiredNativeChatPastes(now) - - expect(existsSync(oldPaste)).toBe(false) - expect(existsSync(newPaste)).toBe(true) - expect(existsSync(outsideOld)).toBe(true) - expect(existsSync(path.join(folder, 'orca-paste-link.png'))).toBe(true) - expect(existsSync(nestedOld)).toBe(true) - }) - - it('does nothing, and does not throw, when the folder is missing', async () => { - rmSync(folder, { recursive: true }) - await expect(sweepExpiredNativeChatPastes()).resolves.toBeUndefined() - }) - - it('keeps a paste longer than one send id stays valid on the host', () => { - expect(NATIVE_CHAT_PASTE_TTL_MS).toBeGreaterThan(AGENT_SESSION_MAX_NEW_OPERATION_AGE_MS) - }) }) diff --git a/src/main/window/native-chat-paste-files.ts b/src/main/window/native-chat-paste-files.ts index a2ef9682426..c96d19fe9f0 100644 --- a/src/main/window/native-chat-paste-files.ts +++ b/src/main/window/native-chat-paste-files.ts @@ -1,16 +1,12 @@ -// Local native-chat pastes live in an Orca-owned folder, so a restored draft can show and send them: -// a restore keeps only files that really are inside it, and old files expire. +// Orca's folder for local native-chat pastes no send claims: a terminal-backed chat's, and every +// paste from before structured chats used their attachment store. Never swept, since nothing tells +// which of them a sent message names; a restore keeps only files that really are inside it. -import { lstat, readdir, realpath, stat, unlink } from 'node:fs/promises' +import { lstat, realpath, stat } from 'node:fs/promises' import path from 'node:path' import { getAppEnvironment } from '../../shared/app-environment' import { NATIVE_CHAT_PASTE_FOLDER } from '../../shared/native-chat-paste-folder' -// Why 30 days: no age bounds what can still name a paste (a queued send is retried with a new id -// after the host's 24 h id window), so this is a judgment. A draft or outbox entry kept longer meets -// its image as a placeholder or a failed send, and a sent paste's file lingers until then. -export const NATIVE_CHAT_PASTE_TTL_MS = 30 * 24 * 60 * 60 * 1000 -const PASTE_FILE_NAME = /^orca-paste-.+\.png$/i const MAX_RESTORED_PASTES = 256 type PathApi = typeof path.posix @@ -106,33 +102,3 @@ async function restoreNativeChatPaste( return refused } } - -/** Deletes pastes older than the TTL: only Orca's paste files, never a link, a folder, or anything - * in a paste folder that is itself a link. Failures are logged and never block startup. */ -export async function sweepExpiredNativeChatPastes(now = Date.now()): Promise { - const folders = await realPasteFolder() - if (!folders) { - return - } - const folder = folders.named - let entries - try { - entries = await readdir(folder, { withFileTypes: true }) - } catch { - return - } - for (const entry of entries) { - if (!entry.isFile() || !PASTE_FILE_NAME.test(entry.name)) { - continue - } - const file = path.join(folder, entry.name) - try { - const info = await lstat(file) - if (info.isFile() && now - info.mtimeMs > NATIVE_CHAT_PASTE_TTL_MS) { - await unlink(file) - } - } catch (error) { - console.warn('[native-chat-pastes] could not expire a paste:', error) - } - } -} diff --git a/src/preload/api/ui-bridge-clipboard-and-window-controls.ts b/src/preload/api/ui-bridge-clipboard-and-window-controls.ts index 333ebd8a3bb..b34ea58eb4c 100644 --- a/src/preload/api/ui-bridge-clipboard-and-window-controls.ts +++ b/src/preload/api/ui-bridge-clipboard-and-window-controls.ts @@ -104,6 +104,8 @@ export const uiClipboardAndWindowControlsApi = { agentSessionAttachment?: AgentSessionAttachmentClipboardTarget /** A native-chat composer paste, kept where its draft can bring it back. */ forNativeChatDraft?: boolean + /** With `forNativeChatDraft`: a structured chat on this machine, whose sends claim its pastes. */ + forStructuredChat?: boolean }): Promise => ipcRenderer.invoke('clipboard:saveImageAsTempFile', args), clipboardHasImage: (): Promise => ipcRenderer.invoke('clipboard:hasImage'), readClipboardFilePaths: (): Promise => ipcRenderer.invoke('clipboard:readFilePaths'), @@ -111,6 +113,12 @@ export const uiClipboardAndWindowControlsApi = { paths: string[] ): Promise<{ path: string; kept: boolean; exists: boolean }[]> => ipcRenderer.invoke('clipboard:restoreNativeChatPastes', paths), + onAgentSessionUploadRemoved: (callback: (uploadDir: string) => void): (() => void) => { + const listener = (_event: Electron.IpcRendererEvent, uploadDir: string): void => + callback(uploadDir) + ipcRenderer.on('app:agentSessionUploadRemoved', listener) + return () => ipcRenderer.removeListener('app:agentSessionUploadRemoved', listener) + }, readClipboardImageThumbnail: (): Promise => ipcRenderer.invoke('clipboard:readImageThumbnail'), writeClipboardText: (text: string): Promise => diff --git a/src/preload/api/ui-window-api.ts b/src/preload/api/ui-window-api.ts index c06dd79dab7..14dad3c88d8 100644 --- a/src/preload/api/ui-window-api.ts +++ b/src/preload/api/ui-window-api.ts @@ -15,6 +15,8 @@ export type UiWindowApi = { agentSessionAttachment?: AgentSessionAttachmentClipboardTarget /** A native-chat composer paste, kept where its draft can bring it back. */ forNativeChatDraft?: boolean + /** With `forNativeChatDraft`: a structured chat on this machine, whose sends claim its pastes. */ + forStructuredChat?: boolean }) => Promise clipboardHasImage: () => Promise /** Paths of files a file manager copied; empty when there are none or the host cannot list them. */ @@ -23,6 +25,8 @@ export type UiWindowApi = { restoreNativeChatPastes: ( paths: string[] ) => Promise<{ path: string; kept: boolean; exists: boolean }[]> + /** A sweep tried to remove this chat-attachment upload folder; drafts naming it check again. */ + onAgentSessionUploadRemoved: (callback: (uploadDir: string) => void) => () => void readClipboardImageThumbnail: () => Promise writeClipboardText: (text: string) => Promise writeTerminalClipboardText: (text: string) => Promise diff --git a/src/renderer/src/app-shell/native-chat-draft-startup.ts b/src/renderer/src/app-shell/native-chat-draft-startup.ts index 4d64bdd708f..f0a5c9bad43 100644 --- a/src/renderer/src/app-shell/native-chat-draft-startup.ts +++ b/src/renderer/src/app-shell/native-chat-draft-startup.ts @@ -5,17 +5,23 @@ import { setNativeChatComposerDraftOwnerResolver, waitForNativeChatComposerDrafts } from '@/components/native-chat/native-chat-composer-draft-store' +import { recheckNativeChatComposerDraftImagesIn } from '@/components/native-chat/native-chat-composer-draft-image-check' // Why bounded: loading drafts is bookkeeping and must never hold startup; a slower load still // fills in every draft not edited meanwhile when it lands. const DRAFT_LOAD_WAIT_MS = 1_500 /** Before any startup step, so a step that fails can't leave the drafts unloaded. */ -export function startNativeChatDraftLoad(): void { +export function startNativeChatDraftLoad(): (() => void) | undefined { setNativeChatComposerDraftOwnerResolver((scopeKey) => resolveNativeChatDraftOwner(useAppStore.getState(), scopeKey) ) + // Before the load: a removal after a draft's restore check must reach it. + const stopRechecks = window.api?.ui?.onAgentSessionUploadRemoved?.( + recheckNativeChatComposerDraftImagesIn + ) void hydrateNativeChatComposerDrafts() + return stopRechecks } /** Startup waits for the drafts alongside the session read, so a composer shows its draft from diff --git a/src/renderer/src/components/native-chat/native-chat-attachment-upload.ts b/src/renderer/src/components/native-chat/native-chat-attachment-upload.ts index 4b095c9a4eb..f82373eee25 100644 --- a/src/renderer/src/components/native-chat/native-chat-attachment-upload.ts +++ b/src/renderer/src/components/native-chat/native-chat-attachment-upload.ts @@ -38,7 +38,8 @@ export type NativeChatRuntimeSessionAttachmentOwner = { } export type NativeChatAttachmentOwner = - | { kind: 'local' } + /** `structuredSessionId`: a structured chat on this machine, whose sends claim its pastes. */ + | { kind: 'local'; structuredSessionId?: string } | NativeChatRuntimeSessionAttachmentOwner /** Refuses client files: a terminal-backed chat on a paired server (its agent may run on that * server's SSH or WSL host, which the server's store cannot reach), or an `ssh:` host, which diff --git a/src/renderer/src/components/native-chat/native-chat-clipboard-image-save.ts b/src/renderer/src/components/native-chat/native-chat-clipboard-image-save.ts index 12d93faa846..774230c7e8d 100644 --- a/src/renderer/src/components/native-chat/native-chat-clipboard-image-save.ts +++ b/src/renderer/src/components/native-chat/native-chat-clipboard-image-save.ts @@ -30,7 +30,13 @@ async function clipboardImageSaveArgs( | { ok: false; notice: string; cause: NativeChatClipboardImageFailureCause } > { if (owner.kind === 'local') { - return { ok: true, args: { forNativeChatDraft: true } } + return { + ok: true, + args: { + forNativeChatDraft: true, + ...(owner.structuredSessionId ? { forStructuredChat: true } : {}) + } + } } // A terminal-backed chat's agent reads the path from the server's own temp dir, as a terminal paste does. if (owner.kind === 'remote') { diff --git a/src/renderer/src/components/native-chat/native-chat-composer-draft-image-check.ts b/src/renderer/src/components/native-chat/native-chat-composer-draft-image-check.ts index efa65db9abf..9d250f48a84 100644 --- a/src/renderer/src/components/native-chat/native-chat-composer-draft-image-check.ts +++ b/src/renderer/src/components/native-chat/native-chat-composer-draft-image-check.ts @@ -1,17 +1,23 @@ -// Which restored draft images are known to be gone. A paste in Orca's paste folder asks main -// whether it is still kept, which holds only for a file really inside that folder; a file a paired -// server stored for the chat is that server's to check, which its claim at send does; any other -// image goes through the existing existence check (the workspace's read rules locally, the host -// over SSH). +// Restored store images are checked only with affirmative execution-host ownership. -import { useEffect, useSyncExternalStore } from 'react' +import { useEffect, useMemo, useSyncExternalStore } from 'react' +import { useAppStore } from '@/store' +import { chatImageAccess } from '@/lib/local-file-access' import { isAgentSessionAttachmentStorePath } from '../../../../shared/agent-session-attachments' +import { + records, + restoredImageChecks, + restoredImageKey, + NO_IMAGE_CHECKS, + notifyScope, + type DraftImageCheck +} from './native-chat-composer-draft-memory' import type { NativeChatComposerDraftImage } from './native-chat-composer-draft-storage' import { isKeptLocalPaste, + resolveNativeChatComposerDraftOwner, + structuredAgentSessionIdOfDraftScope, readNativeChatComposerDraft, - isNativeChatComposerDraftUnverified, - markNativeChatComposerDraftVerified, unavailableNativeChatComposerDraftImage, updateNativeChatComposerDraft } from './native-chat-composer-draft-store' @@ -40,11 +46,12 @@ export async function findMissingNativeChatComposerDraftImages( } await Promise.all( checkable - .filter((image) => !isKeptLocalPaste(image) && !isAgentSessionAttachmentStorePath(image.path)) + .filter((image) => !isKeptLocalPaste(image)) .map(async (image) => { try { const exists = await pathExists({ filePath: image.path, + ...(isAgentSessionAttachmentStorePath(image.path) ? { access: chatImageAccess() } : {}), ...(image.connectionId ? { connectionId: image.connectionId } : {}) }) if (!exists) { @@ -58,55 +65,149 @@ export async function findMissingNativeChatComposerDraftImages( return missing } -const checking = new Set() - -/** Once per restored draft: an image whose file is gone comes back as one to attach again. */ -export async function verifyRestoredNativeChatComposerDraftImages(scopeKey: string): Promise { - if (!isNativeChatComposerDraftUnverified(scopeKey) || checking.has(scopeKey)) { - return - } - checking.add(scopeKey) - try { - await replaceMissingImages(scopeKey) - } finally { - checking.delete(scopeKey) - markNativeChatComposerDraftVerified(scopeKey) - } +function localStoreAuthority(scopeKey: string): string | null { + const stored = readNativeChatComposerDraft(scopeKey).owner + const current = resolveNativeChatComposerDraftOwner(scopeKey) + return structuredAgentSessionIdOfDraftScope(scopeKey) !== null && + (stored ?? current)?.executionHostId === 'local' && + (!current || current.executionHostId === 'local') + ? JSON.stringify([stored, current]) + : null } -async function replaceMissingImages(scopeKey: string): Promise { - const checked = readNativeChatComposerDraft(scopeKey).images - const missing = await findMissingNativeChatComposerDraftImages(checked) - if (missing.size === 0) { +function checkAuthority(image: NativeChatComposerDraftImage, local: string | null): string | null { + return isAgentSessionAttachmentStorePath(image.path) + ? image.connectionId + ? null + : local + : 'path' +} + +function replaceCheck(scopeKey: string, entry: DraftImageCheck, next?: DraftImageCheck): void { + const checks = restoredImageChecks.get(scopeKey) + if (!checks?.includes(entry)) { return } - const gone = new Set(checked.filter((image) => missing.has(image.id)).map(({ path }) => path)) - updateNativeChatComposerDraft( + restoredImageChecks.set( scopeKey, - { - images: readNativeChatComposerDraft(scopeKey).images.map((image) => - missing.has(image.id) && gone.has(image.path) - ? unavailableNativeChatComposerDraftImage(image) - : image - ) - }, - 'immediate' + checks.flatMap((held) => (held === entry ? (next ? [next] : []) : [held])) + ) + if (restoredImageChecks.get(scopeKey)?.length === 0) { + restoredImageChecks.delete(scopeKey) + } + notifyScope(scopeKey) +} + +function attemptIsCurrent(scopeKey: string, entry: DraftImageCheck): boolean { + return ( + !!entry.attempt && + restoredImageChecks.get(scopeKey)?.includes(entry) === true && + readNativeChatComposerDraft(scopeKey).images.some( + (image) => restoredImageKey(image) === restoredImageKey(entry.image) + ) && + entry.attempt.authority === checkAuthority(entry.image, localStoreAuthority(scopeKey)) ) } -/** Whether the scope's restored images still wait on their check, which runs as they arrive: at - * mount, or later when the startup load or another window brings new ones. */ +async function runCheck(scopeKey: string, entry: DraftImageCheck): Promise { + try { + const missing = await findMissingNativeChatComposerDraftImages([entry.image]) + if (attemptIsCurrent(scopeKey, entry) && missing.has(entry.image.id)) { + updateNativeChatComposerDraft( + scopeKey, + { + images: readNativeChatComposerDraft(scopeKey).images.map((image) => + restoredImageKey(image) === restoredImageKey(entry.image) + ? unavailableNativeChatComposerDraftImage(image) + : image + ) + }, + 'immediate' + ) + } + } finally { + if (attemptIsCurrent(scopeKey, entry)) { + replaceCheck(scopeKey, entry) + } + } +} + +export async function verifyRestoredNativeChatComposerDraftImages(scopeKey: string): Promise { + await Promise.all( + (restoredImageChecks.get(scopeKey) ?? NO_IMAGE_CHECKS).map(async (entry) => { + const authority = checkAuthority(entry.image, localStoreAuthority(scopeKey)) + if (entry.attempt && entry.attempt.authority !== authority) { + replaceCheck(scopeKey, entry, { image: entry.image }) + } else if (!entry.attempt && authority !== null) { + const claimed = { image: entry.image, attempt: { authority } } + replaceCheck(scopeKey, entry, claimed) + if (restoredImageChecks.get(scopeKey)?.includes(claimed)) { + await runCheck(scopeKey, claimed) + } + } + }) + ) +} + +function isInsideUploadDir(path: string, uploadDir: string): boolean { + return ( + path.startsWith(uploadDir) && + (path[uploadDir.length] === '/' || path[uploadDir.length] === '\\') + ) +} + +/** A sweep may have removed this local upload folder after a draft's restore check said its image + * was there, so every loaded draft naming it checks again, whether its composer is shown or not. */ +export function recheckNativeChatComposerDraftImagesIn(uploadDir: string): void { + for (const scopeKey of records.keys()) { + const named = readNativeChatComposerDraft(scopeKey).images.filter( + (image) => + image.unavailableName === undefined && + !image.connectionId && + !!image.path && + isInsideUploadDir(image.path, uploadDir) + ) + if (named.length === 0) { + continue + } + // A fresh entry replaces any attempt in flight, whose answer predates the removal. + const keys = new Set(named.map(restoredImageKey)) + restoredImageChecks.set(scopeKey, [ + ...(restoredImageChecks.get(scopeKey) ?? NO_IMAGE_CHECKS).filter( + (entry) => !keys.has(restoredImageKey(entry.image)) + ), + ...named.map((image) => ({ image })) + ]) + notifyScope(scopeKey) + void verifyRestoredNativeChatComposerDraftImages(scopeKey) + } +} + +/** Only an eligible image's finite restore work holds its chip, independent of deferred peers. */ export function useRestoredNativeChatComposerDraftImageCheck( scopeKey: string, subscribe: (listener: () => void) => () => void -): boolean { - const restoring = useSyncExternalStore(subscribe, () => - isNativeChatComposerDraftUnverified(scopeKey) +): ReadonlySet { + const checks = useSyncExternalStore( + subscribe, + () => restoredImageChecks.get(scopeKey) ?? NO_IMAGE_CHECKS + ) + const local = useAppStore(() => localStoreAuthority(scopeKey)) + const pending = useMemo( + () => + new Set( + checks + .filter( + (entry) => + checkAuthority(entry.image, local) !== null && + (isKeptLocalPaste(entry.image) || isAgentSessionAttachmentStorePath(entry.image.path)) + ) + .map((entry) => restoredImageKey(entry.image)) + ), + [checks, local] ) useEffect(() => { - if (restoring) { - void verifyRestoredNativeChatComposerDraftImages(scopeKey) - } - }, [scopeKey, restoring]) - return restoring + void verifyRestoredNativeChatComposerDraftImages(scopeKey) + }, [scopeKey, checks, local]) + return pending } diff --git a/src/renderer/src/components/native-chat/native-chat-composer-draft-lifecycle.test.tsx b/src/renderer/src/components/native-chat/native-chat-composer-draft-lifecycle.test.tsx index 9ef51f4ab49..757007a4852 100644 --- a/src/renderer/src/components/native-chat/native-chat-composer-draft-lifecycle.test.tsx +++ b/src/renderer/src/components/native-chat/native-chat-composer-draft-lifecycle.test.tsx @@ -643,10 +643,6 @@ describe('native-chat composer draft lifecycle', () => { vi.unstubAllGlobals() } - expect(restoreNativeChatPastes).toHaveBeenCalledWith([ - `${folder}/orca-paste-1-ab.png`, - `${folder}/orca-paste-2-ab.png` - ]) expect(seen.api?.attachments.imageAttachments).toEqual([ { id: 'kept', path: `${folder}/orca-paste-1-ab.png` }, { id: 'swept', path: '', unavailableName: 'orca-paste-2-ab.png' } diff --git a/src/renderer/src/components/native-chat/native-chat-composer-draft-load.ts b/src/renderer/src/components/native-chat/native-chat-composer-draft-load.ts index 4912869e333..52c6076bea2 100644 --- a/src/renderer/src/components/native-chat/native-chat-composer-draft-load.ts +++ b/src/renderer/src/components/native-chat/native-chat-composer-draft-load.ts @@ -8,7 +8,7 @@ import { nextSavedAt, notifyScope, records, - unverifiedScopes + reconcileRestoredImageChecks } from './native-chat-composer-draft-memory' import { flushNativeChatComposerDrafts, @@ -80,8 +80,8 @@ function applyLoaded( continue } const { draft, changed } = withAppends(scopeKey, stored, readAtSequence) + reconcileRestoredImageChecks(scopeKey, draft.images, records.get(scopeKey)?.images ?? []) records.set(scopeKey, draft) - unverifiedScopes.add(scopeKey) if (changed) { dirtyScopes.add(scopeKey) } diff --git a/src/renderer/src/components/native-chat/native-chat-composer-draft-memory.ts b/src/renderer/src/components/native-chat/native-chat-composer-draft-memory.ts index 96ba8bc26af..aba96b48b49 100644 --- a/src/renderer/src/components/native-chat/native-chat-composer-draft-memory.ts +++ b/src/renderer/src/components/native-chat/native-chat-composer-draft-memory.ts @@ -1,7 +1,10 @@ // The draft store's memory, shared by its API (the store module) and its persistence. Memory is // the owner of every draft; storage only keeps a copy of it. -import type { StoredNativeChatComposerDraft } from './native-chat-composer-draft-storage' +import type { + NativeChatComposerDraftImage, + StoredNativeChatComposerDraft +} from './native-chat-composer-draft-storage' // Why unsavedText: an adopted launch seed is also parked in the agent's input line, and only this // run's seed knows to replace it, so a reload must not bring the copy back. @@ -19,8 +22,45 @@ export const dirtyScopes = new Set() export const unconfirmed = new Map() // Drafts whose last write storage refused; shown so the user knows they are held in memory only. export const refusedScopes = new Set() -// Records read back from storage this run whose image files have not been checked yet. -export const unverifiedScopes = new Set() +export type DraftImageCheck = { + image: NativeChatComposerDraftImage + /** The object itself is the current attempt token. */ + attempt?: { authority: string } +} +export const restoredImageChecks = new Map() +export const NO_IMAGE_CHECKS: readonly DraftImageCheck[] = [] + +export function restoredImageKey(image: NativeChatComposerDraftImage): string { + return JSON.stringify([image.id, image.path, image.connectionId]) +} + +/** Ordinary edits prune work; restore boundaries register only newly introduced tuples. */ +export function reconcileRestoredImageChecks( + scopeKey: string, + images: readonly NativeChatComposerDraftImage[], + previous: readonly NativeChatComposerDraftImage[] = images +): void { + const current = restoredImageChecks.get(scopeKey) ?? NO_IMAGE_CHECKS + const held = new Set( + images + .filter((image) => image.path && image.unavailableName === undefined) + .map(restoredImageKey) + ) + const before = new Set(previous.map(restoredImageKey)) + const next = current.filter((entry) => held.has(restoredImageKey(entry.image))) + for (const image of images) { + if (held.has(restoredImageKey(image)) && !before.has(restoredImageKey(image))) { + next.push({ image }) + } + } + if (next.length !== current.length || next.some((entry, index) => entry !== current[index])) { + if (next.length) { + restoredImageChecks.set(scopeKey, next) + } else { + restoredImageChecks.delete(scopeKey) + } + } +} export const scopeListeners = new Map void>>() /** How a change before the startup load lands meets the loaded draft: an edit replaces it, an @@ -79,7 +119,7 @@ export function clearDraftMemoryForTests(): void { dirtyScopes.clear() unconfirmed.clear() refusedScopes.clear() - unverifiedScopes.clear() + restoredImageChecks.clear() lastSavedAt = 0 load.hydrated = false load.appendSequence = 0 diff --git a/src/renderer/src/components/native-chat/native-chat-composer-draft-persistence.ts b/src/renderer/src/components/native-chat/native-chat-composer-draft-persistence.ts index dc9e0a3d139..5fa10d2ac53 100644 --- a/src/renderer/src/components/native-chat/native-chat-composer-draft-persistence.ts +++ b/src/renderer/src/components/native-chat/native-chat-composer-draft-persistence.ts @@ -13,7 +13,7 @@ import { records, refusedScopes, unconfirmed, - unverifiedScopes, + reconcileRestoredImageChecks, type AppendBeforeLoad, type DraftAppend, type DraftRecord, @@ -242,14 +242,6 @@ export function deleteLoadingNativeChatComposerDraftsWhere( } } -function addsAnImage( - draft: StoredNativeChatComposerDraft, - previous: DraftRecord | undefined -): boolean { - const held = new Set(previous?.images.map((image) => `${image.id}\0${image.path}`)) - return draft.images.some((image) => !held.has(`${image.id}\0${image.path}`)) -} - // Why: another window of the same app (two web-client tabs) can send or edit this draft; its write // replaces what this window holds unless this window has a change of its own not yet saved. function adoptForeignChange(message: unknown): void { @@ -268,13 +260,11 @@ function adoptForeignChange(message: unknown): void { } const draft = parseStoredNativeChatComposerDraft(value) if (draft) { - // Why: images this window already checked stay checked, so their chips don't flash. - if (addsAnImage(draft, records.get(scopeKey))) { - unverifiedScopes.add(scopeKey) - } + reconcileRestoredImageChecks(scopeKey, draft.images, records.get(scopeKey)?.images ?? []) records.set(scopeKey, draft) } else { records.delete(scopeKey) + reconcileRestoredImageChecks(scopeKey, []) } notifyScope(scopeKey) }) diff --git a/src/renderer/src/components/native-chat/native-chat-composer-draft-store.test.ts b/src/renderer/src/components/native-chat/native-chat-composer-draft-store.test.ts index 4af2cc65d5c..f209dc8ed30 100644 --- a/src/renderer/src/components/native-chat/native-chat-composer-draft-store.test.ts +++ b/src/renderer/src/components/native-chat/native-chat-composer-draft-store.test.ts @@ -51,6 +51,7 @@ const SKILL_DOCUMENT = { } let modules: DraftModules +const originalPreloadApi = window.api // Warm the same draft modules the reload cases exercise. beforeAll(async () => { @@ -65,6 +66,7 @@ beforeEach(async () => { }) afterEach(() => { + Object.assign(window, { api: originalPreloadApi }) vi.useRealTimers() // Each loaded renderer closes its channel, so it hears nothing from the next test. for (const instance of loaded.splice(0)) { @@ -113,7 +115,6 @@ describe('native-chat composer draft store', () => { expect(reloaded.drafts.readNativeChatDraftCache('tab-1:pane')).toBe('typed, then Orca quit') await reloaded.store.nativeChatComposerDraftWritesSettled() expect(storedDraft('tab-1:pane')?.text).toBe('typed, then Orca quit') - expect(localStorage.length).toBe(0) }) it('reports a scope’s writes settled only once storage completes them, and false when refused', async () => { @@ -454,15 +455,6 @@ describe('native-chat composer draft store', () => { expect(reloaded.attachments.readNativeChatAttachmentCache('tab-1:pane')).toEqual([]) }) - it('removes an emptied draft without waiting for a flush', () => { - vi.useFakeTimers() - modules.drafts.writeNativeChatDraftCache('tab-1:pane', 'about to send') - modules.store.flushNativeChatComposerDrafts() - modules.drafts.writeNativeChatDraftCache('tab-1:pane', '') - - expect(storage.drafts.size).toBe(0) - }) - it('loses nothing past the old budget: 200 drafts and over 5M characters all come back', async () => { for (let index = 0; index < 200; index += 1) { modules.drafts.writeNativeChatDraftCache(`tab-${index}:pane`, `${index}`.padEnd(26_000, 'd')) @@ -604,6 +596,8 @@ describe('native-chat composer draft store', () => { 'saved earlier\n\nreturned by Stop' ) expect(reloaded.attachments.readNativeChatAttachmentCache('agent-session:s1')).toEqual(IMAGES) + const memory = await import('./native-chat-composer-draft-memory') + expect(memory.restoredImageChecks.get('agent-session:s1')).toHaveLength(1) expect(storedDraft('agent-session:s1')).toMatchObject({ text: 'saved earlier\n\nreturned by Stop', images: IMAGES @@ -799,24 +793,32 @@ describe('native-chat composer draft store', () => { warn.mockRestore() }) - it('keeps a restored image checked when another window saves the draft with the same image', async () => { - const other = await reload() + it('gives a foreign replacement its own check and keeps completed unchanged images settled', async () => { + const other = modules + modules = await reload() + const checks = await import('./native-chat-composer-draft-image-check') + const memory = await import('./native-chat-composer-draft-memory') + const scope = 'tab-1:pane' + const restored = () => modules.store.readNativeChatComposerDraft(scope) + const old = Promise.withResolvers() + const next = Promise.withResolvers() + const pathExists = vi.fn().mockReturnValueOnce(old.promise).mockReturnValueOnce(next.promise) + Object.assign(window, { api: { fs: { pathExists } } }) other.attachments.appendNativeChatAttachmentCache('tab-1:pane', [IMAGES[0]]) - await vi.waitFor(() => - expect(modules.attachments.readNativeChatAttachmentCache('tab-1:pane')).toEqual([IMAGES[0]]) - ) - modules.store.markNativeChatComposerDraftVerified('tab-1:pane') - + await vi.waitFor(() => expect(restored().images).toEqual([IMAGES[0]])) + const first = checks.verifyRestoredNativeChatComposerDraftImages('tab-1:pane') + const replacement = { ...IMAGES[0], path: '/repo/new.png' } + other.store.updateNativeChatComposerDraft(scope, { images: [replacement] }, 'immediate') + await vi.waitFor(() => expect(restored().images).toEqual([replacement])) + const second = checks.verifyRestoredNativeChatComposerDraftImages('tab-1:pane') + old.resolve(false) + await first + expect(memory.restoredImageChecks.get('tab-1:pane')).toHaveLength(1) + next.reject(new Error('temporarily unavailable')) + await second other.drafts.appendNativeChatDraftCache('tab-1:pane', 'caption') - await vi.waitFor(() => - expect(modules.drafts.readNativeChatDraftCache('tab-1:pane')).toBe('caption') - ) - expect(modules.store.isNativeChatComposerDraftUnverified('tab-1:pane')).toBe(false) - - other.attachments.appendNativeChatAttachmentCache('tab-1:pane', [IMAGES[1]]) - await vi.waitFor(() => - expect(modules.store.isNativeChatComposerDraftUnverified('tab-1:pane')).toBe(true) - ) + await vi.waitFor(() => expect(restored().text).toBe('caption')) + expect(memory.restoredImageChecks.has('tab-1:pane')).toBe(false) }) it('follows another window’s save of a draft, unless a change here is not saved yet', async () => { diff --git a/src/renderer/src/components/native-chat/native-chat-composer-draft-store.ts b/src/renderer/src/components/native-chat/native-chat-composer-draft-store.ts index fb1c2004257..95214f1970e 100644 --- a/src/renderer/src/components/native-chat/native-chat-composer-draft-store.ts +++ b/src/renderer/src/components/native-chat/native-chat-composer-draft-store.ts @@ -24,7 +24,7 @@ import { records, refusedScopes, scopeListeners, - unverifiedScopes, + reconcileRestoredImageChecks, type DraftAppend, type DraftRecord } from './native-chat-composer-draft-memory' @@ -107,17 +107,6 @@ export function subscribeToNativeChatComposerDraft( } } -/** A draft read back from storage this run whose image files have not been checked yet. */ -export function isNativeChatComposerDraftUnverified(scopeKey: string): boolean { - return unverifiedScopes.has(scopeKey) -} - -export function markNativeChatComposerDraftVerified(scopeKey: string): void { - if (unverifiedScopes.delete(scopeKey)) { - notifyScope(scopeKey) - } -} - /** A draft storage refused to save: it is kept in memory only until a later write lands. */ export function isNativeChatComposerDraftUnsaved(scopeKey: string): boolean { return refusedScopes.has(scopeKey) && records.has(scopeKey) @@ -127,7 +116,9 @@ function isEmptyDraft(draft: NativeChatComposerDraft): boolean { return draft.text === '' && draft.images.length === 0 } -export function readNativeChatComposerDraft(scopeKey: string): NativeChatComposerDraft { +export function readNativeChatComposerDraft( + scopeKey: string +): NativeChatComposerDraft & Pick { return records.get(scopeKey) ?? EMPTY_DRAFT } @@ -160,6 +151,7 @@ export function updateNativeChatComposerDraft( } if (isEmptyDraft({ text, images })) { records.delete(scopeKey) + reconcileRestoredImageChecks(scopeKey, []) notifyScope(scopeKey) persistNativeChatComposerDraft(scopeKey, 'immediate', append) return @@ -175,6 +167,7 @@ export function updateNativeChatComposerDraft( ...(unsavedText === undefined ? {} : { unsavedText }) } records.set(scopeKey, record) + reconcileRestoredImageChecks(scopeKey, record.images) notifyScope(scopeKey) persistNativeChatComposerDraft(scopeKey, persist, append) if (!load.hydrated) { @@ -262,6 +255,7 @@ function deleteDraftsWhere( const draft = records.get(scopeKey) if (matches(scopeKey, draft)) { records.delete(scopeKey) + reconcileRestoredImageChecks(scopeKey, []) notifyScope(scopeKey) if (draft) { persistNativeChatComposerDraft(scopeKey, 'immediate') diff --git a/src/renderer/src/components/native-chat/native-chat-composer-drop-scope.test.tsx b/src/renderer/src/components/native-chat/native-chat-composer-drop-scope.test.tsx index 10e1d6bf984..3a534c1c2a8 100644 --- a/src/renderer/src/components/native-chat/native-chat-composer-drop-scope.test.tsx +++ b/src/renderer/src/components/native-chat/native-chat-composer-drop-scope.test.tsx @@ -36,7 +36,11 @@ const intake = vi.hoisted(() => { pick: vi.fn() } }) -vi.mock('@/store', () => ({ useAppStore: { getState: () => ({ tabsByWorktree: {} }) } })) +vi.mock('@/store', () => ({ + useAppStore: Object.assign((select: () => unknown) => select(), { + getState: () => ({ tabsByWorktree: {} }) + }) +})) // Keeps the real notice strings so the silent-failure guards assert what users see. vi.mock('./native-chat-attachment-upload', async (importOriginal) => ({ ...(await importOriginal()), diff --git a/src/renderer/src/components/native-chat/native-chat-external-attachments-destination.test.tsx b/src/renderer/src/components/native-chat/native-chat-external-attachments-destination.test.tsx index a3bf8fce214..88602c487f8 100644 --- a/src/renderer/src/components/native-chat/native-chat-external-attachments-destination.test.tsx +++ b/src/renderer/src/components/native-chat/native-chat-external-attachments-destination.test.tsx @@ -34,7 +34,9 @@ const mocks = vi.hoisted(() => { return state }) vi.mock('@/store', () => ({ - useAppStore: { getState: () => ({ tabsByWorktree: mocks.tabsByWorktree }) } + useAppStore: Object.assign((select: () => unknown) => select(), { + getState: () => ({ tabsByWorktree: mocks.tabsByWorktree }) + }) })) vi.mock('@/i18n/i18n', () => ({ translate: (_key: string, fallback: string) => fallback })) vi.mock('../../../../shared/remote-runtime-pty-id', async (importOriginal) => ({ diff --git a/src/renderer/src/components/native-chat/native-chat-image-paste.ts b/src/renderer/src/components/native-chat/native-chat-image-paste.ts index 3eb6689b034..6b1aca46bf0 100644 --- a/src/renderer/src/components/native-chat/native-chat-image-paste.ts +++ b/src/renderer/src/components/native-chat/native-chat-image-paste.ts @@ -17,8 +17,8 @@ export function isNativeChatPastedImagePath(path: string): boolean { return /^orca-paste-.+\.png$/i.test(base) } -/** True for a local paste Orca keeps in its paste folder, judged from the path alone; main checks - * the real file before a restore keeps it. */ +/** True for a local paste in the folder older builds kept them in, judged from the path alone; + * main checks the real file before a restore keeps it. */ export function isNativeChatKeptPastePath(path: string): boolean { const parts = path.split(/[\\/]/).filter(Boolean) return parts.at(-2) === NATIVE_CHAT_PASTE_FOLDER && isNativeChatPastedImagePath(path) diff --git a/src/renderer/src/components/native-chat/use-native-chat-composer-attachments.test.tsx b/src/renderer/src/components/native-chat/use-native-chat-composer-attachments.test.tsx index 29d8384a846..c44e04c2686 100644 --- a/src/renderer/src/components/native-chat/use-native-chat-composer-attachments.test.tsx +++ b/src/renderer/src/components/native-chat/use-native-chat-composer-attachments.test.tsx @@ -16,6 +16,12 @@ import { NATIVE_FILE_DROP_MAX_PATHS } from '../../../../shared/native-file-drop' import { imageReferenceToken } from '../../../../shared/native-chat-image-reference' import type * as RemoteRuntimePtyIdModule from '../../../../shared/remote-runtime-pty-id' +import { useAppStore } from '@/store' +import * as draftStore from './native-chat-composer-draft-store' +import * as draftStorage from './native-chat-composer-draft-storage' +import { nativeChatImagesHoldSend } from './native-chat-image-reattach' +import { recheckNativeChatComposerDraftImagesIn } from './native-chat-composer-draft-image-check' + vi.mock('@/i18n/i18n', () => ({ translate: (_key: string, fallback: string) => fallback })) @@ -150,6 +156,97 @@ describe('useNativeChatComposerAttachments', () => { ) }) + it('checks a restored store image when its owner becomes local, holding Send and preserving a newer edit', async () => { + const scope = 'agent-session:restored' + const image = { id: 'old', path: '/state/agent-session-attachments/upload/shot.png' } + const storage = draftStorage.createMemoryNativeChatComposerDraftStorage() + storage.drafts.set(scope, { text: '', images: [image, { ...image, id: 'edit' }], savedAt: 1 }) + const paste = { id: 'paste', path: '/state/native-chat-pastes/orca-paste-1-a.png' } + storage.drafts.set('pty-1', { text: '', images: [image, paste], savedAt: 1 }) + draftStorage.setNativeChatComposerDraftStorageForTests(storage) + let owner: draftStorage.NativeChatComposerDraftOwner | undefined + draftStore.setNativeChatComposerDraftOwnerResolver(() => owner) + const exists = Promise.withResolvers() + const pathExists = vi.fn(() => exists.promise) + const originalApi = window.api + Object.assign(window, { + api: { + fs: { pathExists }, + ui: { + restoreNativeChatPastes: async () => [{ path: paste.path, kept: true, exists: true }] + } + } + }) + try { + await draftStore.hydrateNativeChatComposerDrafts() + const terminal = await renderProbe('pty-1') + act(() => terminal.latest().attachResolvedPaths(['/repo/fresh.png'])) + expect(nativeChatImagesHoldSend(terminal.latest().imageAttachments)).toBe(false) + act(() => terminal.root.unmount()) + const probe = await renderProbe(scope, true) + owner = { workspaceId: 'wt', executionHostId: 'runtime:remote' } + act(() => useAppStore.setState({})) + expect(pathExists).not.toHaveBeenCalled() + owner = { workspaceId: 'wt', executionHostId: 'local' } + act(() => useAppStore.setState({})) + expect(nativeChatImagesHoldSend(probe.latest().imageAttachments)).toBe(true) + owner = { workspaceId: 'wt', executionHostId: 'runtime:remote' } + await act(async () => useAppStore.setState({})) + expect(nativeChatImagesHoldSend(probe.latest().imageAttachments)).toBe(false) + owner = { workspaceId: 'wt', executionHostId: 'local' } + act(() => useAppStore.setState({})) + const newer = { id: 'edit', path: '/repo/new.png' } + await act(async () => { + draftStore.updateNativeChatComposerDraft(scope, { images: [image, newer] }, 'immediate') + exists.resolve(false) + }) + expect(probe.latest().imageAttachments).toMatchObject([ + { id: 'old', path: '', unavailableName: 'shot.png' }, + newer + ]) + act(() => probe.root.unmount()) + } finally { + Object.assign(window, { api: originalApi }) + } + }) + + it('marks a restored image Not kept when a later sweep removes its upload, composer shown or not', async () => { + const scope = 'agent-session:swept-later' + const image = { id: 'old', path: '/state/agent-session-attachments/upload/shot.png' } + const other = { id: 'other', path: '/state/agent-session-attachments/upload-2/b.png' } + const storage = draftStorage.createMemoryNativeChatComposerDraftStorage() + storage.drafts.set(scope, { text: 'hi', images: [image, other], savedAt: 1 }) + draftStorage.setNativeChatComposerDraftStorageForTests(storage) + draftStore.setNativeChatComposerDraftOwnerResolver(() => ({ + workspaceId: 'wt', + executionHostId: 'local' + })) + let present = true + const pathExists = vi.fn(async () => present) + const originalApi = window.api + Object.assign(window, { api: { fs: { pathExists }, ui: {} } }) + try { + await draftStore.hydrateNativeChatComposerDrafts() + const probe = await renderProbe(scope, true) + await act(async () => {}) + expect(nativeChatImagesHoldSend(probe.latest().imageAttachments)).toBe(false) + act(() => probe.root.unmount()) + present = false + await act(async () => + recheckNativeChatComposerDraftImagesIn('/state/agent-session-attachments/upload') + ) + expect(draftStore.readNativeChatComposerDraft(scope).images).toMatchObject([ + { id: 'old', path: '', unavailableName: 'shot.png' }, + other + ]) + const reopened = await renderProbe(scope, true) + expect(nativeChatImagesHoldSend(reopened.latest().imageAttachments)).toBe(true) + act(() => reopened.root.unmount()) + } finally { + Object.assign(window, { api: originalApi }) + } + }) + it('holds attached images as chips (deferred to submit) and restores them on remount', async () => { const first = await renderProbe('pty-1') diff --git a/src/renderer/src/components/native-chat/use-native-chat-composer-image-chips.ts b/src/renderer/src/components/native-chat/use-native-chat-composer-image-chips.ts index 27400c08afe..39939a69661 100644 --- a/src/renderer/src/components/native-chat/use-native-chat-composer-image-chips.ts +++ b/src/renderer/src/components/native-chat/use-native-chat-composer-image-chips.ts @@ -1,10 +1,10 @@ import { useCallback, useMemo, useSyncExternalStore } from 'react' import type { NativeChatComposerImageAttachment } from './NativeChatComposerField' import { - isKeptLocalPaste, readNativeChatComposerDraft, subscribeToNativeChatComposerDraft } from './native-chat-composer-draft-store' +import { restoredImageKey } from './native-chat-composer-draft-memory' import { useRestoredNativeChatComposerDraftImageCheck } from './native-chat-composer-draft-image-check' import { nativeChatImageRow, @@ -27,7 +27,10 @@ export function useNativeChatComposerImageChips(attachmentScopeKey: string): { ) // Why: a restored paste shows only once main confirms it is still kept, so until the restore // check is done it waits like a chip still saving, instead of flashing before a placeholder. - const restoring = useRestoredNativeChatComposerDraftImageCheck(attachmentScopeKey, subscribe) + const restoringImages = useRestoredNativeChatComposerDraftImageCheck( + attachmentScopeKey, + subscribe + ) // Chips still on their way live outside the composer, so one a prompt card unmounted comes back // still pending and settles into the draft whichever composer is showing. const pending = useNativeChatPendingAttachments(attachmentScopeKey) @@ -39,10 +42,12 @@ export function useNativeChatComposerImageChips(attachmentScopeKey: string): { } return nativeChatImageRow( settled.map((image) => - restoring && isKeptLocalPaste(image) ? { ...image, pending: true } : withPreview(image) + restoringImages.has(restoredImageKey(image)) + ? { ...image, pending: true } + : withPreview(image) ), pending.map(withPreview) ) - }, [pending, previews, restoring, settled]) + }, [pending, previews, restoringImages, settled]) return { imageAttachments, ...previewActions } } diff --git a/src/renderer/src/components/native-chat/use-native-chat-composer-paste.test.tsx b/src/renderer/src/components/native-chat/use-native-chat-composer-paste.test.tsx index d38ba38ac45..76c6f461d78 100644 --- a/src/renderer/src/components/native-chat/use-native-chat-composer-paste.test.tsx +++ b/src/renderer/src/components/native-chat/use-native-chat-composer-paste.test.tsx @@ -183,6 +183,19 @@ describe('useNativeChatComposerPaste', () => { expect(mocks.saveClipboardImageAsTempFile).toHaveBeenCalledWith({ forNativeChatDraft: true }) }) + it("keeps a local structured chat's paste where that chat's send claims it", async () => { + const probe = await renderProbe({ + resolveAttachmentOwner: () => ({ kind: 'local', structuredSessionId: 'session-1' }) + }) + await act(async () => { + probe.latest().handlePaste(imagePasteEvent()) + }) + expect(mocks.saveClipboardImageAsTempFile).toHaveBeenCalledWith({ + forNativeChatDraft: true, + forStructuredChat: true + }) + }) + it('shows a pending chip before the save resolves', async () => { let resolveSave: (path: string) => void = () => {} mocks.saveClipboardImageAsTempFile.mockReturnValue( diff --git a/src/renderer/src/components/native-chat/use-native-chat-external-attachments.ts b/src/renderer/src/components/native-chat/use-native-chat-external-attachments.ts index a5be39de03a..8523fca63f3 100644 --- a/src/renderer/src/components/native-chat/use-native-chat-external-attachments.ts +++ b/src/renderer/src/components/native-chat/use-native-chat-external-attachments.ts @@ -116,7 +116,7 @@ export function useNativeChatExternalAttachments({ pendingChipsRef.current = pendingChips }, [pendingChips]) - const resolveAttachmentOwner = useCallback(() => { + const resolveAttachmentOwner = useCallback((): NativeChatAttachmentOwner => { const { structuredWorktreeId, structuredSession, terminalTabId } = workspaceRef.current if (structuredWorktreeId && structuredSession?.runtimeEnvironmentId) { return resolveNativeChatRuntimeSessionAttachmentOwner({ @@ -124,9 +124,16 @@ export function useNativeChatExternalAttachments({ runtimeEnvironmentId: structuredSession.runtimeEnvironmentId }) } - return structuredWorktreeId - ? resolveNativeChatAttachmentOwnerForWorktree(useAppStore.getState(), structuredWorktreeId) - : resolveNativeChatAttachmentOwner(useAppStore.getState(), terminalTabId) + if (!structuredWorktreeId) { + return resolveNativeChatAttachmentOwner(useAppStore.getState(), terminalTabId) + } + const owner = resolveNativeChatAttachmentOwnerForWorktree( + useAppStore.getState(), + structuredWorktreeId + ) + return owner.kind === 'local' && structuredSession + ? { kind: 'local', structuredSessionId: structuredSession.sessionId } + : owner }, []) // Captures one intake's owner, destination and chips; its checks and work run against them later. diff --git a/src/renderer/src/web/preload-api/web-ui-api.ts b/src/renderer/src/web/preload-api/web-ui-api.ts index 73309d68846..7d96db7eadb 100644 --- a/src/renderer/src/web/preload-api/web-ui-api.ts +++ b/src/renderer/src/web/preload-api/web-ui-api.ts @@ -178,6 +178,7 @@ export function createWebUiApi(): NonNullable['ui']> { readClipboardFilePaths: async () => [], // Why empty: a browser has no local paste folder, so restored pastes stay to attach again. restoreNativeChatPastes: async () => [], + onAgentSessionUploadRemoved: () => noopUnsubscribe, readClipboardImageThumbnail: () => readClipboardImageThumbnail().catch(() => null), writeClipboardText: writeWebClipboardText, writeTerminalClipboardText: writeWebClipboardText, diff --git a/src/shared/agent-session-attachments.ts b/src/shared/agent-session-attachments.ts index e135edba75b..8b1e83ae7f0 100644 --- a/src/shared/agent-session-attachments.ts +++ b/src/shared/agent-session-attachments.ts @@ -1,9 +1,10 @@ -// Files a client attaches to a structured chat on a paired Orca server: the client uploads the -// bytes, the server keeps them in a store it owns, and the agent only ever sees the server path. +// Files attached to a structured chat: kept in a store owned by the host that runs the chat (a +// paired server's clients upload the bytes; this machine's own pastes are written in place), and +// the agent only ever sees the host path. import type { ImportSkipReason } from './filesystem-import-result-types' -/** Directory name of the server's store, under its userData. Also how a client recognizes a +/** Directory name of a host's store, under its userData. Also how a client recognizes a * stored attachment's path without a round trip. */ export const AGENT_SESSION_ATTACHMENTS_DIR_NAME = 'agent-session-attachments' @@ -48,8 +49,8 @@ export type AgentSessionAttachmentUploadCommitResult = { byteLength: number } -/** Whether `filePath` names a file in some server's attachment store. Only routes a preview read; - * the server re-checks the real path before reading anything. */ +/** Whether `filePath` names a file in some host's attachment store. Only routes a preview read; + * the host re-checks the real path before reading anything. */ export function isAgentSessionAttachmentStorePath(filePath: string): boolean { return filePath.split(/[\\/]/).includes(AGENT_SESSION_ATTACHMENTS_DIR_NAME) }