From 505b4d0bc3581b2bf1eb82db1d3768e15c0fe526 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Sat, 28 Feb 2026 22:58:59 -0800 Subject: [PATCH] Resolve persisted attachment paths across server pipelines - add shared attachment path normalization and resolution helpers - reuse persisted image files for turn start and branch name generation - cover persisted attachment flow in text generation and reactor tests --- apps/server/src/attachmentPaths.ts | 54 +++++++++ .../git/Layers/CodexTextGeneration.test.ts | 63 ++++++++++ .../src/git/Layers/CodexTextGeneration.ts | 49 +++++--- .../Layers/ProjectionPipeline.ts | 19 +-- .../Layers/ProviderCommandReactor.test.ts | 111 +++++++++++++++++- .../Layers/ProviderCommandReactor.ts | 84 ++++++++++++- apps/server/src/wsServer.ts | 18 ++- 7 files changed, 354 insertions(+), 44 deletions(-) create mode 100644 apps/server/src/attachmentPaths.ts diff --git a/apps/server/src/attachmentPaths.ts b/apps/server/src/attachmentPaths.ts new file mode 100644 index 0000000000000000000000000000000000000000..cb0e8e43a862a797434bb8c51f633d991957aae3 --- /dev/null +++ b/apps/server/src/attachmentPaths.ts @@ -0,0 +1,54 @@ +import path from "node:path"; + +import { ATTACHMENTS_ROUTE_PREFIX } from "./projectFaviconRoute.ts"; + +export function normalizeAttachmentRelativePath(rawRelativePath: string): string | null { + const normalized = path.normalize(rawRelativePath).replace(/^[/\\]+/, ""); + if ( + normalized.length === 0 || + normalized.startsWith("..") || + normalized.includes("\0") + ) { + return null; + } + return normalized.replace(/\\/g, "/"); +} + +export function attachmentRouteToRelativePath(dataUrl: string): string | null { + const prefix = `${ATTACHMENTS_ROUTE_PREFIX}/`; + if (!dataUrl.startsWith(prefix)) { + return null; + } + return normalizeAttachmentRelativePath(dataUrl.slice(prefix.length)); +} + +export function resolveAttachmentRelativePath(input: { + readonly stateDir: string; + readonly relativePath: string; +}): string | null { + const normalizedRelativePath = normalizeAttachmentRelativePath(input.relativePath); + if (!normalizedRelativePath) { + return null; + } + + const attachmentsRoot = path.resolve(path.join(input.stateDir, "attachments")); + const filePath = path.resolve(path.join(attachmentsRoot, normalizedRelativePath)); + if (!filePath.startsWith(`${attachmentsRoot}${path.sep}`)) { + return null; + } + return filePath; +} + +export function resolveAttachmentRoutePath(input: { + readonly stateDir: string; + readonly dataUrl: string; +}): string | null { + const relativePath = attachmentRouteToRelativePath(input.dataUrl); + if (!relativePath) { + return null; + } + return resolveAttachmentRelativePath({ + stateDir: input.stateDir, + relativePath, + }); +} diff --git a/apps/server/src/git/Layers/CodexTextGeneration.test.ts b/apps/server/src/git/Layers/CodexTextGeneration.test.ts index 7055ded51966ac79a1bd77fa14e3ea90ea3fbf5e..32a37b1a7c8c724d3f91fa2be9d53f16124b781d 100644 --- a/apps/server/src/git/Layers/CodexTextGeneration.test.ts +++ b/apps/server/src/git/Layers/CodexTextGeneration.test.ts @@ -3,10 +3,28 @@ import { it } from "@effect/vitest"; import { Effect, FileSystem, Layer, Path } from "effect"; import { expect } from "vitest"; +import { type ServerConfigShape, ServerConfig } from "../../config.ts"; import { CodexTextGenerationLive } from "./CodexTextGeneration.ts"; import { TextGenerationError } from "../Errors.ts"; import { TextGeneration } from "../Services/TextGeneration.ts"; +function makeTestServerConfig(stateDir: string): ServerConfigShape { + return { + mode: "web", + port: 0, + host: undefined, + cwd: process.cwd(), + keybindingsConfigPath: "", + stateDir, + staticDir: undefined, + devUrl: undefined, + noBrowser: true, + authToken: undefined, + autoBootstrapProjectFromCwd: false, + logWebSocketEvents: false, + }; +} + function makeFakeCodexBinary(dir: string) { return Effect.gen(function* () { const fs = yield* FileSystem.FileSystem; @@ -311,6 +329,51 @@ it.layer(CodexTextGenerationTestLayer)("CodexTextGenerationLive", (it) => { ), ); + it.effect("resolves persisted attachment URLs to files for codex image inputs", () => + withFakeCodexEnv( + { + output: JSON.stringify({ + branch: "fix/ui-regression", + }), + requireImage: true, + }, + Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const stateDir = yield* fs.makeTempDirectoryScoped({ prefix: "t3code-state-textgen-" }); + const imagePath = path.join( + stateDir, + "attachments", + "thread-1", + "message-1-0.png", + ); + yield* fs.makeDirectory(path.join(stateDir, "attachments", "thread-1"), { + recursive: true, + }); + yield* fs.writeFile(imagePath, Buffer.from("hello")); + + const textGeneration = yield* TextGeneration; + const generated = yield* textGeneration + .generateBranchName({ + cwd: process.cwd(), + message: "Fix layout bug from screenshot.", + attachments: [ + { + type: "image", + name: "bug.png", + mimeType: "image/png", + sizeBytes: 5, + dataUrl: "/attachments/thread-1/message-1-0.png", + }, + ], + }) + .pipe(Effect.provideService(ServerConfig, makeTestServerConfig(stateDir))); + + expect(generated.branch).toBe("fix/ui-regression"); + }), + ), + ); + it.effect("returns null branch when codex returns the wrong object shape", () => withFakeCodexEnv( { diff --git a/apps/server/src/git/Layers/CodexTextGeneration.ts b/apps/server/src/git/Layers/CodexTextGeneration.ts index d19bb81bafe28b995aeb7f88174228622cabfa04..df2afc1aa0af691e056aa37e7fb4122f15a52e5b 100644 --- a/apps/server/src/git/Layers/CodexTextGeneration.ts +++ b/apps/server/src/git/Layers/CodexTextGeneration.ts @@ -3,6 +3,8 @@ import { randomUUID } from "node:crypto"; import { Effect, FileSystem, Layer, Option, Path, Schema, Stream } from "effect"; import { ChildProcess, ChildProcessSpawner } from "effect/unstable/process"; +import { resolveAttachmentRoutePath } from "../../attachmentPaths.ts"; +import { ServerConfig } from "../../config.ts"; import { TextGenerationError } from "../Errors.ts"; import { inferImageExtension, parseBase64DataUrl } from "../../imageMime.ts"; import { @@ -245,6 +247,7 @@ const makeCodexTextGeneration = Effect.gen(function* () { if (!attachments || attachments.length === 0) { return []; } + const serverConfig = yield* Effect.serviceOption(ServerConfig); const imagePaths: string[] = []; for (const [index, attachment] of attachments.entries()) { @@ -253,26 +256,44 @@ const makeCodexTextGeneration = Effect.gen(function* () { } const parsed = parseBase64DataUrl(attachment.dataUrl); - if (!parsed || !parsed.mimeType.startsWith("image/")) { + if (parsed && parsed.mimeType.startsWith("image/")) { + const bytes = Buffer.from(parsed.base64, "base64"); + if (bytes.byteLength === 0) { + continue; + } + + const extension = inferImageExtension({ + mimeType: parsed.mimeType, + fileName: attachment.name, + }); + const imagePath = yield* writeTempBinaryFile( + operation, + `codex-image-${index}`, + bytes, + extension, + ); + imagePaths.push(imagePath); continue; } - const bytes = Buffer.from(parsed.base64, "base64"); - if (bytes.byteLength === 0) { + const persistedPath = + Option.isSome(serverConfig) + ? resolveAttachmentRoutePath({ + stateDir: serverConfig.value.stateDir, + dataUrl: attachment.dataUrl, + }) + : null; + const localPath = persistedPath ?? attachment.dataUrl; + if (!path.isAbsolute(localPath)) { continue; } - - const extension = inferImageExtension({ - mimeType: parsed.mimeType, - fileName: attachment.name, - }); - const imagePath = yield* writeTempBinaryFile( - operation, - `codex-image-${index}`, - bytes, - extension, + const fileInfo = yield* fileSystem.stat(localPath).pipe( + Effect.catch(() => Effect.succeed(null)), ); - imagePaths.push(imagePath); + if (!fileInfo || fileInfo.type !== "File") { + continue; + } + imagePaths.push(localPath); } return imagePaths; }); diff --git a/apps/server/src/orchestration/Layers/ProjectionPipeline.ts b/apps/server/src/orchestration/Layers/ProjectionPipeline.ts index 49ba13d824650f4de4370bddad4e77db432bd59e..15093a62d6f6a5713953d096b84a61b00436f4c0 100644 --- a/apps/server/src/orchestration/Layers/ProjectionPipeline.ts +++ b/apps/server/src/orchestration/Layers/ProjectionPipeline.ts @@ -37,9 +37,8 @@ import { OrchestrationProjectionPipeline, type OrchestrationProjectionPipelineShape, } from "../Services/ProjectionPipeline.ts"; -import { - ATTACHMENTS_ROUTE_PREFIX, -} from "../../projectFaviconRoute.ts"; +import { attachmentRouteToRelativePath } from "../../attachmentPaths.ts"; +import { ATTACHMENTS_ROUTE_PREFIX } from "../../projectFaviconRoute.ts"; import { inferImageExtension, parseBase64DataUrl } from "../../imageMime.ts"; export const ORCHESTRATION_PROJECTOR_NAMES = { @@ -262,18 +261,6 @@ function retainProjectionActivitiesAfterRevert( ); } -function toAttachmentRelativePath(dataUrl: string): string | null { - const prefix = `${ATTACHMENTS_ROUTE_PREFIX}/`; - if (!dataUrl.startsWith(prefix)) { - return null; - } - const relativePath = dataUrl.slice(prefix.length).replace(/^[/\\]+/, ""); - if (relativePath.length === 0 || relativePath.startsWith("..")) { - return null; - } - return relativePath.replace(/\\/g, "/"); -} - function collectThreadAttachmentRelativePaths( threadId: string, messages: ReadonlyArray, @@ -289,7 +276,7 @@ function collectThreadAttachmentRelativePaths( if (attachment.type !== "image") { continue; } - const relativePath = toAttachmentRelativePath(attachment.dataUrl); + const relativePath = attachmentRouteToRelativePath(attachment.dataUrl); if (!relativePath || !relativePath.startsWith(threadPrefix)) { continue; } diff --git a/apps/server/src/orchestration/Layers/ProviderCommandReactor.test.ts b/apps/server/src/orchestration/Layers/ProviderCommandReactor.test.ts index c4644eeb3fc7e1998a688a34456945ed0b43315c..7d4f2b44f38b7ccda21986509bbc92ee24e813cc 100644 --- a/apps/server/src/orchestration/Layers/ProviderCommandReactor.test.ts +++ b/apps/server/src/orchestration/Layers/ProviderCommandReactor.test.ts @@ -1,3 +1,7 @@ +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + import type { ProviderRuntimeEvent } from "@t3tools/contracts"; import { ApprovalRequestId, @@ -13,6 +17,7 @@ import { import { Effect, Exit, Layer, ManagedRuntime, PubSub, Scope, Stream } from "effect"; import { afterEach, describe, expect, it, vi } from "vitest"; +import { type ServerConfigShape, ServerConfig } from "../../config.ts"; import { OrchestrationEventStoreLive } from "../../persistence/Layers/OrchestrationEventStore.ts"; import { OrchestrationCommandReceiptRepositoryLive } from "../../persistence/Layers/OrchestrationCommandReceipts.ts"; import { SqlitePersistenceMemory } from "../../persistence/Layers/Sqlite.ts"; @@ -36,6 +41,23 @@ const asApprovalRequestId = (value: string): ApprovalRequestId => const asMessageId = (value: string): MessageId => MessageId.makeUnsafe(value); const asTurnId = (value: string): TurnId => TurnId.makeUnsafe(value); +function makeTestServerConfig(stateDir: string): ServerConfigShape { + return { + mode: "web", + port: 0, + host: undefined, + cwd: process.cwd(), + keybindingsConfigPath: "", + stateDir, + staticDir: undefined, + devUrl: undefined, + noBrowser: true, + authToken: undefined, + autoBootstrapProjectFromCwd: false, + logWebSocketEvents: false, + }; +} + async function waitFor(predicate: () => boolean, timeoutMs = 2000): Promise { const deadline = Date.now() + timeoutMs; const poll = async (): Promise => { @@ -70,7 +92,7 @@ describe("ProviderCommandReactor", () => { runtime = null; }); - async function createHarness() { + async function createHarness(input?: { readonly stateDir?: string }) { const now = new Date().toISOString(); const runtimeEventPubSub = Effect.runSync(PubSub.unbounded()); let nextSessionIndex = 1; @@ -124,7 +146,7 @@ describe("ProviderCommandReactor", () => { Layer.provide(OrchestrationCommandReceiptRepositoryLive), Layer.provide(SqlitePersistenceMemory), ); - const layer = ProviderCommandReactorLive.pipe( + const liveLayer = ProviderCommandReactorLive.pipe( Layer.provideMerge(orchestrationLayer), Layer.provideMerge(Layer.succeed(ProviderService, service)), Layer.provideMerge(Layer.succeed(GitCore, { renameBranch } as unknown as GitCoreShape)), @@ -135,6 +157,14 @@ describe("ProviderCommandReactor", () => { ), ), ); + const layer = + input?.stateDir !== undefined + ? liveLayer.pipe( + Layer.provideMerge( + Layer.succeed(ServerConfig, makeTestServerConfig(input.stateDir)), + ), + ) + : liveLayer; runtime = ManagedRuntime.make(layer); const engine = await runtime.runPromise(Effect.service(OrchestrationEngineService)); const reactor = await runtime.runPromise(Effect.service(ProviderCommandReactor)); @@ -285,6 +315,83 @@ describe("ProviderCommandReactor", () => { }); }); + it("reuses persisted attachment files for branch generation and turn start", async () => { + const stateDir = fs.mkdtempSync(path.join(os.tmpdir(), "t3code-reactor-attachments-")); + const harness = await createHarness({ stateDir }); + const now = new Date().toISOString(); + + await Effect.runPromise( + harness.engine.dispatch({ + type: "thread.meta.update", + commandId: CommandId.makeUnsafe("cmd-thread-meta-set-temp-branch-persisted"), + threadId: ThreadId.makeUnsafe("thread-1"), + branch: "t3code/abcdef12", + worktreePath: "/tmp/provider-project/.t3/worktrees/t3code-abcdef12", + }), + ); + + await Effect.runPromise( + harness.engine.dispatch({ + type: "thread.turn.start", + commandId: CommandId.makeUnsafe("cmd-turn-start-persisted-attachments"), + threadId: ThreadId.makeUnsafe("thread-1"), + message: { + messageId: asMessageId("user-message-persisted-attachments"), + role: "user", + text: "Fix visual bug from screenshot", + attachments: [ + { + type: "image", + name: "bug.png", + mimeType: "image/png", + sizeBytes: 5, + dataUrl: "data:image/png;base64,SGVsbG8=", + }, + ], + }, + approvalPolicy: "on-request", + sandboxMode: "workspace-write", + createdAt: now, + }), + ); + + await waitFor(() => harness.generateBranchName.mock.calls.length === 1); + await waitFor(() => harness.sendTurn.mock.calls.length === 1); + + const expectedAttachmentPath = path.join( + stateDir, + "attachments", + "thread-1", + "user-message-persisted-attachments-0.png", + ); + + expect(harness.generateBranchName.mock.calls[0]?.[0]).toEqual({ + cwd: "/tmp/provider-project/.t3/worktrees/t3code-abcdef12", + message: "Fix visual bug from screenshot", + attachments: [ + { + type: "image", + name: "bug.png", + mimeType: "image/png", + sizeBytes: 5, + dataUrl: expectedAttachmentPath, + }, + ], + }); + expect(harness.sendTurn.mock.calls[0]?.[0]).toMatchObject({ + attachments: [ + { + type: "image", + name: "bug.png", + mimeType: "image/png", + sizeBytes: 5, + dataUrl: expectedAttachmentPath, + }, + ], + }); + expect(fs.existsSync(expectedAttachmentPath)).toBe(true); + }); + it("skips worktree branch generation after the first user turn", async () => { const harness = await createHarness(); const now = new Date().toISOString(); diff --git a/apps/server/src/orchestration/Layers/ProviderCommandReactor.ts b/apps/server/src/orchestration/Layers/ProviderCommandReactor.ts index 072f76134c52a72c93c1194fc47bac27a3eb91a9..96e12e15c301f42733120fb1859b11894e04ef6c 100644 --- a/apps/server/src/orchestration/Layers/ProviderCommandReactor.ts +++ b/apps/server/src/orchestration/Layers/ProviderCommandReactor.ts @@ -1,3 +1,5 @@ +import fs from "node:fs"; + import { type ChatAttachment, CommandId, @@ -15,8 +17,11 @@ import { import { Cache, Cause, Duration, Effect, Layer, Option, Queue, Stream } from "effect"; import { resolveThreadWorkspaceCwd } from "../../checkpointing/Utils.ts"; +import { resolveAttachmentRelativePath, resolveAttachmentRoutePath } from "../../attachmentPaths.ts"; +import { ServerConfig } from "../../config.ts"; import { GitCore } from "../../git/Services/GitCore.ts"; import { TextGeneration } from "../../git/Services/TextGeneration.ts"; +import { inferImageExtension, parseBase64DataUrl } from "../../imageMime.ts"; import { ProviderService } from "../../provider/Services/ProviderService.ts"; import { OrchestrationEngineService } from "../Services/OrchestrationEngine.ts"; import { @@ -103,6 +108,7 @@ const make = Effect.gen(function* () { const providerService = yield* ProviderService; const git = yield* GitCore; const textGeneration = yield* TextGeneration; + const serverConfig = yield* Effect.serviceOption(ServerConfig); const handledTurnStartKeys = yield* Cache.make({ capacity: HANDLED_TURN_START_KEY_MAX, timeToLive: HANDLED_TURN_START_KEY_TTL, @@ -280,6 +286,74 @@ const make = Effect.gen(function* () { }); }); + const resolvePersistedAttachments = Effect.fnUntraced(function* ( + input: { + readonly threadId: ThreadId; + readonly messageId: string; + readonly attachments: ReadonlyArray | undefined; + }, + ) { + if (!input.attachments || input.attachments.length === 0) { + return input.attachments ?? []; + } + + if (Option.isNone(serverConfig)) { + return input.attachments; + } + const threadSegment = encodeURIComponent(input.threadId); + const messageSegment = encodeURIComponent(input.messageId); + + return yield* Effect.forEach( + Array.from(input.attachments.entries()), + ([index, attachment]) => + Effect.gen(function* () { + if (attachment.type !== "image") { + return attachment; + } + + const resolvedRoutePath = resolveAttachmentRoutePath({ + stateDir: serverConfig.value.stateDir, + dataUrl: attachment.dataUrl, + }); + const resolvedMaterializedPath = + resolvedRoutePath ?? + (() => { + const parsed = parseBase64DataUrl(attachment.dataUrl); + if (!parsed || !parsed.mimeType.startsWith("image/")) { + return null; + } + const extension = inferImageExtension({ + mimeType: parsed.mimeType, + fileName: attachment.name, + }); + return resolveAttachmentRelativePath({ + stateDir: serverConfig.value.stateDir, + relativePath: `${threadSegment}/${messageSegment}-${index}${extension}`, + }); + })(); + + if (!resolvedMaterializedPath) { + return attachment; + } + + const isFile = yield* Effect.sync(() => { + try { + return fs.statSync(resolvedMaterializedPath).isFile(); + } catch { + return false; + } + }); + if (!isFile) { + return attachment; + } + return { + ...attachment, + dataUrl: resolvedMaterializedPath, + } satisfies ChatAttachment; + }), + ); + }); + const maybeGenerateAndRenameWorktreeBranchForFirstTurn = Effect.fnUntraced(function* (input: { readonly threadId: ThreadId; readonly branch: string | null; @@ -381,19 +455,25 @@ const make = Effect.gen(function* () { return; } + const resolvedAttachments = yield* resolvePersistedAttachments({ + threadId: event.payload.threadId, + messageId: message.id, + attachments: message.attachments, + }); + yield* maybeGenerateAndRenameWorktreeBranchForFirstTurn({ threadId: event.payload.threadId, branch: thread.branch, worktreePath: thread.worktreePath, messageId: message.id, messageText: message.text, - ...(message.attachments !== undefined ? { attachments: message.attachments } : {}), + ...(message.attachments !== undefined ? { attachments: resolvedAttachments } : {}), }); yield* sendTurnForThread({ threadId: event.payload.threadId, messageText: message.text, - ...(message.attachments !== undefined ? { attachments: message.attachments } : {}), + ...(message.attachments !== undefined ? { attachments: resolvedAttachments } : {}), ...(event.payload.model !== undefined ? { model: event.payload.model } : {}), ...(event.payload.effort !== undefined ? { effort: event.payload.effort } : {}), approvalPolicy: event.payload.approvalPolicy, diff --git a/apps/server/src/wsServer.ts b/apps/server/src/wsServer.ts index 5f4fb9d616a3d53c8ef6911c5a405167ec9a169b..ac978806d27722db4bb8298859381285859ecf29 100644 --- a/apps/server/src/wsServer.ts +++ b/apps/server/src/wsServer.ts @@ -55,6 +55,7 @@ import { Open } from "./open"; import { ServerConfig } from "./config"; import { GitCore } from "./git/Services/GitCore.ts"; import { ATTACHMENTS_ROUTE_PREFIX, tryHandleProjectFaviconRequest } from "./projectFaviconRoute"; +import { normalizeAttachmentRelativePath, resolveAttachmentRelativePath } from "./attachmentPaths"; /** * ServerShape - Service API for server lifecycle control. @@ -249,21 +250,18 @@ export const createServer = Effect.fn(function* (): Effect.fn.Return< } if (url.pathname.startsWith(ATTACHMENTS_ROUTE_PREFIX)) { - const attachmentsRoot = path.resolve(path.join(serverConfig.stateDir, "attachments")); const rawRelativePath = url.pathname.slice(ATTACHMENTS_ROUTE_PREFIX.length); - const normalizedRelativePath = path.normalize(rawRelativePath).replace(/^[/\\]+/, ""); - - if ( - normalizedRelativePath.length === 0 || - normalizedRelativePath.startsWith("..") || - normalizedRelativePath.includes("\0") - ) { + const normalizedRelativePath = normalizeAttachmentRelativePath(rawRelativePath); + if (!normalizedRelativePath) { respond(400, { "Content-Type": "text/plain" }, "Invalid attachment path"); return; } - const filePath = path.resolve(path.join(attachmentsRoot, normalizedRelativePath)); - if (!filePath.startsWith(`${attachmentsRoot}${path.sep}`)) { + const filePath = resolveAttachmentRelativePath({ + stateDir: serverConfig.stateDir, + relativePath: normalizedRelativePath, + }); + if (!filePath) { respond(400, { "Content-Type": "text/plain" }, "Invalid attachment path"); return; } -- 2.54.0