Skip to content
1 change: 1 addition & 0 deletions apps/server/src/orchestration-v2/EffectOutbox.ts
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ export const OrchestrationEffectRequestV2 = Schema.Union([
Schema.Struct({
type: Schema.Literal("attachment.cleanup"),
attachmentIds: Schema.Array(Schema.String),
skipReferenced: Schema.optional(Schema.Boolean),
}),
Schema.Struct({
type: Schema.Literal("thread-title.generate"),
Expand Down
60 changes: 60 additions & 0 deletions apps/server/src/orchestration-v2/EffectWorker.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ import * as CheckpointRollbackService from "./CheckpointRollbackService.ts";
import * as CheckpointService from "./CheckpointService.ts";
import * as EffectOutbox from "./EffectOutbox.ts";
import * as EffectWorker from "./EffectWorker.ts";
import * as ProjectionStore from "./ProjectionStore.ts";
import * as ResourceCleanupService from "./ResourceCleanupService.ts";
import * as RunFinalizationService from "./RunFinalizationService.ts";
import * as ProviderSessionManager from "./ProviderSessionManager.ts";
import * as ProviderTurnControlService from "./ProviderTurnControlService.ts";
Expand Down Expand Up @@ -81,6 +83,7 @@ function layerExecutorFor(input: {
readonly events: Ref.Ref<ReadonlyArray<string>>;
readonly failFirstStart?: Ref.Ref<boolean>;
readonly threads?: Partial<ThreadManagementService.ThreadManagementService["Service"]>;
readonly projectionStore?: Partial<ProjectionStore.ProjectionStoreV2["Service"]>;
readonly continueAfterRestart?: boolean;
readonly interrupt?: ProviderTurnControlService.ProviderTurnControlServiceV2Shape["interrupt"];
readonly rollback?: CheckpointRollbackService.CheckpointRollbackServiceV2Shape["execute"];
Expand Down Expand Up @@ -164,6 +167,7 @@ function layerExecutorFor(input: {
Layer.mergeAll(
layerDependencies,
Layer.mock(ThreadManagementService.ThreadManagementService)(input.threads ?? {}),
Layer.mock(ProjectionStore.ProjectionStoreV2)(input.projectionStore ?? {}),
ServerSettings.layerTest(
input.continueAfterRestart === true ? { continueThreadsAfterServerUpdate: true } : {},
),
Expand Down Expand Up @@ -638,3 +642,59 @@ it.effect.each(["confirmed", "unconfirmed", "failed"] as const)(
);
}),
);

it.effect.each([
{ name: "a queued edit's cleanup keeps files a later edit restored", skipReferenced: true },
{ name: "thread deletion's cleanup removes every listed file", skipReferenced: undefined },
])("$name", ({ skipReferenced }) =>
Effect.gen(function* () {
const timestamp = DateTime.formatIso(yield* DateTime.now);
const events = yield* Ref.make<ReadonlyArray<string>>([]);
const queuedAttachmentIds = yield* Ref.make<ReadonlyArray<string>>([]);
const removed = yield* Ref.make<ReadonlyArray<string>>([]);
const layer = layerExecutorFor({
events,
projectionStore: {
getThreadAttachmentIds: () =>
Ref.get(queuedAttachmentIds).pipe(Effect.map((ids) => [...ids, "answered"])),
},
});
// The edit that dropped these files committed; a stale edit restores one before cleanup runs,
// and a question answer still uses another.
yield* Ref.set(queuedAttachmentIds, ["restored"]);
yield* Effect.gen(function* () {
const executor = yield* EffectWorker.OrchestrationEffectExecutorV2;
yield* executor.execute({
id: "effect:queued-edit:attachment.cleanup",
commandId: CommandId.make("command:queued-edit"),
threadId,
request: {
type: "attachment.cleanup",
attachmentIds: ["restored", "answered", "dropped"],
...(skipReferenced === undefined ? {} : { skipReferenced }),
},
status: "running",
attemptCount: 1,
availableAt: timestamp,
leaseOwner: "test-worker",
createdAt: timestamp,
updatedAt: timestamp,
completedAt: null,
lastError: null,
});
}).pipe(
Effect.provide(layer),
Effect.provideService(ResourceCleanupService.ResourceCleanupService, {
cleanupScheduledTasks: () => Effect.void,
cleanupTerminals: () => Effect.void,
cleanupPreviews: () => Effect.void,
cleanupDevices: () => Effect.void,
cleanupAttachments: (ids) => Ref.update(removed, (current) => [...current, ...ids]),
}),
);
assert.deepEqual(
yield* Ref.get(removed),
skipReferenced ? ["dropped"] : ["restored", "answered", "dropped"],
);
}),
);
21 changes: 19 additions & 2 deletions apps/server/src/orchestration-v2/EffectWorker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {
import * as RunFinalizationService from "./RunFinalizationService.ts";
import * as ResourceCleanupService from "./ResourceCleanupService.ts";
import * as EffectOutbox from "./EffectOutbox.ts";
import * as ProjectionStore from "./ProjectionStore.ts";
import * as CheckpointRollbackService from "./CheckpointRollbackService.ts";
import * as ProviderSessionManager from "./ProviderSessionManager.ts";
import * as ProviderTurnControlService from "./ProviderTurnControlService.ts";
Expand Down Expand Up @@ -82,6 +83,7 @@ export const layerExecutor: Layer.Layer<
| RuntimeRequestService.RuntimeRequestServiceV2
| ThreadTitleRegenerationService.ThreadTitleRegenerationService
| ThreadManagementService.ThreadManagementService
| ProjectionStore.ProjectionStoreV2
| ServerSettings.ServerSettingsService
> = Layer.effect(
OrchestrationEffectExecutorV2,
Expand All @@ -96,6 +98,7 @@ export const layerExecutor: Layer.Layer<
const threadTitleRegeneration =
yield* ThreadTitleRegenerationService.ThreadTitleRegenerationService;
const threads = yield* ThreadManagementService.ThreadManagementService;
const projectionStore = yield* ProjectionStore.ProjectionStoreV2;
const settings = yield* ServerSettings.ServerSettingsService;
return OrchestrationEffectExecutorV2.of({
execute: Effect.fn("OrchestrationEffectExecutorV2.execute")(function (effect, options) {
Expand Down Expand Up @@ -582,8 +585,21 @@ export const layerExecutor: Layer.Layer<
}),
),
);
case "attachment.cleanup":
return resourceCleanup.cleanupAttachments(effect.request.attachmentIds).pipe(
case "attachment.cleanup": {
const request = effect.request;
return Effect.gen(function* () {
// Another message or answer may share the file, or a stale queued edit may restore it.
const referenced = new Set(
request.skipReferenced
? yield* projectionStore.getThreadAttachmentIds(effect.threadId, {
includeShownByLiveFork: true,
})
: [],
);
yield* resourceCleanup.cleanupAttachments(
request.attachmentIds.filter((id) => !referenced.has(id)),
);
Comment thread
saphid marked this conversation as resolved.
Resolved
}).pipe(
Effect.mapError(
(cause) =>
new OrchestrationEffectExecutionError({
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
Expand All @@ -593,6 +609,7 @@ export const layerExecutor: Layer.Layer<
}),
),
);
}
case "thread-title.generate":
return threadTitleRegeneration
.execute({
Expand Down
25 changes: 24 additions & 1 deletion apps/server/src/orchestration-v2/Orchestrator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11134,6 +11134,7 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio
const dispatchQueuedRunEdit = Effect.fn("Orchestrator.dispatchQueuedRunEdit")(function* (
command: Extract<OrchestrationV2Command, { readonly type: "queued-run.edit" }>,
events: Ref.Ref<Array<OrchestrationV2DomainEvent>>,
effects: Ref.Ref<Array<PendingOrchestrationEffectV2>>,
) {
if (command.text.trim().length === 0) {
return yield* new OrchestratorDispatchError({
Expand Down Expand Up @@ -11227,6 +11228,28 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio
}),
});
}
// Thread deletion only cleans files the thread still references.
const keptAttachmentIds = new Set(
(command.attachments ?? queuedMessage.attachments).map((attachment) => attachment.id),
);
const droppedAttachmentIds = queuedMessage.attachments
.map((attachment) => attachment.id)
.filter((id) => !keptAttachmentIds.has(id));
if (droppedAttachmentIds.length > 0) {
yield* Ref.update(effects, (existing) => [
...existing,
{
id: `effect:${command.commandId}:attachment.cleanup`,
commandId: command.commandId,
threadId: command.threadId,
request: {
type: "attachment.cleanup",
attachmentIds: droppedAttachmentIds,
skipReferenced: true,
},
} satisfies PendingOrchestrationEffectV2,
]);
}
});

const subagentCount = (threadId: ThreadId) =>
Expand Down Expand Up @@ -14662,7 +14685,7 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio
yield* dispatchQueuedRunCancel(command, events);
break;
case "queued-run.edit":
yield* dispatchQueuedRunEdit(command, events);
yield* dispatchQueuedRunEdit(command, events, effects);
break;
case "checkpoint.rollback":
yield* dispatchCheckpointRollback(command, events, effects);
Expand Down
23 changes: 19 additions & 4 deletions apps/server/src/orchestration-v2/ProjectionStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -481,8 +481,10 @@ export interface ProjectionStoreV2Shape {
readonly searchThread: (
input: OrchestrationV2SearchThreadInput,
) => Effect.Effect<OrchestrationV2SearchThreadResult, ProjectionStoreV2Error>;
/** Files thread deletion may remove; `includeShownByLiveFork` adds ones a live fork shows. */
readonly getThreadAttachmentIds: (
threadId: ThreadId,
options?: { readonly includeShownByLiveFork?: boolean },
) => Effect.Effect<ReadonlyArray<string>, ProjectionStoreV2Error>;
readonly getThreadHistoryPage: (
threadId: ThreadId,
Expand Down Expand Up @@ -6168,14 +6170,27 @@ export const layer: Layer.Layer<ProjectionStoreV2, never, SqlClient.SqlClient> =
)
`;

const getThreadAttachmentIds: ProjectionStoreV2Shape["getThreadAttachmentIds"] = (threadId) =>
// Thread deletion keeps what a live fork still shows; a reference check must count it.
const exceptShownByLiveFork = (
threadId: ThreadId,
runIdColumn: string,
options: { readonly includeShownByLiveFork?: boolean } | undefined,
) =>
options?.includeShownByLiveFork === true
? sql``
: sql`AND NOT ${shownByLiveFork(threadId, runIdColumn)}`;

const getThreadAttachmentIds: ProjectionStoreV2Shape["getThreadAttachmentIds"] = (
threadId,
options,
) =>
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}
AND NOT ${shownByLiveFork(threadId, "message.run_id")}
${exceptShownByLiveFork(threadId, "message.run_id", options)}
`,
sql<{ payload_json: string }>`
SELECT item.payload_json
Expand All @@ -6185,7 +6200,7 @@ export const layer: Layer.Layer<ProjectionStoreV2, never, SqlClient.SqlClient> =
AND (item.payload_json LIKE '%htmlRender%'
OR item.payload_json LIKE ${`%${MCP_APP_OUTPUT_KEY}%`}
OR item.payload_json LIKE '%preview_recording_stop%')
AND NOT ${shownByLiveFork(threadId, "item.run_id")}
${exceptShownByLiveFork(threadId, "item.run_id", options)}
`,
sql<{ id: string }>`
SELECT DISTINCT json_extract(attachment.value, '$.id') AS id
Expand All @@ -6194,7 +6209,7 @@ export const layer: Layer.Layer<ProjectionStoreV2, never, SqlClient.SqlClient> =
json_each(question.value) AS attachment
WHERE item.thread_id = ${threadId}
AND item.type = 'user_input_request'
AND NOT ${shownByLiveFork(threadId, "item.run_id")}
${exceptShownByLiveFork(threadId, "item.run_id", options)}
`,
// A guest's waiting files are pending uploads that no message row holds yet.
sql<{ id: string }>`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ it.effect.each([
Layer.mock(ThreadManagement.ThreadManagementService)({
recoverDelegatedTask: () => Effect.void,
}),
Layer.mock(ProjectionStore.ProjectionStoreV2)({}),
ServerSettings.layerTest(),
Layer.mock(ProviderTurnStart.ProviderTurnStartServiceV2)({
start: () => Effect.sync(() => calls.push("start")).pipe(Effect.asVoid),
Expand Down
49 changes: 49 additions & 0 deletions apps/server/src/orchestration-v2/ThreadMessageIntake.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
EventId,
GuestMessageId,
MessageId,
RunId,
RuntimeRequestId,
ThreadId,
TurnItemId,
Expand Down Expand Up @@ -426,3 +427,51 @@ it.effect("sends a guest message with the guest's files, never ones the client n
expect(command.attachments).toEqual([guestFile]);
}).pipe(Effect.provide(layerIntakeTest)),
);

it.effect("refuses a queued edit that keeps a file another edit already deleted", () =>
Effect.gen(function* () {
const config = yield* ServerConfig.ServerConfig;
const storedFile = (name: string) => ({
type: "image" as const,
id: ChatAttachmentId.make(createAttachmentId("thread-queued-edit")!),
name,
mimeType: "image/png",
sizeBytes: 3,
});
const kept = storedFile("kept.png");
const deleted = storedFile("deleted.png");
NodeFS.writeFileSync(
NodePath.join(config.attachmentsDir, `${kept.id}.png`),
new Uint8Array([1, 2, 3]),
);
const captured: OrchestrationV2ServerCommand[] = [];
const edit = (attachments: ReadonlyArray<typeof kept>) =>
dispatchCommand({
type: "queued-run.edit",
commandId: CommandId.make(`queued-edit-${attachments.length}`),
threadId: ThreadId.make("thread-queued-edit"),
runId: RunId.make("run-queued-edit"),
text: "Edited from a stale view.",
attachments,
}).pipe(
Effect.provide(
Layer.mock(ThreadManagementService.ThreadManagementService)({
dispatch: layerFailingDispatchHandler(captured),
hasReceipt: (commandId) =>
Effect.succeed(captured.some((command) => command.commandId === commandId)),
}),
),
Effect.result,
);

yield* edit([kept]);
expect(captured).toHaveLength(1);
const result = yield* edit([kept, deleted]);
expect(result._tag).toBe("Failure");
expect(captured).toHaveLength(1);
// A retry of the accepted edit replays its receipt even after a later cleanup removed its file.
NodeFS.rmSync(NodePath.join(config.attachmentsDir, `${kept.id}.png`));
yield* edit([kept]);
expect(captured).toHaveLength(2);
}).pipe(Effect.provide(layerIntakeTest)),
);
19 changes: 19 additions & 0 deletions apps/server/src/orchestration-v2/ThreadMessageIntake.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { remapComposerContextAttachments } from "@t3tools/shared/composerContext
import { appendUserInputAttachmentPaths } from "../provider/userInputAttachments.ts";
import type { ChatAttachment, OrchestrationV2Command } from "@t3tools/contracts";
import * as Effect from "effect/Effect";
import * as FileSystem from "effect/FileSystem";
import * as Schema from "effect/Schema";
import { resolveAttachmentPath } from "../attachmentStore.ts";
import * as ServerConfig from "../config.ts";
Expand Down Expand Up @@ -158,6 +159,24 @@ export const dispatchCommand = Effect.fn("ThreadMessageIntake.dispatchCommand")(
(command.type !== "queued-run.edit" || command.attachments === undefined)
)
return yield* threads.dispatch(command);
// An accepted edit's retry replays its receipt even if a later cleanup removed its files.
if (command.type === "queued-run.edit" && !(yield* threads.hasReceipt(command.commandId))) {
const config = yield* ServerConfig.ServerConfig;
const fileSystem = yield* FileSystem.FileSystem;
// A stale view can keep a file that another client's edit dropped and deleted.
for (const attachment of command.attachments ?? []) {
if (AttachmentClaims.attachmentIsPendingUpload(attachment)) continue;
const path = resolveAttachmentPath({ attachmentsDir: config.attachmentsDir, attachment });
if (
path !== null &&
!(yield* fileSystem.exists(path).pipe(Effect.orElseSucceed(() => false)))
Comment thread
macroscopeapp[bot] marked this conversation as resolved.
Resolved
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
) {
return yield* new AttachmentClaims.AttachmentClaimError({
message: `Attachment '${attachment.name}' cannot be sent: attachment not found.`,
});
}
}
}
const claimed = yield* AttachmentClaims.claimPendingAttachments({
threadId: command.threadId,
attachments: command.attachments ?? [],
Expand Down
Loading
Loading