diff --git a/apps/server/src/orchestration-v2/Orchestrator.ts b/apps/server/src/orchestration-v2/Orchestrator.ts index 3157f55cacfa6..fa301bf896c65 100644 --- a/apps/server/src/orchestration-v2/Orchestrator.ts +++ b/apps/server/src/orchestration-v2/Orchestrator.ts @@ -14187,7 +14187,7 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio Effect.flatMap((sideChatIds) => sideChatIds.reduce( (locked, sideChatId) => threadDispatch.withLock(sideChatId, locked), - dispatchWithReceiptEffect(command, sideChatIds), + threadDispatch.withDeletionLock(dispatchWithReceiptEffect(command, sideChatIds)), ), ), ), diff --git a/apps/server/src/orchestration-v2/ProjectionStore.ts b/apps/server/src/orchestration-v2/ProjectionStore.ts index 0c4976dc90598..195d8363bb330 100644 --- a/apps/server/src/orchestration-v2/ProjectionStore.ts +++ b/apps/server/src/orchestration-v2/ProjectionStore.ts @@ -1450,6 +1450,53 @@ function inheritedRunFork(thread: Pick; + readonly files: ReadonlyArray<{ + readonly threadId: ThreadId; + readonly runOrdinal: number | null; + readonly id: string; + }>; +}): Array { + const threads = new Map(input.threads.map((thread) => [thread.id, thread])); + const filesByThread = Map.groupBy(input.files, (file) => file.threadId); + const shownBy = (threadId: ThreadId) => { + const shown = new Set(); + const seen = new Set(); + let current: ThreadId | undefined = threadId; + let cutoff = Number.POSITIVE_INFINITY; + while (current !== undefined && !seen.has(current)) { + seen.add(current); + for (const file of filesByThread.get(current) ?? []) { + // Runless rows count as shown: over-keeping only delays cleanup. + if (file.runOrdinal === null || file.runOrdinal <= cutoff) shown.add(file.id); + } + const source: RunForkFamilyThread["source"] = threads.get(current)?.source ?? null; + if (source === null) break; + cutoff = source.runOrdinal; + current = source.threadId; + } + return shown; + }; + const kept = new Set( + input.threads.flatMap((thread) => + thread.deleted || thread.id === input.threadId ? [] : [...shownBy(thread.id)], + ), + ); + return [...shownBy(input.threadId)].filter((id) => !kept.has(id)); +} + function buildVisibleTurnItems(input: { readonly projection: OrchestrationV2ThreadProjection; readonly sourceProjection: OrchestrationV2ThreadProjection | null; @@ -5661,14 +5708,99 @@ export const layer: Layer.Layer = Effect.mapError(controlReadError(threadId)), ); + // The thread, its run-fork ancestors, and every run fork below them, found through the indexed + // fork transfers rather than a scan of all threads. Deleted members stay: forks read through them. + const selectRunForkFamily = (threadId: ThreadId) => + sql<{ + thread_id: ThreadId; + deleted_at: string | null; + source_thread_id: ThreadId | null; + source_run_ordinal: number | null; + }>` + WITH RECURSIVE ancestor(thread_id) AS ( + SELECT ${threadId} + UNION + SELECT json_extract(thread.payload_json, '$.forkedFrom.threadId') + FROM ancestor + JOIN orchestration_v2_projection_threads AS thread ON thread.thread_id = ancestor.thread_id + WHERE json_extract(thread.payload_json, '$.forkedFrom.type') = 'run' + AND json_extract(thread.payload_json, '$.lineage.sideChat') IS NOT 1 + ), family(thread_id) AS ( + SELECT thread_id FROM ancestor + UNION + SELECT fork.thread_id + FROM family + JOIN orchestration_v2_projection_context_transfers AS transfer + ON transfer.source_thread_id = family.thread_id AND transfer.type = 'fork' + JOIN orchestration_v2_projection_threads AS fork + ON fork.thread_id = transfer.target_thread_id + WHERE json_extract(fork.payload_json, '$.forkedFrom.type') = 'run' + AND json_extract(fork.payload_json, '$.forkedFrom.threadId') = family.thread_id + AND json_extract(fork.payload_json, '$.lineage.sideChat') IS NOT 1 + ) + SELECT + thread.thread_id, + thread.deleted_at, + source_run.thread_id AS source_thread_id, + source_run.ordinal AS source_run_ordinal + FROM family + JOIN orchestration_v2_projection_threads AS thread ON thread.thread_id = family.thread_id + LEFT JOIN orchestration_v2_projection_runs AS source_run + ON json_extract(thread.payload_json, '$.forkedFrom.type') = 'run' + AND json_extract(thread.payload_json, '$.lineage.sideChat') IS NOT 1 + AND source_run.run_id = json_extract(thread.payload_json, '$.forkedFrom.runId') + AND source_run.thread_id = json_extract(thread.payload_json, '$.forkedFrom.threadId') + `; + + // Prompt files and question-answer files, with the run that bounds which forks show them. + const selectThreadFiles = (threadIds: ReadonlyArray) => + sql<{ thread_id: ThreadId; run_ordinal: number | null; id: string }>` + SELECT message.thread_id, run.ordinal AS run_ordinal, + json_extract(attachment.value, '$.id') AS id + FROM orchestration_v2_projection_messages AS message + JOIN json_each(message.payload_json, '$.attachments') AS attachment + LEFT JOIN orchestration_v2_projection_runs AS run ON run.run_id = message.run_id + WHERE message.thread_id IN (SELECT value FROM json_each(${encodeIdList(threadIds)})) + UNION + SELECT item.thread_id, run.ordinal AS run_ordinal, + json_extract(attachment.value, '$.id') AS id + FROM orchestration_v2_projection_turn_items AS item + JOIN json_each(item.payload_json, '$.questionAnswer.attachmentsByQuestionId') AS question + JOIN json_each(question.value) AS attachment + LEFT JOIN orchestration_v2_projection_runs AS run ON run.run_id = item.run_id + WHERE item.thread_id IN (SELECT value FROM json_each(${encodeIdList(threadIds)})) + AND item.type = 'user_input_request' + `; + const getThreadAttachmentIds: ProjectionStoreV2Shape["getThreadAttachmentIds"] = (threadId) => Effect.all([ - sql<{ id: string }>` - SELECT DISTINCT json_extract(attachment.value, '$.id') AS id - FROM orchestration_v2_projection_messages AS message, - json_each(message.payload_json, '$.attachments') AS attachment - WHERE message.thread_id = ${threadId} - `, + selectRunForkFamily(threadId).pipe( + Effect.flatMap((family) => + selectThreadFiles([threadId, ...family.map((thread) => thread.thread_id)]).pipe( + Effect.map((files) => + unsharedThreadFileIds({ + threadId, + threads: family.map((thread) => ({ + id: thread.thread_id, + deleted: thread.deleted_at !== null, + source: + thread.source_thread_id === null || thread.source_run_ordinal === null + ? null + : { + threadId: thread.source_thread_id, + runOrdinal: thread.source_run_ordinal, + }, + })), + files: files.map((file) => ({ + threadId: file.thread_id, + runOrdinal: file.run_ordinal, + id: file.id, + })), + }), + ), + ), + ), + ), sql<{ payload_json: string }>` SELECT payload_json FROM orchestration_v2_projection_turn_items @@ -5676,14 +5808,6 @@ export const layer: Layer.Layer = AND type = 'dynamic_tool' AND (payload_json LIKE '%htmlRender%' OR payload_json LIKE ${`%${MCP_APP_OUTPUT_KEY}%`} OR payload_json LIKE '%preview_recording_stop%') - `, - sql<{ id: string }>` - SELECT DISTINCT json_extract(attachment.value, '$.id') AS id - FROM orchestration_v2_projection_turn_items AS item, - json_each(item.payload_json, '$.questionAnswer.attachmentsByQuestionId') AS question, - json_each(question.value) AS attachment - WHERE item.thread_id = ${threadId} - AND item.type = 'user_input_request' `, // A guest's waiting files are pending uploads that no message row holds yet. sql<{ id: string }>` @@ -5695,11 +5819,10 @@ export const layer: Layer.Layer = AND json_extract(message.value, '$.status') = 'pending' `, ]).pipe( - Effect.map(([messages, renders, answerFiles, guestFiles]) => [ + Effect.map(([files, renders, guestFiles]) => [ ...new Set([ - ...messages.map((row) => row.id), + ...files, ...guestFiles.map((row) => row.id), - ...answerFiles.map((row) => row.id), ...threadHtmlRenderAttachmentIds( threadId, renders.map((row) => { diff --git a/apps/server/src/orchestration-v2/ThreadCommandExecutor.ts b/apps/server/src/orchestration-v2/ThreadCommandExecutor.ts index 8bd1cc38d2de2..3d3c6c9ff999c 100644 --- a/apps/server/src/orchestration-v2/ThreadCommandExecutor.ts +++ b/apps/server/src/orchestration-v2/ThreadCommandExecutor.ts @@ -1,11 +1,26 @@ import type { ThreadId } from "@t3tools/contracts"; import * as KeyedLock from "@t3tools/shared/KeyedLock"; import * as Context from "effect/Context"; +import * as Effect from "effect/Effect"; import * as Layer from "effect/Layer"; +import * as Semaphore from "effect/Semaphore"; export class ThreadCommandExecutor extends Context.Service< ThreadCommandExecutor, - KeyedLock.KeyedLock + KeyedLock.KeyedLock & { + /** + * Serializes deletion planning through commit because run forks show files their sources own. + * Take it inside the thread lock; never take a thread lock while holding it. + */ + readonly withDeletionLock: (effect: Effect.Effect) => Effect.Effect; + } >()("t3/orchestration-v2/ThreadCommandExecutor") {} -export const layer = Layer.effect(ThreadCommandExecutor, KeyedLock.make()); +export const layer = Layer.effect( + ThreadCommandExecutor, + Effect.gen(function* () { + const threads = yield* KeyedLock.make(); + const deletion = yield* Semaphore.make(1); + return { ...threads, withDeletionLock: deletion.withPermit }; + }), +); diff --git a/apps/server/src/orchestration-v2/runtimeLayer.test.ts b/apps/server/src/orchestration-v2/runtimeLayer.test.ts index a49e323770cc1..ccafe793918af 100644 --- a/apps/server/src/orchestration-v2/runtimeLayer.test.ts +++ b/apps/server/src/orchestration-v2/runtimeLayer.test.ts @@ -48,6 +48,7 @@ import * as Stream from "effect/Stream"; import * as TestClock from "effect/testing/TestClock"; import * as SqlClient from "effect/sql/SqlClient"; +import { createAttachmentId } from "../attachmentStore.ts"; import * as CheckpointStore from "../checkpointing/CheckpointStore.ts"; import * as GitWorkflow from "../git/GitWorkflowService.ts"; import * as ServerConfig from "../config.ts"; @@ -2497,6 +2498,130 @@ it.layer(layerLegacyImportTest)("OrchestrationV2 legacy import", (it) => { ); }); +const imageAttachment = (threadId: ThreadId, name: string) => ({ + type: "image" as const, + id: createAttachmentId(threadId, "png")!, + name, + mimeType: "image/png", + sizeBytes: 8, +}); + +const writeCompletedUserRun = Effect.fn("test.writeCompletedUserRun")(function* (input: { + readonly threadId: ThreadId; + readonly key: string; + readonly ordinal: number; + readonly attachments: ReadonlyArray>; +}) { + const eventSink = yield* EventSink.EventSinkV2; + const now = yield* DateTime.now; + const runId = RunId.make(`image-fork-run:${input.key}`); + const messageId = MessageId.make(`image-fork-message:${input.key}`); + yield* eventSink.write({ + events: [ + { + id: EventId.make(`image-fork-run:${input.key}`), + type: "run.created", + threadId: input.threadId, + runId, + occurredAt: now, + payload: { + id: runId, + threadId: input.threadId, + ordinal: input.ordinal, + providerInstanceId: modelSelection.instanceId, + modelSelection, + providerThreadId: null, + userMessageId: messageId, + rootNodeId: null, + activeAttemptId: null, + status: "completed", + requestedAt: now, + startedAt: now, + completedAt: now, + checkpointId: null, + contextHandoffId: null, + }, + }, + { + id: EventId.make(`image-fork-message:${input.key}`), + type: "message.updated", + threadId: input.threadId, + runId, + occurredAt: now, + payload: { + id: messageId, + threadId: input.threadId, + runId, + nodeId: null, + role: "user", + text: "What is in this screenshot?", + attachments: [...input.attachments], + streaming: false, + createdBy: "user", + creationSource: "web", + createdAt: now, + updatedAt: now, + }, + }, + ], + }); + return runId; +}); + +// A run fork stores no message rows; it shows its source's prompt, image included, by reading through. +const seedImagePromptRunFork = Effect.fn("test.seedImagePromptRunFork")(function* (key: string) { + const orchestrator = yield* Orchestrator.OrchestratorV2; + const sourceThreadId = ThreadId.make(`image-fork-source:${key}`); + const forkThreadId = ThreadId.make(`image-fork-target:${key}`); + const attachment = imageAttachment(sourceThreadId, "screenshot.png"); + yield* orchestrator.dispatch({ + type: "thread.create", + commandId: CommandId.make(`image-fork-create:${key}`), + threadId: sourceThreadId, + projectId: ProjectId.make(`image-fork-project:${key}`), + title: "Image source", + modelSelection, + runtimeMode: "full-access", + interactionMode: "default", + branch: null, + worktreePath: null, + createdBy: "user", + creationSource: "web", + }); + const runId = yield* writeCompletedUserRun({ + threadId: sourceThreadId, + key, + ordinal: 1, + attachments: [attachment], + }); + yield* orchestrator.dispatch({ + type: "thread.fork", + commandId: CommandId.make(`image-fork-fork:${key}`), + sourceThreadId, + targetThreadId: forkThreadId, + sourcePoint: { type: "run", runId }, + createdBy: "user", + creationSource: "web", + }); + return { sourceThreadId, forkThreadId, attachmentId: attachment.id }; +}); + +const deleteThreadCleanup = Effect.fn("test.deleteThreadCleanup")(function* (threadId: ThreadId) { + const orchestrator = yield* Orchestrator.OrchestratorV2; + const commandId = CommandId.make(`image-fork-delete:${threadId}`); + yield* orchestrator.dispatch({ type: "thread.delete", commandId, threadId }); + return yield* queuedAttachmentCleanup(commandId); +}); + +const queuedAttachmentCleanup = Effect.fn("test.queuedAttachmentCleanup")(function* ( + commandId: CommandId, +) { + const outbox = yield* EffectOutbox.EffectOutboxV2; + return (yield* outbox.listByCommandId(commandId)).flatMap((effect) => + effect.request.type === "attachment.cleanup" ? effect.request.attachmentIds : [], + ); +}); + it.layer(layerTest)("OrchestrationV2LayerLive lifecycle", (it) => { it.effect("applies lifecycle commands idempotently and emits archive/removal shell deltas", () => Effect.gen(function* () { @@ -2828,6 +2953,122 @@ it.layer(layerTest)("OrchestrationV2LayerLive lifecycle", (it) => { }), ); + it.effect("deleting a source keeps what its live fork shows and releases later files", () => + Effect.gen(function* () { + const { sourceThreadId, attachmentId } = yield* seedImagePromptRunFork("after-fork-run"); + const later = imageAttachment(sourceThreadId, "later.png"); + yield* writeCompletedUserRun({ + threadId: sourceThreadId, + key: "after-fork-run-later", + ordinal: 2, + attachments: [later], + }); + const cleaned = yield* deleteThreadCleanup(sourceThreadId); + assert.include(cleaned, later.id); + assert.notInclude(cleaned, attachmentId, "deleted an image its live fork still shows"); + }), + ); + + it.effect("a fork of a fork keeps the file its grandparent shows until it is deleted", () => + Effect.gen(function* () { + const orchestrator = yield* Orchestrator.OrchestratorV2; + const { sourceThreadId, forkThreadId, attachmentId } = + yield* seedImagePromptRunFork("fork-chain"); + const grandchildThreadId = ThreadId.make("image-fork-chain-grandchild"); + const forkRunId = yield* writeCompletedUserRun({ + threadId: forkThreadId, + key: "fork-chain-fork", + ordinal: 2, + attachments: [], + }); + yield* orchestrator.dispatch({ + type: "thread.fork", + commandId: CommandId.make("image-fork-chain-fork-of-fork"), + sourceThreadId: forkThreadId, + targetThreadId: grandchildThreadId, + sourcePoint: { type: "run", runId: forkRunId }, + createdBy: "user", + creationSource: "web", + }); + assert.notInclude(yield* deleteThreadCleanup(sourceThreadId), attachmentId); + assert.notInclude(yield* deleteThreadCleanup(forkThreadId), attachmentId); + assert.include(yield* deleteThreadCleanup(grandchildThreadId), attachmentId); + }), + ); + + it.effect("concurrently deleting a source and its last fork still cleans the shared file", () => + Effect.gen(function* () { + const orchestrator = yield* Orchestrator.OrchestratorV2; + const projectionStore = yield* ProjectionStore.ProjectionStoreV2; + const executor = yield* ThreadCommandExecutor.ThreadCommandExecutor; + const { sourceThreadId, forkThreadId, attachmentId } = + yield* seedImagePromptRunFork("concurrent-delete"); + const sourceRead = yield* Deferred.make(); + const releaseSource = yield* Deferred.make(); + const getThreadAttachmentIds = projectionStore.getThreadAttachmentIds; + const withDeletionLock = executor.withDeletionLock; + let deletionLockRequests = 0; + // The source's delete holds its read until the fork's delete has read too or is queued behind it. + const readSpy = vi + .spyOn(projectionStore, "getThreadAttachmentIds") + .mockImplementation((threadId) => + getThreadAttachmentIds(threadId).pipe( + Effect.tap(() => + threadId === sourceThreadId + ? Deferred.succeed(sourceRead, undefined).pipe( + Effect.andThen(Deferred.await(releaseSource)), + ) + : Deferred.succeed(releaseSource, undefined), + ), + ), + ); + const observeDeletionLock: ThreadCommandExecutor.ThreadCommandExecutor["Service"]["withDeletionLock"] = + (effect) => + ++deletionLockRequests === 2 + ? Deferred.succeed(releaseSource, undefined).pipe( + Effect.andThen(withDeletionLock(effect)), + ) + : withDeletionLock(effect); + const lockSpy = vi + .spyOn(executor, "withDeletionLock") + .mockImplementation(observeDeletionLock); + const deleteSourceCommandId = CommandId.make("image-fork-concurrent-delete-source"); + const deleteForkCommandId = CommandId.make("image-fork-concurrent-delete-fork"); + yield* Effect.gen(function* () { + const deleteSource = yield* orchestrator + .dispatch({ + type: "thread.delete", + commandId: deleteSourceCommandId, + threadId: sourceThreadId, + }) + .pipe(Effect.forkChild({ startImmediately: true })); + yield* Deferred.await(sourceRead); + const deleteFork = yield* orchestrator + .dispatch({ + type: "thread.delete", + commandId: deleteForkCommandId, + threadId: forkThreadId, + }) + .pipe(Effect.forkChild({ startImmediately: true })); + yield* Fiber.join(deleteSource); + yield* Fiber.join(deleteFork); + }).pipe( + Effect.ensuring( + Effect.sync(() => { + readSpy.mockRestore(); + lockSpy.mockRestore(); + }), + ), + ); + assert.notInclude(yield* queuedAttachmentCleanup(deleteSourceCommandId), attachmentId); + assert.include( + yield* queuedAttachmentCleanup(deleteForkCommandId), + attachmentId, + "concurrent deletes of a source and its last fork leaked the shared image", + ); + }), + ); + it.effect("keeps the branch pull request when linking another pull request", () => Effect.gen(function* () { const orchestrator = yield* Orchestrator.OrchestratorV2; diff --git a/apps/server/src/project/ProjectService.ts b/apps/server/src/project/ProjectService.ts index 70f12adf05f1f..c9c2a9c787dd5 100644 --- a/apps/server/src/project/ProjectService.ts +++ b/apps/server/src/project/ProjectService.ts @@ -465,22 +465,26 @@ export const make = Effect.gen(function* () { commandId: CommandId.make(`${input.commandId}:delete-thread:${threadId}`), threadId, }; - const now = yield* DateTime.now; - const plan = yield* planThreadDeletion({ - command, - projection, - attachmentIds: yield* threadProjections.getThreadAttachmentIds(threadId), - now, - idAllocator, - }); - const committed = yield* eventSink.commitCommand({ - commandId: command.commandId, - commandType: command.type, - threadId, - acceptedAt: now, - events: plan.events, - effects: plan.effects, - }); + const committed = yield* threadCommands.withDeletionLock( + Effect.gen(function* () { + const now = yield* DateTime.now; + const plan = yield* planThreadDeletion({ + command, + projection, + attachmentIds: yield* threadProjections.getThreadAttachmentIds(threadId), + now, + idAllocator, + }); + return yield* eventSink.commitCommand({ + commandId: command.commandId, + commandType: command.type, + threadId, + acceptedAt: now, + events: plan.events, + effects: plan.effects, + }); + }), + ); if ( committed.receipt.threadId !== command.threadId || committed.receipt.commandType !== command.type