diff --git a/.task-cache.json b/.task-cache.json new file mode 100644 index 000000000000..0e62827f9d64 --- /dev/null +++ b/.task-cache.json @@ -0,0 +1,3 @@ +{ + "endpoint": "https://t3-task-cache.pinglabs.workers.dev/projects/t3code" +} diff --git a/apps/coder-gateway/src/server.ts b/apps/coder-gateway/src/server.ts index 4acbff35af2c..bfce6727a222 100644 --- a/apps/coder-gateway/src/server.ts +++ b/apps/coder-gateway/src/server.ts @@ -106,7 +106,7 @@ function sendText( "Cache-Control": "no-store", "Content-Type": contentType, "Content-Security-Policy": - "default-src 'self'; base-uri 'none'; form-action 'none'; frame-ancestors 'none'; style-src 'self' 'unsafe-inline'; script-src 'self' 'wasm-unsafe-eval' 'sha256-66N88M2Gs4tvXNc8z6k+OKKok8OCKne3d83NpL7KJ9A='; connect-src 'self' blob: https: http: ws://127.0.0.1:*; img-src 'self' data: blob: https: http:; media-src 'self' blob: https: http:; worker-src 'self' blob:; object-src 'self' blob:; frame-src 'self' blob:", + "default-src 'self'; base-uri 'none'; form-action 'none'; frame-ancestors 'none'; style-src 'self' 'unsafe-inline'; script-src 'self' 'wasm-unsafe-eval' 'sha256-+xUlzIW26WOhLd/WV5CUp/KDzSwFacmCLHCiJxm72Yw='; connect-src 'self' blob: https: http: ws://127.0.0.1:*; img-src 'self' data: blob: https: http:; media-src 'self' blob: https: http:; worker-src 'self' blob:; object-src 'self' blob:; frame-src 'self' blob:", "X-Content-Type-Options": "nosniff", "X-Frame-Options": "DENY", "Cross-Origin-Resource-Policy": "same-origin", diff --git a/apps/coder-helper/src/bin.ts b/apps/coder-helper/src/bin.ts index f0d489a34a67..5e55f6478512 100644 --- a/apps/coder-helper/src/bin.ts +++ b/apps/coder-helper/src/bin.ts @@ -75,6 +75,19 @@ export const coderHelperStdioLayer = RpcServer.layer( Layer.provide(Layer.succeed(Logger.LogToStderr)(true)), ); -if (import.meta.main) { +// Coder: upstream's `bin.ts` dispatch for `DocumentText.ts`, which runs office document text +// extraction in a child process of the server's own entry point; here that is the helper bundle. +if (import.meta.main && process.argv[2] === "__document-text") { + const { extractDocumentText } = await import("t3/src/provider/documentTextWorker.ts"); + try { + process.stdout.write(await extractDocumentText(process.argv[3]!, process.argv[4]!)); + } catch (error) { + if (error instanceof Error && "code" in error && /^E[A-Z0-9]+$/.test(String(error.code))) { + throw error; + } + // Exit 2 identifies document rejections; OS and process failures remain retryable. + process.exitCode = 2; + } +} else if (import.meta.main) { Layer.launch(coderHelperStdioLayer).pipe(NodeRuntime.runMain); } diff --git a/apps/server/integration/SqlStatementCounter.integration.ts b/apps/server/integration/SqlStatementCounter.integration.ts index cdbe148adda2..0c215ba79b69 100644 --- a/apps/server/integration/SqlStatementCounter.integration.ts +++ b/apps/server/integration/SqlStatementCounter.integration.ts @@ -5,11 +5,7 @@ export interface SqlStatementCounter { readonly count: () => number; } -/** - * Install the same counter on every runtime under test, otherwise statements - * run by background reactors and statements run by request handlers land in - * different counters. - */ +/** Install on every test runtime to include background reactor statements. */ export function makeSqlStatementCounter(): SqlStatementCounter { let statements = 0; const tracer = Tracer.make({ diff --git a/apps/server/integration/transferBudgetV2.integration.test.ts b/apps/server/integration/transferBudgetV2.integration.test.ts index 75b0be5ae57a..8417d852acfd 100644 --- a/apps/server/integration/transferBudgetV2.integration.test.ts +++ b/apps/server/integration/transferBudgetV2.integration.test.ts @@ -16,7 +16,9 @@ import { EnvironmentAuthenticatedPrincipal, ORCHESTRATION_V2_WS_METHODS, OrchestrationV2RpcSchemas, + OrchestrationV2ThreadBoundedSnapshot, OrchestrationV2ThreadDetailSnapshot, + OrchestrationV2ThreadHistoryPage, OrchestrationV2ShellSnapshot, OrchestrationV2GetThreadProjectionError, OrchestrationV2GetShellSnapshotError, @@ -63,11 +65,23 @@ import { type TransferBudgetRun, } from "./TransferBudgetReport.integration.ts"; import { TRANSFER_HISTORY_TURN_COUNT } from "./fixtures/transferBudget.ts"; -import { THREAD_ID, threadCreated, turnEvents } from "./TransferBudgetV2Fixture.integration.ts"; +import { + LONG_TURN_THREAD_ID, + longTurnThreadBatches, + THREAD_ID, + threadCreated, + turnEvents, +} from "./TransferBudgetV2Fixture.integration.ts"; const decodeThreadSnapshot = Schema.decodeUnknownEffect( Schema.fromJsonString(Schema.toCodecJson(OrchestrationV2ThreadDetailSnapshot)), ); +const decodeBoundedSnapshot = Schema.decodeUnknownEffect( + Schema.fromJsonString(Schema.toCodecJson(OrchestrationV2ThreadBoundedSnapshot)), +); +const decodeHistoryPage = Schema.decodeUnknownEffect( + Schema.fromJsonString(Schema.toCodecJson(OrchestrationV2ThreadHistoryPage)), +); const decodeShellSnapshot = Schema.decodeUnknownEffect( Schema.fromJsonString(Schema.toCodecJson(OrchestrationV2ShellSnapshot)), ); @@ -87,6 +101,10 @@ const layerManagement = Layer.unwrap( getThreadSnapshot: (id) => projections.getThreadSnapshot(id).pipe(Effect.orDie), getThreadSnapshotWindow: (id, options) => projections.getThreadSnapshotWindow(id, options).pipe(Effect.orDie), + getThreadHistoryPage: (id, cursor, throughEntryId, conversationOnly) => + projections + .getThreadHistoryPage(id, cursor, throughEntryId, conversationOnly) + .pipe(Effect.orDie), getThreadShell: (id) => projections.getThreadShell(id).pipe(Effect.orDie), getShellSnapshot: (options) => projections.getShellSnapshot(options).pipe(Effect.orDie), listActiveSubagentThreadIds: () => @@ -331,7 +349,29 @@ it.live("reports V2 thread HTTP and WebSocket transfer budgets", () => const shellMode = yield* synchronized(rsq); const reconnectThread = { ...resumedThread.recorder.totals(), mode: threadMode }; const reconnectShell = { ...resumedShell.recorder.totals(), mode: shellMode }; + const reconnectSqlStatements = counter.count() - reconnectSqlStart; assert.equal(threadMode, "snapshot"); + + for (const events of longTurnThreadBatches(provider)) yield* sink.write({ events }); + const longTurnColdOpen = yield* measureHttpGet({ + url: `${base}/api/orchestration/threads/${LONG_TURN_THREAD_ID}/bounded?compactTurnItems=1`, + headers: { [ORCHESTRATION_PROTOCOL_HEADER]: String(ORCHESTRATION_PROTOCOL_VERSION) }, + }); + assert.equal(longTurnColdOpen.status, 200); + const coldOpen = yield* decodeBoundedSnapshot( + Buffer.from(longTurnColdOpen.decodedBody).toString(), + ); + const cursor = coldOpen.historyCursor; + if (cursor === null) return yield* Effect.die("Expected older long-turn history"); + const longTurnHistoryPage = yield* measureHttpGet({ + url: `${base}/api/orchestration/threads/${LONG_TURN_THREAD_ID}/history?cursor=${encodeURIComponent(cursor)}`, + headers: { [ORCHESTRATION_PROTOCOL_HEADER]: String(ORCHESTRATION_PROTOCOL_VERSION) }, + }); + assert.equal(longTurnHistoryPage.status, 200); + const historyPage = yield* decodeHistoryPage( + Buffer.from(longTurnHistoryPage.decodedBody).toString(), + ); + assert.isTrue(historyPage.hasMoreHistory); return { provider, threadSnapshot, @@ -342,7 +382,12 @@ it.live("reports V2 thread HTTP and WebSocket transfer budgets", () => measuredTurnSqlStatements, reconnectThread, reconnectShell, - reconnectSqlStatements: counter.count() - reconnectSqlStart, + reconnectSqlStatements, + longTurnColdOpen: { + http: longTurnColdOpen, + rows: coldOpen.projection.visibleTurnItems.length, + }, + longTurnHistoryPage: { http: longTurnHistoryPage, rows: historyPage.items.length }, } satisfies TransferBudgetRun; }).pipe( Effect.provide(Layer.fresh(layerServices).pipe(Layer.provideMerge(NodeServices.layer))), diff --git a/apps/server/package.json b/apps/server/package.json index fe316ed3fe8e..2a309f13d3b4 100644 --- a/apps/server/package.json +++ b/apps/server/package.json @@ -17,11 +17,14 @@ "@t3tools/source-control-core": "workspace:*", "@t3tools/source-control-definitions": "workspace:*", "@t3tools/source-control-gitlab": "workspace:*", + "character-entities": "2.0.2", "effect": "catalog:", "node-pty": "1.2.0-beta.15", + "saxes": "6.0.0", "stream-chain": "^4.2.5", "stream-json": "3.6.0", - "yaml": "catalog:" + "yaml": "catalog:", + "yauzl": "^3.4.0" }, "devDependencies": { "@anthropic-ai/claude-agent-sdk": "0.3.276", @@ -33,6 +36,7 @@ "@t3tools/shared": "workspace:*", "@t3tools/source-control-testing": "workspace:*", "@types/node": "catalog:", + "@types/yauzl": "^3.4.0", "effect-acp": "workspace:*", "effect-codex-app-server": "workspace:*", "vite-plus": "catalog:" diff --git a/apps/server/scripts/acp-mock-agent.ts b/apps/server/scripts/acp-mock-agent.ts index 7a749b501bd3..e237cbe4ed10 100644 --- a/apps/server/scripts/acp-mock-agent.ts +++ b/apps/server/scripts/acp-mock-agent.ts @@ -52,6 +52,7 @@ const emitGrokMonitorPostTurnPoll = process.env.T3_ACP_EMIT_GROK_MONITOR_POST_TU const emitGrokBackgroundTaskStarted = process.env.T3_ACP_EMIT_GROK_BACKGROUND_TASK_STARTED === "1"; const emitGrokBackgroundTaskPoll = process.env.T3_ACP_GROK_BACKGROUND_TASK_COMPLETION === "poll"; const emitForeignSessionUpdates = process.env.T3_ACP_EMIT_FOREIGN_SESSION_UPDATES === "1"; +const omitInitializeModelState = process.env.T3_ACP_OMIT_INITIALIZE_MODEL_STATE === "1"; const waitForResumeRelease = process.env.T3_ACP_WAIT_FOR_RESUME_RELEASE === "1"; const completeFirstPromptOnCancel = process.env.T3_ACP_COMPLETE_FIRST_PROMPT_ON_CANCEL === "1"; const floodStderr = process.env.T3_ACP_FLOOD_STDERR === "1"; @@ -429,8 +430,7 @@ function modeState(): AcpCompat.SessionModeState { }; } -// Mirrors the real Grok ACP: it advertises versioned model ids, never the CLI's own -// "grok-build" product name. +// Grok ACP advertises versioned model ids, never the CLI product name. const grokAcpModels: ReadonlyArray = [ { modelId: process.env.T3_ACP_GROK_MODEL ?? "grok-4.6", @@ -576,7 +576,7 @@ const program = Effect.gen(function* () { return { protocolVersion: 2, info: { name: "t3-acp-mock-agent", version: "0.0.0" }, - _meta: { modelState: modelState() }, + ...(omitInitializeModelState ? {} : { _meta: { modelState: modelState() } }), capabilities: { session: { ...(supportsSessionLifecycle ? { fork: {}, additionalDirectories: {} } : {}), @@ -600,8 +600,7 @@ const program = Effect.gen(function* () { }), ); - // Mirrors the real Antigravity agent: the API key method reads - // GEMINI_API_KEY from the process environment and rejects when it is missing. + // Antigravity requires GEMINI_API_KEY for API key authentication. yield* agent.handleAuthenticate((request) => Effect.gen(function* () { if (antigravityProfile) { @@ -938,21 +937,20 @@ const program = Effect.gen(function* () { }, }); }); - yield* Effect.sleep("1200 millis") - .pipe( - Effect.andThen( - Effect.sync(() => { - writeJsonRpcNotification("_x.ai/task_completed", { - sessionId: cancelledSessionId, - update: { - sessionUpdate: "task_completed", - task_snapshot: { task_id: "task-bg-1", command: "sleep 30" }, - }, - }); - }), - ), - ) - .pipe(Effect.forkDetach); + yield* Effect.sleep("1200 millis").pipe( + Effect.andThen( + Effect.sync(() => { + writeJsonRpcNotification("_x.ai/task_completed", { + sessionId: cancelledSessionId, + update: { + sessionUpdate: "task_completed", + task_snapshot: { task_id: "task-bg-1", command: "sleep 30" }, + }, + }); + }), + ), + Effect.forkDetach, + ); } }), ); @@ -962,6 +960,124 @@ const program = Effect.gen(function* () { const requestedSessionId = String(request.sessionId ?? sessionId); beginAcpMockPrompt(cancelledSessions, requestedSessionId); promptCount += 1; + const idleError = process.env.T3_ACP_IDLE_ERROR; + if (idleError === "grok-background" && promptCount === 1) { + // The structured completion lands before the prompt settles, so it is buffered. + const taskId = "call-idle-error-background"; + writeJsonRpcNotification("session/update", { + sessionId: requestedSessionId, + update: { + sessionUpdate: "tool_call", + toolCallId: taskId, + title: "run_terminal_command", + kind: "execute", + status: "in_progress", + rawInput: { command: "sleep 1; echo done" }, + }, + }); + writeJsonRpcNotification("session/update", { + sessionId: requestedSessionId, + update: { + sessionUpdate: "tool_call_update", + toolCallId: taskId, + status: "completed", + rawOutput: { + type: "BackgroundTaskStarted", + task_id: taskId, + task_type: "bash", + status: "running", + command: "sleep 1; echo done", + }, + }, + }); + writeJsonRpcNotification("_x.ai/task_completed", { + sessionId: requestedSessionId, + update: { + sessionUpdate: "task_completed", + task_snapshot: { task_id: taskId, kind: "bash", exit_code: 0, output: "done\n" }, + }, + }); + writeJsonRpcNotification("session/update", { + sessionId: requestedSessionId, + update: { sessionUpdate: "state_update", state: "idle", stopReason: "error" }, + }); + return {}; + } + if (idleError !== undefined && promptCount === 1) { + const toolCallId = "tool-call-idle-error-1"; + writeJsonRpcNotification("session/update", { + sessionId: requestedSessionId, + update: { + sessionUpdate: "tool_call_update", + toolCallId, + title: "Terminal", + kind: "execute", + status: "in_progress", + rawInput: { command: ["long-running-tool"] }, + }, + }); + writeJsonRpcNotification("session/update", { + sessionId: requestedSessionId, + update: { + sessionUpdate: "tool_call_update", + toolCallId: "tool-call-idle-error-monitor", + title: "Monitor", + kind: "execute", + status: "in_progress", + rawInput: { command: ["background-monitor"] }, + }, + }); + writeJsonRpcNotification("session/update", { + sessionId: requestedSessionId, + update: { + sessionUpdate: "tool_call_update", + toolCallId: "tool-call-idle-error-subagent", + title: "Subagent", + kind: "other", + status: "in_progress", + rawInput: {}, + }, + }); + writeJsonRpcNotification("session/update", { + sessionId: "mock-child-session-idle-error", + update: { + sessionUpdate: "tool_call_update", + toolCallId: "tool-call-idle-error-child", + title: "Child tool", + kind: "execute", + status: "in_progress", + rawInput: { command: ["child-tool"] }, + }, + }); + writeJsonRpcNotification("session/update", { + sessionId: requestedSessionId, + update: { + sessionUpdate: "state_update", + state: "idle", + stopReason: "error", + ...(idleError === "details" + ? { error: { code: -32000, message: "Missing Authentication header" } } + : {}), + }, + }); + // The agent outlives the failed prompt, so its subagent finishes afterwards. + yield* Effect.sleep("50 millis").pipe( + Effect.andThen( + Effect.sync(() => + writeJsonRpcNotification("session/update", { + sessionId: requestedSessionId, + update: { + sessionUpdate: "tool_call_update", + toolCallId: "tool-call-idle-error-subagent", + status: "completed", + }, + }), + ), + ), + Effect.forkDetach, + ); + return {}; + } if ( process.env.T3_ACP_CRASH_PROMPT === "1" && request.prompt.some((part) => part.type === "text" && part.text === "crash now") @@ -1464,11 +1580,9 @@ const program = Effect.gen(function* () { const detachedCommand = runningCommandIgnoresTerm ? 'trap "" TERM; bash -c \'trap "" TERM; while :; do sleep 1; done\' & child=$!; printf "%s %s %s\n" "$PPID" "$$" "$child" > "$1"; echo published; wait "$child"' : 'sleep 120 & child=$!; printf "%s %s %s\n" "$PPID" "$$" "$child" > "$1"; echo published; wait "$child"'; - // Nested bash publishes "$PPID $$ $child" once it starts. Do not - // write the launcher PID alone here: that races with bash and can - // clobber the triple that interrupt tests wait for. It then prints a - // line, so an agent that exits after launch only exits once the - // triple exists instead of racing a slow launcher start. + // Only nested bash writes the "$PPID $$ $child" triple; writing the launcher PID + // here would race it. The launcher's printed line keeps the agent alive until the + // triple exists. const detachedLauncher = NodeChildProcess.spawn( process.execPath, ["-e", launcher, "bash", "-c", detachedCommand, "bash", runningCommandPidPath], @@ -2322,7 +2436,7 @@ const program = Effect.gen(function* () { } if (emitXAiPlanMdWrite) { - // Match Grok's real session layout so isGrokPlanMarkdownPath accepts it. + // isGrokPlanMarkdownPath requires Grok's session layout. const planRoot = process.env.T3_ACP_PLAN_ROOT ?? "/tmp/mock-home/.grok"; const planPath = `${planRoot}/sessions/${requestedSessionId}/plan.md`; const planBody = "# Mock plan\n\n- Write the feature\n- Add a test\n- Ship it\n"; diff --git a/apps/server/src/assets/AssetAccess.test.ts b/apps/server/src/assets/AssetAccess.test.ts index 6ffbaad8dce9..5b34e06a61f3 100644 --- a/apps/server/src/assets/AssetAccess.test.ts +++ b/apps/server/src/assets/AssetAccess.test.ts @@ -208,6 +208,22 @@ describe("AssetAccess", () => { ["recording.webm", "video/webm"], ["report.html", "text/html"], ["report.pdf", "application/pdf"], + ["notes.odt", "application/vnd.oasis.opendocument.text"], + ["budget.ods", "application/vnd.oasis.opendocument.spreadsheet"], + ["slides.odp", "application/vnd.oasis.opendocument.presentation"], + ["notes.rtf", "application/rtf"], + ["book.epub", "application/epub+zip"], + ["analysis.ipynb", "application/x-ipynb+json"], + ["tone.wav", "audio/wav"], + ["tone.webm-audio", "audio/webm"], + ["archive.zip", "application/zip"], + ["archive.jar", "application/java-archive"], + ["archive.tar", "application/x-tar"], + ["archive.gz", "application/gzip"], + ["archive.tgz", "application/gzip"], + ["data.sqlite", "application/vnd.sqlite3"], + ["data.sqlite3", "application/vnd.sqlite3"], + ["data.db", "application/vnd.sqlite3"], ] as const) { const filePath = path.join(outside, name); yield* fs.writeFileString(filePath, "media"); @@ -232,6 +248,65 @@ describe("AssetAccess", () => { }).pipe(Effect.provide(layerTest)), ); + it.effect("downloads any linked file with the same exact-file identity checks as media", () => + Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const root = yield* fs.makeTempDirectoryScoped({ prefix: "t3-download-root-" }); + const outside = yield* fs.makeTempDirectoryScoped({ prefix: "t3-download-outside-" }); + for (const name of [ + "data.csv", + "report.docx", + "sheet.xlsx", + "slides.pptx", + "archive.zip", + "no-extension", + ]) { + for (const directory of [root, outside]) { + const filePath = path.join(directory, name); + yield* fs.writeFileString(filePath, "shared file bytes"); + const resource = { + _tag: "media-file" as const, + threadId: ThreadId.make("thread-1"), + path: directory === root ? name : filePath, + disposition: "attachment" as const, + }; + const result = yield* issueAssetUrl({ resource, workspaceRoot: root }); + const suffix = result.relativeUrl.slice(`${ASSET_ROUTE_PREFIX}/`.length); + const separator = suffix.indexOf("/"); + const token = suffix.slice(0, separator); + const asset = yield* resolveAsset(token, suffix.slice(separator + 1)); + expect(asset).toMatchObject({ + kind: "file", + download: true, + fileName: name, + mimeType: "application/octet-stream", + }); + if (asset?.kind !== "file") throw new Error("Expected a downloadable file"); + const response = HttpServerResponse.toWeb(yield* assetFileResponse(asset)); + expect(response.headers.get("content-disposition")).toContain("attachment;"); + expect(response.headers.get("content-type")).toBe("application/octet-stream"); + expect(yield* Effect.promise(() => response.text())).toBe("shared file bytes"); + expect(yield* resolveAsset(token, "sibling.docx")).toBeNull(); + expect(yield* resolveAsset(token, `../${name}`)).toBeNull(); + expect(yield* resolveAsset(`${token}tampered`, name)).toBeNull(); + yield* fs.rename(filePath, `${filePath}.original`); + yield* fs.writeFileString(filePath, "replacement bytes"); + expect(yield* resolveAsset(token, name)).toBeNull(); + } + } + const directoryError = yield* issueAssetUrl({ + resource: { + _tag: "media-file", + threadId: ThreadId.make("thread-1"), + path: root, + disposition: "attachment", + }, + }).pipe(Effect.flip); + expect(directoryError._tag).toBe("AssetWorkspaceAssetNotFoundError"); + }).pipe(Effect.provide(layerTest)), + ); + it.effect("resolves relative media paths from the thread workspace, including outside it", () => Effect.gen(function* () { const fs = yield* FileSystem.FileSystem; @@ -258,6 +333,179 @@ describe("AssetAccess", () => { }).pipe(Effect.provide(layerTest)), ); + it.effect("previews draft audio and video from relative workspace paths", () => + Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const root = yield* fs.makeTempDirectoryScoped({ prefix: "t3-draft-media-" }); + for (const [name, mimeType] of [ + ["video.mp4", "video/mp4"], + ["audio.wav", "audio/wav"], + ] as const) { + const filePath = path.join(root, name); + yield* fs.writeFileString(filePath, "media bytes"); + const result = yield* issueAssetUrl({ + resource: { _tag: "draft-workspace-file", cwd: root, path: name }, + workspaceRoot: root, + }); + expect(result.fileRevision).toEqual(expect.any(String)); + const suffix = result.relativeUrl.slice(`${ASSET_ROUTE_PREFIX}/`.length); + const separator = suffix.indexOf("/"); + const token = suffix.slice(0, separator); + const asset = yield* resolveAsset(token, suffix.slice(separator + 1)); + expect(asset).toMatchObject({ kind: "file", path: yield* fs.realPath(filePath), mimeType }); + if (asset?.kind !== "file") throw new Error("Expected draft media file"); + for (const [range, status, body] of [ + [undefined, 200, "media bytes"], + ["bytes=2-5", 206, "dia "], + ] as const) { + const resolved = yield* resolveAsset(token, suffix.slice(separator + 1)); + if (resolved?.kind !== "file") throw new Error("Expected draft media file"); + const response = HttpServerResponse.toWeb(yield* assetFileResponse(resolved, range)); + expect(response.status).toBe(status); + expect(response.headers.get("content-type")).toBe(mimeType); + expect(response.headers.get("content-length")).toBe(String(body.length)); + if (range) expect(response.headers.get("content-range")).toBe("bytes 2-5/11"); + expect(yield* Effect.promise(() => response.text())).toBe(body); + } + for (const requestedPath of [ + name === "video.mp4" ? "audio.wav" : "video.mp4", + `../${name}`, + ]) { + expect(yield* resolveAsset(token, requestedPath)).toBeNull(); + } + expect(yield* resolveAsset(`${token}tampered`, name)).toBeNull(); + } + }).pipe(Effect.provide(layerTest)), + ); + + it.effect( + "rejects draft media escapes and disallowed types without enabling non-draft media", + () => + Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const directory = yield* fs.makeTempDirectoryScoped({ prefix: "t3-draft-boundary-" }); + const root = path.join(directory, "workspace"); + yield* fs.makeDirectory(root); + for (const name of ["video.mp4", "audio.wav", "secret.txt"]) { + yield* fs.writeFileString(path.join(root, name), "workspace bytes"); + yield* fs.writeFileString(path.join(directory, name), "outside bytes"); + } + for (const [requestedPath, tag] of [ + ["../video.mp4", "AssetWorkspacePathValidationError"], + ["../audio.wav", "AssetWorkspacePathValidationError"], + ["secret.txt", "AssetPreviewTypeValidationError"], + ] as const) { + const error = yield* issueAssetUrl({ + resource: { _tag: "draft-workspace-file", cwd: root, path: requestedPath }, + workspaceRoot: root, + }).pipe(Effect.flip); + expect(error._tag).toBe(tag); + } + for (const name of ["video.mp4", "audio.wav"]) { + const error = yield* issueAssetUrl({ + resource: { _tag: "workspace-file", threadId: ThreadId.make("thread-1"), path: name }, + workspaceRoot: root, + }).pipe(Effect.flip); + expect(error._tag).toBe("AssetPreviewTypeValidationError"); + } + }).pipe(Effect.provide(layerTest)), + ); + + it.effect.skipIf(!symlinksSupported)( + "rejects draft media symlinks escaping the workspace or targeting disallowed types", + () => + Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const directory = yield* fs.makeTempDirectoryScoped({ prefix: "t3-draft-symlink-" }); + const root = path.join(directory, "workspace"); + yield* fs.makeDirectory(root); + const outside = path.join(directory, "outside.mp4"); + const secret = path.join(root, "secret.txt"); + yield* fs.writeFileString(outside, "outside bytes"); + yield* fs.writeFileString(secret, "private bytes"); + for (const [name, target] of [ + ["escape.mp4", outside], + ["disguised.wav", secret], + ] as const) { + yield* fs.symlink(target, path.join(root, name)); + const error = yield* issueAssetUrl({ + resource: { _tag: "draft-workspace-file", cwd: root, path: name }, + workspaceRoot: root, + }).pipe(Effect.flip); + expect(["AssetWorkspaceAssetNotFoundError", "AssetPreviewTypeValidationError"]).toContain( + error._tag, + ); + } + }).pipe(Effect.provide(layerTest)), + ); + + it.effect.skipIf(!symlinksSupported)( + "preserves absolute draft previews through stable symlinks", + () => + Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const root = yield* fs.makeTempDirectoryScoped({ prefix: "t3-draft-absolute-" }); + for (const extension of ["png", "mp4", "wav"]) { + const target = path.join(root, `target.${extension}`); + const alias = path.join(root, `alias.${extension}`); + yield* fs.writeFileString(target, "preview bytes"); + yield* fs.symlink(target, alias); + const result = yield* issueAssetUrl({ + resource: { _tag: "draft-workspace-file", cwd: root, path: alias }, + workspaceRoot: root, + }); + const suffix = result.relativeUrl.slice(`${ASSET_ROUTE_PREFIX}/`.length); + const separator = suffix.indexOf("/"); + expect( + yield* resolveAsset(suffix.slice(0, separator), suffix.slice(separator + 1)), + ).toMatchObject({ kind: "file", path: yield* fs.realPath(target) }); + } + }).pipe(Effect.provide(layerTest)), + ); + + it.effect.skipIf(!symlinksSupported)( + "rejects a draft media path swapped after workspace containment validation", + () => + Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const directory = yield* fs.makeTempDirectoryScoped({ prefix: "t3-draft-swap-" }); + const root = path.join(directory, "workspace"); + yield* fs.makeDirectory(root); + const filePath = path.join(root, "video.mp4"); + const outside = path.join(directory, "outside.mp4"); + yield* fs.writeFileString(filePath, "workspace video"); + yield* fs.writeFileString(outside, "outside video"); + const canonicalFile = yield* fs.realPath(filePath); + let swapped = false; + const swappingFileSystem = FileSystem.FileSystem.of({ + ...fs, + stat: Effect.fn(function* (requestedPath) { + const info = yield* fs.stat(requestedPath); + if (requestedPath === canonicalFile && !swapped) { + swapped = true; + yield* fs.remove(filePath); + yield* fs.symlink(outside, filePath); + } + return info; + }), + }); + const result = yield* issueAssetUrl({ + resource: { _tag: "draft-workspace-file", cwd: root, path: "video.mp4" }, + workspaceRoot: root, + }).pipe(Effect.result, Effect.provideService(FileSystem.FileSystem, swappingFileSystem)); + expect(swapped).toBe(true); + expect(result).toMatchObject({ + _tag: "Failure", + failure: { _tag: "AssetWorkspaceAssetNotFoundError" }, + }); + }).pipe(Effect.provide(layerTest)), + ); + it.effect("resolves home-relative media paths independently of the workspace", () => Effect.gen(function* () { const fs = yield* FileSystem.FileSystem; @@ -668,9 +916,63 @@ describe("AssetAccess", () => { kind: "file", path: canonicalCssPath, }); + for (const extension of ["otf", "ttf", "woff", "woff2"]) { + const fontName = `preview.${extension}`; + const fontPath = path.join(root, fontName); + yield* fileSystem.writeFileString(fontPath, "font bytes"); + const canonicalFontPath = yield* fileSystem.realPath(fontPath); + for (const resource of [ + { _tag: "workspace-file" as const, threadId: ThreadId.make("thread-1"), path: htmlPath }, + { _tag: "draft-workspace-file" as const, cwd: root, path: "report.html" }, + ]) { + const url = yield* issueAssetUrl({ resource, workspaceRoot: root }); + const suffix = url.relativeUrl.slice(`${ASSET_ROUTE_PREFIX}/`.length); + expect(yield* resolveAsset(suffix.slice(0, suffix.indexOf("/")), fontName)).toEqual({ + kind: "file", + path: canonicalFontPath, + }); + } + } expect(yield* resolveAsset(token, "../secret.txt")).toBeNull(); expect(yield* resolveAsset(token, ".env")).toBeNull(); expect(yield* resolveAsset(`${token}tampered`, "report.html")).toBeNull(); + for (const extension of [ + "odt", + "ods", + "odp", + "rtf", + "epub", + "ipynb", + "zip", + "jar", + "tar", + "gz", + "tgz", + "sqlite", + "sqlite3", + "db", + "docx", + "xlsx", + "pptx", + "bmp", + "heic", + "tif", + "tiff", + "weba", + "mmd", + "mermaid", + ]) { + const siblingName = `private.${extension}`; + yield* fileSystem.writeFileString(path.join(root, siblingName), "private sibling"); + for (const resource of [ + { _tag: "workspace-file" as const, threadId: ThreadId.make("thread-1"), path: htmlPath }, + { _tag: "draft-workspace-file" as const, cwd: root, path: "report.html" }, + ]) { + const url = yield* issueAssetUrl({ resource, workspaceRoot: root }); + const suffix = url.relativeUrl.slice(`${ASSET_ROUTE_PREFIX}/`.length); + expect(yield* resolveAsset(suffix.slice(0, suffix.indexOf("/")), siblingName)).toBeNull(); + } + } }).pipe(Effect.provide(layerTest)), ); @@ -708,6 +1010,77 @@ describe("AssetAccess", () => { }).pipe(Effect.provide(layerTest)), ); + it.effect("issues exact workspace URLs for every non-HTML preview", () => + Effect.gen(function* () { + const fileSystem = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const root = yield* fileSystem.makeTempDirectoryScoped({ + prefix: "t3-asset-image-workspace-", + }); + const assetsDirectory = path.join(root, "assets"); + yield* fileSystem.makeDirectory(assetsDirectory, { recursive: true }); + yield* fileSystem.writeFileString( + path.join(assetsDirectory, "private.docx"), + "private document", + ); + for (const extension of [ + "pdf", + "odt", + "ods", + "odp", + "rtf", + "epub", + "ipynb", + "png", + "bmp", + "heic", + "tif", + "tiff", + "docx", + "xlsx", + "pptx", + "ttf", + "otf", + "woff", + "woff2", + "zip", + "jar", + "tar", + "gz", + "tgz", + "sqlite", + "sqlite3", + "db", + ]) { + const name = `preview.${extension}`; + const filePath = path.join(assetsDirectory, name); + const siblingName = `other.${extension}`; + yield* fileSystem.writeFileString(filePath, "preview"); + yield* fileSystem.writeFileString( + path.join(assetsDirectory, siblingName), + "private sibling", + ); + const canonicalPath = yield* fileSystem.realPath(filePath); + for (const resource of [ + { + _tag: "workspace-file" as const, + threadId: ThreadId.make("thread-1"), + path: filePath, + }, + { _tag: "draft-workspace-file" as const, cwd: root, path: `assets/${name}` }, + ]) { + const result = yield* issueAssetUrl({ resource, workspaceRoot: root }); + const suffix = result.relativeUrl.slice(`${ASSET_ROUTE_PREFIX}/`.length); + const token = suffix.slice(0, suffix.indexOf("/")); + expect(yield* resolveAsset(token, name)).toEqual({ kind: "file", path: canonicalPath }); + expect(yield* resolveAsset(token, siblingName)).toBeNull(); + expect(yield* resolveAsset(token, "private.docx")).toBeNull(); + expect(yield* resolveAsset(token, `../${name}`)).toBeNull(); + } + } + }).pipe(Effect.provide(layerTest)), + ); + it.effect("previews workspace files with literal fragment characters in their paths", () => Effect.gen(function* () { const fileSystem = yield* FileSystem.FileSystem; @@ -716,7 +1089,27 @@ describe("AssetAccess", () => { const directory = path.join(root, "assets#archive"); yield* fileSystem.makeDirectory(directory); - for (const name of ["icon#v2.png", "report#draft.pdf"]) { + for (const name of [ + "icon#v2.png", + "report#draft.pdf", + "report.docx", + "workbook.xlsx", + "slides.pptx", + "notes.odt", + "budget.ods", + "slides.odp", + "notes.rtf", + "book.epub", + "analysis.ipynb", + "specimen.ttf", + "specimen.otf", + "specimen.woff", + "specimen.woff2", + "image.bmp", + "image.heic", + "image.tif", + "image.tiff", + ]) { const filePath = path.join(directory, name); yield* fileSystem.writeFileString(filePath, "preview fixture"); const canonicalFile = yield* fileSystem.realPath(filePath); @@ -793,13 +1186,65 @@ describe("AssetAccess", () => { }).pipe(Effect.provide(layerTest)), ); + it.effect("serves document attachments inline when a viewer requests it", () => + Effect.gen(function* () { + const config = yield* ServerConfig.ServerConfig; + const fileSystem = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + yield* fileSystem.makeDirectory(config.attachmentsDir, { recursive: true }); + for (const [extension, mimeType] of [ + ["pdf", "application/pdf"], + ["svg", "image/svg+xml"], + ["bmp", "image/bmp"], + ["ttf", "font/ttf"], + ["otf", "font/otf"], + ["woff", "font/woff"], + ["woff2", "font/woff2"], + ["weba", "audio/webm"], + ["zip", "application/zip"], + ["jar", "application/java-archive"], + ["tar", "application/x-tar"], + ["gz", "application/gzip"], + ["tgz", "application/gzip"], + ["sqlite", "application/vnd.sqlite3"], + ["sqlite3", "application/vnd.sqlite3"], + ["db", "application/vnd.sqlite3"], + ] as const) { + const attachmentId = `thread-1-00000000-0000-4000-8000-000000000001-${extension}`; + const attachmentPath = path.join(config.attachmentsDir, `${attachmentId}.${extension}`); + yield* fileSystem.writeFile(attachmentPath, new Uint8Array([1, 2, 3])); + + const result = yield* issueAssetUrl({ + resource: { + _tag: "attachment", + attachmentId, + fileName: `report.${extension}`, + mimeType: "text/html", + disposition: "inline", + }, + }); + const suffix = result.relativeUrl.slice(`${ASSET_ROUTE_PREFIX}/`.length); + const separatorIndex = suffix.indexOf("/"); + + expect( + yield* resolveAsset(suffix.slice(0, separatorIndex), suffix.slice(separatorIndex + 1)), + ).toEqual({ + kind: "file", + path: attachmentPath, + fileName: `report.${extension}`, + mimeType, + }); + } + }).pipe(Effect.provide(layerTest)), + ); + it.effect("keeps inline requests for other attachment types as downloads", () => Effect.gen(function* () { const config = yield* ServerConfig.ServerConfig; const fileSystem = yield* FileSystem.FileSystem; const path = yield* Path.Path; - const attachmentId = "thread-1-00000000-0000-4000-8000-000000000002-zip"; - const attachmentPath = path.join(config.attachmentsDir, `${attachmentId}.zip`); + const attachmentId = "thread-1-00000000-0000-4000-8000-000000000002-7z"; + const attachmentPath = path.join(config.attachmentsDir, `${attachmentId}.7z`); yield* fileSystem.makeDirectory(config.attachmentsDir, { recursive: true }); yield* fileSystem.writeFile(attachmentPath, new Uint8Array([1, 2, 3])); @@ -807,7 +1252,7 @@ describe("AssetAccess", () => { resource: { _tag: "attachment", attachmentId, - fileName: "archive.zip", + fileName: "archive.7z", mimeType: "text/html", disposition: "inline", }, diff --git a/apps/server/src/assets/AssetAccess.ts b/apps/server/src/assets/AssetAccess.ts index aca9fe1b047c..855d64618e1d 100644 --- a/apps/server/src/assets/AssetAccess.ts +++ b/apps/server/src/assets/AssetAccess.ts @@ -16,12 +16,11 @@ import { TurnItemId, } from "@t3tools/contracts"; import { - audioMimeTypeFromExtension, hostPreviewMimeTypeFromExtension, - isWorkspaceImagePreviewPath, + isWorkspaceAudioPreviewPath, isWorkspacePreviewEntryPath, + isWorkspaceVideoPreviewPath, WORKSPACE_BROWSER_PREVIEW_EXTENSIONS, - WORKSPACE_IMAGE_PREVIEW_EXTENSIONS, } from "@t3tools/shared/filePreview"; import { IMAGE_DIMENSIONS_HEADER_BYTES, @@ -73,18 +72,28 @@ const HTML_ATTACHMENT_TOKEN_BUCKET_MS = 30 * 60 * 1000; const PROJECT_FAVICON_VERSION_PREFIX = "v"; const PROJECT_FAVICON_ICNS_MIN_SIZE = 96; const INLINE_VIDEO_MIME_TYPE_PATTERN = /^video\/[\w!#$&^.+-]+$/i; -// The extension comes from the attachment id the server assigned, never from -// the client's mime type. +// Use server-assigned attachment extensions, never client MIME types. const INLINE_PREVIEW_MIME_TYPES: Record = { + svg: "image/svg+xml", pdf: "application/pdf", html: "text/html", htm: "text/html", }; const inlinePreviewMimeTypeForExtension = (extension: string) => - INLINE_PREVIEW_MIME_TYPES[extension] ?? audioMimeTypeFromExtension(`.${extension}`) ?? undefined; + INLINE_PREVIEW_MIME_TYPES[extension] ?? + hostPreviewMimeTypeFromExtension(`.${extension}`) ?? + undefined; const PREVIEW_ASSET_EXTENSIONS = new Set([ ...WORKSPACE_BROWSER_PREVIEW_EXTENSIONS, - ...WORKSPACE_IMAGE_PREVIEW_EXTENSIONS, + // Keep HTML dependency grants independent of newly supported file viewers. + ".avif", + ".gif", + ".ico", + ".jpeg", + ".jpg", + ".png", + ".svg", + ".webp", ".css", ".js", ".mjs", @@ -113,6 +122,7 @@ const AssetClaimsSchema = Schema.Union([ version: Schema.Literal(1), kind: Schema.Literal("media-file-exact"), filePath: Schema.String, + download: Schema.optionalKey(Schema.Boolean), device: Schema.String, inode: Schema.String, expiresAt: Schema.Number, @@ -201,8 +211,7 @@ const optionOnNotFound = ( export const readMcpAppHtml = Effect.fn("AssetAccess.readMcpAppHtml")(function* (filePath: string) { const fileSystem = yield* FileSystem.FileSystem; - // Stored snapshots also contain a bounded CSP meta tag (up to 32 domains - // per directive). Bound the read even if the attachment grows after minting. + // Allow space for stored CSP tags, but bound reads if the attachment grows after minting. const limit = MCP_APP_MAX_HTML_BYTES + 128 * 1024; const chunks = yield* Stream.runCollect(fileSystem.stream(filePath, { bytesToRead: limit + 1 })); const size = chunks.reduce((total, chunk) => total + chunk.byteLength, 0); @@ -230,7 +239,7 @@ const readToolOutputImage = Effect.fn("AssetAccess.readToolOutputImage")(functio : { mimeType: image.mimeType, bytes: Buffer.from(image.data, "base64") }; }); -/** Serves an `.icns` favicon as its embedded PNG, since browsers cannot draw it. */ +// Browsers cannot draw .icns; serve its embedded PNG. const projectFaviconAsset = Effect.fn("AssetAccess.projectFaviconAsset")(function* ( filePath: string, ) { @@ -305,17 +314,13 @@ const resolveCanonicalWorkspaceFileForRequest = (input: { const HEADER_IMAGE_EXTENSIONS = new Set([".png", ".jpg", ".jpeg", ".gif", ".webp"]); -/** From the identity-checked, non-blocking handle the caller already holds. */ const readImageDimensionsFromOpenFile = (filePath: string, file: OpenMediaFile) => readMediaFileHeader(filePath, file, IMAGE_DIMENSIONS_HEADER_BYTES).pipe( Effect.map(readImageDimensions), Effect.orElseSucceed((): ImageDimensions | null => null), ); -/** - * Opens through `openMediaFile` so a path swapped for a FIFO cannot block the - * request; a regular open would wait for a writer that never comes. - */ +// Non-blocking open prevents swapped-in FIFOs from hanging the request. const readImageDimensionsFromHeader = (filePath: string) => openMediaFile(filePath).pipe( Effect.flatMap((file) => @@ -328,6 +333,7 @@ const readImageDimensionsFromHeader = (filePath: string) => const finalizeAbsoluteMediaFileAsset = Effect.fn("AssetAccess.finalizeAbsoluteMediaFileAsset")( function* (input: { readonly requestedPath: string; + readonly expectedCanonicalPath?: string; readonly workspaceRoot: string | undefined; readonly resource: AssetResource; readonly expiresAt: number; @@ -353,13 +359,20 @@ const finalizeAbsoluteMediaFileAsset = Effect.fn("AssetAccess.finalizeAbsoluteMe (cause) => new AssetWorkspaceAssetInspectionError({ resource: input.resource, cause }), ), ); - if (!canonicalFile) { + // The workspace caller pins its containment-checked path; never follow a + // replacement symlink to a different file during this second resolution. + if ( + !canonicalFile || + (input.expectedCanonicalPath !== undefined && canonicalFile !== input.expectedCanonicalPath) + ) { return yield* new AssetWorkspaceAssetNotFoundError({ resource: input.resource, checkedPath: input.requestedPath, }); } - if (hostPreviewMimeTypeFromExtension(path.extname(canonicalFile)) === null) { + const download = + input.resource._tag === "media-file" && input.resource.disposition === "attachment"; + if (!download && hostPreviewMimeTypeFromExtension(path.extname(canonicalFile)) === null) { return yield* new AssetPreviewTypeValidationError({ resource: input.resource }); } const wantsDimensions = HEADER_IMAGE_EXTENSIONS.has(path.extname(canonicalFile).toLowerCase()); @@ -394,6 +407,7 @@ const finalizeAbsoluteMediaFileAsset = Effect.fn("AssetAccess.finalizeAbsoluteMe version: 1 as const, kind: "media-file-exact" as const, filePath: canonicalFile, + ...(download ? { download: true } : {}), ...opened.identity, expiresAt: input.expiresAt, }, @@ -428,7 +442,11 @@ const finalizeWorkspaceFileAsset = Effect.fn("AssetAccess.finalizeWorkspaceFileA }), ), ); - if (!isWorkspacePreviewEntryPath(resolved.relativePath)) { + const isDraftMedia = + input.resource._tag === "draft-workspace-file" && + (isWorkspaceAudioPreviewPath(resolved.relativePath) || + isWorkspaceVideoPreviewPath(resolved.relativePath)); + if (!isDraftMedia && !isWorkspacePreviewEntryPath(resolved.relativePath)) { return yield* new AssetPreviewTypeValidationError({ resource: input.resource, }); @@ -450,6 +468,13 @@ const finalizeWorkspaceFileAsset = Effect.fn("AssetAccess.finalizeWorkspaceFileA resource: input.resource, }); } + if (isDraftMedia) { + return yield* finalizeAbsoluteMediaFileAsset({ + ...input, + requestedPath: canonicalFile, + expectedCanonicalPath: canonicalFile, + }); + } const canonicalWorkspaceRoot = yield* fileSystem.realPath(input.workspaceRoot).pipe( Effect.mapError( (cause) => @@ -465,7 +490,7 @@ const finalizeWorkspaceFileAsset = Effect.fn("AssetAccess.finalizeWorkspaceFileA ? yield* readImageDimensionsFromHeader(canonicalFile) : null; return { - claims: isWorkspaceImagePreviewPath(resolved.relativePath) + claims: !/\.html?$/i.test(resolved.relativePath) ? { version: 1 as const, kind: "workspace-file-exact" as const, @@ -482,6 +507,7 @@ const finalizeWorkspaceFileAsset = Effect.fn("AssetAccess.finalizeWorkspaceFileA }, fileName: path.basename(resolved.relativePath), imageDimensions, + fileRevision: undefined, }; }, ); @@ -594,6 +620,7 @@ export const issueAssetUrl = Effect.fn("AssetAccess.issueAssetUrl")(function* (i claims = finalized.claims; fileName = finalized.fileName; imageDimensions = finalized.imageDimensions; + fileRevision = finalized.fileRevision; break; } case "attachment": { @@ -790,9 +817,7 @@ export const issueAssetUrl = Effect.fn("AssetAccess.issueAssetUrl")(function* (i claims = { ...claims, expiresAt }; } if (claims.kind === "attachment" && claims.mimeType === "text/html") { - // HTML pages never change, so mints in the same window sign the same URL and a reload - // can answer it from the browser cache. Images keep a new URL per mint, since their - // load-error retries rely on one. + // Stable URLs let immutable HTML reloads hit the cache; image retries need fresh URLs. expiresAt = Math.ceil(expiresAt / HTML_ATTACHMENT_TOKEN_BUCKET_MS) * HTML_ATTACHMENT_TOKEN_BUCKET_MS; claims = { ...claims, expiresAt }; @@ -907,7 +932,9 @@ export const resolveAsset = Effect.fn("AssetAccess.resolveAsset")(function* ( Effect.orElseSucceed(() => null), ); if (canonicalFile !== claims.filePath) return null; - const mimeType = hostPreviewMimeTypeFromExtension(path.extname(canonicalFile)); + const mimeType = claims.download + ? "application/octet-stream" + : hostPreviewMimeTypeFromExtension(path.extname(canonicalFile)); if (!mimeType) return null; const file = yield* openMediaFile(canonicalFile, claims).pipe( Effect.tapError((cause) => @@ -916,7 +943,13 @@ export const resolveAsset = Effect.fn("AssetAccess.resolveAsset")(function* ( Effect.orElseSucceed(() => null), ); return file - ? ({ kind: "file", path: canonicalFile, mimeType, file } satisfies ResolvedAsset) + ? ({ + kind: "file", + path: canonicalFile, + mimeType, + file, + ...(claims.download ? { download: true, fileName: path.basename(canonicalFile) } : {}), + } satisfies ResolvedAsset) : null; } if (claims.kind === "workspace-file-exact") { diff --git a/apps/server/src/assets/AttachmentUpload.test.ts b/apps/server/src/assets/AttachmentUpload.test.ts index 1eb60a1544b2..d4181c202f5d 100644 --- a/apps/server/src/assets/AttachmentUpload.test.ts +++ b/apps/server/src/assets/AttachmentUpload.test.ts @@ -1,19 +1,27 @@ +import * as DocumentText from "../provider/DocumentText.ts"; // @effect-diagnostics nodeBuiltinImport:off import * as NodeFS from "node:fs"; import * as NodePath from "node:path"; import * as NodeServices from "@effect/platform-node/NodeServices"; import { describe, expect, it } from "@effect/vitest"; +import { ChatAttachmentId } from "@t3tools/contracts"; import * as Deferred from "effect/Deferred"; import * as Effect from "effect/Effect"; import * as Fiber from "effect/Fiber"; +import * as FileSystem from "effect/FileSystem"; import * as Layer from "effect/Layer"; +import * as PlatformError from "effect/PlatformError"; import * as Schema from "effect/Schema"; import * as Stream from "effect/Stream"; import * as ServerSecretStore from "../auth/ServerSecretStore.ts"; import { base64UrlEncode, signPayload } from "../auth/utils.ts"; import * as ServerConfig from "../config.ts"; +import { + claimPendingAttachments, + releaseClaimedAttachments, +} from "../orchestration-v2/AttachmentClaims.ts"; import { ATTACHMENT_UPLOAD_ROUTE_PREFIX, deletePendingAttachment, @@ -24,7 +32,7 @@ import { const layerTest = ServerSecretStore.layer.pipe( Layer.provideMerge(ServerConfig.layerTest(process.cwd(), { prefix: "t3-attachment-upload-" })), - Layer.provideMerge(NodeServices.layer), + Layer.provideMerge(DocumentText.layer.pipe(Layer.provideMerge(NodeServices.layer))), ); const uploadInput = { @@ -47,6 +55,83 @@ const encodeLegacyAttachmentUploadClaims = Schema.encodeEffect( ); describe("AttachmentUpload", () => { + it.effect.each([false, true])( + "persists and replays immutable uploads (hard links unsupported: %s)", + (unsupported) => + Effect.gen(function* () { + const config = yield* ServerConfig.ServerConfig; + const fileSystem = yield* FileSystem.FileSystem; + const store = (claims: Parameters[0], bytes: Uint8Array) => + storeAttachmentUpload(claims, bytes).pipe( + Effect.provideService( + FileSystem.FileSystem, + unsupported + ? { + ...fileSystem, + link: () => + Effect.fail( + PlatformError.systemError({ + _tag: "Unknown", + module: "FileSystem", + method: "link", + }), + ), + } + : fileSystem, + ), + ); + const bytes = new TextEncoder().encode( + JSON.stringify({ + cells: [{ cell_type: "markdown", source: ["# Cargo\n", "Violet crates: 19"] }], + }), + ); + const issued = yield* issueAttachmentUploadUrl({ + type: "file", + name: "cargo.ipynb", + mimeType: "application/x-ipynb+json", + sizeBytes: bytes.length, + }); + const claims = yield* validateAttachmentUploadToken( + issued.relativeUrl.slice(`${ATTACHMENT_UPLOAD_ROUTE_PREFIX}/`.length), + ); + if (!claims) throw new Error("Expected upload claims"); + expect(yield* store(claims, bytes)).toEqual({ ok: true }); + expect(yield* store(claims, bytes)).toEqual({ ok: true }); + const path = NodePath.join(config.attachmentsDir, `${issued.attachmentId}.ipynb`); + expect(NodeFS.readFileSync(`${path}.txt`, "utf8")).toContain("Violet crates: 19"); + const replacement = new TextEncoder().encode( + new TextDecoder().decode(bytes).replace("Violet crates: 19", "Violet crates: 28"), + ); + expect(yield* store(claims, replacement)).toMatchObject({ + ok: false, + status: 409, + }); + expect(NodeFS.readFileSync(path)).toEqual(Buffer.from(bytes)); + expect(NodeFS.readFileSync(`${path}.txt`, "utf8")).toContain("Violet crates: 19"); + const claimed = yield* claimPendingAttachments({ + threadId: "document-thread", + attachments: [ + { + type: "file", + id: ChatAttachmentId.make(issued.attachmentId), + name: claims.name, + mimeType: claims.mimeType, + sizeBytes: bytes.length, + }, + ], + }); + expect(claimed.claimedPaths).toHaveLength(2); + expect(NodeFS.readFileSync(claimed.claimedPaths[1]!, "utf8")).toContain( + "Violet crates: 19", + ); + yield* deletePendingAttachment(issued.attachmentId); + expect(NodeFS.existsSync(path)).toBe(false); + expect(NodeFS.existsSync(`${path}.txt`)).toBe(false); + expect(NodeFS.existsSync(claimed.claimedPaths[1]!)).toBe(true); + yield* releaseClaimedAttachments(claimed.claimedPaths); + expect(NodeFS.existsSync(claimed.claimedPaths[1]!)).toBe(false); + }).pipe(Effect.provide(layerTest)), + ); it.effect("rejects tampered and malformed upload tokens", () => Effect.gen(function* () { const issued = yield* issueAttachmentUploadUrl(uploadInput); @@ -139,6 +224,47 @@ describe("AttachmentUpload", () => { }).pipe(Effect.provide(layerTest)), ); + it.effect("stores MIME-identified SVG files with a previewable signed extension", () => + Effect.gen(function* () { + const config = yield* ServerConfig.ServerConfig; + const bytes = new TextEncoder().encode(''); + for (const name of ["diagram", "diagram.bin"]) { + const issued = yield* issueAttachmentUploadUrl({ + type: "file", + name, + mimeType: "image/svg+xml", + sizeBytes: bytes.length, + }); + const token = issued.relativeUrl.slice(`${ATTACHMENT_UPLOAD_ROUTE_PREFIX}/`.length); + const claims = yield* validateAttachmentUploadToken(token); + if (!claims) throw new Error("Expected valid SVG upload claims."); + expect(claims.name).toBe(`${name}.svg`); + expect(issued.attachmentId).toMatch(/-svg$/); + expect(yield* storeAttachmentUpload(claims, bytes)).toEqual({ ok: true }); + expect( + NodeFS.readFileSync(NodePath.join(config.attachmentsDir, `${issued.attachmentId}.svg`)), + ).toEqual(Buffer.from(bytes)); + const claimed = yield* claimPendingAttachments({ + threadId: "svg-upload-thread", + attachments: [ + { + type: "file", + id: ChatAttachmentId.make(issued.attachmentId), + name, + mimeType: "image/svg+xml", + sizeBytes: bytes.length, + }, + ], + }); + expect(claimed.attachments[0]?.name).toBe(`${name}.svg`); + expect(NodeFS.readFileSync(claimed.claimedPaths[0]!)).toEqual(Buffer.from(bytes)); + yield* releaseClaimedAttachments(claimed.claimedPaths); + yield* deletePendingAttachment(issued.attachmentId); + } + expect(NodeFS.readdirSync(config.attachmentsDir)).toEqual([]); + }).pipe(Effect.provide(layerTest)), + ); + it.effect("removes partial streamed uploads that exceed their signed size", () => Effect.gen(function* () { const config = yield* ServerConfig.ServerConfig; diff --git a/apps/server/src/assets/AttachmentUpload.ts b/apps/server/src/assets/AttachmentUpload.ts index efbe6a5ba6a8..f3cc445abf55 100644 --- a/apps/server/src/assets/AttachmentUpload.ts +++ b/apps/server/src/assets/AttachmentUpload.ts @@ -1,14 +1,19 @@ +// @effect-diagnostics nodeBuiltinImport:off -- exclusive copy and streaming hashes are filesystem boundaries missing from Effect's API. +import * as NodeCrypto from "node:crypto"; +import * as NodeFSP from "node:fs/promises"; import { ATTACHMENT_UPLOAD_URL_TTL_MS, type AttachmentCreateUploadUrlInput, AttachmentUploadSigningKeyError, } from "@t3tools/contracts"; +import { isSvgAttachment } from "@t3tools/shared/image"; import * as Clock from "effect/Clock"; import * as Crypto from "effect/Crypto"; import * as Effect from "effect/Effect"; import * as FileSystem from "effect/FileSystem"; import * as Option from "effect/Option"; import * as Path from "effect/Path"; +import * as PlatformError from "effect/PlatformError"; import * as Schema from "effect/Schema"; import * as Stream from "effect/Stream"; import type * as HttpServerRequest from "effect/http/HttpServerRequest"; @@ -31,6 +36,7 @@ import { import * as ServerSecretStore from "../auth/ServerSecretStore.ts"; import * as ServerConfig from "../config.ts"; import { inferImageExtension } from "../imageMime.ts"; +import * as DocumentText from "../provider/DocumentText.ts"; export const ATTACHMENT_UPLOAD_ROUTE_PREFIX = "/api/attachments/upload"; @@ -93,8 +99,12 @@ export const issueAttachmentUploadUrl = Effect.fn("AttachmentUpload.issueUrl")(f } const attachmentType = input.type ?? "image"; + const name = + attachmentType === "file" && isSvgAttachment(input) && !/\.svg$/i.test(input.name) + ? `${input.name}.svg` + : input.name; const attachmentId = createPendingAttachmentId( - attachmentType === "file" ? attachmentFileExtension(input.name) : undefined, + attachmentType === "file" ? attachmentFileExtension(name) : undefined, ); const expiresAt = nowMs + ATTACHMENT_UPLOAD_URL_TTL_MS; const encodedPayload = base64UrlEncode( @@ -103,7 +113,7 @@ export const issueAttachmentUploadUrl = Effect.fn("AttachmentUpload.issueUrl")(f kind: "attachment-upload", type: attachmentType, attachmentId, - name: input.name, + name, mimeType: input.mimeType, sizeBytes: input.sizeBytes, expiresAt, @@ -200,7 +210,51 @@ export const storeAttachmentUpload = Effect.fn("AttachmentUpload.store")(functio detail: `Body was ${receivedBytes} bytes, expected ${claims.sizeBytes}.`, } satisfies StoreAttachmentUploadResult; } - yield* fileSystem.rename(partPath, finalPath); + // Publishing without replacement keeps an upload and its cached text immutable. + const published = yield* fileSystem.link(partPath, finalPath).pipe( + Effect.map(() => true), + Effect.catch((error) => { + if (error.reason._tag === "AlreadyExists") return Effect.succeed(false); + return Effect.tryPromise({ + try: () => NodeFSP.copyFile(partPath, finalPath, NodeFSP.constants.COPYFILE_EXCL), + catch: (cause) => + PlatformError.systemError({ + _tag: + cause instanceof Error && "code" in cause && cause.code === "EEXIST" + ? "AlreadyExists" + : "Unknown", + module: "FileSystem", + method: "copyFile", + pathOrDescriptor: finalPath, + cause, + }), + }).pipe( + Effect.as(true), + Effect.catch((error) => + error.reason._tag === "AlreadyExists" ? Effect.succeed(false) : Effect.fail(error), + ), + ); + }), + ); + if (!published) { + if (Number((yield* fileSystem.stat(finalPath)).size) !== receivedBytes) { + return { ok: false, status: 409, detail: "Attachment already uploaded." }; + } + const hashes = yield* Effect.forEach([partPath, finalPath], (file) => + fileSystem.stream(file).pipe( + Stream.runFold( + () => NodeCrypto.createHash("sha256"), + (hash, chunk) => hash.update(chunk), + ), + Effect.map((hash) => hash.digest("hex")), + ), + ); + if (hashes[0] !== hashes[1]) { + return { ok: false, status: 409, detail: "Attachment already uploaded." }; + } + } + const documents = yield* DocumentText.DocumentText; + yield* documents.ensure(finalPath); return { ok: true } satisfies StoreAttachmentUploadResult; }).pipe( Effect.catch((cause) => @@ -239,4 +293,5 @@ export const deletePendingAttachment = Effect.fn("AttachmentUpload.deletePending const fileSystem = yield* FileSystem.FileSystem; yield* fileSystem.remove(attachmentPath, { force: true }).pipe(Effect.orElseSucceed(() => {})); + yield* fileSystem.remove(`${attachmentPath}.txt`, { force: true }).pipe(Effect.ignore); }); diff --git a/apps/server/src/assets/MediaFile.ts b/apps/server/src/assets/MediaFile.ts index 9b6baef64e95..069ff048dd10 100644 --- a/apps/server/src/assets/MediaFile.ts +++ b/apps/server/src/assets/MediaFile.ts @@ -77,10 +77,7 @@ export const openMediaFile = Effect.fn("openMediaFile")(function* ( ) { return null; } - // Callers canonicalise with Effect's FileSystem.realPath, which is - // Node's JS realpath. fs/promises.realpath is the native binding and - // on Windows also expands 8.3 short names, so a path that is already - // canonical by the caller's rules would still look swapped here. + // Match FileSystem.realPath's JS behavior; native realpath expands Windows 8.3 names too. if ((await realpathLikeFileSystem(filePath)) !== filePath) return null; const after = await NodeFSP.lstat(filePath, { bigint: true }); if (!after.isFile() || info.dev !== after.dev || info.ino !== after.ino) return null; diff --git a/apps/server/src/assets/icns.ts b/apps/server/src/assets/icns.ts index 897caf01fd20..08a03f326c87 100644 --- a/apps/server/src/assets/icns.ts +++ b/apps/server/src/assets/icns.ts @@ -4,9 +4,7 @@ const ICNS_HEADER_BYTES = 8; const ICNS_MAGIC = 0x69636e73; const PNG_MAGIC = 0x89504e47; -/** - * Returns a copy so the response does not pin the whole `.icns` buffer. - */ +/** Returns a copy to avoid retaining the whole .icns buffer. */ export function extractIcnsPng(bytes: Uint8Array, minSize: number): Uint8Array | null { const data = new DataView(bytes.buffer, bytes.byteOffset, bytes.byteLength); if (bytes.length < ICNS_HEADER_BYTES || data.getUint32(0) !== ICNS_MAGIC) return null; diff --git a/apps/server/src/attachmentStore.test.ts b/apps/server/src/attachmentStore.test.ts index 0cd6d89aeb43..75ab03d4254f 100644 --- a/apps/server/src/attachmentStore.test.ts +++ b/apps/server/src/attachmentStore.test.ts @@ -166,16 +166,18 @@ describe("attachmentStore", () => { const uuid = "00000000-0000-4000-8000-000000000002"; const pendingPath = NodePath.join(attachmentsDir, `pending-${uuid}.png`); const pendingFilePath = NodePath.join(attachmentsDir, `pending-${uuid}-pdf.pdf`); + const sidecarPath = `${pendingFilePath}.txt`; const threadPath = NodePath.join(attachmentsDir, `thread-1-${uuid}.png`); const partialPath = NodePath.join(attachmentsDir, `${uuid}.part`); - for (const filePath of [pendingPath, pendingFilePath, threadPath, partialPath]) { + for (const filePath of [pendingPath, pendingFilePath, sidecarPath, threadPath, partialPath]) { NodeFS.writeFileSync(filePath, Buffer.from("pixels")); NodeFS.utimesSync(filePath, oldTimeSeconds, oldTimeSeconds); } - expect(sweepStalePendingAttachments({ attachmentsDir, nowMs: now })).toEqual({ deleted: 3 }); + expect(sweepStalePendingAttachments({ attachmentsDir, nowMs: now })).toEqual({ deleted: 4 }); expect(NodeFS.existsSync(pendingPath)).toBe(false); expect(NodeFS.existsSync(pendingFilePath)).toBe(false); + expect(NodeFS.existsSync(sidecarPath)).toBe(false); expect(NodeFS.existsSync(partialPath)).toBe(false); expect(NodeFS.existsSync(threadPath)).toBe(true); } finally { diff --git a/apps/server/src/attachmentStore.ts b/apps/server/src/attachmentStore.ts index 9e708da15a88..0ce0a4874a12 100644 --- a/apps/server/src/attachmentStore.ts +++ b/apps/server/src/attachmentStore.ts @@ -43,8 +43,7 @@ export function toSafeThreadAttachmentSegment(threadId: string): string | null { export function attachmentFileExtension(fileName: string): string { const extension = NodePath.extname(fileName).toLowerCase(); - // ".part" is reserved for in-flight uploads; a stored "archive.part" would - // look stale to sweepStalePendingAttachments and get deleted. + // .part is reserved for pending uploads; the stale-upload sweep would delete stored files. if (extension === ".part" || !/^\.[a-z0-9]{1,10}$/.test(extension)) { return ".bin"; } @@ -103,12 +102,7 @@ export function createDeterministicAttachmentId( return `${threadSegment}-${uuid}`; } -/** - * A retry resends the same bytes and keeps its id. Editing a queued message - * reuses the message id, so the bytes also stop a new image from taking an - * earlier image's id and file. Clients cache attachment images by id, which - * relies on ids never changing bytes. - */ +/** Retries keep ids; changed bytes must get new ids because clients cache images by id. */ export function chatAttachmentStableKey( messageId: string, index: number, @@ -145,10 +139,7 @@ function legacyAttachmentRelativePath(attachmentId: string): string | null { : null; } -/** - * Only ids minted for this thread count, so deleting a fork never removes its - * source's pages. - */ +/** Only this thread's ids qualify, preserving source pages when deleting a fork. */ export function threadHtmlRenderAttachmentIds( threadId: string, items: Iterable<{ readonly toolName: string | null | undefined; readonly output?: unknown }>, @@ -303,7 +294,9 @@ export function sweepStalePendingAttachments(input: { for (const entry of entries) { const isPartial = entry.endsWith(".part"); if (!isPartial) { - const attachmentId = parseAttachmentIdFromRelativePath(entry); + const attachmentId = parseAttachmentIdFromRelativePath( + entry.endsWith(".txt") && entry.slice(0, -4).includes(".") ? entry.slice(0, -4) : entry, + ); if ( !attachmentId || parseThreadSegmentFromAttachmentId(attachmentId) !== PENDING_ATTACHMENT_THREAD_SEGMENT diff --git a/apps/server/src/auth/ServerSecretStore.ts b/apps/server/src/auth/ServerSecretStore.ts index 020413ee8cd0..100e0e19ca2a 100644 --- a/apps/server/src/auth/ServerSecretStore.ts +++ b/apps/server/src/auth/ServerSecretStore.ts @@ -150,7 +150,7 @@ export class ServerSecretStore extends Context.Service< } >()("t3/auth/ServerSecretStore") {} -/** @public Service construction is part of the canonical Effect module API. */ +/** @public */ export const make = Effect.gen(function* () { const crypto = yield* Crypto.Crypto; const fileSystem = yield* FileSystem.FileSystem; diff --git a/apps/server/src/checkpointing/CheckpointDiffQuery.ts b/apps/server/src/checkpointing/CheckpointDiffQuery.ts index 05f80d0e46d3..fb4513c2a16f 100644 --- a/apps/server/src/checkpointing/CheckpointDiffQuery.ts +++ b/apps/server/src/checkpointing/CheckpointDiffQuery.ts @@ -80,7 +80,7 @@ function buildTurnDiffResult( }; } -/** @public Service construction is part of the canonical Effect module API. */ +/** @public */ export const make = Effect.gen(function* () { const threads = yield* ThreadManagement.ThreadManagementService; const checkpointStore = yield* CheckpointStore.CheckpointStore; diff --git a/apps/server/src/checkpointing/CheckpointStore.ts b/apps/server/src/checkpointing/CheckpointStore.ts index 33c268891505..3c7e8183e12d 100644 --- a/apps/server/src/checkpointing/CheckpointStore.ts +++ b/apps/server/src/checkpointing/CheckpointStore.ts @@ -63,10 +63,7 @@ export class CheckpointStore extends Context.Service< input: DiffCheckpointsInput, ) => Effect.Effect; - /** - * Returns null when HEAD did not move or a checkpoint does not record HEAD. - * Then every changed path belongs to the turn. - */ + /** Null means HEAD is unchanged or unrecorded; all changed paths then belong to the turn. */ readonly listAuthoredPaths: ( input: ListAuthoredPathsInput, ) => Effect.Effect | null, CheckpointStoreError>; @@ -77,7 +74,7 @@ export class CheckpointStore extends Context.Service< } >()("t3/checkpointing/CheckpointStore") {} -/** @public Service construction is part of the canonical Effect module API. */ +/** @public */ export const make = Effect.gen(function* () { const vcsRegistry = yield* VcsDriverRegistry.VcsDriverRegistry; diff --git a/apps/server/src/checkpointing/Diffs.ts b/apps/server/src/checkpointing/Diffs.ts index e45a0ab63c46..c832b62d13ba 100644 --- a/apps/server/src/checkpointing/Diffs.ts +++ b/apps/server/src/checkpointing/Diffs.ts @@ -17,7 +17,6 @@ export function parseTurnDiffFilesFromNumstat(numstat: string): ReadonlyArray; -}): string | undefined { - const worktreeCwd = input.thread.worktreePath ?? undefined; - if (worktreeCwd) { - return worktreeCwd; - } - - return input.projects.find((project) => project.id === input.thread.projectId)?.workspaceRoot; -} diff --git a/apps/server/src/coderAssets.test.ts b/apps/server/src/coderAssets.test.ts index c4b8d147b250..a81a62813226 100644 --- a/apps/server/src/coderAssets.test.ts +++ b/apps/server/src/coderAssets.test.ts @@ -15,6 +15,7 @@ import { makeServeAsset } from "./coderAssets.ts"; import * as ServerConfig from "./config.ts"; import * as Orchestrator from "./orchestration-v2/Orchestrator.ts"; import * as ProjectFaviconResolver from "./project/ProjectFaviconResolver.ts"; +import * as DocumentText from "./provider/DocumentText.ts"; import * as T3ProjectFileLoader from "./project/T3ProjectFileLoader.ts"; import * as WorkspacePaths from "./workspace/WorkspacePaths.ts"; @@ -23,6 +24,7 @@ const layerConfig = ServerConfig.ServerConfig.layerTest(process.cwd(), { }); const layerTest = Layer.mergeAll( Layer.mock(Orchestrator.OrchestratorV2)({ getTurnItem: () => Effect.succeed(null) }), + Layer.mock(DocumentText.DocumentText)({ ensure: () => Effect.succeed(undefined) }), layerConfig, WorkspacePaths.layer, ServerSecretStore.layer.pipe(Layer.provide(layerConfig)), diff --git a/apps/server/src/coderAssets.ts b/apps/server/src/coderAssets.ts index ef558bf71cba..bbe963532eed 100644 --- a/apps/server/src/coderAssets.ts +++ b/apps/server/src/coderAssets.ts @@ -36,6 +36,7 @@ import { layerAssetRoute, layerAttachmentUploadRoute } from "./http.ts"; import type * as Orchestrator from "./orchestration-v2/Orchestrator.ts"; import type * as ProjectFaviconResolver from "./project/ProjectFaviconResolver.ts"; import type * as WorkspacePaths from "./workspace/WorkspacePaths.ts"; +import type * as DocumentText from "./provider/DocumentText.ts"; const ASSET_REQUEST_ORIGIN = "http://helper.invalid"; /** How long a body may wait for its request, or a finished body's id is remembered. */ @@ -125,6 +126,7 @@ export const makeServeAsset = Effect.gen(function* () { | ServerSecretStore.ServerSecretStore | ProjectFaviconResolver.ProjectFaviconResolver | WorkspacePaths.WorkspacePaths + | DocumentText.DocumentText >(); const platform = yield* Layer.build(NodeHttpPlatform.layer); const serve = (input: CoderServeAssetInput): Stream.Stream => diff --git a/apps/server/src/coderEnvironment.ts b/apps/server/src/coderEnvironment.ts index 5611ed3155a2..72d18fedae3a 100644 --- a/apps/server/src/coderEnvironment.ts +++ b/apps/server/src/coderEnvironment.ts @@ -58,6 +58,7 @@ export const CODER_ENVIRONMENT_CAPABILITIES = { projectProviderInstanceOverrides: true, threadSnooze: true, environmentThemes: true, + providerWorkspaceSnapshotsOnDemand: true, threadPinning: true, threadPinReorder: true, threadActiveReorder: true, diff --git a/apps/server/src/coderServer.ts b/apps/server/src/coderServer.ts index 83ffff4c66ab..0692fb5b0173 100644 --- a/apps/server/src/coderServer.ts +++ b/apps/server/src/coderServer.ts @@ -1,6 +1,7 @@ // Coder: the helper's runtime layer. It composes the layers `server.ts` carries from upstream with // the Coder provider, orchestration, startup, and RPC wiring that replace upstream's HTTP server. import * as ModelManifest from "./provider/ModelManifest.ts"; +import * as DocumentText from "./provider/DocumentText.ts"; import * as ProviderMaintenanceRunner from "./provider/providerMaintenanceRunner.ts"; import { FetchHttpClient } from "effect/http"; import * as StorageCleanup from "./storageCleanup.ts"; @@ -334,6 +335,9 @@ export const makeCoderRuntimeLayer = () => { Layer.provideMerge(VcsProcess.layer), // Every agent and terminal spawn reads this, so it sits below everything. Layer.provideMerge(AgentScopeLive.layer), + // As upstream's platform services: attachment uploads, agent attachments, and the project + // toolkit read office documents' extracted text. + Layer.provideMerge(DocumentText.layer), ); // Upstream provides the agent-session scanner beside its WS layer; the helper RPC layer // takes it here. diff --git a/apps/server/src/coderWs.ts b/apps/server/src/coderWs.ts index 00f03663b929..df9f1a86e85d 100644 --- a/apps/server/src/coderWs.ts +++ b/apps/server/src/coderWs.ts @@ -131,7 +131,6 @@ export const makeHandlers = ({ serverCommandId, refreshGitStatus }: UpstreamHelp historyCursor: bounded.historyCursor, hasMoreHistory: bounded.hasMoreHistory, latestLocalTurnOrdinal: bounded.latestLocalTurnOrdinal, - payloadBudgetExceeded: bounded.payloadBudgetExceeded, }; if (exceedsHelperStdioFrame(result)) { return yield* new OrchestrationV2GetThreadProjectionError({ diff --git a/apps/server/src/environmentTheme.ts b/apps/server/src/environmentTheme.ts index 3554739c159e..c070a1b46cac 100644 --- a/apps/server/src/environmentTheme.ts +++ b/apps/server/src/environmentTheme.ts @@ -40,7 +40,7 @@ const isEnvironmentThemeId = Schema.is(EnvironmentThemeId); const THEME_FILE_SUFFIX = ".json"; const MAX_THEME_FILES = 32; -/** Exported so the publish path cannot accept a file the watcher will skip. */ +/** Shared with publishing so it cannot accept files the watcher skips. */ export const MAX_THEME_FILE_BYTES = 32 * 1024; const MAX_THEME_TOTAL_BYTES = 192 * 1024; @@ -54,10 +54,7 @@ export class EnvironmentThemeService extends Context.Service< { readonly current: Effect.Effect>; - /** - * The subscription is acquired before the current set is read, so a publish - * landing while a client connects is delivered rather than lost. - */ + /** Subscribes before reading to retain changes during connection. */ readonly streamChanges: Stream.Stream>; } >()("t3/environmentTheme/EnvironmentThemeService") {} @@ -90,13 +87,10 @@ export const readPublishedThemes = Effect.fn(function* (themesDir: string) { for (const entry of entries.toSorted()) { if (!entry.endsWith(THEME_FILE_SUFFIX)) continue; const id = entry.slice(0, -THEME_FILE_SUFFIX.length); - // A reserved id is either shadowed by a built-in on the client or captures - // clients that never chose it, so it is not publishable. + // Reserved ids shadow built-ins or capture clients that never chose them. if (!isEnvironmentThemeId(id) || UNPUBLISHABLE_THEME_IDS.has(id)) continue; - // Counts files examined, not themes accepted: capping the output would - // let a directory of malformed files be opened, read, and decoded in full - // on every refresh and every client connect. + // Cap examined files, not accepted themes, to bound work on malformed directories. examined += 1; if (examined > MAX_THEME_FILES) { yield* Effect.logWarning( @@ -146,21 +140,10 @@ export const readPublishedThemes = Effect.fn(function* (themesDir: string) { const make = Effect.gen(function* () { const { environmentThemesDir } = yield* ServerConfig.ServerConfig; const fs = yield* FileSystem.FileSystem; - /** - * Every observed set carries a sequence number, so a subscriber can drop - * queued events that predate the snapshot it started from. Without it a - * publish landing between subscribing and reading replays after the newer - * value and walks clients backwards onto stale colors. - */ + // Sequence numbers let subscribers drop queued events older than their initial snapshot. const changes = yield* PubSub.sliding(1); const published = yield* Ref.make({ seq: 0, themes: [] }); - /** - * Guards the whole read/compare/publish, not just the state update. The - * directory read is async, so two concurrent refreshes can finish out of - * order and a slower read of an older set would publish under a higher - * sequence -- which the subscriber filter, ordering publications rather than - * observations, could not then drop. - */ + // Serialize read/compare/publish so slow older reads cannot publish with newer sequence numbers. const refreshSemaphore = yield* Semaphore.make(1); const watcherScope = yield* Scope.make("sequential"); yield* Effect.addFinalizer(() => Scope.close(watcherScope, Exit.void)); @@ -170,8 +153,7 @@ const make = Effect.gen(function* () { const themes = yield* readPublishedThemes(environmentThemesDir).pipe( Effect.provideService(FileSystem.FileSystem, fs), ); - // Structural equality over the whole decoded value: a hand-rolled field - // list here silently drops republishes for any field it forgets. + // Compare the whole value so new fields cannot silently miss republishes. const [changed, next] = yield* Ref.modify( published, (previous): readonly [readonly [boolean, PublishedThemes], PublishedThemes] => { @@ -185,8 +167,6 @@ const make = Effect.gen(function* () { }), ); - // The directory is created up front so the watcher has something to attach - // to before the first publisher writes into it. yield* fs.makeDirectory(environmentThemesDir, { recursive: true }).pipe(Effect.ignoreCause()); const watchable = yield* Effect.sync(() => { @@ -200,9 +180,7 @@ const make = Effect.gen(function* () { } }); - // Debounced for the same reason settings watching is: a theme script emits - // several events per save and `fs.watch` can fire before the content is - // flushed. + // A save emits several events, and fs.watch can fire before content is flushed. const watchEvents = watchable ? fs.watch(environmentThemesDir).pipe(Stream.debounce(Duration.millis(100))) : Stream.empty; diff --git a/apps/server/src/git/GitManager.test.ts b/apps/server/src/git/GitManager.test.ts index 370863026e83..56a7c4111181 100644 --- a/apps/server/src/git/GitManager.test.ts +++ b/apps/server/src/git/GitManager.test.ts @@ -209,10 +209,7 @@ function initRepo( yield* runGit(cwd, ["init", "--initial-branch=main"]); yield* runGit(cwd, ["config", "user.email", "test@example.com"]); yield* runGit(cwd, ["config", "user.name", "Test User"]); - // Hermetic line endings: machine-global `core.autocrlf=true` on Windows - // makes Git check out CRLF, which breaks exact-content assertions that - // write LF with Node. Local config overrides the global so the fixture - // (and its linked worktrees, which share config) always checks out LF. + // Override global autocrlf on Windows for LF assertions, including linked worktrees. yield* runGit(cwd, ["config", "core.autocrlf", "false"]); yield* fs.writeFileString(NodePath.join(cwd, "README.md"), "hello\n"); yield* runGit(cwd, ["add", "README.md"]); @@ -1604,9 +1601,7 @@ it.layer(layerGitManagerTest)("GitManager", (it) => { yield* runGit(repoDir, ["checkout", "-b", "feature/merged-branch-deleted"]); yield* runGit(repoDir, ["push", "-u", "origin", "feature/merged-branch-deleted"]); - // GitHub commonly deletes a pull request's head branch after merge. Git - // removes the remote-tracking ref, but preserves the local branch's - // remote and merge configuration as evidence that it was published. + // Deleting a merged head removes its tracking ref but leaves remote/merge config proving publication. yield* runGit(repoDir, ["push", "origin", "--delete", "feature/merged-branch-deleted"]); const configuredRemote = yield* runGit(repoDir, [ "config", @@ -3668,7 +3663,7 @@ it.layer(layerGitManagerTest)("GitManager", (it) => { yield* initRepo(repoDir); const forkDir = yield* createBareRemote(); yield* runGit(repoDir, ["remote", "add", "fork-seed", forkDir]); - // Origin is the base repository; a lone remote would be the base itself. + // A lone remote would be treated as the base repository. yield* runGit(repoDir, [ "remote", "add", diff --git a/apps/server/src/git/GitManager.ts b/apps/server/src/git/GitManager.ts index 4afab2478db1..df315a913e58 100644 --- a/apps/server/src/git/GitManager.ts +++ b/apps/server/src/git/GitManager.ts @@ -133,10 +133,7 @@ export class GitManager extends Context.Service< input: VcsStatusInput, options?: GitRemoteStatusOptions, ) => Effect.Effect; - /** - * `refresh` asks the host again. `freshGitState` rereads the branch's git - * state without forcing a host refresh; use it before persisting a result. - */ + /** refresh queries the host; freshGitState rereads only git state and is required before persisting. */ readonly branchPullRequest: ( input: { readonly cwd: string; readonly branch: string }, options?: { readonly refresh?: boolean; readonly freshGitState?: boolean }, @@ -179,11 +176,8 @@ const PR_THREAD_COMMAND_OUTPUT_CHARS = 2_000; const isSourceControlProviderError = Schema.is(SourceControlProviderError); /** - * A hosting provider rejects a throttled request immediately, so caching every - * failure for a flat 20s made a rate-limited poller re-ask *faster* than a - * healthy one does (which waits PR_LOOKUP_CACHE_TTL), turning a transient 429 - * into sustained pressure. Backing off per branch keeps the retry rate below - * the healthy rate once a branch has failed more than a couple of times. + * Throttled lookups fail fast, so a flat failure TTL would retry faster than a healthy + * lookup refreshes. Backing off per branch keeps a 429 from becoming sustained pressure. */ function prLookupFailureTtl(consecutiveFailures: number): Duration.Duration { const exponent = Math.max(0, consecutiveFailures - 1); @@ -1001,11 +995,9 @@ export const make = Effect.gen(function* () { if (repositoryNameWithOwner.length === 0) { yield* fetchChangeRequestHeadIntoBranch(cwd, pullRequest.number, localBranch).pipe( - // Without the host's head ref, a head in the same repository is a branch on the - // primary remote, so it is fetched by name instead, but only when that remote is the - // pull request's own repository: a host can find a change request by number outside - // the checkout's repository. Only while it is open, too: the branch of a closed one - // may have moved on past the head it was closed with. + // Fall back to a branch fetch only for an open PR in the primary remote's repository. + // A host can resolve a PR number from another repository, so verify the repository first. + // Closed PR branches may have moved past the head recorded at closure. Effect.catch((cause) => pullRequest.isCrossRepository === true || pullRequest.state !== "open" ? Effect.fail(cause) @@ -1282,11 +1274,7 @@ export const make = Effect.gen(function* () { return null; } - // Both sides must be resolved before treating a mismatch as real: - // `readConfigValueNullable` swallows any git-config read failure into - // `null`, so a transient failure to resolve the *current* remote URL must - // read as "unknown", not as "no remote" — the latter would otherwise drop - // an already-known PR badge on every hiccup. + // Config read failures become null; require both remote URLs before treating a mismatch as real. if (lastKnown.headRemoteUrlKey !== null && current.headRemoteUrlKey !== null) { return lastKnown.headRemoteUrlKey === current.headRemoteUrlKey ? lastKnown.pr : null; } @@ -1311,8 +1299,7 @@ export const make = Effect.gen(function* () { }, refreshMissingPullRequest = false, ) { - // Keyed by (cwd, branch) only: the upstream ref changing (e.g. a first - // `push -u`) must not orphan the fallback value for the same branch. + // Upstream changes must not orphan the same branch's fallback value. const branchKey = `${cwd}\u0000${details.branch}`; const cacheKey = prLookupCacheKey(cwd, details); if (refreshMissingPullRequest) { @@ -1614,8 +1601,7 @@ export const make = Effect.gen(function* () { } satisfies BranchHeadContext; }); - // Remote names may contain slashes, so refs are matched literally per remote - // instead of with a glob. + // Remote names can contain slashes; match refs literally. const findRemoteTrackingRemote = Effect.fn("findRemoteTrackingRemote")(function* ( cwd: string, branch: string, @@ -1714,11 +1700,7 @@ export const make = Effect.gen(function* () { }) .pipe( Effect.flatMap((result) => { - // `merge-base --is-ancestor` reports 0 for ancestor and 1 - // for not-ancestor; any other non-zero exit is a failed - // probe, not a negative result. A failed probe rejects the - // whole inference instead of letting a surviving probe win - // by default and attach the wrong fork's PR. + // Exit 1 means non-ancestor; other failures must reject inference rather than pick another fork. if (result.exitCode === 0) return Effect.succeed(remote.name); if (result.exitCode === 1) return Effect.succeed(null); return new GitCommandError({ @@ -1741,10 +1723,7 @@ export const make = Effect.gen(function* () { return remoteUrl === null ? null : remoteName; }); - // `git worktree add -b feature origin/main` makes the new local branch track - // origin/main. That upstream is the branch's base, not its published PR head. - // Looking up PRs for it can attach an old reverse merge from main and - // auto-settle an unrelated feature thread. + // worktree add can set origin/main as upstream; this base is not the branch's published PR head. const resolveLookupHeadContext = Effect.fn("resolveLookupHeadContext")(function* ( cwd: string, details: { @@ -2389,9 +2368,7 @@ export const make = Effect.gen(function* () { }); return mergeGitStatusParts(local, remote); }); - // A merged or closed answer can settle a thread or remove its worktree, so - // branchPullRequest confirms one against fresh reads, and discovery asks for - // fresh reads (`freshGitState`) before it persists an answer. + // Closed PR results can settle threads or remove worktrees; confirm fresh git state before persisting. const prLookupGitStateByKey = new Map< string, { readonly readAtMs: number; readonly value: string | null } @@ -2587,9 +2564,7 @@ export const make = Effect.gen(function* () { if (Option.isSome(cached)) yield* Cache.invalidate(prLookupCache, cacheKey); } let cached = yield* getPrLookup(cacheKey); - // The cached head context may have resolved on a different remote than - // the saved upstream: a branch tracking origin/main but pushed to a fork - // is looked up on the fork. Verify against the remote the lookup used. + // Lookup may use a fork instead of the saved upstream; verify the remote actually queried. const identityRemoteName = (headContext: BranchHeadContext) => headContext.remoteName ?? remoteName ?? undefined; const currentIdentity = yield* resolvePrLookupRepositoryIdentity( @@ -2647,8 +2622,7 @@ export const make = Effect.gen(function* () { closedAt: latest.closedAt ?? null, mergedAt: latest.mergedAt ?? null, headSha: latest.headSha ?? null, - // Hosting CLIs can select an upstream repository instead of origin. - // The returned PR URL names the repository that actually owns it. + // Hosting CLIs may target upstream; the PR URL identifies its actual repository. repositoryKey: pullRequestRepositoryKey(latest.url), }; }); @@ -2810,17 +2784,13 @@ export const make = Effect.gen(function* () { const localPullRequestBranch = resolvePullRequestWorktreeLocalBranchName(pullRequestWithRemoteInfo); - // Git refuses to move a branch that is checked out in a worktree, so the - // reuse paths cannot go through materializePullRequestHeadBranch and instead - // advance the checkout from inside the worktree. + // Git refuses to move checked-out branches; advance reused checkouts from inside their worktree. const reuseExistingWorktree = Effect.fn("reuseExistingWorktree")(function* ( worktreePath: string, checkedOutBranch: string, ) { if (checkedOutBranch !== localPullRequestBranch) { - // findLocalHeadBranch also accepts a branch that merely shares the head's bare name — - // a fork PR opened from "main" matches the user's own local main. That checkout is - // somebody else's work, so it keeps its tracking config and nothing else. + // Bare-name matches may be unrelated local branches; preserve their work and tracking config. return { pullRequest, branch: localPullRequestBranch, @@ -2829,9 +2799,7 @@ export const make = Effect.gen(function* () { }; } - // Read before ensureExistingWorktreeUpstream: it force-updates the remote-tracking ref, - // and once that has jumped to a rewritten head there is no way left to tell a checkout - // that holds nothing of its own from one carrying local commits. + // Capture upstream before its forced fetch so rewritten heads remain distinguishable from local commits. const upstreamCommitBeforeFetch = yield* gitCore .resolveCommit({ cwd: worktreePath, revision: "@{upstream}" }) .pipe( @@ -2888,8 +2856,7 @@ export const make = Effect.gen(function* () { ), ); - // Only when the checkout actually moved: another thread may be running in this worktree, - // and re-running the setup script under it buys nothing when the code did not change. + // Setup can disrupt other threads using this worktree; rerun only when the checkout moved. if (refreshed.moved) { yield* maybeRunSetupScript(worktreePath); } diff --git a/apps/server/src/git/GitWorkflowService.ts b/apps/server/src/git/GitWorkflowService.ts index a7e78c9235da..21d111956a5e 100644 --- a/apps/server/src/git/GitWorkflowService.ts +++ b/apps/server/src/git/GitWorkflowService.ts @@ -333,7 +333,7 @@ export const make = Effect.gen(function* () { Effect.map((result) => result.exitCode === 0), ), hasRefNamed: (input) => - // No ref name holds a pattern character, and one here would widen the match. + // Pattern characters would widen the ref match. /[*?[\\]/.test(input.refName) ? Effect.succeed(false) : ensureGitCommand("GitWorkflowService.hasRefNamed", input.cwd).pipe( diff --git a/apps/server/src/htmlRender/HtmlRender.ts b/apps/server/src/htmlRender/HtmlRender.ts index c866481d338d..fa1e9a59870f 100644 --- a/apps/server/src/htmlRender/HtmlRender.ts +++ b/apps/server/src/htmlRender/HtmlRender.ts @@ -64,11 +64,8 @@ export class HtmlRenderViewportHeightError extends Schema.TaggedError|$)|]*(?:>|$)([\s\S]*?)(?:<\/style\s*>|$)|\sstyle\s*=\s*(?:"([^"]*)(?:"|$)|'([^']*)(?:'|$)|([^\s>]+))/gi; // The lookbehind skips base64 runs, such as an inlined image that happens to contain "1vh". @@ -146,10 +143,7 @@ const dataUriPrefix = (path: string) => const latin1 = (bytes: Uint8Array, start: number, end: number) => String.fromCharCode(...bytes.subarray(start, end)); -/** - * Whether file bytes are an image, whatever the file is named, so a symlink or - * renamed file cannot carry other data, such as a secret, into a page. - */ +// Validate bytes, not filenames, so renamed files or symlinks cannot inline secrets. const isImageBytes = (bytes: Uint8Array) => { const head = latin1(bytes, 0, 12); if ( @@ -237,8 +231,7 @@ const inlineLocalImages = Effect.fn("HtmlRender.inlineLocalImages")(function* (h if (pageBytes > MAX_PAGE_BYTES) { return yield* new HtmlRenderPageTooLargeError({ sizeBytes: pageBytes }); } - // Files can grow after `stat`. Each read stops one byte past the image - // limit, and reading stops once the images read so far cannot fit the page. + // Files can grow after stat; bound each read and stop when accumulated images cannot fit. let readBytes = 0; const images = yield* Effect.forEach( files.filter((file) => file.size !== undefined), diff --git a/apps/server/src/http.test.ts b/apps/server/src/http.test.ts index 0a41e756d8ef..43e64b4b49c5 100644 --- a/apps/server/src/http.test.ts +++ b/apps/server/src/http.test.ts @@ -281,9 +281,22 @@ describe("video asset byte ranges", () => { const image = HttpServerResponse.toWeb( yield* assetFileResponse({ path: file, mimeType: "image/png" }, "bytes=0-1"), ); - expect(image.status).toBe(200); - expect(image.headers.has("accept-ranges")).toBe(false); - expect(yield* Effect.promise(() => image.text())).toBe("0123456789"); + expect(image.status).toBe(206); + expect(image.headers.get("accept-ranges")).toBe("bytes"); + expect(yield* Effect.promise(() => image.text())).toBe("01"); + for (const mimeType of ["text/plain", "text/csv", "application/octet-stream"]) { + for (const [range, expected, status] of [ + ["bytes=2-5", "2345", 206], + ["bytes=-3", "789", 206], + ["bytes=10-", "", 416], + ] as const) { + const result = HttpServerResponse.toWeb( + yield* assetFileResponse({ path: file, mimeType }, range), + ); + expect(result.status).toBe(status); + expect(yield* Effect.promise(() => result.text())).toBe(expected); + } + } }).pipe(Effect.provide(layerFileResponse)), ); diff --git a/apps/server/src/http.ts b/apps/server/src/http.ts index 9b4548283bf4..a6193987776a 100644 --- a/apps/server/src/http.ts +++ b/apps/server/src/http.ts @@ -18,15 +18,12 @@ import { } from "./assets/AttachmentUpload.ts"; const SVG_CONTENT_SECURITY_POLICY = "default-src 'none'; style-src 'unsafe-inline'; sandbox"; -// The sandbox gives the document an opaque origin: scripts run, but same-origin -// cookies, storage, and API calls are out of reach. No modals: agent HTML can -// open without a click (inline renders, and mobile loads it as the top document), -// and must not raise blocking dialogs. +// Opaque origin blocks cookies, storage and API access while allowing scripts. +// Agent HTML can open without a click, so disallow blocking modals. const HTML_CONTENT_SECURITY_POLICY = "sandbox allow-scripts allow-forms allow-popups allow-downloads"; -// Types a browser may render as a document if a proxy strips the disposition -// header. Downloads of these fall back to octet-stream. +// Proxies may strip disposition; force document-capable downloads to octet-stream. const DOWNLOAD_MIME_TYPE_PATTERN = /^[\w!#$&^.+-]+\/[\w!#$&^.+-]+$/; const isSafeDownloadMimeType = (mimeType: string): boolean => DOWNLOAD_MIME_TYPE_PATTERN.test(mimeType) && @@ -156,15 +153,13 @@ export const assetFileResponse = Effect.fn("assetFileResponse")(function* ( const mediaInfo = mediaFile ? yield* statMediaFile(asset.path, mediaFile) : undefined; const isMedia = /^(?:audio|video)\//i.test(headers["Content-Type"] ?? ""); if (isMedia) { - // Do not invite conditional range requests with validators that cannot establish - // byte-for-byte identity. Attachment media carries no `file`, and must not - // outlive the signed URL that granted it either. + // Without file identity, validators cannot guarantee byte equality; attachments must also expire with their URL. headers["Cache-Control"] = "private, no-store"; } let status = 200; let offset = 0n; let bytesToRead: bigint | undefined; - if (isMedia) { + { headers["Accept-Ranges"] = "bytes"; // If-Range requires a matching validator. A full response is safe when we cannot validate it. if (method === "GET" && rangeHeader && ifRangeHeader === undefined) { diff --git a/apps/server/src/imageMime.ts b/apps/server/src/imageMime.ts index b180dda51a09..338bbd62ce59 100644 --- a/apps/server/src/imageMime.ts +++ b/apps/server/src/imageMime.ts @@ -45,10 +45,7 @@ function isBase64Whitespace(code: number): boolean { return code === 0x0d || code === 0x0a || code === 0x20; } -// Data URLs carry the full image payload, so this parser must never run a -// regex across the payload: V8's regex engine borrows the JS call stack, and -// matching a multi-megabyte string from a deep call stack (e.g. inside fiber -// execution) throws "Maximum call stack size exceeded". +// V8 regexes use the JS stack; scanning image payloads can overflow a deep fiber stack. export function parseBase64DataUrl( dataUrl: string, ): { readonly mimeType: string; readonly base64: string } | null { @@ -100,8 +97,7 @@ export function parseBase64DataUrl( if (base64.length === 0 || base64.length % 4 !== 0) return null; const firstPad = base64.indexOf("="); if (firstPad !== -1) { - // '=' is only valid as one or two trailing padding characters; Node's - // decoder would otherwise silently truncate at the first '='. + // Node silently truncates at the first =; allow only one or two trailing padding characters. if (base64.length - firstPad > 2) return null; for (let index = firstPad; index < base64.length; index += 1) { if (base64.charCodeAt(index) !== 0x3d) return null; diff --git a/apps/server/src/keybindings.ts b/apps/server/src/keybindings.ts index 7fbfdbe9e651..bf87ee2eabc0 100644 --- a/apps/server/src/keybindings.ts +++ b/apps/server/src/keybindings.ts @@ -548,8 +548,7 @@ const make = Effect.gen(function* () { } } - // Keep only defaults that fit, before persisted rules so existing - // custom shortcuts retain priority when their conditions overlap. + // Put defaults before persisted rules so custom shortcuts win overlapping conditions. const availableSlots = Math.max(0, MAX_KEYBINDINGS_COUNT - customConfig.length); const defaultsToAppend = missingDefaults.slice(0, availableSlots); const skippedDefaults = missingDefaults.slice(availableSlots); @@ -594,8 +593,7 @@ const make = Effect.gen(function* () { const revalidateAndEmitSafely = revalidateAndEmit.pipe(Effect.ignoreCause({ log: true })); - // Editors emit multiple events per save (truncate, write, rename) and - // `fs.watch` can fire before the content has been flushed to disk. + // Saves emit multiple events; fs.watch may fire before content is flushed. const debouncedKeybindingsEvents = fs.watch(keybindingsConfigDir).pipe( Stream.filter((event) => { return ( diff --git a/apps/server/src/mcp/McpInvocationContext.ts b/apps/server/src/mcp/McpInvocationContext.ts index f830da3d4a03..cc45af6bae4d 100644 --- a/apps/server/src/mcp/McpInvocationContext.ts +++ b/apps/server/src/mcp/McpInvocationContext.ts @@ -10,6 +10,7 @@ import { } from "@t3tools/contracts"; import * as Context from "effect/Context"; import * as Effect from "effect/Effect"; +import * as Fiber from "effect/Fiber"; const ALL_MCP_CAPABILITIES = [ "preview", @@ -24,6 +25,8 @@ export interface McpThreadCaller { readonly threadId: ThreadId; readonly providerSessionId: string; readonly providerInstanceId: ProviderInstanceId; + /** A delegated child task; its parent owns the work it hands back. */ + readonly subagent?: boolean; } export interface McpClientCaller { @@ -32,18 +35,11 @@ export interface McpClientCaller { readonly access: AuthMcpClientAccess; } -/** - * A read-only client never reaches a write (`McpToolAccess` refuses it first), - * so it maps to the lowest mode rather than to nothing. - */ +/** Read-only clients map to the lowest mode because McpToolAccess blocks writes first. */ export const clientRuntimeModeCeiling = (client: McpClientCaller | undefined): RuntimeMode => client === undefined || client.access === "read-only" ? "approval-required" : client.access; -/** - * A thread caller's omitted target falls back to its own thread; a client - * caller has no own thread, so tools that act as the caller (delegate_task, - * preview, worktree handoff) need `thread`. - */ +/** Thread callers default to their own thread; client callers need thread for tools that act as the caller. */ export interface McpInvocationScope { readonly environmentId: EnvironmentId; readonly capabilities: ReadonlySet; @@ -58,6 +54,16 @@ export class McpInvocationContext extends Context.Service< McpInvocationScope >()("t3/mcp/McpInvocationContext") {} +/** + * `McpSchema.EnabledWhen` for tools a delegated child must not see or call. + * EnabledWhen only receives the MCP client profile, so read the T3 caller from the request fiber. + */ +export const hiddenFromSubagents = (): boolean => { + const fiber = Fiber.getCurrent(); + const scope = fiber && Context.getOrUndefined(fiber.context, McpInvocationContext); + return scope?.thread?.subagent !== true; +}; + export type McpCapabilityError = C extends "preview" ? PreviewAutomationUnavailableError : McpCapabilityUnavailableError; diff --git a/apps/server/src/mcp/McpSessionRegistry.testkit.ts b/apps/server/src/mcp/McpSessionRegistry.testkit.ts index 1ac02357a878..84676e7ccb1b 100644 --- a/apps/server/src/mcp/McpSessionRegistry.testkit.ts +++ b/apps/server/src/mcp/McpSessionRegistry.testkit.ts @@ -19,6 +19,7 @@ export const layer = Layer.succeed( browserToolsAvailable: true, }, }), + // @effect-diagnostics-next-line effectSucceedWithVoid:off - This optional result requires undefined, not the wider void type. resolve: () => Effect.succeed(undefined), touch: () => Effect.void, revokeProviderSession: () => Effect.void, diff --git a/apps/server/src/mcp/McpSessionRegistry.ts b/apps/server/src/mcp/McpSessionRegistry.ts index 23b30a8ccd03..7b9605efcde9 100644 --- a/apps/server/src/mcp/McpSessionRegistry.ts +++ b/apps/server/src/mcp/McpSessionRegistry.ts @@ -33,6 +33,7 @@ export interface McpCredentialRequest { readonly providerInstanceId: ProviderInstanceId; readonly browserToolsAvailable?: boolean; readonly capabilities?: ReadonlySet; + readonly subagent?: boolean; } export interface McpIssuedCredential { @@ -133,6 +134,7 @@ const makeWithOptions = Effect.fn("McpSessionRegistry.make")(function* ( threadId: ThreadId.make(request.threadId), providerSessionId, providerInstanceId: ProviderInstanceId.make(request.providerInstanceId), + subagent: request.subagent === true, }, client: undefined, // Upstream's scope, so credential reuse sees the capabilities it compares. Preview and @@ -262,4 +264,6 @@ export const layer = Layer.effect(McpSessionRegistry, make); export const touchActiveMcpThread = (threadId: ThreadId): Effect.Effect => activeMcpSessionRegistry ? activeMcpSessionRegistry.touch(threadId) : Effect.void; -export const __testing = { make: makeWithOptions }; +export const __testing = { + make: makeWithOptions, +}; diff --git a/apps/server/src/mcp/McpToolAccess.ts b/apps/server/src/mcp/McpToolAccess.ts index 5cc8dc0c38b8..1eb4cf8c12f8 100644 --- a/apps/server/src/mcp/McpToolAccess.ts +++ b/apps/server/src/mcp/McpToolAccess.ts @@ -27,10 +27,7 @@ import { unavailable, } from "./threadAccess.ts"; -/** - * Their private constructors only stop the type checker; this also stops - * `Reflect.construct` and friends at runtime. - */ +/** Private constructors only enforce types; the token also blocks runtime construction. */ const builtHere: unique symbol = Symbol("t3/mcp/McpToolAccess/builtHere"); const refuseOutsideConstruction = (token: symbol) => { @@ -55,11 +52,7 @@ let handlersLayer: , EX, RX>( layer: Layer.Layer, EX, RX>, ) => HandlersLayer; -/** - * Effect's own registration functions accept any handler, so the - * `t3code/no-raw-mcp-registration` lint rule keeps Effect's `McpServer` inside - * McpHttpServer. - */ +/** t3code/no-raw-mcp-registration confines Effect's unchecked registration API to McpHttpServer. */ export class Declaration { readonly #handle: Handler; private constructor(token: typeof builtHere, handle: Handler) { @@ -109,7 +102,7 @@ export const readsAsCaller = (handle: (params: P) => Effect.Effect(handle: (params: P) => Effect.Effect) => declare((params: P) => requireThreadCaller.pipe( - Effect.flatMap(() => writingCaller), + Effect.andThen(writingCaller), Effect.flatMap(() => handle(params)), ), ); @@ -117,10 +110,7 @@ export const actsAsCaller = (handle: (params: P) => Effect.Effect(handle: (params: P) => Effect.Effect) => declare((params: P) => orchestratingCaller.pipe(Effect.flatMap(() => handle(params)))); -/** - * Unlike `writes` it does not need thread control; the handler checks its - * capability, as the capability toolkits do. - */ +/** Requires handler capability checks, not thread control. */ export const writesWithCapability = (handle: (params: P) => Effect.Effect) => declare((params: P) => writingCaller.pipe(Effect.flatMap(() => handle(params)))); @@ -140,9 +130,7 @@ export const writesThreads = ( yield* assertTargetWithinLimits(caller.limits, target); } } - // The target's user can raise its modes after the check above; the - // orchestrator checks again under the thread's lock and records its - // refusal here, since handlers wrap dispatch errors their own way. + // Modes can rise after this check; the orchestrator rechecks under lock and records refusal before handlers wrap errors. const refused = yield* Ref.make(undefined); return yield* handle(params).pipe( Effect.provideService(DispatchModeLimit, { ...caller.limits, refused }), @@ -196,10 +184,7 @@ export const startsThreads = ( const fullAccessRequired = "Changing projects or environment settings needs a live full-access/default calling thread or a full-access client."; -/** - * `check` re-checks the caller wherever the handler waits before writing, such - * as for a lock, since the caller's modes can change meanwhile. - */ +/** Recheck after waits before writing; caller modes may have changed. */ export const writesEnvironment = ( handle: ( params: P, diff --git a/apps/server/src/mcp/OrchestratorMcpService.activity.test.ts b/apps/server/src/mcp/OrchestratorMcpService.activity.test.ts index 9d2f10bc6c02..d7f9746f76ea 100644 --- a/apps/server/src/mcp/OrchestratorMcpService.activity.test.ts +++ b/apps/server/src/mcp/OrchestratorMcpService.activity.test.ts @@ -468,7 +468,6 @@ it("readThread and sendToThread reach threads in other projects", async () => { .pipe(Effect.flip); expect(staleDelete.code).toBe("parent_not_active"); - // A background item keeps only its owning instance live, even after a switch. const backgroundTasks = [ { taskId: "background-subagent", @@ -512,7 +511,6 @@ it("readThread and sendToThread reach threads in other projects", async () => { .pipe(Effect.flip); expect(mismatched.code).toBe("parent_not_active"); - // A fresh active run wins over the earlier inactive projection and empty roster. parentShell = liveThreadShell(parentThreadId); const restarted = yield* service.sendToThread(makeScope(), { threadId: foreignThreadId, diff --git a/apps/server/src/mcp/OrchestratorMcpService.test.ts b/apps/server/src/mcp/OrchestratorMcpService.test.ts index bba4e816caca..6ce02cbb32e2 100644 --- a/apps/server/src/mcp/OrchestratorMcpService.test.ts +++ b/apps/server/src/mcp/OrchestratorMcpService.test.ts @@ -5,6 +5,7 @@ import { EnvironmentId, MessageId, NodeId, + ORCHESTRATOR_MCP_RUNNING_TASK_NEXT, type OrchestratorMcpCapabilitiesResult, OrchestratorMcpTarget, type OrchestrationV2ThreadShell, @@ -89,7 +90,7 @@ describe("OrchestratorMcpService", () => { ], } as unknown as OrchestrationV2ThreadProjection; const childProjection = { - thread: { id: childThreadId }, + thread: { id: childThreadId, title: "Child" }, runs: [ { id: childRunId, @@ -223,7 +224,7 @@ describe("OrchestratorMcpService", () => { ], } as unknown as OrchestrationV2ThreadProjection; const childProjection = { - thread: { id: childThreadId }, + thread: { id: childThreadId, title: "Child" }, runs: [ { id: RunId.make("run:mcp-restart-child"), @@ -292,6 +293,114 @@ describe("OrchestratorMcpService", () => { }), ); + it.effect("tells the parent to end its turn only while its task still runs", () => + Effect.gen(function* () { + const parentThreadId = ThreadId.make("thread:mcp-next-parent"); + const childThreadId = ThreadId.make("thread:mcp-next-child"); + const taskId = NodeId.make("node:mcp-next-task"); + let finished = false; + let pendingRequest = false; + const parentProjection = () => + ({ + thread: { id: parentThreadId }, + runs: [], + contextTransfers: [], + subagents: [ + { + id: taskId, + threadId: parentThreadId, + origin: "app_owned", + childThreadId, + driver: "codex", + model: "gpt-5.6-terra", + status: finished ? "completed" : "running", + result: finished ? "done" : null, + completionDelivery: { state: "pending" }, + }, + ], + }) as unknown as OrchestrationV2ThreadProjection; + const childRun = () => ({ + id: RunId.make("run:mcp-next-child"), + ordinal: 1, + status: finished ? ("completed" as const) : ("running" as const), + delegatedTaskId: taskId, + }); + const childProjection = () => + ({ + thread: { id: childThreadId, title: "Child task" }, + runs: [childRun()], + contextTransfers: [], + messages: [], + subagents: [], + providerThreads: [], + runtimeRequests: pendingRequest ? [{ status: "pending" }] : [], + nodes: [], + turnItems: [], + }) as unknown as OrchestrationV2ThreadProjection; + const layerDependencies = Layer.mergeAll( + NodeServices.layer, + Layer.mock(ThreadManagementService.ThreadManagementService)({ + getThreadRecords: (threadId) => + Effect.succeed(threadId === parentThreadId ? parentProjection() : childProjection()), + getThreadShell: (threadId) => Effect.succeed(liveThreadShell(threadId)), + getProjectThreadRecords: () => Effect.succeed(childProjection()), + waitForThread: () => + Effect.succeed({ + threadId: childThreadId, + run: childRun() as unknown as OrchestrationV2ThreadProjection["runs"][number], + timedOut: !finished, + }), + dispatch: () => Effect.succeed({} as never), + }), + ServerSettings.layerTest(), + Layer.mock(ProviderRegistry.ProviderRegistry)({ getProviders: Effect.succeed([]) }), + Layer.mock(ProviderAdapterRegistry.ProviderAdapterRegistryV2)({ + list: () => Effect.succeed([]), + }), + Layer.mock(ProjectService.ProjectService)({}), + Layer.mock(SecretRequests.SecretRequests)({}), + Layer.mock(ScheduledTaskService.ScheduledTaskService)({}), + ); + const scope: McpInvocationScope = { + environmentId: EnvironmentId.make("environment:mcp-next"), + requestNamespace: "provider-session:mcp-next", + thread: { + threadId: parentThreadId, + providerSessionId: "provider-session:mcp-next", + providerInstanceId: ProviderInstanceId.make("codex"), + }, + client: undefined, + capabilities: new Set(["orchestration"]), + issuedAt: 1, + }; + + yield* Effect.gen(function* () { + const service = yield* OrchestratorMcpService.OrchestratorMcpService; + const running = yield* service.taskStatus(scope, taskId); + assert.equal(running.status, "running"); + assert.equal(running.next, ORCHESTRATOR_MCP_RUNNING_TASK_NEXT); + const waiting = yield* service.waitForThread(scope, { threadId: childThreadId }); + assert.isTrue(waiting.timedOut); + assert.equal(waiting.next, ORCHESTRATOR_MCP_RUNNING_TASK_NEXT); + + // A child waiting on a question or approval needs an answer, not a wake. + pendingRequest = true; + const asking = yield* service.waitForThread(scope, { threadId: childThreadId }); + assert.isTrue(asking.timedOut); + assert.notProperty(asking, "next"); + pendingRequest = false; + + finished = true; + const completed = yield* service.taskStatus(scope, taskId); + assert.equal(completed.status, "completed"); + assert.notProperty(completed, "next"); + const waited = yield* service.waitForThread(scope, { threadId: childThreadId }); + assert.equal(waited.status, "completed"); + assert.notProperty(waited, "next"); + }).pipe(Effect.provide(layerOrchestratorService.pipe(Layer.provide(layerDependencies)))); + }), + ); + it.effect("does not dispose delivery when a nonterminal task has no active child run", () => Effect.gen(function* () { const parentThreadId = ThreadId.make("thread:mcp-cancel-parent"); @@ -316,7 +425,7 @@ describe("OrchestratorMcpService", () => { ], } as unknown as OrchestrationV2ThreadProjection; const childProjection = { - thread: { id: childThreadId }, + thread: { id: childThreadId, title: "Child" }, runs: [], contextTransfers: [], messages: [], @@ -398,9 +507,9 @@ describe("OrchestratorMcpService", () => { }, ], }) as unknown as OrchestrationV2ThreadProjection; - // A finished run whose binding to the follow-up was lost (#19002). + // A finished run whose binding to the follow-up was lost. const childProjection = { - thread: { id: childThreadId }, + thread: { id: childThreadId, title: "Child" }, runs: childState === "no runs" ? [] @@ -503,7 +612,7 @@ describe("OrchestratorMcpService", () => { ], } as unknown as OrchestrationV2ThreadProjection; const childProjection = { - thread: { id: childThreadId }, + thread: { id: childThreadId, title: "Child" }, runs: [{ id: childRunId, status: "running" }], contextTransfers: [], messages: [], @@ -583,7 +692,7 @@ describe("OrchestratorMcpService", () => { ], } as unknown as OrchestrationV2ThreadProjection; const childProjection = { - thread: { id: childThreadId }, + thread: { id: childThreadId, title: "Child" }, runs: [{ id: childRunId, status: "running" }], contextTransfers: [], messages: [], @@ -669,7 +778,7 @@ describe("OrchestratorMcpService", () => { ], } as unknown as OrchestrationV2ThreadProjection; const childProjection = { - thread: { id: childThreadId }, + thread: { id: childThreadId, title: "Child" }, runs: [{ id: childRunId, status: "running" }], contextTransfers: [], messages: [], @@ -764,7 +873,7 @@ describe("OrchestratorMcpService", () => { [ childThreadId, { - thread: { id: childThreadId }, + thread: { id: childThreadId, title: "Child" }, runs: [{ id: RunId.make("run:mcp-cancel-grandchild-child"), status: "running" }], contextTransfers: [], messages: [], @@ -992,7 +1101,7 @@ describe("OrchestratorMcpService provider resolution", () => { }) as unknown as OrchestrationV2ThreadProjection; const childProjection = { - thread: { id: childThreadId }, + thread: { id: childThreadId, title: "Child" }, runs: [], contextTransfers: [], messages: [], diff --git a/apps/server/src/mcp/OrchestratorMcpService.ts b/apps/server/src/mcp/OrchestratorMcpService.ts index d8ac0ad8ebf8..f0c97e321ce1 100644 --- a/apps/server/src/mcp/OrchestratorMcpService.ts +++ b/apps/server/src/mcp/OrchestratorMcpService.ts @@ -5,6 +5,7 @@ import { MessageId, type ModelSelection, NodeId, + ORCHESTRATOR_MCP_RUNNING_TASK_NEXT, TurnItemId, type OrchestrationV2Run, type OrchestrationV2Subagent, @@ -67,6 +68,7 @@ import { } from "@t3tools/contracts"; import { getProviderOptionCurrentValue } from "@t3tools/shared/model"; import { runRanAfter } from "@t3tools/shared/orchestrationV2ThreadError"; +import { formatThreadLink } from "@t3tools/shared/threadLinks"; import { normalizeThreadPullRequestKey, visibleThreadPullRequests, @@ -173,9 +175,7 @@ export interface OrchestratorMcpServiceShape { scope: McpInvocationScope, input: OrchestratorMcpDeleteScheduledTaskInput, ) => Effect.Effect; - /** - * The value never reaches the agent: the result is a status. - */ + /** Only status reaches the agent, never this value. */ readonly requestSecret: ( scope: McpInvocationScope, input: OrchestratorMcpRequestSecretInput, @@ -213,13 +213,7 @@ function failure(code: OrchestratorMcpFailure["code"], message: string): Orchest return new OrchestratorMcpFailure({ code, message }); } -/** - * A caller's thread is live while it is not archived, runs on the caller's - * provider instance, and has an active run or a background subagent that - * instance started. A background subagent outlives its run and calls tools - * with its thread's credential, which is revoked when the provider process - * goes away. - */ +/** Background subagents retain credentials until provider exit; archived threads and other instances cannot act. */ export const isLiveCallerShell = ( shell: Pick< OrchestrationV2ThreadShell, @@ -376,7 +370,6 @@ function providerConstraints( constraints.push("No V2 provider adapter is registered."); } if (provider === undefined) return constraints; - // Driver availability comes from the registry, even when health probes are skipped. if (!isProviderAvailable(provider)) { constraints.push(provider.unavailableReason ?? "Provider driver is unavailable."); } @@ -395,11 +388,7 @@ function providerConstraints( return constraints; } -/** - * Models without descriptors skip the descriptor checks (mirroring how model slugs are only - * validated when the provider advertises models), but duplicate ids always fail: downstream - * consumers disagree on whether the first or last value of a duplicated id wins. - */ +/** Skip checks without descriptors; reject duplicate IDs because consumers disagree on precedence. */ function invalidOptionSelections( selections: ReadonlyArray, descriptors: ReadonlyArray | undefined, @@ -847,7 +836,6 @@ function threadSnooze( }; } -/** The same links `list_thread_pull_requests` reports, without the snapshot details. */ function threadPullRequests( links: OrchestrationV2ThreadShell["pullRequests"], ): OrchestratorMcpThreadPullRequest[] { @@ -868,6 +856,7 @@ function listItemFromShell( return { threadId: shell.id, title: shell.title, + link: formatThreadLink(shell.id, shell.title), createdBy: shell.createdBy, creationSource: shell.creationSource, status: shell.activityRunStatus ?? shell.status, @@ -900,6 +889,7 @@ function threadDetail( threadId: projection.thread.id, projectId: projection.thread.projectId, title: projection.thread.title, + link: formatThreadLink(projection.thread.id, projection.thread.title), createdBy: projection.thread.createdBy, creationSource: projection.thread.creationSource, status: active?.status ?? latest?.status ?? "idle", @@ -1227,10 +1217,7 @@ const make = Effect.gen(function* () { return { ...caller, target } as const; }); - /** - * A thread caller writes to another thread only while its own run is live, - * so a credential that outlived its session cannot reach across threads. - */ + /** Cross-thread writes require a live caller run; stale session credentials cannot authorize them. */ const assertLiveCallerForOtherThread = ( scope: McpInvocationScope, parent: Pick | undefined, @@ -1306,11 +1293,7 @@ const make = Effect.gen(function* () { const loadOrchestrationCapableInstanceIds = () => providerAdapters.list().pipe(Effect.map((instanceIds) => new Set(instanceIds))); - /** - * Provider snapshots only re-probe while a client is in the foreground, so an - * unattended agent can see a provider as unavailable after it was fixed. - * Re-probe the requested instance once before refusing it. - */ + /** Background agents miss foreground-only health probes; re-probe once before refusing. */ const assertTargetEnabledForProject = ( parent: Pick, commandId: CommandId, @@ -1347,13 +1330,7 @@ const make = Effect.gen(function* () { readonly parent: Pick; readonly target: OrchestratorMcpTarget | undefined; readonly providers: ReadonlyArray; - /** - * Set only when the command that will dispatch this target already has a receipt: that - * dispatch can only replay the recorded result, never start anything new, so filtering - * candidates by the project's current settings would just refuse a retry that does no new - * work. Never set this for a resolution whose dispatch uses a different command id than the - * one checked for a receipt. - */ + /** Receipt replays bypass current settings because they start no work. Dispatch must reuse the checked command ID. */ readonly skipProjectEnablementFilter?: boolean; }): Effect.fn.Return { const requestedInstanceId = input.target?.providerInstanceId; @@ -1620,9 +1597,10 @@ const make = Effect.gen(function* () { instanceId: task.providerInstanceId, model: task.model, }; - const response = { + const result = { taskId: task.id, childThreadId: task.childThreadId, + link: formatThreadLink(task.childThreadId, childControls.thread.title), childRunId: childRun?.id ?? null, childNodeId: task.id, status, @@ -1655,6 +1633,10 @@ const make = Effect.gen(function* () { : pendingTaskApproval(childControls, task.id, childRun), waitTimedOut, } satisfies OrchestratorMcpDelegateTaskResult; + const response: OrchestratorMcpDelegateTaskResult = + endsTaskWait(result) || task.completionDelivery?.state === "disposed" + ? result + : { ...result, next: ORCHESTRATOR_MCP_RUNNING_TASK_NEXT }; if ( acknowledgeTerminal && isTerminalTaskStatus(status) && @@ -1697,8 +1679,7 @@ const make = Effect.gen(function* () { "orchestration_error", `Unable to watch delegated task ${taskId}: ${errorMessage(error)}`, ); - // Sequences are global, so one cursor taken before the first read - // replays anything either thread records after it. + // One global cursor before both reads captures later events from either thread. const afterSequence = yield* threadManagement .getThreadEventSequence(scope.thread.threadId) .pipe(Effect.mapError(streamError)); @@ -1906,9 +1887,7 @@ const make = Effect.gen(function* () { : input.bindToCurrentThread && parent !== undefined ? parent.thread.id : null; - // Rebinding changes where runs execute, so the workspace strategy must - // follow: unbinding a root-strategy task would otherwise run loose - // prompts in the shared project checkout. + // Update workspace strategy on rebinding so unbound root tasks cannot run in the shared checkout. const workspaceStrategy = input.bindToCurrentThread === undefined ? existing.workspaceStrategy @@ -1989,6 +1968,7 @@ const make = Effect.gen(function* () { label: input.label, reason: input.reason, ...(input.placeholder === undefined ? {} : { placeholder: input.placeholder }), + ...(input.delivery === "file" ? { delivery: "file" as const } : {}), secretStatus, }) .pipe( @@ -2066,6 +2046,12 @@ const make = Effect.gen(function* () { ); yield* Effect.annotateCurrentSpan({ "secret_request.status": status }); if (status !== "saved") return { status }; + if (input.delivery === "file") { + const file = yield* secretRequests + .saveToFile({ threadId, turnItemId }) + .pipe(Effect.mapError((error) => failure("orchestration_error", error.message))); + return { status, file }; + } const secretRef = yield* secretRequests.savedRef({ threadId: threadId, turnItemId }); if (Option.isNone(secretRef)) { return yield* failure( @@ -2458,29 +2444,27 @@ const make = Effect.gen(function* () { ), ), ); - // A follow-up that lost its child run (#19002) stays running on the parent, while its + // A follow-up that lost its child run stays running on the parent, while its // status here comes from the child's last run. Stop on the parent cancels that task. const cancelOrphan = Effect.gen(function* () { if (parentTask?.runId == null || !isOrchestrationV2WorkActive(parentTask.status)) return false; - yield* threadManagement - .dispatch({ - type: "subagent.stop", - commandId: stableCommandId({ - scope, - requestKey: key, - operation: "cancel-task-orphan", - }), - threadId: scope.thread.threadId, - runId: parentTask.runId, - subagentId: parentTask.id, - }) - .pipe(Effect.mapError(threadManagementFailure)); - const { subagents } = yield* threadManagement - .getThreadRecords(scope.thread.threadId, ["subagents"]) - .pipe(Effect.mapError(threadManagementFailure)); + yield* threadManagement.dispatch({ + type: "subagent.stop", + commandId: stableCommandId({ + scope, + requestKey: key, + operation: "cancel-task-orphan", + }), + threadId: scope.thread.threadId, + runId: parentTask.runId, + subagentId: parentTask.id, + }); + const { subagents } = yield* threadManagement.getThreadRecords(scope.thread.threadId, [ + "subagents", + ]); return subagents.some((task) => task.id === input.taskId && task.status === "cancelled"); - }); + }).pipe(Effect.mapError(threadManagementFailure)); if (isTerminalTaskStatus(current.status)) { // The child can still run work for another turn, so it is stopped either way. yield* stopWithinLimit; @@ -2676,6 +2660,7 @@ const make = Effect.gen(function* () { ); return { threadId, + link: formatThreadLink(threadId, projection.thread.title), runId: run?.id ?? null, status: run?.status ?? "idle", title: projection.thread.title, @@ -2867,6 +2852,7 @@ const make = Effect.gen(function* () { ); return { threadId: input.threadId, + link: formatThreadLink(input.threadId, target.thread.title), messageId, runId: result.run.id, ...(result.run.delegatedTaskId === undefined ? {} : { taskId: result.run.delegatedTaskId }), @@ -2875,7 +2861,7 @@ const make = Effect.gen(function* () { } satisfies OrchestratorMcpThreadSendResult; }), waitForThread: Effect.fn("OrchestratorMcpService.waitForThread")(function* (scope, input) { - const { target } = yield* loadScopedThread(scope, input.threadId); + const { parent, target } = yield* loadScopedThread(scope, input.threadId); const result = yield* threadManagement .waitForThread({ projectId: target.thread.projectId, @@ -2887,11 +2873,33 @@ const make = Effect.gen(function* () { ), }) .pipe(Effect.mapError(threadManagementFailure)); + const status = result.run === null ? "idle" : runOutcome(result.run); + const taskId = result.run?.delegatedTaskId; + const ownedTask = + result.timedOut && + status !== "waiting" && + parent?.subagents.some( + (task) => + task.id === taskId && + task.origin === "app_owned" && + task.threadId === parent.thread.id && + task.childThreadId === input.threadId && + task.completionDelivery?.state !== "disposed", + ) === true; + // A child waiting on a question or approval needs an answer, not a wake. + const childRequests = ownedTask + ? (yield* threadManagement + .getThreadRecords(input.threadId, ["runtimeRequests"]) + .pipe(Effect.mapError(threadManagementFailure))).runtimeRequests + : []; + const wakesCaller = + ownedTask && !childRequests.some((request) => request.status === "pending"); return { threadId: input.threadId, runId: result.run?.id ?? null, - status: result.run === null ? "idle" : runOutcome(result.run), + status, timedOut: result.timedOut, + ...(wakesCaller ? { next: ORCHESTRATOR_MCP_RUNNING_TASK_NEXT } : {}), } satisfies OrchestratorMcpThreadWaitResult; }), interruptThread: Effect.fn("OrchestratorMcpService.interruptThread")(function* (scope, input) { diff --git a/apps/server/src/mcp/TaskCancelNativeSubagent.integration.test.ts b/apps/server/src/mcp/TaskCancelNativeSubagent.integration.test.ts index 9c36a26045dc..504ae749a78a 100644 --- a/apps/server/src/mcp/TaskCancelNativeSubagent.integration.test.ts +++ b/apps/server/src/mcp/TaskCancelNativeSubagent.integration.test.ts @@ -50,9 +50,6 @@ const driver = ProviderDriverKind.make("claudeAgent"); const instanceId = ProviderInstanceId.make("claudeAgent"); const modelSelection = { instanceId, model: "claude-test" }; -// A delegated Claude child ends its turn while a background native subagent it -// started keeps running (#17154). - it.effect("task_cancel ends a delegated child's background native subagent", () => Effect.scoped( Effect.gen(function* () { diff --git a/apps/server/src/mcp/ThreadMetadataMcpService.ts b/apps/server/src/mcp/ThreadMetadataMcpService.ts index e05200fd8eda..421d728461eb 100644 --- a/apps/server/src/mcp/ThreadMetadataMcpService.ts +++ b/apps/server/src/mcp/ThreadMetadataMcpService.ts @@ -203,8 +203,7 @@ const make = Effect.gen(function* () { const metadataEvent = dispatched.storedEvents.find( (stored) => stored.event.type === "thread.metadata-updated", ); - // A retried commandId replays the stored events of the first dispatch; - // compaction may have deleted them by then, so answer from the projection. + // Retries may find stored events compacted away; answer from the projection. const thread = metadataEvent?.event.type === "thread.metadata-updated" ? metadataEvent.event.payload diff --git a/apps/server/src/mcp/WorktreeMcpService.test.ts b/apps/server/src/mcp/WorktreeMcpService.test.ts index 3902068894d6..3be8705cc0bb 100644 --- a/apps/server/src/mcp/WorktreeMcpService.test.ts +++ b/apps/server/src/mcp/WorktreeMcpService.test.ts @@ -348,13 +348,9 @@ const expectTypedFailure = (exit: Exit.Exit, expected: object) expect(reason.error).toMatchObject(expected); }; -// Resolves the service once from the harness layer; used by tests that must -// make several calls against the SAME instance (the in-flight guard lives in -// the layer's closure, so a fresh layer per call would never see it). +// Resolve once so calls share the in-flight guard. const resolveService = (harness: ReturnType) => - Effect.gen(function* () { - return yield* WorktreeMcpService.WorktreeMcpService; - }).pipe(Effect.provide(harness.layer)); + WorktreeMcpService.WorktreeMcpService.pipe(Effect.provide(harness.layer)); const runHandoff = ( harness: ReturnType, @@ -371,7 +367,6 @@ const runStatus = (harness: ReturnType) => return yield* service.status(harness.scope); }).pipe(Effect.provide(harness.layer)); -// Decodes tool arguments the way the MCP tool does before calling the service. const decodeHandoffInput = Schema.decodeUnknownEffect(WorktreeMcpHandoffInput); describe("t3_worktree_handoff", () => { diff --git a/apps/server/src/mcp/WorktreeMcpService.ts b/apps/server/src/mcp/WorktreeMcpService.ts index 461b7044ba87..8f02ee154642 100644 --- a/apps/server/src/mcp/WorktreeMcpService.ts +++ b/apps/server/src/mcp/WorktreeMcpService.ts @@ -63,9 +63,7 @@ const make = Effect.gen(function* () { const setupScriptRunner = yield* ProjectSetupScriptRunner.ProjectSetupScriptRunner; const vcsStatusBroadcaster = yield* VcsStatusBroadcaster.VcsStatusBroadcaster; - // Serializes handoffs per thread: two concurrent calls could otherwise both - // pass the worktreePath === null check and each create a worktree, leaving - // one untracked on disk. + // Serialize handoffs per thread to avoid creating two worktrees and orphaning one. const handoffThreadsInFlight = new Set(); const requireThreadScope = (scope: McpInvocationScope) => @@ -225,14 +223,7 @@ const make = Effect.gen(function* () { const ids = yield* handoffIds(scope); - // uninterruptibleMask: only the potentially slow worktree creation itself - // stays interruptible (restore). From the moment it succeeds, through the - // binding, continuation queue, setup script launch, and result - // construction, there is no interruptible gap: a client cancel can - // therefore neither orphan the created worktree before the rollback is - // armed, nor skip setting up a worktree the thread was just bound to - // (once the binding commits, the scheduled session detach can sever this - // request's connection and interrupt the fiber). + // Mask interruption after worktree creation through setup; binding can detach and interrupt the caller. return yield* Effect.uninterruptibleMask((restore) => Effect.gen(function* () { const worktree = yield* restore( @@ -273,10 +264,7 @@ const make = Effect.gen(function* () { ).pipe(Effect.ignoreCause({ log: true })); const recheckAndBind = Effect.gen(function* () { - // The projection was read before the potentially slow git work - // above; a concurrent binding (for example from the UI) could have - // attached the thread in the meantime. Re-check before committing so - // the race cannot leave a second, untracked worktree. + // A concurrent UI binding may have landed during git work; do not orphan a second worktree. const recheck = yield* loadThread(scope); if (recheck.thread.worktreePath !== null) { return yield* alreadyInWorktree(recheck.thread.worktreePath); @@ -309,18 +297,13 @@ const make = Effect.gen(function* () { ), ); }).pipe( - // Interrupt-only causes skip the removal: the binding may have - // committed, and force-deleting a worktree the thread now points at - // would be worse than leaking one. + // Do not remove on interruption alone; the thread binding may already have committed. Effect.onError((cause) => Cause.hasInterruptsOnly(cause) ? Effect.void : removeCreatedWorktree, ), ); - // Queue the continuation right after the binding commits: the detach - // that the metadata update schedules will terminate the calling - // session, and a durably queued message is what guarantees the thread - // re-launches inside the worktree. + // Queue continuation immediately after binding; detachment terminates the caller, so relaunch needs a durable message. const queueContinuation: Effect.Effect = Effect.suspend(() => input.continuationPrompt === undefined @@ -401,11 +384,7 @@ const make = Effect.gen(function* () { function* (callerScope, input) { yield* requireCapability(callerScope); const scope = yield* requireThreadScope(callerScope); - // uninterruptibleMask: the guard acquisition and the registration of the - // releasing finalizer happen with no interruptible gap in between. An - // interrupt landing between a bare add() and the start of an ensured - // effect would otherwise leak the guard entry and block every future - // handoff for this thread until restart. + // Acquire and register cleanup atomically so cancellation cannot strand the guard until restart. return yield* Effect.uninterruptibleMask((restore) => Effect.suspend(() => { if (handoffThreadsInFlight.has(scope.thread.threadId)) { diff --git a/apps/server/src/mcp/threadAccess.ts b/apps/server/src/mcp/threadAccess.ts index ba8dc4c3e576..9f3a8a57c05b 100644 --- a/apps/server/src/mcp/threadAccess.ts +++ b/apps/server/src/mcp/threadAccess.ts @@ -102,12 +102,7 @@ export const assertTargetWithinLimits = ( Effect.asVoid, ); -/** - * A thread caller acts only while it owns a live run, or a background - * subagent that run started, of a thread that is not archived, so a provider - * token that outlived its session cannot act. An OAuth client has no run; its - * session and ceiling are its authority. - */ +/** Thread callers need a live run or its background subagent in an unarchived thread. OAuth clients rely on their session and ceiling. */ export function assertLiveCaller({ caller, scope }: Caller) { if (caller === undefined) return Effect.void; return OrchestrationMcp.isLiveCallerShell(caller, scope.thread?.providerInstanceId) diff --git a/apps/server/src/mcp/toolkits/attachment/input.ts b/apps/server/src/mcp/toolkits/attachment/input.ts index f29d00d1f9f9..1f37edbbf6d4 100644 --- a/apps/server/src/mcp/toolkits/attachment/input.ts +++ b/apps/server/src/mcp/toolkits/attachment/input.ts @@ -2,8 +2,7 @@ import { ChatFileAttachment, ChatImageAttachment } from "@t3tools/contracts"; import * as Schema from "effect/Schema"; import * as Struct from "effect/Struct"; -// Screenshot provenance is app-owned. Existing attachments recover it from stored -// metadata; exposing its recursive accessibility tree breaks model tool schemas. +// Keep screenshot provenance in app metadata; its recursive tree breaks model tool schemas. export const McpAttachmentInput = Schema.Union([ ChatImageAttachment.mapFields(Struct.omit(["source"])), ChatFileAttachment, diff --git a/apps/server/src/mcp/toolkits/environment/tools.ts b/apps/server/src/mcp/toolkits/environment/tools.ts index c73bfbcc3f2d..a844c8070fc2 100644 --- a/apps/server/src/mcp/toolkits/environment/tools.ts +++ b/apps/server/src/mcp/toolkits/environment/tools.ts @@ -41,7 +41,7 @@ const shared = { const EnvironmentReadTool = Tool.make("t3_environment_read", { ...shared, description: - "Read this server's identity and selected environment preferences. Provider/model availability is exposed by orchestrator_capabilities. Writing instructions are limited to 4,000 characters.", + "Read this server's identity and preferences. Providers and models: `orchestrator_capabilities`.", success: Schema.Struct({ environmentId: ExecutionEnvironmentDescriptor.fields.environmentId, label: Schema.String, @@ -55,7 +55,7 @@ const EnvironmentReadTool = Tool.make("t3_environment_read", { const EnvironmentPreferencesTool = Tool.make("t3_environment_preferences_update", { ...shared, description: - "Update selected environment-wide preferences through normal settings persistence and notifications. Requires a live full-access/default calling thread. Omitted fields are preserved; empty customInstructions clears them.", + "Update environment-wide preferences. Live full-access or default caller thread only. Omitted fields are unchanged; `customInstructions: ''` clears it.", parameters: Schema.Struct({ defaultThreadEnvMode: ServerSettingsPatch.fields.defaultThreadEnvMode, newWorktreesStartFromOrigin: ServerSettingsPatch.fields.newWorktreesStartFromOrigin, diff --git a/apps/server/src/mcp/toolkits/html/tools.ts b/apps/server/src/mcp/toolkits/html/tools.ts index 3c93ecc8b584..a8010fab4f52 100644 --- a/apps/server/src/mcp/toolkits/html/tools.ts +++ b/apps/server/src/mcp/toolkits/html/tools.ts @@ -22,21 +22,18 @@ const Html = Schema.String.check(Schema.isMinLength(1), Schema.isMaxLength(512_0 // Coder: `html_render` only. `html_preview` screenshots pages in a downloaded headless browser, // which T3 Coder does not install, so the render description does not point agents at it. The // height caps a frame only on upstream's mobile app, so the description omits that case. -const PAGE_RULES = - 'Write one self-contained document with inline
Research
Results', + ); + const doc = new DOMParser().parseFromString(html, "text/html"); + expect(doc.querySelector("td")?.textContent).toBe("Research"); + expect(doc.images).toHaveLength(1); + expect(doc.images[0]!.getAttribute("src")).toBe("data:image/png;base64,iVBORw0KGgo="); + expect(doc.querySelector("script,style,iframe,a,[onerror]")).toBeNull(); + expect(html).not.toContain("https://"); + }); +}); +it("opens a known empty attachment even when its range response is inaccessible", async () => { + refresh.mockResolvedValue("https://unreachable.invalid/empty.txt"); + await using screen = await render( + , + ); + await vi.waitFor( + () => expect(screen.container.textContent).toContain("This file is empty."), + 10_000, + ); +}); + +it.each(["draft", "thread"] as const)( + "reads an unknown-size %s workspace file instead of treating it as empty", + async (source) => { + refresh.mockResolvedValue("https://example.test/binary.txt"); + using _fetch = stubGlobal( + "fetch", + vi.fn(async () => new Response("invalid\0text")), + ); + await using screen = await render( + , + ); + await vi.waitFor(() => + expect(screen.container.textContent).toContain( + "This file contains binary data and cannot be shown as text.", + ), + ); + expect(fetch).toHaveBeenCalled(); + expect(screen.container.textContent).not.toContain("This file is empty."); + }, +); + +it("refreshes authorization and retries a failed streamed attachment", async () => { + const bad = URL.createObjectURL(new Blob(["invalid\0text"])); + const good = URL.createObjectURL(new Blob(["recovered text\n"])); + refresh.mockReset().mockResolvedValueOnce(bad).mockResolvedValue(good); + try { + await using screen = await render( + , + ); + await vi.waitFor(() => + expect(screen.container.textContent).toContain("This file contains binary data."), + ); + await act(() => + screen.container.querySelector('[role="alert"] button')!.click(), + ); + await vi.waitFor(() => expect(screen.container.textContent).toContain("recovered text")); + expect(refresh).toHaveBeenCalledTimes(2); + } finally { + URL.revokeObjectURL(bad); + URL.revokeObjectURL(good); + } +}); + +it.each([ + ["custom/sheet1.xml", 250001], + ["/xl/custom/sheet1.bin", 250001], + ["custom/valid.xml", 1], +] as const)( + "bounds the worksheet relationship target %s at row %i before the reader allocates its grid", + async (target, row) => { + const { default: JSZip } = await import("jszip"); + const zip = new JSZip(); + zip.file( + "xl/workbook.xml", + '', + ); + zip.file( + "xl/_rels/workbook.xml.rels", + ``, + ); + zip.file( + target.startsWith("/") ? target.slice(1) : `xl/${target}`, + `worksheet value`, + ); + const file = await zip.generateAsync({ type: "blob", compression: "DEFLATE" }); + const src = URL.createObjectURL(file); + try { + await using screen = await render( +
+ +
, + ); + await vi.waitFor( + () => + expect(screen.container.textContent).toContain( + row > 250000 ? "This workbook exceeds the preview limit" : "worksheet value", + ), + { timeout: 10_000 }, + ); + } finally { + URL.revokeObjectURL(src); + } + }, +); + +it("copies every indexed line rather than the mounted preview rows", async () => { + const text = Array.from({ length: 10_000 }, (_, i) => `line-${i}`).join("\n"); + const copyContents = { current: null as (() => Promise) | null }; + await using screen = await render( + , + ); + await vi.waitFor(() => expect(screen.container.textContent).toContain("line-0")); + await vi.waitFor(async () => expect(await copyContents.current!()).toBe(text)); + const first = copyContents.current!(); + await expect(copyContents.current!()).rejects.toThrow("Another copy is in progress."); + expect(await first).toBe(text); + expect(await copyContents.current!()).toBe(text); + expect(screen.container.querySelector('[role="alert"]')).toBeNull(); +}); + +it.each(["archive", "sqlite"] as const)( + "reauthorizes and retries a failed %s attachment", + async (kind) => { + const bad = URL.createObjectURL(new Blob(["invalid"])); + let bytes: Uint8Array; + if (kind === "archive") + bytes = new Uint8Array( + await new JSZip().file("recovered.txt", "ok").generateAsync({ type: "uint8array" }), + ); + else { + const [{ default: initSqlJs }, { default: wasmUrl }] = await Promise.all([ + import("sql.js"), + import("sql.js/dist/sql-wasm.wasm?url"), + ]); + const SQL = await initSqlJs({ locateFile: () => wasmUrl }); + const database = new SQL.Database(); + database.run( + "CREATE TABLE recovered (value TEXT); INSERT INTO recovered VALUES('retry succeeded')", + ); + bytes = new Uint8Array(database.export()); + database.close(); + } + const good = URL.createObjectURL(new Blob([bytes])); + refresh.mockReset().mockResolvedValueOnce(bad).mockResolvedValue(good); + try { + await using screen = await render( + , + ); + await vi.waitFor( + () => expect(screen.container.querySelector('[role="alert"] button')).not.toBeNull(), + 10_000, + ); + await act(() => + screen.container.querySelector('[role="alert"] button')!.click(), + ); + await vi.waitFor( + () => + expect(screen.container.textContent).toContain( + kind === "archive" ? "recovered.txt" : "retry succeeded", + ), + 10_000, + ); + expect(refresh).toHaveBeenCalledTimes(2); + } finally { + URL.revokeObjectURL(bad); + URL.revokeObjectURL(good); + } + }, +); + +it("loads a font attachment through the lazy preview", async () => { + const { default: fontUrl } = + await import("../../../node_modules/@betteroffice/fonts/assets/Carlito-Regular.ttf?url"); + const file = await (await fetch(fontUrl)).blob(); + await using screen = await render( + , + ); + await vi.waitFor(() => { + const frame = screen.container.querySelector("iframe"); + expect(frame?.title).toBe("Carlito.ttf"); + expect(frame?.srcdoc).toContain("@font-face"); + expect(frame?.srcdoc).toContain("The quick brown fox"); + }); + expect(screen.container.querySelector('[role="alert"]')).toBeNull(); +}); + +it("toggles a Mermaid attachment between diagram and source through the lazy preview", async () => { + const text = "flowchart LR\nA --> B"; + const file = new Blob([text], { type: "text/vnd.mermaid" }); + await using screen = await render( + , + ); + await vi.waitFor(() => + expect(screen.container.querySelector("[data-markdown-mermaid] svg path")).not.toBeNull(), + ); + await screen.getByRole("button", { name: "Expand diagram" }).click(); + await vi.waitFor(() => { + const image = document.querySelector('[role="dialog"] img'); + expect(image?.alt).toBe("Mermaid diagram"); + expect(image?.naturalWidth).toBeGreaterThan(0); + }); + await page.getByRole("button", { name: "Close image preview" }).click(); + await vi.waitFor(() => expect(document.querySelector('[role="dialog"]')).toBeNull()); + await screen.getByRole("button", { name: "Show Mermaid source" }).click(); + await vi.waitFor(() => expect(screen.container.querySelector("pre")?.textContent).toBe(text)); + await screen.getByRole("button", { name: "Show rendered diagram" }).click(); + await vi.waitFor(() => + expect(screen.container.querySelector("[data-markdown-mermaid] svg path")).not.toBeNull(), + ); + expect(screen.container.querySelector('[role="alert"]')).toBeNull(); +}); + +it("navigates, searches, zooms, and selects mixed-height PDF pages", async () => { + const objects = [ + "<< /Type /Catalog /Pages 2 0 R >>", + "<< /Type /Pages /Count 3 /Kids [3 0 R 5 0 R 7 0 R] >>", + ]; + for (const [index, title] of [ + "Quarterly outlook", + "Delivery milestones", + "Budget and risks", + ].entries()) { + const text = `/GS1 gs BT /F1 24 Tf 30 ${index === 0 ? 70 : 700} Td ${index === 0 ? "(Quarter) Tj /F2 24 Tf (ly outlook) Tj" : `(${title}) Tj`} ET`; + objects.push( + `<< /Type /Page /Parent 2 0 R /MediaBox [0 0 612 ${index === 0 ? 140 : 792}] /Rotate ${index === 2 ? 90 : 0} /Resources << /Font << /F1 9 0 R /F2 10 0 R >> /ExtGState << /GS1 << /TR << /FunctionType 2 /Domain [0 1] /C0 [0] /C1 [1] /N 2 >> >> >> >> /Contents ${4 + index * 2} 0 R >>`, + `<< /Length ${text.length} >>\nstream\n${text}\nendstream`, + ); + } + objects.push("<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica >>"); + objects.push("<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica-Bold >>"); + let source = "%PDF-1.4\n"; + const offsets = [0]; + for (const [index, object] of objects.entries()) { + offsets.push(source.length); + source += `${index + 1} 0 obj\n${object}\nendobj\n`; + } + const xref = source.length; + source += `xref\n0 11\n0000000000 65535 f \n${offsets + .slice(1) + .map((offset) => `${String(offset).padStart(10, "0")} 00000 n \n`) + .join("")}trailer\n<< /Size 11 /Root 1 0 R >>\nstartxref\n${xref}\n%%EOF`; + const file = new Blob([source], { type: "application/pdf" }); + const { default: PdfViewer } = await import("./PdfViewer"); + const url = URL.createObjectURL(file); + try { + await using screen = await render( +
+ +
, + ); + await vi.waitFor(() => expect(screen.container.textContent).toContain("1 / 3 pages"), { + timeout: 15_000, + }); + const region = screen.container.querySelector('[role="region"]')!; + await vi.waitFor(() => { + expect(region.scrollWidth).toBe(region.clientWidth); + expect(region.querySelector("canvas")!.getBoundingClientRect().width).toBeLessThan( + region.clientWidth, + ); + expect(screen.container.textContent).toContain("Fit width"); + }); + await act(() => { + (screen.container.firstElementChild as HTMLElement).style.width = "500px"; + }); + await vi.waitFor(() => + expect(region.querySelector("canvas")!.getBoundingClientRect().width).toBe(468), + ); + await screen.getByRole("button", { name: "Next page", exact: true }).click(); + await vi.waitFor(() => expect(screen.container.textContent).toContain("2 / 3 pages")); + await screen.getByRole("button", { name: "Previous page", exact: true }).click(); + await vi.waitFor(() => expect(screen.container.textContent).toContain("1 / 3 pages")); + await screen.getByRole("button", { name: "Zoom in", exact: true }).click(); + await vi.waitFor(() => expect(region.scrollWidth).toBeGreaterThan(region.clientWidth)); + await screen.getByRole("button", { name: "Fit width", exact: true }).click(); + await vi.waitFor(() => expect(region.scrollWidth).toBe(region.clientWidth)); + await screen.getByRole("searchbox", { name: "Find in PDF" }).fill("Quarterly outlook"); + await vi.waitFor(() => { + expect(screen.container.querySelector('[role="status"]')?.textContent).toBe("1/1"); + expect( + Array.from(region.querySelectorAll(".pdf-find-match")).map((span) => span.textContent), + ).toEqual(["Quarter", "ly outlook"]); + }); + await screen.getByRole("searchbox", { name: "Find in PDF" }).fill("Budget"); + await vi.waitFor(() => expect(screen.container.textContent).toContain("3 / 3 pages")); + await screen.getByRole("button", { name: "Zoom in", exact: true }).click(); + await vi.waitFor(() => expect(screen.container.textContent).toContain("3 / 3 pages")); + await vi.waitFor(() => { + const span = Array.from(screen.container.querySelectorAll("[data-pdf-text-layer] span")).find( + (node) => node.textContent === "Budget and risks", + ); + expect(span).toBeDefined(); + const textBounds = span!.getBoundingClientRect(); + const pageBounds = span!.parentElement!.getBoundingClientRect(); + expect(textBounds.width).toBeGreaterThan(0); + expect(textBounds.height).toBeGreaterThan(0); + expect(textBounds.left).toBeGreaterThanOrEqual(pageBounds.left - 1); + expect(textBounds.top).toBeGreaterThanOrEqual(pageBounds.top - 1); + expect(textBounds.right).toBeLessThanOrEqual(pageBounds.right + 1); + expect(textBounds.bottom).toBeLessThanOrEqual(pageBounds.bottom + 1); + const range = document.createRange(); + range.selectNodeContents(span!); + const selection = window.getSelection()!; + selection.removeAllRanges(); + selection.addRange(range); + expect(selection.toString()).toBe("Budget and risks"); + expect(getComputedStyle(span!).userSelect).toBe("text"); + selection.removeAllRanges(); + }); + await screen.getByRole("button", { name: "Fit width", exact: true }).click(); + await vi.waitFor(() => expect(screen.container.textContent).toContain("3 / 3 pages")); + await screen.getByRole("searchbox", { name: "Find in PDF" }).fill(" "); + await vi.waitFor(() => + expect(screen.container.querySelector('[role="status"]')?.textContent).toBe("0/0"), + ); + } finally { + URL.revokeObjectURL(url); + } +}); +it("mounts a bounded set of saved notebook outputs as the reader scrolls", async () => { + const notebook = { + nbformat: 4, + metadata: {}, + cells: Array.from({ length: 20 }, (_, cell) => ({ + cell_type: "code", + source: [], + metadata: {}, + outputs: Array.from({ length: 100 }, (_, output) => ({ + output_type: "display_data", + data: { "text/html": `

saved ${cell}-${output}

` }, + })), + })), + }; + const file = new Blob([JSON.stringify(notebook)]); + await using screen = await render( +
+ +
, + ); + await vi.waitFor(() => + expect(screen.container.querySelectorAll("iframe").length).toBeGreaterThan(0), + ); + expect(screen.container.querySelectorAll("iframe").length).toBeLessThanOrEqual(16); + const first = screen.container.querySelector("iframe")!; + expect(first.srcdoc).toContain("saved 0-0"); + const viewport = screen.container.querySelector(".scrollbar-gutter-both")!; + await act(() => { + viewport.scrollTop = viewport.scrollHeight; + }); + await vi.waitFor(() => { + expect( + [...screen.container.querySelectorAll("iframe")].some((frame) => + frame.srcdoc.includes("saved 19-99"), + ), + ).toBe(true); + expect(first.isConnected).toBe(false); + }); + expect(screen.container.querySelectorAll("iframe").length).toBeLessThanOrEqual(16); + await act(() => { + viewport.scrollTop = 0; + }); + await vi.waitFor(() => + expect(screen.container.querySelector("iframe")?.srcdoc).toContain("saved 0-0"), + ); +}); + +it.each([ + [String.raw`{\rtf1\uc1\u945{\b bold}Hello}`, "αboldHello"], + [String.raw`{\rtf1\uc1{\u945}Hello}`, "αHello"], + [ + String.raw`{\rtf1\ansi{\fonttbl{\f0\fcharset204 Cyrillic;}{\f1\fcharset0 Latin;}}\f0\'c0{\f1\'c0}\'c1}`, + "АÀБ", + ], + [String.raw`{\rtf1\ansi\deff1{\fonttbl{\f1\fcharset204 Cyrillic;}}\'c0\plain\'c1}`, "АБ"], +])("retains RTF grouped Unicode and font text in %s", async (source, expected) => { + const { openRtf } = await import("./extendedDocumentPreview"); + const result = openRtf(new TextEncoder().encode(source).buffer); + if (result.type !== "document") throw new Error("Expected rich text document"); + const html = result.pages.map((page) => page.html).join(""); + expect(new DOMParser().parseFromString(html, "text/html").body.textContent).toBe(expected); +}); + +it.each(["odt", "odp"] as const)("preserves rendered %s spaces and tabs", async (kind) => { + const archive = new JSZip().file( + "content.xml", + `${kind === "odt" ? "" : ""}ABC${kind === "odt" ? "" : ""}`, + ); + const bytes = await archive.generateAsync({ type: "arraybuffer" }); + const worker = new Worker(new URL("./officePreview.worker.ts", import.meta.url), { + type: "module", + }); + try { + const result = await new Promise((resolve) => { + worker.addEventListener( + "message", + (event: MessageEvent) => resolve(event.data), + { once: true }, + ); + worker.postMessage({ type: "open", kind, bytes }, [bytes]); + }); + if (result.type !== "document") throw new Error("Expected document"); + await using screen = await render( + , + ); + await vi.waitFor(() => { + const doc = screen.container.querySelector("iframe")?.contentDocument; + const spans = doc?.querySelectorAll("span[data-odf-whitespace]"); + expect(doc?.querySelector("p")?.textContent).toBe("A B\tC"); + expect(spans).toHaveLength(2); + for (const span of spans!) { + expect(doc!.defaultView!.getComputedStyle(span).whiteSpace).toBe("pre-wrap"); + expect(span.getBoundingClientRect().width).toBeGreaterThan(0); + } + expect(spans![0]!.getBoundingClientRect().width).toBeGreaterThan(10); + }); + } finally { + worker.terminate(); + } +}); + +it.each(["rtf", "epub", "ods"] as const)( + "preserves %s document text through the worker and attachment viewer", + async (kind) => { + const archive = new JSZip(); + if (kind === "epub") { + archive.file( + "META-INF/container.xml", + '', + ); + archive.file( + "book.opf", + '', + ); + archive.file( + "chapter.xhtml", + "Handbook

first
second
third


", + ); + } else if (kind === "ods") { + archive.file( + "content.xml", + 'Northnested noteprivate noterevised from 120', + ); + } + const file = + kind === "rtf" + ? new Blob([ + String.raw`{\rtf1\ansi 10\endash 20 \emdash \bullet \lquote a\rquote \ldblquote b\rdblquote\par Name\cell Value\cell\row 12\cell 34\cell\row{\*\private hidden\endash\cell\row}Unicode:\u8211\endash end}`, + ]) + : await archive.generateAsync({ type: "blob" }); + const bytes = await file.arrayBuffer(); + const worker = new Worker(new URL("./officePreview.worker.ts", import.meta.url), { + type: "module", + }); + try { + const result = await new Promise((resolve) => { + worker.addEventListener( + "message", + (event: MessageEvent) => resolve(event.data), + { once: true }, + ); + worker.postMessage({ type: "open", kind, bytes }, [bytes]); + }); + if (kind === "ods") { + expect(result.type).toBe("ods"); + if (result.type !== "ods") throw new Error("Expected spreadsheet"); + expect(result.sheets[0]!.rows).toEqual([["North", "150"]]); + } else { + expect(result.type).toBe("document"); + if (result.type !== "document") throw new Error("Expected document"); + const body = new DOMParser().parseFromString(result.pages[0]!.html, "text/html").body; + if (kind === "epub") { + expect(body.querySelectorAll("br")).toHaveLength(2); + expect(body.querySelectorAll("hr")).toHaveLength(1); + } else { + expect(body.textContent).toContain("10–20 —•‘a’“b”"); + expect(body.textContent).toContain("Name\tValue\t12\t34\t"); + expect(body.textContent).toContain("Unicode:–end"); + expect(body.textContent).not.toContain("hidden"); + expect(body.querySelectorAll("br")).toHaveLength(3); + } + } + } finally { + worker.terminate(); + } + await using screen = await render( +
+ +
, + ); + await vi.waitFor(() => { + if (kind === "ods") { + expect(screen.container.textContent).toContain("North"); + expect(screen.container.textContent).toContain("150"); + expect(screen.container.textContent).not.toContain("private note"); + expect(screen.container.textContent).toContain("1 rows · 2 columns"); + expect(screen.container.textContent).not.toContain("Limited to 500"); + } else { + const frame = screen.container.querySelector("iframe"); + expect(frame?.srcdoc).toContain(kind === "rtf" ? "10–20" : "first
second
third"); + } + }); + expect(screen.container.querySelector('[role="alert"]')).toBeNull(); + }, +); + +it.each(["epub", "odt", "ipynb", "html", "encoded-html"] as const)( + "bounds %s image pixels before returning browser markup", + async (kind) => { + const canvas = new OffscreenCanvas(2, 3); + canvas.getContext("2d"); + const blob = await canvas.convertToBlob({ type: "image/png" }); + for (const width of [2, 12000]) { + const image = await blob.arrayBuffer(); + new DataView(image).setUint32(16, width); + new DataView(image).setUint32(20, width === 2 ? 3 : 12000); + const archive = new JSZip(); + archive.file("cover.png", image); + if (kind === "epub") { + archive.file( + "META-INF/container.xml", + '', + ); + archive.file( + "book.opf", + '', + ); + archive.file( + "chapter.xhtml", + '

retained caption

', + ); + } else + archive.file( + "content.xml", + 'retained caption', + ); + const base64 = btoa(String.fromCharCode(...new Uint8Array(image))); + const bytes = + kind === "ipynb" || kind === "html" || kind === "encoded-html" + ? new TextEncoder().encode( + JSON.stringify({ + nbformat: 4, + cells: [ + { + cell_type: "code", + source: "plot()", + outputs: [ + { + data: { + ...(kind === "html" || kind === "encoded-html" + ? { + "text/html": `
retained caption
`, + } + : { "image/png": base64 }), + "text/plain": "retained caption", + }, + }, + ], + }, + ], + }), + ).buffer + : await archive.generateAsync({ type: "arraybuffer" }); + const worker = new Worker(new URL("./officePreview.worker.ts", import.meta.url), { + type: "module", + }); + try { + const result = await new Promise((resolve) => { + worker.addEventListener( + "message", + (event: MessageEvent) => resolve(event.data), + { once: true }, + ); + worker.postMessage( + { + type: "open", + kind: kind === "html" || kind === "encoded-html" ? "ipynb" : kind, + bytes, + }, + [bytes], + ); + }); + if (result.type === "notebook") { + const output = result.cells[0]!.outputs[0]!; + expect(output.kind).toBe( + width === 2 ? (kind === "html" || kind === "encoded-html" ? "html" : "image") : "text", + ); + if (output.kind === "html") { + const rendered = new DOMParser().parseFromString( + sanitizeDocumentHtml(output.text), + "text/html", + ); + expect(rendered.images).toHaveLength(1); + expect(rendered.querySelector("td")!.textContent).toBe("retained caption"); + } + if (width !== 2) expect(result.cells[0]!.outputs[0]!.text).toBe("retained caption"); + } else { + expect(result.type).toBe("document"); + if (result.type !== "document") throw new Error("Expected document"); + expect(result.pages[0]!.html.includes(" { + const { BinaryDataFactory } = await import("./pdfDocument"); + const factory = new BinaryDataFactory(); + const fetchAsset = vi.fn( + async () => + new Response("app", { + headers: { "content-type": "text/html; charset=utf-8" }, + }), + ); + using _fetch = stubGlobal("fetch", fetchAsset); + await expect(factory.fetch({ kind, filename })).rejects.toThrow( + `PDF ${directory} asset unavailable.`, + ); + fetchAsset.mockImplementation(async () => new Response(new Uint8Array([1, 2, 3]))); + await expect(factory.fetch({ kind, filename })).resolves.toEqual(new Uint8Array([1, 2, 3])); + await expect(factory.fetch({ kind, filename: "missing" })).rejects.toThrow( + `Unsupported PDF ${directory} asset.`, + ); +}); diff --git a/apps/web/src/components/files/AttachmentFilePreview.tsx b/apps/web/src/components/files/AttachmentFilePreview.tsx index 16b8c279023b..d582bfa1591e 100644 --- a/apps/web/src/components/files/AttachmentFilePreview.tsx +++ b/apps/web/src/components/files/AttachmentFilePreview.tsx @@ -1,11 +1,15 @@ import { filePreviewDelimiter } from "@t3tools/shared/delimitedPreview"; -import type { EnvironmentId, ScopedThreadRef } from "@t3tools/contracts"; +import type { AssetResource, EnvironmentId, ScopedThreadRef } from "@t3tools/contracts"; import { formatAttachmentSize } from "@t3tools/client-runtime/state/attachments"; import { readFilePreviewResponse } from "@t3tools/client-runtime/file-preview"; import { settlePromise } from "@t3tools/client-runtime/state/runtime"; import * as AsyncResult from "effect/reactivity/AsyncResult"; import * as Cause from "effect/Cause"; -import { filePreviewKind, FILE_TEXT_PREVIEW_MAX_BYTES } from "@t3tools/shared/filePreview"; +import { + filePreviewKind, + officePreviewKind, + FILE_TEXT_PREVIEW_MAX_BYTES, +} from "@t3tools/shared/filePreview"; import { ChevronRightIcon, DownloadIcon, Trash2Icon, WrapTextIcon, XIcon } from "lucide-react"; import { Check, Code2, Copy, Eye, Table2 } from "lucide"; import { lazy, Suspense, useEffect, useRef, useState } from "react"; @@ -19,6 +23,8 @@ import { useCopyToClipboard } from "~/hooks/useCopyToClipboard"; import { useClientSettings, updateClientSettings } from "~/hooks/useSettings"; import { cn } from "~/lib/utils"; +import { BinaryFilePreview } from "./BinaryFilePreview"; +import { PreviewImage } from "./PreviewImage"; import { AudioPreview } from "./AudioPreview"; import { BrowserDocumentFrame } from "./BrowserDocumentFrame"; import { DelimitedTablePreview } from "./DelimitedTablePreview"; @@ -30,6 +36,20 @@ import { FileSurfaceNotice, } from "./fileSurfaceChrome"; +const LargeTextViewer = lazy(() => import("./LargeTextViewer")); + +const OfficeFilePreview = lazy(() => + import("./OfficeFilePreview").then((module) => ({ default: module.OfficeFilePreview })), +); + +const FontFilePreview = lazy(() => + import("./FontFilePreview").then((module) => ({ default: module.FontFilePreview })), +); + +const MermaidFilePreview = lazy(() => + import("./MermaidFilePreview").then((module) => ({ default: module.MermaidFilePreview })), +); + const SourcePreview = lazy(() => import("./ReadOnlySourcePreview")); const STALE_URL_MS = 5 * 60_000; @@ -42,7 +62,11 @@ export function ReadOnlySourcePreview(props: { name: string; text: string }) { ); } -function renderedToggleLabel(mode: "markdown" | "html" | "table", rendered: boolean): string { +function renderedToggleLabel( + mode: "markdown" | "mermaid" | "html" | "table", + rendered: boolean, +): string { + if (mode === "mermaid") return rendered ? "Show Mermaid source" : "Show rendered diagram"; if (mode === "markdown") return rendered ? "Show markdown source" : "Show rendered markdown"; if (mode === "table") return rendered ? "Show source" : "Show table"; return rendered ? "Show HTML source" : "Show rendered page"; @@ -53,7 +77,12 @@ export function AttachmentFilePreview(props: { mimeType: string; sizeBytes: number; file?: Blob | null; - asset?: { environmentId: EnvironmentId; attachmentId: string }; + asset?: + | { environmentId: EnvironmentId; attachmentId: string } + | { + environmentId: EnvironmentId; + resource: Extract; + }; htmlRender?: boolean; threadRef?: ScopedThreadRef; origin?: string; @@ -61,22 +90,39 @@ export function AttachmentFilePreview(props: { onClose?: () => void; }) { const kind = filePreviewKind(props); + const officeKind = officePreviewKind(props); const delimiter = filePreviewDelimiter(props); const renderedMode = - kind === "markdown" ? "markdown" : kind === "html" ? "html" : delimiter ? "table" : null; - const attachmentId = props.asset?.attachmentId; + kind === "mermaid" + ? "mermaid" + : kind === "markdown" + ? "markdown" + : kind === "html" + ? "html" + : delimiter + ? "table" + : null; + const attachmentId = + props.asset && "attachmentId" in props.asset ? props.asset.attachmentId : undefined; const { copyToClipboard, isCopied } = useCopyToClipboard({ target: props.name }); // useAssetUrlRefresh keys its callback on these objects, so they must stay cached. - const resource = attachmentId - ? { - _tag: "attachment" as const, - attachmentId, - fileName: props.name, - mimeType: props.mimeType, - disposition: "inline" as const, - } + const resource = + props.asset && "resource" in props.asset + ? props.asset.resource + : attachmentId + ? { + _tag: "attachment" as const, + attachmentId, + fileName: props.name, + mimeType: props.mimeType, + disposition: "inline" as const, + } + : null; + const downloadResource = resource + ? resource._tag === "draft-workspace-file" + ? resource + : { ...resource, disposition: "attachment" as const } : null; - const downloadResource = resource ? { ...resource, disposition: "attachment" as const } : null; const refresh = useAssetUrlRefresh( props.file ? null : (props.asset?.environmentId ?? null), props.file ? null : resource, @@ -90,6 +136,7 @@ export function AttachmentFilePreview(props: { const [content, setContent] = useState<{ text: string; truncated: boolean } | null>(null); const [error, setError] = useState(null); const [contentError, setContentError] = useState(null); + const copyContents = useRef<(() => Promise) | null>(null); const authorizedAt = useRef(0); useEffect(() => { if (!props.file) return; @@ -121,9 +168,14 @@ export function AttachmentFilePreview(props: { // oxlint-disable-next-line react/exhaustive-effect-dependencies -- Retry must reauthorize the remote file. }, [props.file, refresh, revision]); const url = props.file ? localUrl : remoteUrl; - const needsText = kind === "text" || kind === "markdown" || (kind === "html" && !rendered); + const streamsText = + kind === "text" && + (props.sizeBytes > FILE_TEXT_PREVIEW_MAX_BYTES || + (props.sizeBytes === 0 && (!resource || resource._tag === "attachment"))); + const needsText = + kind === "text" || kind === "markdown" || kind === "mermaid" || (kind === "html" && !rendered); useEffect(() => { - if (!needsText || !url) return; + if (!needsText || streamsText || !url) return; const controller = new AbortController(); // oxlint-disable-next-line react/set-state-in-effect -- A new external resource must clear the previous response before loading. setContent(null); @@ -156,15 +208,15 @@ export function AttachmentFilePreview(props: { setContentError(cause instanceof Error ? cause.message : "Could not load this file."); }); return () => controller.abort(); - }, [url, needsText, revision, props.sizeBytes, props.file, refresh]); + }, [url, needsText, streamsText, revision, props.sizeBytes, props.file, refresh]); const failure = error ?? (needsText ? contentError : null); const wordWrap = useClientSettings((settings) => settings.wordWrap); const showsRawText = failure === null && needsText && - content !== null && + (content !== null || streamsText) && !(delimiter && rendered) && - !(kind === "markdown" && rendered); + !((kind === "markdown" || kind === "mermaid") && rendered); const save = () => { setSaving(true); @@ -177,8 +229,7 @@ export function AttachmentFilePreview(props: { if (!response.ok) throw new Error("The file could not be loaded. Try again."); file = await response.blob(); } - // A Blob keeps cross-origin downloads inside the desktop client instead of - // navigating its custom app scheme to an external browser. + // Blob downloads keep desktop's custom app scheme from navigating to an external browser. const downloadUrl = URL.createObjectURL(file); // Scheduled before the click so the URL is revoked even if the click throws. setTimeout(() => URL.revokeObjectURL(downloadUrl), 30_000); @@ -207,11 +258,27 @@ export function AttachmentFilePreview(props: { setRevision((value) => value + 1); }} /> + ) : streamsText && url ? ( + }> + setRevision((value) => value + 1)} + delimiter={rendered ? delimiter : null} + copyContents={copyContents} + /> + ) : !url || (needsText && !content) ? ( ) : needsText && content ? ( delimiter && rendered ? ( + ) : kind === "mermaid" && rendered ? ( + }> + + ) : kind === "markdown" && rendered ? ( @@ -219,6 +286,27 @@ export function AttachmentFilePreview(props: { ) : ( ) + ) : kind === "archive" || kind === "sqlite" ? ( + setRevision((value) => value + 1)} + /> + ) : kind === "font" ? ( + }> + + + ) : officeKind ? ( + }> + + ) : kind === "pdf" || kind === "html" ? ( ) : kind === "image" ? ( - // oxlint-disable-next-line t3code/require-centered-scroll-gutter -- The image is capped at max-h-full max-w-full, so this never scrolls. -
- {props.name} setError("Unable to load image.")} - /> -
+ setError("Unable to load image.")} + /> ) : (

No preview for this file

@@ -298,10 +384,33 @@ export function AttachmentFilePreview(props: { ) : null} - {content ? ( + {content || streamsText ? ( copyToClipboard(content.text, undefined)} + label={isCopied ? "Copied" : content?.truncated ? "Copy preview" : "Copy contents"} + onPress={() => { + if (content) { + void copyToClipboard(content.text, undefined); + return; + } + const copy = copyContents.current; + if (!copy) { + toastManager.add({ + type: "error", + title: "Could not copy file", + description: "Wait for the file to finish loading before copying.", + }); + return; + } + void copy() + .then((text) => copyToClipboard(text, undefined)) + .catch((cause: unknown) => + toastManager.add({ + type: "error", + title: "Could not copy file", + description: cause instanceof Error ? cause.message : "Try again.", + }), + ); + }} > diff --git a/apps/web/src/components/files/AudioPreview.tsx b/apps/web/src/components/files/AudioPreview.tsx index 23d0d2db15c3..735a56e524e0 100644 --- a/apps/web/src/components/files/AudioPreview.tsx +++ b/apps/web/src/components/files/AudioPreview.tsx @@ -1,12 +1,38 @@ -import { useState } from "react"; +import { LoaderCircleIcon, PauseIcon, PlayIcon, TriangleAlertIcon } from "lucide-react"; +import { createContext, useContext, useRef, useState } from "react"; -export function AudioPreview(props: { +import { useStableCallback } from "~/hooks/useStableCallback"; +import { ContextChip, ContextChipAction, ContextChipLabel } from "../ContextChip"; +import { middleTruncateAttachmentName } from "../composerInlineChip"; + +export const AudioPreviewUnmountContext = createContext<((audio: HTMLAudioElement) => void) | null>( + null, +); + +type AudioPreviewProps = { readonly src: string; readonly name: string; - /** Re-signed URLs with the same revision keep the loaded audio and its position. */ readonly revision?: string | null; readonly onError?: () => void; -}) { + readonly compact?: boolean; + readonly failed?: boolean; + readonly onOpen?: (() => void) | undefined; + readonly onRetry?: () => void; +}; + +export function AudioPreview(props: AudioPreviewProps) { + return props.compact ? ( + + ) : ( + + ); +} + +function StandardAudioPreview(props: AudioPreviewProps) { + const onUnmount = useContext(AudioPreviewUnmountContext); + const audioRef = useStableCallback((audio: HTMLAudioElement | null) => { + if (audio) return () => onUnmount?.(audio); + }); const [loadedSource, setLoadedSource] = useState<{ src: string; revision: string } | null>(null); const [playing, setPlaying] = useState(false); const revision = props.revision ?? null; @@ -18,6 +44,7 @@ export function AudioPreview(props: { return (