Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
2 changes: 2 additions & 0 deletions apps/server/src/orchestration-v2/EffectWorker.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ 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 RunFinalizationService from "./RunFinalizationService.ts";
import * as ProviderSessionManager from "./ProviderSessionManager.ts";
import * as ProviderTurnControlService from "./ProviderTurnControlService.ts";
Expand Down Expand Up @@ -168,6 +169,7 @@ function layerExecutorFor(input: {
Layer.mergeAll(
layerDependencies,
Layer.mock(ThreadManagementService.ThreadManagementService)(input.threads ?? {}),
Layer.mock(ProjectionStore.ProjectionStoreV2)({}),
ServerSettings.layerTest(
input.continueAfterRestart === true ? { continueThreadsAfterServerUpdate: true } : {},
),
Expand Down
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 @@ -22,6 +22,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 @@ -84,6 +85,7 @@ export const layerExecutor: Layer.Layer<
| RuntimeRequestService.RuntimeRequestServiceV2
| ThreadTitleRegenerationService.ThreadTitleRegenerationService
| ThreadManagementService.ThreadManagementService
| ProjectionStore.ProjectionStoreV2
| ServerSettings.ServerSettingsService
> = Layer.effect(
OrchestrationEffectExecutorV2,
Expand All @@ -98,6 +100,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 @@ -584,8 +587,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 @@ -595,6 +611,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 @@ -11529,6 +11529,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 @@ -11622,6 +11623,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 @@ -15113,7 +15136,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 @@ -501,8 +501,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 @@ -6252,14 +6254,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 @@ -6269,7 +6284,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 @@ -6278,7 +6293,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 @@ -159,6 +160,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