diff --git a/.agents/upstream-review.md b/.agents/upstream-review.md index 718e2a5fa..b58bc4c01 100644 --- a/.agents/upstream-review.md +++ b/.agents/upstream-review.md @@ -61,6 +61,7 @@ Historical groups are indexed in the linked archive. This file migration changes | Mobile Android: provider setup removal, Material You, Keychain group, Android notifications / `6c583620ff7ad3235b135af7107c0543467eecfa` | Full ten-source list in #456, from #9721 through #10416 | Adopted #9721 (mobile Antigravity setup from the #440 port removed; Pylon's Prime host maintenance status kept), #3665 (team via `$(AppIdentifierPrefix)`), #10691, #10692, #10859 and #10416 (FCM relay delivery, Android ongoing activity, `/v2/client/devices`, additive relay migration). #10598 and #10620 already covered: Pylon's exporter fixed launcher cropping and its splash mark fits the Android 12+ mask; T3 background artwork layers left to the maintainer. Two version bumps skipped. Adaptations: Pylon EAS OTA gate, package IDs, signed-bundle Keychain group, relay hourly cron, Effect `TaggedErrorClass`; notification switches need Firebase config and a registered push token; the Hermes cookie assertion was restored when integrating after #460 carried #10851's Effect patch fix. Firebase/FCM not provisioned; rollback needs a forward fix. Cursor unchanged. | [Mobile Android #456](https://github.com/pylon-code/pylon/pull/456); 258 focused tests, five package typechecks, scoped lint/format, generated-theme and icon checks. | | Chat view, composer and timeline fixes / `6c583620ff7ad3235b135af7107c0543467eecfa` | Full 11-source list in [#457](https://github.com/pylon-code/pylon/pull/457), from #10256 through #10768; `7220dfe2c949476eaa7d21eccbcd3a0ce0eddb49` (#9871) lands separately | Adopted English tooltip dates, visible task row states, projection-owned checkpoint lookups, anchored toolbar labels, remembered Fast mode, `thread.stop`, proactive panels on entry, scroll-to-end clearance, minimap turn navigation and the composer footer/bar loading fixes. Pylon adaptations: revert buttons keep the exact rollback proof (`deriveRollbackTargets` over server messages only, so optimistic sends never claim a target (after #460 moved `/feedback` results into composer banners, feedback adds no timeline rows); target re-derived on click) instead of inferred counts; Stop, `thread.stop` and a new palette action share one interrupt that also stops turns awaiting provider admission; the implicit Normal Fast default applies only to interactive composers, not background text generation; visible status labels replace the screen-reader-only prefixes; `reportsContextWindow` is set for Codex, Claude and Prime daemon (not Prime ACP, Cursor, Grok, OpenCode or Antigravity) and reads `contextWindowIndicatorEnabled`. Pylon's locked workspace label keeps its unmotioned markup. Mobile unchanged: the stop shortcut and palette action, minimap navigation and reserved meter slot are web/desktop surfaces. Cursor unchanged. | [Web composer #457](https://github.com/pylon-code/pylon/pull/457); 853 focused tests, web/server/contracts/client-runtime typechecks, scoped lint/format. | | Remote media fallback, recording transfer, browser context menus, MCP snapshot output and macOS installer art / `6c583620ff7ad3235b135af7107c0543467eecfa` | `a01b227d6f37d1cfed7a2f47aaace2f72ea76ce3` (#10619), `9e37f0c291974d084a59d7b9f165a1cb056e6043` (#10572), `b5f7fa0ede2a0d791226e6474c9cc4374dd89cc9` (#10670), `061543e9e5b54ec0048725c37d52fef2962df173` (#10501), `991526383f95eefdd66ffd2853136c6dd34ae008` (#10632), `5d14c0e9686d8bedf47dd93dd3811fd639255c7e` (#10819), `0fe4c99ee6df4cbb7a9064d2d86ece65ecef5eb3` (#10820) | All seven adopted. Desktop retries missing drive or POSIX absolute media from remote threads (relative images arrive workspace-resolved) against its primary environment; UNC and device paths never fall back, client or server. Agent recordings upload once to a remote requesting environment, while the desktop's own primary environment gets the saved path with no copy (older desktops get an update-required error). Pylon adaptations: transferred recordings and `save: true` screenshots live in `browser-artifacts//` so rollbacks keep them and thread deletion removes them; `preview_snapshot` keeps upstream's read-only/idempotent hints because saves stay in that per-thread store; Codex `tool_timeout_sec` and OpenCode `timeout` are 180 s for Pylon's MCP server, Claude and ACP providers keep their defaults. #10501 uses `Schema.TaggedErrorClass` for Pylon's Effect. DMG layout, sizes and art follow upstream (Nightly aurora raster has no marks); both variants carry the Pylon mark and "Drag Pylon into Applications." instead of T3's wordmark; titles, artifact names and signing unchanged. Split from window capture #8103, which lands separately. Cursor unchanged. | [Desktop fixes #462](https://github.com/pylon-code/pylon/pull/462); shared 6, client-runtime 50, server MCP/asset/config/pipeline/adapter 308 + router 5, web 79, desktop 126 and build-script 80 tests; contracts, shared, client-runtime, t3, web, desktop, mobile and scripts typechecks; scoped lint/format. | +| Files in question answers / `6c583620ff7ad3235b135af7107c0543467eecfa` | `7220dfe2c949476eaa7d21eccbcd3a0ce0eddb49` (#9871) | Adopted across contracts, server, web, desktop and mobile: `attachmentsByQuestionId` on respond commands and events, the `questionAttachments` capability, normalizer claims, `user-input.answer-submitted` history, provider answer path lines in `ProviderService`, and a separate `projection.attachment-cleanup` cursor that retains answer files through reverts and deletes. Per adapter: Codex, Claude, Cursor, Grok and OpenCode receive path lines; Antigravity questions never accept custom answers, so the decider refuses their attachments; Prime asks no user-input questions. Pylon adaptation: normalizer and failed-dispatch cleanup keep `thread.input-queue.follow-up`; mobile readiness uses the #10404 upload helpers from #460 as upstream does. Pylon diverges on cleanup mechanics, which upstream shares at the frozen head: the live path writes the cleanup cursor with the projector cursors at the last finished cleanup; bootstrap selects only revert/delete rows past it without decoding payloads, lists the attachments directory once and skips threads without files (1,000 deleted threads with 3,000 files: 28.8 s to 2 ms); a database without the row starts at its lowest projector cursor; file errors are logged and retried once on the next start instead of pinning the cursor; revert retention reads only answer activities; unused `minLastAppliedSequence` removed. Mobile discards question drafts only on live thread data and explains a paste refused by an older server. Claude, Cursor, Grok and OpenCode are code-path verified; upstream live-tested Codex. Cursor unchanged. | [Question attachments #463](https://github.com/pylon-code/pylon/pull/463); 868 focused tests (28 files: server 16, web 7, mobile 5), five package typechecks, scoped lint/format, before/after bootstrap probe. | ## Deferred register diff --git a/apps/mobile/package.json b/apps/mobile/package.json index dee50460a..93d1c0ae9 100644 --- a/apps/mobile/package.json +++ b/apps/mobile/package.json @@ -129,6 +129,7 @@ "@effect/vitest": "catalog:", "@pierre/trees": "1.0.0-beta.4", "@types/react": "~19.2.0", + "@types/react-dom": "~19.2.3", "babel-preset-expo": "~57.0.9", "tailwindcss": "^4.0.0", "typescript": "catalog:" diff --git a/apps/mobile/src/features/threads/PendingUserInputCard.tsx b/apps/mobile/src/features/threads/PendingUserInputCard.tsx index e7642dd50..3c54ff945 100644 --- a/apps/mobile/src/features/threads/PendingUserInputCard.tsx +++ b/apps/mobile/src/features/threads/PendingUserInputCard.tsx @@ -1,3 +1,4 @@ +import { QuestionAttachments } from "./QuestionAttachments"; import type { ApprovalRequestId, UserInputQuestion } from "@t3tools/contracts"; import { useCallback, useRef } from "react"; import { Platform, Pressable, ScrollView, View, type LayoutChangeEvent } from "react-native"; @@ -15,7 +16,7 @@ import Animated, { import { USER_INPUT_TOGGLE_DURATION_MS } from "./pendingUserInputLayout"; import { SymbolView } from "../../components/AppSymbol"; -import { AppText as Text, AppTextInput as TextInput } from "../../components/AppText"; +import { AppText as Text } from "../../components/AppText"; import { ControlPill } from "../../components/ControlPill"; import { cn } from "../../lib/cn"; import { @@ -307,18 +308,17 @@ export function PendingUserInputCard(props: PendingUserInputCardProps) { ); })} - {question.allowCustomAnswer !== false ? ( - - props.onChangeCustomAnswer(props.pendingUserInput.requestId, question.id, value) - } - onFocus={() => props.onInputFocusChange?.(true)} - onBlur={() => props.onInputFocusChange?.(false)} - placeholder="Or type a custom answer" - className="min-h-[54px] rounded-2xl border border-input-border bg-input px-3.5 py-3 font-sans text-base text-foreground" - /> - ) : null} + + props.onChangeCustomAnswer(props.pendingUserInput.requestId, question.id, value) + } + onInputFocusChange={props.onInputFocusChange} + /> ); })} diff --git a/apps/mobile/src/features/threads/QuestionAnswerHistory.test.tsx b/apps/mobile/src/features/threads/QuestionAnswerHistory.test.tsx new file mode 100644 index 000000000..9e8581eb0 --- /dev/null +++ b/apps/mobile/src/features/threads/QuestionAnswerHistory.test.tsx @@ -0,0 +1,58 @@ +import { ApprovalRequestId, EnvironmentId } from "@t3tools/contracts"; +import type { ReactNode } from "react"; +import { renderToStaticMarkup } from "react-dom/server"; +import { describe, expect, it, vi } from "vite-plus/test"; + +vi.mock("react-native", () => ({ + View: "div", + Pressable: ({ children }: { children: ReactNode }) => , + Image: "img", + Linking: { openURL: vi.fn() }, +})); +vi.mock("../../components/AppText", () => ({ AppText: "span" })); +vi.mock("../../state/assets", () => ({ useAssetUrl: () => null })); + +import { QuestionAnswerHistory } from "./QuestionAnswerHistory"; + +describe("QuestionAnswerHistory", () => { + it.each([{}, { text: "Text-only answer", file: "Answer with a file" }])( + "renders attachment-only questions alongside text answers: %j", + (answers) => { + const markup = renderToStaticMarkup( + , + ); + expect(markup.match(/Provide a spec/g)).toHaveLength(1); + expect(markup.match(/spec\.txt/g)).toHaveLength(1); + expect(markup).toContain("Provide a screenshot"); + expect(markup).toContain("shot.png"); + for (const answer of Object.values(answers)) expect(markup).toContain(answer); + }, + ); +}); diff --git a/apps/mobile/src/features/threads/QuestionAnswerHistory.tsx b/apps/mobile/src/features/threads/QuestionAnswerHistory.tsx new file mode 100644 index 000000000..2ff0a3b37 --- /dev/null +++ b/apps/mobile/src/features/threads/QuestionAnswerHistory.tsx @@ -0,0 +1,71 @@ +import type { + EnvironmentId, + UserInputAttachmentAnswerPayload, + UserInputAttachments, +} from "@t3tools/contracts"; +import { Image, Linking, Pressable, View } from "react-native"; +import { AppText as Text } from "../../components/AppText"; +import { useAssetUrl } from "../../state/assets"; + +function AnswerFile(props: { + environmentId: EnvironmentId; + attachment: UserInputAttachments[string][number]; +}) { + const url = useAssetUrl(props.environmentId, { + _tag: "attachment", + attachmentId: props.attachment.id, + }); + return ( + { + if (url) void Linking.openURL(url); + }} + className="gap-1" + > + {props.attachment.type === "image" && url ? ( + + ) : null} + {props.attachment.name} + + ); +} + +export function QuestionAnswerHistory(props: { + environmentId: EnvironmentId; + answer: UserInputAttachmentAnswerPayload; +}) { + return ( + + {[ + ...new Set([ + ...Object.keys(props.answer.answers), + ...Object.keys(props.answer.attachmentsByQuestionId), + ]), + ].map((questionId) => ( + + {props.answer.questionTextById?.[questionId] ? ( + + {props.answer.questionTextById[questionId]} + + ) : null} + + {[props.answer.answers[questionId]] + .flat() + .filter((value): value is string => typeof value === "string") + .join(", ")} + + {(props.answer.attachmentsByQuestionId[questionId] ?? []).map((attachment) => ( + + ))} + + ))} + + ); +} diff --git a/apps/mobile/src/features/threads/QuestionAttachments.test.tsx b/apps/mobile/src/features/threads/QuestionAttachments.test.tsx new file mode 100644 index 000000000..0f7118511 --- /dev/null +++ b/apps/mobile/src/features/threads/QuestionAttachments.test.tsx @@ -0,0 +1,102 @@ +import type { ReactNode } from "react"; +import { renderToStaticMarkup } from "react-dom/server"; +import { beforeEach, describe, expect, it, vi } from "vite-plus/test"; + +const fixture = vi.hoisted(() => ({ + questionAttachments: true, + onPaste: undefined as ((payload: { type: "images"; uris: Array }) => void) | undefined, +})); +vi.mock("react-native", () => ({ Alert: { alert: vi.fn() }, View: "div" })); +vi.mock("expo-paste-input", () => ({ + TextInputWrapper: (props: { onPaste: typeof fixture.onPaste; children: ReactNode }) => { + fixture.onPaste = props.onPaste; + return props.children; + }, +})); +vi.mock("@effect/atom-react", () => ({ useAtomValue: () => ({}) })); +vi.mock("../../components/AppText", () => ({ AppTextInput: "input" })); +vi.mock("../../components/ComposerAttachmentButton", () => ({ + ComposerAttachmentButton: () => null, +})); +vi.mock("../../components/ComposerAttachmentStrip", () => ({ + ComposerAttachmentStrip: () => null, +})); +vi.mock("../../lib/composerImages", () => ({ + convertPastedImagesToAttachments: vi.fn(async () => []), + pickComposerFiles: vi.fn(), + pickComposerMedia: vi.fn(), +})); +vi.mock("../../state/atom-registry", () => ({ + appAtomRegistry: { get: () => ({}), set: vi.fn() }, +})); +vi.mock("../../state/use-composer-drafts", () => ({ + appendComposerDraftAttachments: vi.fn(), + composerDraftsAtom: "drafts", + removeComposerDraftAttachment: vi.fn(), + releaseUnusedComposerAttachmentFiles: vi.fn(), +})); +vi.mock("../../state/use-thread-selection", () => ({ + useThreadSelection: () => ({ + selectedThread: { environmentId: "environment-1", id: "thread-1" }, + }), +})); +vi.mock("../../state/entities", () => ({ + useServerConfigs: () => + new Map([ + [ + "environment-1", + { environment: { capabilities: { questionAttachments: fixture.questionAttachments } } }, + ], + ]), +})); + +import { ApprovalRequestId } from "@t3tools/contracts"; +import { Alert } from "react-native"; +import { convertPastedImagesToAttachments } from "../../lib/composerImages"; +import { QuestionAttachments } from "./QuestionAttachments"; + +function pasteImage() { + const question = { + id: "question-1", + header: "Screenshot", + question: "Show the error", + options: [], + allowCustomAnswer: true, + }; + renderToStaticMarkup( + {}} + />, + ); + fixture.onPaste?.({ type: "images", uris: ["file:///pasted.png"] }); +} + +beforeEach(() => { + vi.mocked(Alert.alert).mockClear(); + vi.mocked(convertPastedImagesToAttachments).mockClear(); + fixture.onPaste = undefined; +}); + +describe("QuestionAttachments paste", () => { + it("tells the user to update a server without question attachments", () => { + fixture.questionAttachments = false; + pasteImage(); + expect(Alert.alert).toHaveBeenCalledWith( + "Could not paste image", + "Update this server to send files with question answers.", + ); + expect(convertPastedImagesToAttachments).not.toHaveBeenCalled(); + }); + + it("stages the image when the server supports question attachments", () => { + fixture.questionAttachments = true; + pasteImage(); + expect(Alert.alert).not.toHaveBeenCalled(); + expect(convertPastedImagesToAttachments).toHaveBeenCalledOnce(); + }); +}); diff --git a/apps/mobile/src/features/threads/QuestionAttachments.tsx b/apps/mobile/src/features/threads/QuestionAttachments.tsx new file mode 100644 index 000000000..62d56e0e8 --- /dev/null +++ b/apps/mobile/src/features/threads/QuestionAttachments.tsx @@ -0,0 +1,189 @@ +import { TextInputWrapper } from "expo-paste-input"; +import { AppTextInput as TextInput } from "../../components/AppText"; +import { useNativePaste } from "../../lib/useNativePaste"; +import { convertPastedImagesToAttachments } from "../../lib/composerImages"; +import { + PROVIDER_SEND_TURN_MAX_ATTACHMENTS, + type ApprovalRequestId, + type UserInputQuestion, +} from "@t3tools/contracts"; +import { useAtomValue } from "@effect/atom-react"; +import { Alert, View } from "react-native"; +import { useEffect, useRef } from "react"; +import { ComposerAttachmentButton } from "../../components/ComposerAttachmentButton"; +import { ComposerAttachmentStrip } from "../../components/ComposerAttachmentStrip"; +import { pickComposerFiles, pickComposerMedia } from "../../lib/composerImages"; +import { useThreadSelection } from "../../state/use-thread-selection"; +import { useServerConfigs } from "../../state/entities"; +import { appAtomRegistry } from "../../state/atom-registry"; +import { + appendComposerDraftAttachments, + composerDraftsAtom, + removeComposerDraftAttachment, + releaseUnusedComposerAttachmentFiles, +} from "../../state/use-composer-drafts"; +import { + changeQuestionAttachmentPreparation, + questionAttachmentDraftKey, + questionAttachmentPreparationAtom, +} from "../../state/question-attachments"; + +export function QuestionAttachments(props: { + requestId: ApprovalRequestId; + question: UserInputQuestion; + questions: ReadonlyArray; + disabled: boolean; + value: string; + onChangeText: (value: string) => void; + onInputFocusChange?: ((focused: boolean) => void) | undefined; +}) { + const { selectedThread } = useThreadSelection(); + const configs = useServerConfigs(); + const drafts = useAtomValue(composerDraftsAtom); + const scopeKey = JSON.stringify([ + selectedThread?.environmentId, + selectedThread?.id, + props.requestId, + props.question.id, + ]); + const pickerScope = useRef<{ key: string; active: boolean } | null>(null); + useEffect(() => { + const scope = { key: scopeKey, active: true }; + pickerScope.current = scope; + return () => { + scope.active = false; + }; + }, [scopeKey]); + const append = ( + key: string, + attachments: Parameters[1], + ) => { + if (!selectedThread) return 0; + const current = appAtomRegistry.get(composerDraftsAtom); + const otherCount = props.questions.reduce((count, question) => { + const target = questionAttachmentDraftKey( + selectedThread.environmentId, + selectedThread.id, + props.requestId, + question.id, + ); + return target === key ? count : count + (current[target]?.attachments.length ?? 0); + }, 0); + return appendComposerDraftAttachments(key, attachments, { + maxAttachments: Math.max(0, PROVIDER_SEND_TURN_MAX_ATTACHMENTS - otherCount), + }); + }; + const paste = useNativePaste((uris) => { + const scope = pickerScope.current; + if (!selectedThread || props.disabled) return; + const environmentConfig = configs.get(selectedThread.environmentId); + if (!environmentConfig?.environment.capabilities.questionAttachments) { + // Without a config the server has not answered yet, so it may well support files. + Alert.alert( + "Could not paste image", + environmentConfig + ? "Update this server to send files with question answers." + : "Connect to this environment before adding files to an answer.", + ); + return; + } + const key = questionAttachmentDraftKey( + selectedThread.environmentId, + selectedThread.id, + props.requestId, + props.question.id, + ); + changeQuestionAttachmentPreparation(key, 1); + void convertPastedImagesToAttachments({ + uris, + existingCount: appAtomRegistry.get(composerDraftsAtom)[key]?.attachments.length ?? 0, + }) + .then(async (images) => { + if ( + scope?.active && + (appAtomRegistry.get(questionAttachmentPreparationAtom)[key] ?? 0) > 0 + ) { + if (append(key, images) > 0) + Alert.alert("Could not paste image", "Too many attachments."); + } else await releaseUnusedComposerAttachmentFiles(images); + }) + .catch((error) => + Alert.alert("Could not paste image", error instanceof Error ? error.message : "Try again."), + ) + .finally(() => changeQuestionAttachmentPreparation(key, -1)); + }); + if (!selectedThread || props.question.allowCustomAnswer === false) return null; + const { environmentId, id: threadId } = selectedThread; + const capabilities = configs.get(environmentId)?.environment.capabilities; + const canAttach = capabilities?.questionAttachments === true; + const key = questionAttachmentDraftKey( + environmentId, + threadId, + props.requestId, + props.question.id, + ); + const attachments = drafts[key]?.attachments ?? []; + const pick = async (kind: "media" | "files") => { + const scope = pickerScope.current; + changeQuestionAttachmentPreparation(key, 1); + try { + const existingCount = appAtomRegistry.get(composerDraftsAtom)[key]?.attachments.length ?? 0; + const result = + kind === "files" + ? await pickComposerFiles({ + existingCount, + maxBytes: capabilities?.fileAttachments?.maxUploadBytes, + }) + : await pickComposerMedia({ + existingCount, + maxVideoBytes: capabilities?.fileAttachments?.maxUploadBytes, + }); + const picked = "files" in result ? result.files : result.attachments; + // Resolution on another client clears the reservation while the picker is open. + if ( + !scope?.active || + (appAtomRegistry.get(questionAttachmentPreparationAtom)[key] ?? 0) === 0 + ) { + await releaseUnusedComposerAttachmentFiles(picked); + return; + } + const rejected = append(key, picked); + if (result.error || rejected > 0) + Alert.alert("Could not attach file", result.error ?? "Too many attachments."); + } catch (error) { + Alert.alert("Could not attach file", error instanceof Error ? error.message : "Try again."); + } finally { + changeQuestionAttachmentPreparation(key, -1); + } + }; + return ( + + {canAttach ? ( + pick("media")} + onPickFiles={() => pick("files")} + /> + ) : null} + { + if (!props.disabled) removeComposerDraftAttachment(key, id); + }} + /> + + props.onInputFocusChange?.(true)} + onBlur={() => props.onInputFocusChange?.(false)} + placeholder="Or type a custom answer" + className="min-h-[54px] rounded-2xl border border-input-border bg-input px-3.5 py-3 font-sans text-base text-foreground" + /> + + + ); +} diff --git a/apps/mobile/src/features/threads/thread-work-log.tsx b/apps/mobile/src/features/threads/thread-work-log.tsx index 5ecd753be..ec4c3deaf 100644 --- a/apps/mobile/src/features/threads/thread-work-log.tsx +++ b/apps/mobile/src/features/threads/thread-work-log.tsx @@ -1,3 +1,4 @@ +import { QuestionAnswerHistory } from "./QuestionAnswerHistory"; import * as Haptics from "expo-haptics"; import { Image } from "expo-image"; import { type AppSymbolName, SymbolView } from "../../components/AppSymbol"; @@ -859,13 +860,19 @@ const ThreadWorkLogRow = memo(function ThreadWorkLogRow( - {expanded && (fullDetail || viewedImagePath) ? ( + {expanded && (fullDetail || viewedImagePath || row.workEntry.questionAnswer) ? ( + {row.workEntry.questionAnswer ? ( + + ) : null} {viewedImagePath ? ( {props.renderImage({ href: viewedImagePath, alt: null, title: null })} diff --git a/apps/mobile/src/lib/composerAttachmentUploadQueue.test.ts b/apps/mobile/src/lib/composerAttachmentUploadQueue.test.ts index ba70a2845..8bf67456c 100644 --- a/apps/mobile/src/lib/composerAttachmentUploadQueue.test.ts +++ b/apps/mobile/src/lib/composerAttachmentUploadQueue.test.ts @@ -1,5 +1,10 @@ -import { EnvironmentId } from "@t3tools/contracts"; +import { ApprovalRequestId, EnvironmentId, ThreadId } from "@t3tools/contracts"; import { describe, expect, it, vi } from "vite-plus/test"; +import { + questionAttachmentDraftKey, + questionAttachmentDraftPrefix, +} from "../state/question-attachments"; +vi.mock("../state/atom-registry", () => ({ appAtomRegistry: {} })); import { composerAttachmentUploadBlockReason, @@ -217,6 +222,26 @@ describe("composer attachment upload queue", () => { }); describe("draft upload scope and offline submission", () => { + it("uploads question drafts in their environment without matching other thread prefixes", () => { + const requestId = ApprovalRequestId.make("request:1"); + for (const environment of [environmentId, EnvironmentId.make("remote:server")]) { + const threadId = ThreadId.make("thread:1"); + const prefix = questionAttachmentDraftPrefix(environment, threadId); + const key = questionAttachmentDraftKey(environment, threadId, requestId, "question:1"); + expect(composerDraftEnvironmentId(key, [])).toBe(environment); + expect(key.startsWith(prefix)).toBe(true); + for (const suffix of ["-extra", ":extra", "/extra", "%extra"]) { + const otherKey = questionAttachmentDraftKey( + environment, + ThreadId.make(`${threadId}${suffix}`), + requestId, + "question:1", + ); + expect(otherKey.startsWith(prefix)).toBe(false); + expect(composerDraftEnvironmentId(otherKey, [])).toBe(environment); + } + } + }); it("resolves thread, new-task, and queued-task drafts without crossing environments", () => { expect(composerDraftEnvironmentId("environment-1:thread", [])).toBe(environmentId); expect(composerDraftEnvironmentId("new-task:environment-1:project", [])).toBe(environmentId); diff --git a/apps/mobile/src/lib/threadActivity.test.ts b/apps/mobile/src/lib/threadActivity.test.ts index 5d1e6cc19..32f59bf6f 100644 --- a/apps/mobile/src/lib/threadActivity.test.ts +++ b/apps/mobile/src/lib/threadActivity.test.ts @@ -2,6 +2,7 @@ import { derivePendingRequests } from "@t3tools/client-runtime/pending-requests" import { describe, expect, it } from "vite-plus/test"; import { + ApprovalRequestId, EventId, MessageId, ProjectId, @@ -3753,3 +3754,72 @@ describe("quiet timeline: nested agents", () => { ]); }); }); + +it("accepts ready attachment-only answers while preserving selected options", () => { + const question = { + id: "q", + header: "Spec", + question: "Provide a specification", + options: [{ label: "Yes", description: "Approve" }], + multiSelect: false, + }; + expect(buildPendingUserInputAnswers([question], { q: { attachmentCount: 1 } })).toEqual({ + q: "", + }); + expect( + buildPendingUserInputAnswers([question], { + q: { attachmentCount: 1, selectedOptionValues: ["Yes"] }, + }), + ).toEqual({ q: "Yes" }); + expect( + buildPendingUserInputAnswers([question], { + q: { attachmentCount: 1, attachmentsBlocked: true }, + }), + ).toBeNull(); + expect( + buildPendingUserInputAnswers([{ ...question, allowCustomAnswer: false }], { + q: { attachmentCount: 1 }, + }), + ).toBeNull(); +}); + +it("makes attachment-only question answers expandable in the mobile feed", () => { + const answer = { + requestId: ApprovalRequestId.make("question-request"), + answers: { q: "" }, + questionTextById: { q: "Attach the specification" }, + attachmentsByQuestionId: { + q: [ + { + type: "file" as const, + id: "question-file", + name: "spec.txt", + mimeType: "text/plain", + sizeBytes: 4, + }, + ], + }, + }; + const thread = makeThread({ + id: ThreadId.make("thread-answer"), + projectId: ProjectId.make("project-answer"), + title: "Answer history", + activities: [ + makeActivity({ + id: EventId.make("answer-submitted"), + createdAt: "2026-09-08T00:00:00.000Z", + kind: "user-input.answer-submitted", + summary: "Answered questions", + payload: answer, + }), + ], + }); + const [group] = buildThreadFeed(thread); + expect(group?.type).toBe("activity-group"); + if (group?.type !== "activity-group") return; + expect(group.activities[0]).toMatchObject({ + canExpand: true, + workEntry: { questionAnswer: answer }, + }); + expect(group.activities[0]?.getFullDetail()).toBeNull(); +}); diff --git a/apps/mobile/src/lib/threadActivity.ts b/apps/mobile/src/lib/threadActivity.ts index b45d94f91..3c5a3a498 100644 --- a/apps/mobile/src/lib/threadActivity.ts +++ b/apps/mobile/src/lib/threadActivity.ts @@ -1,8 +1,10 @@ +import * as Option from "effect/Option"; +import * as Schema from "effect/Schema"; import { requestKindFromRequestType, type PendingApproval, } from "@t3tools/client-runtime/pending-requests"; -import { isToolLifecycleItemType } from "@t3tools/contracts"; +import { UserInputAttachmentAnswerPayload, isToolLifecycleItemType } from "@t3tools/contracts"; import type { OrchestrationLatestTurn, OrchestrationThread, @@ -44,6 +46,8 @@ export type { PendingApproval, PendingUserInput } from "@t3tools/client-runtime/ export interface PendingUserInputDraftAnswer { readonly selectedOptionValues?: ReadonlyArray; readonly customAnswer?: string; + readonly attachmentCount?: number; + readonly attachmentsBlocked?: boolean; } export interface ThreadFeedActivity { @@ -81,6 +85,7 @@ export interface ThreadFeedActivity { } export interface WorkLogEntry { + readonly questionAnswer?: UserInputAttachmentAnswerPayload; id: string; createdAt: string; turnId: TurnId | null; @@ -310,6 +315,7 @@ function resolvePendingUserInputAnswer( question: UserInputQuestion, draft: PendingUserInputDraftAnswer | undefined, ): string | ReadonlyArray | null { + if (draft?.attachmentsBlocked) return null; const customAnswer = question.allowCustomAnswer === false ? null : normalizeDraftAnswer(draft?.customAnswer); if (customAnswer) { @@ -318,9 +324,16 @@ function resolvePendingUserInputAnswer( const selectedOptionValues = normalizeSelectedOptionValues(question, draft?.selectedOptionValues); if (question.multiSelect) { - return selectedOptionValues.length > 0 ? selectedOptionValues : null; + return selectedOptionValues.length > 0 + ? selectedOptionValues + : question.allowCustomAnswer !== false && (draft?.attachmentCount ?? 0) > 0 + ? "" + : null; } - return selectedOptionValues[0] ?? null; + return ( + selectedOptionValues[0] ?? + (question.allowCustomAnswer !== false && (draft?.attachmentCount ?? 0) > 0 ? "" : null) + ); } /** Codex children settle via task.updated (idle/failed/interrupted), never @@ -456,6 +469,8 @@ function isPlanBoundaryToolActivity(activity: OrchestrationThreadActivity): bool return typeof payload?.detail === "string" && payload.detail.startsWith("ExitPlanMode:"); } +const decodeQuestionAttachmentAnswer = Schema.decodeUnknownOption(UserInputAttachmentAnswerPayload); + function toDerivedWorkLogEntry(activity: OrchestrationThreadActivity): DerivedWorkLogEntry { const payload = activity.payload && typeof activity.payload === "object" @@ -503,6 +518,11 @@ function toDerivedWorkLogEntry(activity: OrchestrationThreadActivity): DerivedWo ? "info" : activity.tone, sourceActivityKind: activity.kind, + ...(() => { + if (activity.kind !== "user-input.answer-submitted") return {}; + const answer = decodeQuestionAttachmentAnswer(activity.payload); + return Option.isSome(answer) ? { questionAnswer: answer.value } : {}; + })(), }; const toolCallId = asTrimmedString(payload?.toolCallId) ?? asTrimmedString(asRecord(payload?.data)?.toolCallId); @@ -992,6 +1012,7 @@ function buildWorkEntryExpandedBody(entry: WorkLogEntry): string | null { * for every row (see the deferred-expansion test). */ function workEntryCanExpand(entry: WorkLogEntry): boolean { + if (entry.questionAnswer) return true; if (entry.agentSpawn) return agentSpawnMembers(entry.agentSpawn).length > 0; if (workLogEntryIsToolLike(entry) && workEntryIndicatesToolFailure(entry)) return true; if (entry.itemType === "mcp_tool_call" && entry.toolData !== undefined) return true; diff --git a/apps/mobile/src/state/question-attachments.ts b/apps/mobile/src/state/question-attachments.ts new file mode 100644 index 000000000..313e39b31 --- /dev/null +++ b/apps/mobile/src/state/question-attachments.ts @@ -0,0 +1,29 @@ +import type { ApprovalRequestId, EnvironmentId, ThreadId } from "@t3tools/contracts"; +import { Atom } from "effect/unstable/reactivity"; +import { appAtomRegistry } from "./atom-registry"; + +export function questionAttachmentDraftPrefix( + environmentId: EnvironmentId, + threadId: ThreadId, +): string { + return `${environmentId}:question-${encodeURIComponent(JSON.stringify(threadId))}-`; +} +export function questionAttachmentDraftKey( + environmentId: EnvironmentId, + threadId: ThreadId, + requestId: ApprovalRequestId, + questionId: string, +): string { + return `${questionAttachmentDraftPrefix(environmentId, threadId)}${encodeURIComponent(JSON.stringify([requestId, questionId]))}`; +} +export const questionAttachmentPreparationAtom = Atom.make>({}).pipe( + Atom.keepAlive, +); +export function changeQuestionAttachmentPreparation(key: string, delta: number): void { + const counts = appAtomRegistry.get(questionAttachmentPreparationAtom); + if (delta < 0 && !(key in counts)) return; + appAtomRegistry.set(questionAttachmentPreparationAtom, { + ...counts, + [key]: Math.max(0, (counts[key] ?? 0) + delta), + }); +} diff --git a/apps/mobile/src/state/use-composer-drafts.ts b/apps/mobile/src/state/use-composer-drafts.ts index 8559f2d79..0ba84f82f 100644 --- a/apps/mobile/src/state/use-composer-drafts.ts +++ b/apps/mobile/src/state/use-composer-drafts.ts @@ -944,7 +944,7 @@ export function appendComposerDraftText(draftKey: string, value: string): void { export function appendComposerDraftAttachments( draftKey: string, attachments: ReadonlyArray, - options?: { readonly allowOverflow?: boolean }, + options?: { readonly allowOverflow?: boolean; readonly maxAttachments?: number }, ): number { if (attachments.length === 0) { return 0; @@ -954,7 +954,13 @@ export function appendComposerDraftAttachments( const existing = normalizeDraft(current[draftKey]); const remaining = options?.allowOverflow ? attachments.length - : Math.max(0, PROVIDER_SEND_TURN_MAX_ATTACHMENTS - existing.attachments.length); + : Math.max( + 0, + Math.min( + PROVIDER_SEND_TURN_MAX_ATTACHMENTS, + options?.maxAttachments ?? PROVIDER_SEND_TURN_MAX_ATTACHMENTS, + ) - existing.attachments.length, + ); const accepted = attachments.slice(0, remaining); rejected = attachments.slice(remaining); if (accepted.length === 0) { diff --git a/apps/mobile/src/state/use-selected-thread-requests.test.tsx b/apps/mobile/src/state/use-selected-thread-requests.test.tsx new file mode 100644 index 000000000..6888926ec --- /dev/null +++ b/apps/mobile/src/state/use-selected-thread-requests.test.tsx @@ -0,0 +1,190 @@ +import { renderToStaticMarkup } from "react-dom/server"; +import { beforeEach, describe, expect, it, vi } from "vite-plus/test"; + +const fixture = vi.hoisted(() => ({ + drafts: {} as Record, + uploads: {} as Record, + preparations: {} as Record, + preparationAtom: Symbol("preparation"), + threadStatus: "live", + questionPending: true, + effects: [] as Array<() => void>, +})); +vi.mock("react", async (importOriginal) => ({ + ...(await importOriginal()), + // Server rendering skips effects; collect them and run them after the render. + useEffect: (effect: () => void) => { + fixture.effects.push(effect); + }, +})); +vi.mock("react-native", () => ({ Alert: { alert: vi.fn() } })); +vi.mock("./atom-registry", () => ({ + appAtomRegistry: { + get: (atom: unknown) => (atom === fixture.preparationAtom ? fixture.preparations : {}), + set: vi.fn(), + }, +})); +vi.mock("@effect/atom-react", () => ({ + useAtomValue: (atom: unknown) => + atom === "drafts" + ? fixture.drafts + : atom === "uploads" + ? fixture.uploads + : atom === fixture.preparationAtom + ? fixture.preparations + : {}, +})); +vi.mock("./use-composer-drafts", () => ({ + composerDraftsAtom: "drafts", + clearComposerDraft: vi.fn(), +})); +vi.mock("./composer-attachment-uploads", async () => ({ + ...(await import("../lib/composerAttachmentUploadQueue")), + composerAttachmentUploadsAtom: "uploads", +})); +vi.mock("./question-attachments", async (importOriginal) => ({ + ...(await importOriginal()), + questionAttachmentPreparationAtom: fixture.preparationAtom, +})); +vi.mock("./entities", () => ({ + useServerConfigs: () => + new Map([ + [ + "environment-1", + { + environment: { + capabilities: { + questionAttachments: true, + attachmentUploads: true, + fileAttachments: { maxUploadBytes: 20_000_000 }, + }, + }, + }, + ], + ]), +})); +vi.mock("./threads", () => ({ threadEnvironment: {} })); +vi.mock("./use-atom-command", () => ({ useAtomCommand: () => vi.fn() })); +vi.mock("./use-thread-selection", () => ({ + useThreadSelection: () => ({ + selectedThread: { environmentId: "environment-1", id: "thread-1" }, + }), +})); +vi.mock("./use-thread-detail", async () => { + const Option = await import("effect/Option"); + return { + useSelectedThreadDetailState: () => ({ + status: fixture.threadStatus, + data: Option.some({ + activities: fixture.questionPending + ? [ + { + id: "request-activity", + kind: "user-input.requested", + createdAt: "2026-09-08T00:00:00Z", + payload: { + requestId: "request-1", + questions: ["first", "second"].map((id) => ({ + id, + header: id, + question: `Attach ${id} file`, + options: [], + allowCustomAnswer: true, + })), + }, + }, + ] + : [], + }), + }), + }; +}); + +import { ApprovalRequestId, EnvironmentId, ThreadId } from "@t3tools/contracts"; +import { questionAttachmentDraftKey } from "./question-attachments"; +import { clearComposerDraft } from "./use-composer-drafts"; +import { useSelectedThreadRequests } from "./use-selected-thread-requests"; + +const environmentId = EnvironmentId.make("environment-1"); +const key = (question: string) => + questionAttachmentDraftKey( + environmentId, + ThreadId.make("thread-1"), + ApprovalRequestId.make("request-1"), + question, + ); +function submitButtonMarkup() { + function Probe() { + const { activePendingUserInputAnswers } = useSelectedThreadRequests(); + return ; + } + const markup = renderToStaticMarkup(); + for (const effect of fixture.effects.splice(0)) effect(); + return markup; +} +beforeEach(() => { + vi.mocked(clearComposerDraft).mockClear(); + fixture.threadStatus = "live"; + fixture.questionPending = true; + fixture.preparations = {}; + fixture.drafts = Object.fromEntries( + ["first", "second"].map((id) => [ + key(id), + { + attachments: [ + { + id, + type: "file", + name: `${id}.txt`, + mimeType: "text/plain", + sizeBytes: 4, + fileUri: `file:///${id}.txt`, + }, + ], + }, + ]), + ); + fixture.uploads = { "environment-1:first": { status: "ready" } }; +}); +describe("question attachment submission readiness", () => { + it.each([ + undefined, + { status: "uploading", progress: 0.5 }, + { status: "failed", reason: "Offline" }, + ])("keeps Submit disabled until all question uploads finish: %j", (state) => { + if (state) fixture.uploads["environment-1:second"] = state; + expect(submitButtonMarkup()).toContain("disabled"); + fixture.uploads["environment-1:second"] = { status: "ready" }; + expect(submitButtonMarkup()).not.toContain("disabled"); + }); + it("ignores an upload in another environment", () => { + fixture.uploads["environment-1:second"] = { status: "ready" }; + fixture.uploads["environment-2:second"] = { status: "uploading", progress: 0.5 }; + expect(submitButtonMarkup()).not.toContain("disabled"); + }); + it("waits for attachment preparation even when uploads are ready", () => { + fixture.uploads["environment-1:second"] = { status: "ready" }; + fixture.preparations[key("first")] = 1; + expect(submitButtonMarkup()).toContain("disabled"); + }); +}); +describe("question attachment draft cleanup", () => { + it.each(["cached", "synchronizing"])( + "keeps drafts while a %s snapshot does not show the question yet", + (status) => { + fixture.threadStatus = status; + fixture.questionPending = false; + submitButtonMarkup(); + expect(clearComposerDraft).not.toHaveBeenCalled(); + }, + ); + it("keeps drafts for a question that is still pending", () => { + submitButtonMarkup(); + expect(clearComposerDraft).not.toHaveBeenCalled(); + }); + it("discards drafts once live data shows the question resolved", () => { + fixture.questionPending = false; + submitButtonMarkup(); + expect(vi.mocked(clearComposerDraft).mock.calls).toEqual([[key("first")], [key("second")]]); + }); +}); diff --git a/apps/mobile/src/state/use-selected-thread-requests.ts b/apps/mobile/src/state/use-selected-thread-requests.ts index 576526808..e2167a674 100644 --- a/apps/mobile/src/state/use-selected-thread-requests.ts +++ b/apps/mobile/src/state/use-selected-thread-requests.ts @@ -1,4 +1,17 @@ import { derivePendingRequests } from "@t3tools/client-runtime/pending-requests"; +import { useServerConfigs } from "./entities"; +import { Alert } from "react-native"; +import { + questionAttachmentDraftKey, + questionAttachmentDraftPrefix, + questionAttachmentPreparationAtom, +} from "./question-attachments"; +import { composerDraftsAtom, clearComposerDraft } from "./use-composer-drafts"; +import { + composerAttachmentUploadBlockReason, + composerAttachmentsStillUploading, + composerAttachmentUploadsAtom, +} from "./composer-attachment-uploads"; import { useAtomValue } from "@effect/atom-react"; import { useCallback, useEffect, useMemo, useRef, useState } from "react"; @@ -10,6 +23,7 @@ import { type SessionInteractionResponse, } from "@t3tools/contracts"; import * as Cause from "effect/Cause"; +import * as Option from "effect/Option"; import { Atom } from "effect/unstable/reactivity"; import { threadEnvironment } from "../state/threads"; @@ -37,7 +51,7 @@ import { foldSessionInteractionActivities, } from "../lib/sessionInteractions"; import { appAtomRegistry } from "./atom-registry"; -import { useSelectedThreadDetail } from "./use-thread-detail"; +import { useSelectedThreadDetailState } from "./use-thread-detail"; import { useThreadSelection } from "./use-thread-selection"; import { useAtomCommand } from "./use-atom-command"; @@ -108,9 +122,12 @@ export function useSelectedThreadRequests() { "thread user input dismissal", ); const { selectedThread: selectedThreadShell } = useThreadSelection(); - const selectedThread = useSelectedThreadDetail(); + const selectedThreadState = useSelectedThreadDetailState(); + const selectedThread = Option.getOrNull(selectedThreadState.data); + const selectedThreadLive = selectedThreadState.status === "live"; const userInputDraftsByRequestKey = useAtomValue(userInputDraftsByRequestKeyAtom); const [respondingApprovalId, setRespondingApprovalId] = useState(null); + const userInputResponsesInFlight = useRef(new Set()); const [respondingUserInputId, setRespondingUserInputId] = useState( null, ); @@ -142,11 +159,88 @@ export function useSelectedThreadRequests() { ); const activePendingApproval = activePendingApprovals[0] ?? null; const activePendingUserInput = activePendingUserInputs[0] ?? null; + const questionServerConfigs = useServerConfigs(); + const attachmentDrafts = useAtomValue(composerDraftsAtom); + const preparationCounts = useAtomValue(questionAttachmentPreparationAtom); + const uploadStates = useAtomValue(composerAttachmentUploadsAtom); + useEffect(() => { + // A cached snapshot can predate the question, so only live data may discard its drafts. + if (!selectedThreadLive || !selectedThreadShell || !selectedThread) return; + const prefix = questionAttachmentDraftPrefix( + selectedThreadShell.environmentId, + selectedThreadShell.id, + ); + const retained = new Set( + activePendingUserInputs.flatMap((request) => + request.questions.map((question) => + questionAttachmentDraftKey( + selectedThreadShell.environmentId, + selectedThreadShell.id, + request.requestId, + question.id, + ), + ), + ), + ); + const counts = { ...appAtomRegistry.get(questionAttachmentPreparationAtom) }; + let changed = false; + for (const key of new Set([...Object.keys(attachmentDrafts), ...Object.keys(counts)])) { + if (!key.startsWith(prefix) || retained.has(key)) continue; + if (attachmentDrafts[key]) clearComposerDraft(key); + if (key in counts) { + delete counts[key]; + changed = true; + } + } + if (changed) appAtomRegistry.set(questionAttachmentPreparationAtom, counts); + }, [ + activePendingUserInputs, + attachmentDrafts, + selectedThread, + selectedThreadLive, + selectedThreadShell, + ]); const activePendingUserInputDrafts = activePendingUserInput && selectedThreadShell - ? (userInputDraftsByRequestKey[ - scopedRequestKey(selectedThreadShell.environmentId, activePendingUserInput.requestId) - ] ?? {}) + ? Object.fromEntries( + activePendingUserInput.questions.map((question) => { + const key = questionAttachmentDraftKey( + selectedThreadShell.environmentId, + selectedThreadShell.id, + activePendingUserInput.requestId, + question.id, + ); + const attachments = attachmentDrafts[key]?.attachments ?? []; + const uploadInput = { + environmentId: selectedThreadShell.environmentId, + attachments, + serverConfig: questionServerConfigs.get(selectedThreadShell.environmentId) ?? null, + states: uploadStates, + }; + return [ + question.id, + { + ...userInputDraftsByRequestKey[ + scopedRequestKey( + selectedThreadShell.environmentId, + activePendingUserInput.requestId, + ) + ]?.[question.id], + attachmentCount: attachments.length, + attachmentsBlocked: + (attachments.length > 0 && + uploadInput.serverConfig?.environment.capabilities.questionAttachments !== + true) || + (preparationCounts[key] ?? 0) > 0 || + composerAttachmentsStillUploading(uploadInput) || + composerAttachmentUploadBlockReason({ + ...uploadInput, + connected: true, + }) !== null, + }, + ]; + }), + ) : {}; const activePendingUserInputAnswers = activePendingUserInput ? buildPendingUserInputAnswers(activePendingUserInput.questions, activePendingUserInputDrafts) @@ -205,6 +299,52 @@ export function useSelectedThreadRequests() { return; } + const responseKey = questionAttachmentDraftKey( + selectedThreadShell.environmentId, + selectedThreadShell.id, + activePendingUserInput.requestId, + "", + ); + if (userInputResponsesInFlight.current.has(responseKey)) return; + const attachmentsByQuestionId = new Map< + string, + import("@t3tools/contracts").UserInputAttachments[string] + >(); + for (const question of activePendingUserInput.questions) { + const key = questionAttachmentDraftKey( + selectedThreadShell.environmentId, + selectedThreadShell.id, + activePendingUserInput.requestId, + question.id, + ); + if ((appAtomRegistry.get(questionAttachmentPreparationAtom)[key] ?? 0) > 0) return; + const attachments = appAtomRegistry.get(composerDraftsAtom)[key]?.attachments ?? []; + if (attachments.length === 0) continue; + if ( + attachments.some( + (attachment) => + !attachment.uploadedAttachmentId || + attachment.uploadEnvironmentId !== selectedThreadShell.environmentId, + ) + ) { + Alert.alert( + "Attachments are not ready", + "Wait for uploads to finish, or retry failed uploads.", + ); + return; + } + attachmentsByQuestionId.set( + question.id, + attachments.map((attachment) => ({ + type: attachment.type, + id: attachment.uploadedAttachmentId!, + name: attachment.name, + mimeType: attachment.mimeType, + sizeBytes: attachment.sizeBytes, + })), + ); + } + userInputResponsesInFlight.current.add(responseKey); setRespondingUserInputId(activePendingUserInput.requestId); const result = await respondToUserInput({ environmentId: selectedThreadShell.environmentId, @@ -212,8 +352,12 @@ export function useSelectedThreadRequests() { threadId: selectedThreadShell.id, requestId: activePendingUserInput.requestId, answers: activePendingUserInputAnswers, + ...(attachmentsByQuestionId.size > 0 + ? { attachmentsByQuestionId: Object.fromEntries(attachmentsByQuestionId) } + : {}), }, }); + userInputResponsesInFlight.current.delete(responseKey); setRespondingUserInputId((current) => current === activePendingUserInput.requestId ? null : current, ); diff --git a/apps/server/src/environment/ServerEnvironment.ts b/apps/server/src/environment/ServerEnvironment.ts index 6686f9632..ddbad1531 100644 --- a/apps/server/src/environment/ServerEnvironment.ts +++ b/apps/server/src/environment/ServerEnvironment.ts @@ -160,6 +160,7 @@ export const make = Effect.gen(function* () { repositoryIdentity: true, connectionProbe: true, attachmentUploads: true, + questionAttachments: true, fileAttachments: { maxUploadBytes: PROVIDER_SEND_TURN_MAX_FILE_BYTES }, pullRequests: true, threadSettlement: true, diff --git a/apps/server/src/orchestration/Layers/OrchestrationEngine.test.ts b/apps/server/src/orchestration/Layers/OrchestrationEngine.test.ts index 6eee5e19b..071d5c32e 100644 --- a/apps/server/src/orchestration/Layers/OrchestrationEngine.test.ts +++ b/apps/server/src/orchestration/Layers/OrchestrationEngine.test.ts @@ -423,6 +423,17 @@ describe("OrchestrationEngine", () => { threadId, requestId, answers: { "0": "pnpm", "1": "Example" }, + attachmentsByQuestionId: { + "1": [ + { + type: "file" as const, + id: "thread-1-00000000-0000-4000-8000-0000000000aa-txt", + name: "spec.txt", + mimeType: "text/plain", + sizeBytes: 4, + }, + ], + }, createdAt: "2026-01-01T00:00:02.000Z", }; await expect( @@ -440,8 +451,9 @@ describe("OrchestrationEngine", () => { (message) => message.role === "user", ); expect(userMessages).toHaveLength(1); + expect(userMessages?.[0]?.attachments).toEqual(response.attachmentsByQuestionId["1"]); expect(userMessages?.[0]?.text).toBe( - "Which package manager?\npnpm\n\nWhat should it be named?\nExample", + "Which package manager?\npnpm\n\nWhat should it be named?\nExample\nAttached file: spec.txt (thread-1-00000000-0000-4000-8000-0000000000aa-txt)", ); expect( after.threads[0]?.activities.find((activity) => activity.kind === "user-input.resolved") diff --git a/apps/server/src/orchestration/Layers/ProjectionPipeline.test.ts b/apps/server/src/orchestration/Layers/ProjectionPipeline.test.ts index 90377e0e2..a0a147405 100644 --- a/apps/server/src/orchestration/Layers/ProjectionPipeline.test.ts +++ b/apps/server/src/orchestration/Layers/ProjectionPipeline.test.ts @@ -19,11 +19,16 @@ import * as Effect from "effect/Effect"; import * as FileSystem from "effect/FileSystem"; import * as Layer from "effect/Layer"; 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 * as SqlClient from "effect/unstable/sql/SqlClient"; import { makeSqlStatementCounter } from "../../../integration/SqlStatementCounter.integration.ts"; -import { createPendingAttachmentId } from "../../attachmentStore.ts"; +import { + createPendingAttachmentId, + resolveThreadBrowserArtifactsDir, +} from "../../attachmentStore.ts"; import { claimPreviewRecording } from "../../mcp/toolkits/preview/handlers.ts"; import { OrchestrationCommandReceiptRepositoryLive } from "../../persistence/Layers/OrchestrationCommandReceipts.ts"; import { OrchestrationEventStoreLive } from "../../persistence/Layers/OrchestrationEventStore.ts"; @@ -413,7 +418,7 @@ it.layer(BaseTestLayer)("OrchestrationProjectionPipeline", (it) => { FROM projection_state ORDER BY projector ASC `; - assert.equal(stateRows.length, Object.keys(ORCHESTRATION_PROJECTOR_NAMES).length); + assert.equal(stateRows.length, Object.keys(ORCHESTRATION_PROJECTOR_NAMES).length + 1); for (const row of stateRows) { assert.equal(row.lastAppliedSequence, 3); } @@ -1353,14 +1358,18 @@ it.layer( yield* projectionPipeline.bootstrap; yield* projectionPipeline.bootstrap; - assert.deepEqual( - yield* projectionState.listAll(), - cursorsBeforeFailure.map((cursor) => ({ + assert.deepEqual(yield* projectionState.listAll(), [ + { + projector: "projection.attachment-cleanup", + lastAppliedSequence: pendingEvent.sequence, + updatedAt: pendingEvent.occurredAt, + }, + ...cursorsBeforeFailure.map((cursor) => ({ ...cursor, lastAppliedSequence: pendingEvent.sequence, updatedAt: pendingEvent.occurredAt, })), - ); + ]); const replayedMessages = yield* sql<{ readonly text: string }>` SELECT text FROM projection_thread_messages WHERE message_id = 'message-rollback' `; @@ -1553,10 +1562,53 @@ it.layer( }, }); + const answerKeepId = "thread-revert-files-00000000-0000-4000-8000-000000000006-txt"; + const answerRemoveId = "thread-revert-files-00000000-0000-4000-8000-000000000007-txt"; + for (const [id, turnId] of [ + [answerKeepId, "turn-keep"], + [answerRemoveId, "turn-remove"], + ] as const) { + yield* appendAndProject({ + type: "thread.activity-appended", + eventId: EventId.make(`answer-${id}`), + aggregateKind: "thread", + aggregateId: threadId, + occurredAt: now, + commandId: CommandId.make(`answer-${id}`), + causationEventId: null, + correlationId: CorrelationId.make(`answer-${id}`), + metadata: {}, + payload: { + threadId, + activity: { + id: EventId.make(`answer-${id}`), + kind: "user-input.answer-submitted", + tone: "info", + summary: "Answer with file", + createdAt: now, + turnId: TurnId.make(turnId), + payload: { + requestId: ApprovalRequestId.make(id), + answers: { q: "See file" }, + attachmentsByQuestionId: { + q: [ + { type: "file", id, name: "answer.txt", mimeType: "text/plain", sizeBytes: 6 }, + ], + }, + }, + }, + }, + }); + } const keepPath = path.join(attachmentsDir, `${keepAttachmentId}.png`); const keepFilePath = path.join(attachmentsDir, `${keepFileAttachmentId}.pdf`); const removePath = path.join(attachmentsDir, `${removeAttachmentId}.png`); yield* fileSystem.makeDirectory(attachmentsDir, { recursive: true }); + yield* fileSystem.writeFileString(path.join(attachmentsDir, `${answerKeepId}.txt`), "answer"); + yield* fileSystem.writeFileString( + path.join(attachmentsDir, `${answerRemoveId}.txt`), + "answer", + ); yield* fileSystem.writeFileString(keepPath, "keep"); yield* fileSystem.writeFileString(keepFilePath, "keep"); yield* fileSystem.writeFileString(removePath, "remove"); @@ -1649,9 +1701,33 @@ it.layer( assert.isTrue(yield* exists(keepPath)); assert.isTrue(yield* exists(keepFilePath)); + assert.isTrue(yield* exists(path.join(attachmentsDir, `${answerKeepId}.txt`))); + assert.isFalse(yield* exists(path.join(attachmentsDir, `${answerRemoveId}.txt`))); assert.isFalse(yield* exists(removePath)); assert.isTrue(yield* exists(laterPath)); assert.isTrue(yield* exists(otherThreadPath)); + + // Replay message and activity history from different cursors, as during a projection rebuild. + yield* sql`DELETE FROM projection_thread_messages WHERE thread_id = ${threadId}`; + yield* sql`DELETE FROM projection_thread_activities WHERE thread_id = ${threadId}`; + yield* sql`UPDATE projection_state SET last_applied_sequence = 0 + WHERE projector IN ('projection.thread-messages', 'projection.thread-activities', 'projection.threads')`; + yield* fileSystem.writeFileString(removePath, "remove"); + yield* fileSystem.writeFileString( + path.join(attachmentsDir, `${answerRemoveId}.txt`), + "answer", + ); + yield* sql`CREATE TRIGGER fail_bootstrap_thread BEFORE UPDATE ON projection_threads + BEGIN SELECT RAISE(FAIL, 'forced bootstrap failure'); END`; + yield* projectionPipeline.bootstrap.pipe(Effect.flip); + assert.isTrue(yield* exists(removePath)); + yield* sql`DROP TRIGGER fail_bootstrap_thread`; + yield* projectionPipeline.bootstrap; + assert.isTrue(yield* exists(keepPath)); + assert.isTrue(yield* exists(laterPath)); + assert.isTrue(yield* exists(path.join(attachmentsDir, `${answerKeepId}.txt`))); + assert.isFalse(yield* exists(removePath)); + assert.isFalse(yield* exists(path.join(attachmentsDir, `${answerRemoveId}.txt`))); }), ); }); @@ -2111,6 +2187,383 @@ it.layer(Layer.fresh(makeProjectionPipelinePrefixedTestLayer("t3-projection-atta }, ); +const cleanupBacklogNow = "2026-01-01T00:00:00.000Z"; + +const appendCleanupProject = Effect.fn("appendCleanupProject")(function* (projectId: ProjectId) { + const eventStore = yield* OrchestrationEventStore; + return yield* eventStore.append({ + type: "project.created", + eventId: EventId.make(`evt-create-${projectId}`), + aggregateKind: "project", + aggregateId: projectId, + occurredAt: cleanupBacklogNow, + commandId: CommandId.make(`cmd-create-${projectId}`), + causationEventId: null, + correlationId: null, + metadata: {}, + payload: { + projectId, + title: "Cleanup backlog", + workspaceRoot: "/tmp/project-cleanup-backlog", + defaultModelSelection: null, + scripts: [], + createdAt: cleanupBacklogNow, + updatedAt: cleanupBacklogNow, + }, + }); +}); + +const appendCleanupThreadCreated = Effect.fn("appendCleanupThreadCreated")(function* ( + projectId: ProjectId, + threadId: ThreadId, +) { + const eventStore = yield* OrchestrationEventStore; + return yield* eventStore.append({ + type: "thread.created", + eventId: EventId.make(`evt-create-${threadId}`), + aggregateKind: "thread", + aggregateId: threadId, + occurredAt: cleanupBacklogNow, + commandId: CommandId.make(`cmd-create-${threadId}`), + causationEventId: null, + correlationId: null, + metadata: {}, + payload: { + threadId, + projectId, + title: "Cleanup backlog thread", + modelSelection: { instanceId: ProviderInstanceId.make("codex"), model: "gpt-5-codex" }, + runtimeMode: "full-access", + branch: null, + worktreePath: null, + createdAt: cleanupBacklogNow, + updatedAt: cleanupBacklogNow, + }, + }); +}); + +const appendCleanupThreadDeleted = Effect.fn("appendCleanupThreadDeleted")(function* ( + threadId: ThreadId, +) { + const eventStore = yield* OrchestrationEventStore; + return yield* eventStore.append({ + type: "thread.deleted", + eventId: EventId.make(`evt-delete-${threadId}`), + aggregateKind: "thread", + aggregateId: threadId, + occurredAt: cleanupBacklogNow, + commandId: CommandId.make(`cmd-delete-${threadId}`), + causationEventId: null, + correlationId: null, + metadata: {}, + payload: { threadId, deletedAt: cleanupBacklogNow }, + }); +}); + +// Every projector is current; attachment cleanup still has everything after `cleanupSequence`. +const seedCleanupBacklog = Effect.fn("seedCleanupBacklog")(function* ( + projectorSequence: number, + cleanupSequence: number, +) { + const sql = yield* SqlClient.SqlClient; + yield* sql`DELETE FROM projection_state`; + for (const projector of Object.values(ORCHESTRATION_PROJECTOR_NAMES)) { + yield* sql` + INSERT INTO projection_state (projector, last_applied_sequence, updated_at) + VALUES (${projector}, ${projectorSequence}, ${cleanupBacklogNow}) + `; + } + yield* sql` + INSERT INTO projection_state (projector, last_applied_sequence, updated_at) + VALUES ('projection.attachment-cleanup', ${cleanupSequence}, ${cleanupBacklogNow}) + `; +}); + +const readCleanupCursor = Effect.gen(function* () { + const sql = yield* SqlClient.SqlClient; + return yield* sql<{ readonly lastAppliedSequence: number }>` + SELECT last_applied_sequence AS "lastAppliedSequence" FROM projection_state + WHERE projector = 'projection.attachment-cleanup' + `; +}); + +const cleanupDirectoryListings = { count: 0, failPaths: new Set() }; +const CountingFileSystemLayer = Layer.effect( + FileSystem.FileSystem, + Effect.gen(function* () { + const fileSystem = yield* FileSystem.FileSystem; + return { + ...fileSystem, + readDirectory: (...args: Parameters) => { + cleanupDirectoryListings.count += 1; + if (cleanupDirectoryListings.failPaths.has(args[0])) { + return Effect.fail( + PlatformError.systemError({ + _tag: "Unknown", + module: "FileSystem", + method: "readDirectory", + description: "listing denied for the test", + }), + ); + } + return fileSystem.readDirectory(...args); + }, + }; + }), +).pipe(Layer.provide(NodeServices.layer)); + +it.layer( + Layer.fresh( + OrchestrationProjectionPipelineLive.pipe( + Layer.provideMerge(OrchestrationEventStoreLive), + Layer.provideMerge( + ServerConfig.layerTest(process.cwd(), { prefix: "t3-projection-attachments-backlog-" }), + ), + Layer.provideMerge(SqlitePersistenceMemory), + Layer.provideMerge(CountingFileSystemLayer), + Layer.provideMerge(NodeServices.layer), + ), + ), +)("OrchestrationProjectionPipeline attachment cleanup backlog", (it) => { + it.effect("bounds bootstrap cleanup by threads that still have files", () => + Effect.gen(function* () { + const fileSystem = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const projectionPipeline = yield* OrchestrationProjectionPipeline; + const { attachmentsDir, browserArtifactsDir } = yield* ServerConfig; + const projectId = ProjectId.make("project-cleanup-backlog"); + const deletedThreadCount = 200; + + yield* appendCleanupProject(projectId); + for (let index = 0; index < deletedThreadCount; index += 1) { + const threadId = ThreadId.make(`thread-backlog-${index}`); + yield* appendCleanupThreadCreated(projectId, threadId); + yield* appendCleanupThreadDeleted(threadId); + } + const filesThreadId = ThreadId.make("thread-backlog-files"); + yield* appendCleanupThreadCreated(projectId, filesThreadId); + const lastEvent = yield* appendCleanupThreadDeleted(filesThreadId); + + const deletedFilePath = path.join( + attachmentsDir, + "thread-backlog-files-00000000-0000-4000-8000-000000000001.png", + ); + const keptFilePaths = Array.from({ length: 50 }, (_, index) => + path.join( + attachmentsDir, + `thread-backlog-kept-00000000-0000-4000-8000-${String(index).padStart(12, "0")}.png`, + ), + ); + yield* fileSystem.makeDirectory(attachmentsDir, { recursive: true }); + for (const filePath of [deletedFilePath, ...keptFilePaths]) { + yield* fileSystem.writeFileString(filePath, "file"); + } + // A deleted thread whose only files are browser artifacts still gets cleaned. + const artifactsOnlyDir = resolveThreadBrowserArtifactsDir({ + browserArtifactsDir, + threadId: "thread-backlog-7", + })!; + yield* fileSystem.makeDirectory(artifactsOnlyDir, { recursive: true }); + yield* fileSystem.writeFileString(path.join(artifactsOnlyDir, "recording.webm"), "video"); + yield* seedCleanupBacklog(lastEvent.sequence, 0); + + cleanupDirectoryListings.count = 0; + const counter = makeSqlStatementCounter(); + yield* projectionPipeline.bootstrap.pipe(Effect.withTracer(counter.tracer)); + + // One listing per cleanup directory (attachments, browser artifacts), and statements + // that do not grow with threads that have no files. + assert.strictEqual(cleanupDirectoryListings.count, 2); + assert.isBelow(counter.count(), deletedThreadCount / 4); + assert.isFalse(yield* exists(deletedFilePath)); + assert.isFalse(yield* exists(artifactsOnlyDir)); + for (const filePath of keptFilePaths) { + assert.isTrue(yield* exists(filePath)); + } + assert.deepEqual(yield* readCleanupCursor, [{ lastAppliedSequence: lastEvent.sequence }]); + + // With the cursor current, a restart lists nothing. + cleanupDirectoryListings.count = 0; + yield* projectionPipeline.bootstrap; + assert.strictEqual(cleanupDirectoryListings.count, 0); + }), + ); +}); + +it.layer( + Layer.fresh( + OrchestrationProjectionPipelineLive.pipe( + Layer.provideMerge(OrchestrationEventStoreLive), + Layer.provideMerge( + ServerConfig.layerTest(process.cwd(), { prefix: "t3-projection-artifacts-listing-" }), + ), + Layer.provideMerge(SqlitePersistenceMemory), + Layer.provideMerge(CountingFileSystemLayer), + Layer.provideMerge(NodeServices.layer), + ), + ), +)("OrchestrationProjectionPipeline attachment cleanup backlog", (it) => { + it.effect("removes a deleted thread's browser artifacts when they cannot be listed", () => + Effect.gen(function* () { + const fileSystem = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const projectionPipeline = yield* OrchestrationProjectionPipeline; + const { browserArtifactsDir } = yield* ServerConfig; + const projectId = ProjectId.make("project-artifacts-listing"); + const threadId = ThreadId.make("thread-artifacts-listing"); + + yield* appendCleanupProject(projectId); + yield* appendCleanupThreadCreated(projectId, threadId); + const lastEvent = yield* appendCleanupThreadDeleted(threadId); + const artifactsDir = resolveThreadBrowserArtifactsDir({ browserArtifactsDir, threadId })!; + yield* fileSystem.makeDirectory(artifactsDir, { recursive: true }); + yield* fileSystem.writeFileString(path.join(artifactsDir, "recording.webm"), "video"); + yield* seedCleanupBacklog(lastEvent.sequence, 0); + + // The cursor still moves to the head at startup, so a failed listing must not skip removal. + cleanupDirectoryListings.failPaths.add(browserArtifactsDir); + yield* projectionPipeline.bootstrap.pipe( + Effect.ensuring( + Effect.sync(() => cleanupDirectoryListings.failPaths.delete(browserArtifactsDir)), + ), + ); + assert.isFalse(yield* exists(artifactsDir)); + assert.isTrue(yield* exists(browserArtifactsDir)); + assert.deepEqual(yield* readCleanupCursor, [{ lastAppliedSequence: lastEvent.sequence }]); + }), + ); + + it.effect("keeps finished cleanup when a projector without cleanup lacks a cursor", () => + Effect.gen(function* () { + const fileSystem = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const sql = yield* SqlClient.SqlClient; + const projectionPipeline = yield* OrchestrationProjectionPipeline; + const { attachmentsDir } = yield* ServerConfig; + const projectId = ProjectId.make("project-new-projector"); + const threadId = ThreadId.make("thread-new-projector"); + + yield* appendCleanupProject(projectId); + yield* appendCleanupThreadCreated(projectId, threadId); + const lastEvent = yield* appendCleanupThreadDeleted(threadId); + yield* seedCleanupBacklog(lastEvent.sequence, lastEvent.sequence); + // A projector added in a later release starts without a row. + yield* sql` + DELETE FROM projection_state + WHERE projector = ${ORCHESTRATION_PROJECTOR_NAMES.pendingApprovals} + `; + const laterFilePath = path.join( + attachmentsDir, + "thread-new-projector-00000000-0000-4000-8000-000000000001.png", + ); + yield* fileSystem.makeDirectory(attachmentsDir, { recursive: true }); + yield* fileSystem.writeFileString(laterFilePath, "not part of the finished cleanup"); + + yield* projectionPipeline.bootstrap; + + assert.isTrue(yield* exists(laterFilePath)); + assert.deepEqual(yield* readCleanupCursor, [{ lastAppliedSequence: lastEvent.sequence }]); + }), + ); +}); + +it.layer( + Layer.fresh(makeProjectionPipelinePrefixedTestLayer("t3-projection-attachments-undecodable-")), +)("OrchestrationProjectionPipeline attachment cleanup backlog", (it) => { + it.effect("starts when an old event no longer decodes", () => + Effect.gen(function* () { + const fileSystem = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const projectionPipeline = yield* OrchestrationProjectionPipeline; + const eventStore = yield* OrchestrationEventStore; + const sql = yield* SqlClient.SqlClient; + const { attachmentsDir } = yield* ServerConfig; + const projectId = ProjectId.make("project-cleanup-undecodable"); + const threadId = ThreadId.make("thread-undecodable"); + + yield* appendCleanupProject(projectId); + yield* appendCleanupThreadCreated(projectId, threadId); + // An old message whose payload today's schema rejects, and a delete with an unreadable id. + yield* sql` + INSERT INTO orchestration_events ( + event_id, aggregate_kind, stream_id, stream_version, event_type, occurred_at, + command_id, causation_event_id, correlation_id, actor_kind, payload_json, metadata_json + ) + VALUES + ('evt-legacy-message', 'thread', ${threadId}, 1, 'thread.message-sent', + ${cleanupBacklogNow}, NULL, NULL, NULL, 'client', '{"legacy":true}', '{}'), + ('evt-unreadable-delete', 'thread', ' ', 0, 'thread.deleted', + ${cleanupBacklogNow}, NULL, NULL, NULL, 'client', '{}', '{}') + `; + const deleted = yield* appendCleanupThreadDeleted(threadId); + const readError = yield* Stream.runDrain(eventStore.readFromSequence(0)).pipe(Effect.flip); + assert.strictEqual(readError._tag, "PersistenceDecodeError"); + + const attachmentPath = path.join( + attachmentsDir, + "thread-undecodable-00000000-0000-4000-8000-000000000001.png", + ); + yield* fileSystem.makeDirectory(attachmentsDir, { recursive: true }); + yield* fileSystem.writeFileString(attachmentPath, "file"); + yield* seedCleanupBacklog(deleted.sequence, 0); + + yield* projectionPipeline.bootstrap; + + assert.isFalse(yield* exists(attachmentPath)); + assert.deepEqual(yield* readCleanupCursor, [{ lastAppliedSequence: deleted.sequence }]); + }), + ); +}); + +it.layer( + Layer.fresh(makeProjectionPipelinePrefixedTestLayer("t3-projection-attachments-file-failure-")), +)("OrchestrationProjectionPipeline attachment cleanup backlog", (it) => { + it.effect("cleans past a file that cannot be removed without pinning the cursor", () => + Effect.gen(function* () { + const fileSystem = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const projectionPipeline = yield* OrchestrationProjectionPipeline; + const { attachmentsDir } = yield* ServerConfig; + const projectId = ProjectId.make("project-cleanup-file-failure"); + const blockedThreadId = ThreadId.make("thread-blocked-file"); + const laterThreadId = ThreadId.make("thread-after-blocked"); + + yield* appendCleanupProject(projectId); + for (const threadId of [blockedThreadId, laterThreadId]) { + yield* appendCleanupThreadCreated(projectId, threadId); + } + yield* appendCleanupThreadDeleted(blockedThreadId); + const lastEvent = yield* appendCleanupThreadDeleted(laterThreadId); + + const blockedPath = path.join( + attachmentsDir, + "thread-blocked-file-00000000-0000-4000-8000-000000000001.png", + ); + const siblingPath = path.join( + attachmentsDir, + "thread-blocked-file-00000000-0000-4000-8000-000000000002.png", + ); + const laterPath = path.join( + attachmentsDir, + "thread-after-blocked-00000000-0000-4000-8000-000000000003.png", + ); + // Removing a nonempty directory as a file fails. + yield* fileSystem.makeDirectory(blockedPath, { recursive: true }); + yield* fileSystem.writeFileString(path.join(blockedPath, "keep.txt"), "keep"); + yield* fileSystem.writeFileString(siblingPath, "file"); + yield* fileSystem.writeFileString(laterPath, "file"); + yield* seedCleanupBacklog(lastEvent.sequence, 0); + + yield* projectionPipeline.bootstrap; + + assert.isTrue(yield* exists(blockedPath)); + assert.isFalse(yield* exists(siblingPath)); + assert.isFalse(yield* exists(laterPath)); + assert.deepEqual(yield* readCleanupCursor, [{ lastAppliedSequence: lastEvent.sequence }]); + }), + ); +}); + it.layer(BaseTestLayer)("OrchestrationProjectionPipeline", (it) => { it.effect("replays a bootstrap backlog larger than the event store default limit", () => Effect.gen(function* () { @@ -2285,6 +2738,15 @@ it.layer(BaseTestLayer)("OrchestrationProjectionPipeline", (it) => { }, }); + yield* sql`CREATE TRIGGER fail_later_stream_projector BEFORE UPDATE ON projection_state + WHEN NEW.projector = 'projection.threads' + BEGIN SELECT RAISE(FAIL, 'forced later projector failure'); END`; + yield* projectionPipeline.bootstrap.pipe(Effect.flip); + const committedMessage = yield* sql<{ readonly text: string }>` + SELECT text FROM projection_thread_messages WHERE message_id = 'message-a' + `; + assert.deepEqual(committedMessage, [{ text: "hello world" }]); + yield* sql`DROP TRIGGER fail_later_stream_projector`; yield* projectionPipeline.bootstrap; yield* projectionPipeline.bootstrap; @@ -4255,7 +4717,7 @@ const engineLayer = it.layer( Layer.provideMerge(OrchestrationProjectionSnapshotQueryLive), Layer.provide(ThreadBackgroundLiveness.layer), Layer.provide(ThreadPlanProgress.layer), - Layer.provide(OrchestrationProjectionPipelineLive), + Layer.provideMerge(OrchestrationProjectionPipelineLive), Layer.provide(OrchestrationEventStoreLive), Layer.provide(OrchestrationCommandReceiptRepositoryLive), Layer.provide(RepositoryIdentityResolver.layer), @@ -4549,6 +5011,8 @@ engineLayer("OrchestrationProjectionPipeline via engine dispatch", (it) => { const projectId = ProjectId.make("project-outer-rollback"); const threadId = ThreadId.make("thread-outer-rollback"); const cleanupFailureThreadId = ThreadId.make("thread-cleanup-failure"); + const cleanupAfterFailureThreadId = ThreadId.make("thread-cleanup-after-failure"); + const cleanupRetryThreadId = ThreadId.make("thread-cleanup-retry"); const commandId = CommandId.make("cmd-outer-rollback-delete"); const attachmentPath = path.join( attachmentsDir, @@ -4558,6 +5022,14 @@ engineLayer("OrchestrationProjectionPipeline via engine dispatch", (it) => { attachmentsDir, "thread-cleanup-failure-00000000-0000-4000-8000-000000000001.png", ); + const retriedAttachmentPath = path.join( + attachmentsDir, + "thread-cleanup-retry-00000000-0000-4000-8000-000000000001.png", + ); + const cleanedAfterFailurePath = path.join( + attachmentsDir, + "thread-cleanup-after-failure-00000000-0000-4000-8000-000000000001.png", + ); yield* engine.dispatch({ type: "project.create", @@ -4567,7 +5039,12 @@ engineLayer("OrchestrationProjectionPipeline via engine dispatch", (it) => { workspaceRoot: "/tmp/project-outer-rollback", createdAt, }); - for (const id of [threadId, cleanupFailureThreadId]) { + for (const id of [ + threadId, + cleanupFailureThreadId, + cleanupAfterFailureThreadId, + cleanupRetryThreadId, + ]) { yield* engine.dispatch({ type: "thread.create", commandId: CommandId.make(`cmd-create-${id}`), @@ -4623,12 +5100,19 @@ engineLayer("OrchestrationProjectionPipeline via engine dispatch", (it) => { assert.deepEqual(rolledBackReceipts, []); yield* sql`DROP TRIGGER fail_attachment_command_receipt`; + const sequenceBeforeDelete = cursorsBeforeFailure.find( + (cursor) => cursor.projector === "projection.projects", + )!.lastAppliedSequence; const result = yield* engine.dispatch(deleteCommand); + // Cleanup runs after commit, so its cursor rides with the next command's projector cursors. assert.deepEqual( yield* readCursors, cursorsBeforeFailure.map((cursor) => ({ ...cursor, - lastAppliedSequence: result.sequence, + lastAppliedSequence: + cursor.projector === "projection.attachment-cleanup" + ? sequenceBeforeDelete + : result.sequence, })), ); assert.isFalse(yield* exists(attachmentPath)); @@ -4644,20 +5128,53 @@ engineLayer("OrchestrationProjectionPipeline via engine dispatch", (it) => { ]); // Removing a nonempty directory as a file fails after the command commits. - yield* fileSystem.makeDirectory(blockedAttachmentPath); - yield* fileSystem.writeFileString(path.join(blockedAttachmentPath, "keep.txt"), "keep"); + for (const blockedPath of [blockedAttachmentPath, retriedAttachmentPath]) { + yield* fileSystem.makeDirectory(blockedPath); + yield* fileSystem.writeFileString(path.join(blockedPath, "keep.txt"), "keep"); + } const cleanupFailureCommandId = CommandId.make("cmd-cleanup-failure-delete"); yield* engine.dispatch({ type: "thread.delete", commandId: cleanupFailureCommandId, threadId: cleanupFailureThreadId, }); + // A later cleanup that succeeds must not carry the cursor past the failed one. + yield* fileSystem.writeFileString(cleanedAfterFailurePath, "remove this attachment"); + yield* engine.dispatch({ + type: "thread.delete", + commandId: CommandId.make("cmd-cleanup-after-failure-delete"), + threadId: cleanupAfterFailureThreadId, + }); + assert.isFalse(yield* exists(cleanedAfterFailurePath)); + const retryDelete = yield* engine.dispatch({ + type: "thread.delete", + commandId: CommandId.make("cmd-cleanup-retry-delete"), + threadId: cleanupRetryThreadId, + }); assert.isTrue(yield* exists(blockedAttachmentPath)); + assert.isTrue(yield* exists(retriedAttachmentPath)); const cleanupFailureReceipts = yield* sql<{ readonly status: string }>` SELECT status FROM orchestration_command_receipts WHERE command_id = ${cleanupFailureCommandId} `; assert.deepEqual(cleanupFailureReceipts, [{ status: "accepted" }]); + + const pipeline = yield* OrchestrationProjectionPipeline; + const cleanupCursor = sql<{ readonly lastAppliedSequence: number }>` + SELECT last_applied_sequence AS "lastAppliedSequence" FROM projection_state + WHERE projector = 'projection.attachment-cleanup' + `; + // A failed cleanup holds the cursor at the last cleanup that finished. + assert.deepEqual(yield* cleanupCursor, [{ lastAppliedSequence: result.sequence }]); + + // The next start retries both deletes once: the cleared one succeeds, and the persistent + // failure is logged and left behind instead of pinning the cursor. + yield* fileSystem.remove(retriedAttachmentPath, { recursive: true }); + yield* fileSystem.writeFileString(retriedAttachmentPath, "retry this attachment"); + yield* pipeline.bootstrap; + assert.isFalse(yield* exists(retriedAttachmentPath)); + assert.isTrue(yield* exists(blockedAttachmentPath)); + assert.deepEqual(yield* cleanupCursor, [{ lastAppliedSequence: retryDelete.sequence }]); }), ); }); diff --git a/apps/server/src/orchestration/Layers/ProjectionPipeline.ts b/apps/server/src/orchestration/Layers/ProjectionPipeline.ts index b2ecb0bb5..5b0d14a12 100644 --- a/apps/server/src/orchestration/Layers/ProjectionPipeline.ts +++ b/apps/server/src/orchestration/Layers/ProjectionPipeline.ts @@ -1,6 +1,8 @@ import { ApprovalRequestId, isImportedAgentSessionMessageId, + NonNegativeInt, + UserInputAttachmentAnswerPayload, type ChatAttachment, type OrchestrationEvent, type OrchestrationSessionStatus, @@ -12,6 +14,7 @@ import * as FileSystem from "effect/FileSystem"; import * as Layer from "effect/Layer"; import * as Option from "effect/Option"; import * as Path from "effect/Path"; +import * as Schema from "effect/Schema"; import * as Stream from "effect/Stream"; import * as SqlClient from "effect/unstable/sql/SqlClient"; @@ -19,7 +22,10 @@ import { toPersistenceSqlError, type ProjectionRepositoryError } from "../../per import { OrchestrationEventStore } from "../../persistence/Services/OrchestrationEventStore.ts"; import { ProjectionPendingApprovalRepository } from "../../persistence/Services/ProjectionPendingApprovals.ts"; import { ProjectionProjectRepository } from "../../persistence/Services/ProjectionProjects.ts"; -import { ProjectionStateRepository } from "../../persistence/Services/ProjectionState.ts"; +import { + type ProjectionState, + ProjectionStateRepository, +} from "../../persistence/Services/ProjectionState.ts"; import { ProjectionThreadActivityRepository } from "../../persistence/Services/ProjectionThreadActivities.ts"; import { type ProjectionThreadActivity } from "../../persistence/Services/ProjectionThreadActivities.ts"; import { @@ -336,6 +342,8 @@ function retainProjectionProposedPlansAfterRevert( ); } +const decodeQuestionAttachmentAnswer = Schema.decodeUnknownOption(UserInputAttachmentAnswerPayload); + function collectThreadAttachmentRelativePaths( threadId: string, messages: ReadonlyArray, @@ -360,126 +368,98 @@ function collectThreadAttachmentRelativePaths( return relativePaths; } -const runAttachmentSideEffects = Effect.fn("runAttachmentSideEffects")(function* ( - sideEffects: AttachmentSideEffects, -) { - const serverConfig = yield* Effect.service(ServerConfig); - const fileSystem = yield* Effect.service(FileSystem.FileSystem); - const path = yield* Effect.service(Path.Path); - - const attachmentsRootDir = serverConfig.attachmentsDir; - const readAttachmentRootEntries = fileSystem - .readDirectory(attachmentsRootDir, { recursive: false }) - .pipe(Effect.orElseSucceed(() => [] as Array)); - - const removeDeletedThreadAttachmentEntry = Effect.fn("removeDeletedThreadAttachmentEntry")( - function* (threadSegment: string, entry: string) { - const normalizedEntry = entry.replace(/^[/\\]+/, "").replace(/\\/g, "/"); - if (normalizedEntry.length === 0 || normalizedEntry.includes("/")) { - return; - } - const attachmentId = parseAttachmentIdFromRelativePath(normalizedEntry); - if (!attachmentId) { - return; - } - const attachmentThreadSegment = parseThreadSegmentFromAttachmentId(attachmentId); - if (!attachmentThreadSegment || attachmentThreadSegment !== threadSegment) { - return; - } - yield* fileSystem.remove(path.join(attachmentsRootDir, normalizedEntry), { - force: true, - }); - }, - ); +/** Cursor row in `projection_state` for attachment cleanup. It is not a projector. */ +const ATTACHMENT_CLEANUP_CURSOR = "projection.attachment-cleanup"; - const deleteThreadAttachments = Effect.fn("deleteThreadAttachments")(function* ( - threadId: string, - ) { - const threadSegment = toSafeThreadAttachmentSegment(threadId); - const threadBrowserArtifactsDir = resolveThreadBrowserArtifactsDir({ - browserArtifactsDir: serverConfig.browserArtifactsDir, +/** One thread's file cleanup, merged from its latest revert and delete. */ +interface AttachmentCleanupTarget { + readonly threadId: string; + /** Remove every file unless the thread was re-created after this sequence. */ + readonly deletedAtSequence: number | null; + /** Keep only files the thread's current messages and answers reference. */ + readonly pruned: boolean; +} + +function attachmentCleanupTargets( + event: OrchestrationEvent, + sideEffects: AttachmentSideEffects, +): Array { + const targets: Array = [...sideEffects.deletedThreadIds].map( + (threadId) => ({ threadId, - }); - if (!threadSegment || !threadBrowserArtifactsDir) { - yield* Effect.logWarning("skipping attachment cleanup for unsafe thread id", { - threadId, - }); - return; + deletedAtSequence: event.sequence, + pruned: sideEffects.prunedThreadRelativePaths.has(threadId), + }), + ); + for (const threadId of sideEffects.prunedThreadRelativePaths.keys()) { + if (!sideEffects.deletedThreadIds.has(threadId)) { + targets.push({ threadId, deletedAtSequence: null, pruned: true }); } + } + return targets; +} - yield* fileSystem.remove(threadBrowserArtifactsDir, { recursive: true, force: true }); - const entries = yield* readAttachmentRootEntries; - yield* Effect.forEach( - entries, - (entry) => removeDeletedThreadAttachmentEntry(threadSegment, entry), - { - concurrency: 1, - }, - ); - }); - - const pruneThreadAttachmentEntry = Effect.fn("pruneThreadAttachmentEntry")(function* ( - threadSegment: string, - keptThreadRelativePaths: Set, - entry: string, - ) { +/** Groups attachment directory entries by the thread segment in their attachment ids. */ +function groupAttachmentFilesByThreadSegment( + entries: ReadonlyArray, +): Map> { + const filesBySegment = new Map>(); + for (const entry of entries) { const relativePath = entry.replace(/^[/\\]+/, "").replace(/\\/g, "/"); if (relativePath.length === 0 || relativePath.includes("/")) { - return; + continue; } const attachmentId = parseAttachmentIdFromRelativePath(relativePath); - if (!attachmentId) { - return; - } - const attachmentThreadSegment = parseThreadSegmentFromAttachmentId(attachmentId); - if (!attachmentThreadSegment || attachmentThreadSegment !== threadSegment) { - return; + const threadSegment = attachmentId ? parseThreadSegmentFromAttachmentId(attachmentId) : null; + if (!threadSegment) { + continue; } - - const absolutePath = path.join(attachmentsRootDir, relativePath); - const fileInfo = yield* fileSystem.stat(absolutePath).pipe(Effect.orElseSucceed(() => null)); - if (!fileInfo || fileInfo.type !== "File") { - return; + const files = filesBySegment.get(threadSegment); + if (files) { + files.push(relativePath); + } else { + filesBySegment.set(threadSegment, [relativePath]); } + } + return filesBySegment; +} - if (!keptThreadRelativePaths.has(relativePath)) { - yield* fileSystem.remove(absolutePath, { force: true }); - } - }); - - const pruneThreadAttachments = Effect.fn("pruneThreadAttachments")(function* ( - threadId: string, - keptThreadRelativePaths: Set, - ) { - if (sideEffects.deletedThreadIds.has(threadId)) { - return; +/** + * Projectors whose replay records attachment cleanup (deletes, and message or answer pruning). + * Only their cursors can lower where cleanup restarts; adding an unrelated projector must not + * re-run every historical revert against today's references. + */ +const ATTACHMENT_CLEANUP_PROJECTOR_NAMES: ReadonlyArray = [ + ORCHESTRATION_PROJECTOR_NAMES.threads, + ORCHESTRATION_PROJECTOR_NAMES.threadMessages, + ORCHESTRATION_PROJECTOR_NAMES.threadActivities, +]; + +/** The lowest cursor among the named projectors, or undefined while any of them has none. */ +function lowestProjectorCursor( + states: ReadonlyArray, + names: ReadonlyArray = Object.values(ORCHESTRATION_PROJECTOR_NAMES), +): ProjectionState | undefined { + const byProjector = new Map(states.map((state) => [state.projector, state])); + let lowest: ProjectionState | undefined; + for (const name of names) { + const state = byProjector.get(name); + if (!state) { + return undefined; } - - const threadSegment = toSafeThreadAttachmentSegment(threadId); - if (!threadSegment) { - yield* Effect.logWarning("skipping attachment prune for unsafe thread id", { threadId }); - return; + if (!lowest || state.lastAppliedSequence < lowest.lastAppliedSequence) { + lowest = state; } + } + return lowest; +} - const entries = yield* readAttachmentRootEntries; - yield* Effect.forEach( - entries, - (entry) => pruneThreadAttachmentEntry(threadSegment, keptThreadRelativePaths, entry), - { concurrency: 1 }, - ); - }); - - yield* Effect.forEach(sideEffects.deletedThreadIds, deleteThreadAttachments, { - concurrency: 1, - }); - - yield* Effect.forEach( - sideEffects.prunedThreadRelativePaths.entries(), - ([threadId, keptThreadRelativePaths]) => - pruneThreadAttachments(threadId, keptThreadRelativePaths), - { concurrency: 1 }, - ); +const AttachmentCleanupEventRow = Schema.Struct({ + sequence: NonNegativeInt, + type: Schema.Literals(["thread.reverted", "thread.deleted"]), + threadId: ThreadId, }); +const decodeAttachmentCleanupEventRow = Schema.decodeUnknownOption(AttachmentCleanupEventRow); const makeOrchestrationProjectionPipeline = Effect.fn("makeOrchestrationProjectionPipeline")( function* () { @@ -1193,7 +1173,7 @@ const makeOrchestrationProjectionPipeline = Effect.fn("makeOrchestrationProjecti const applyThreadActivitiesProjection: ProjectorDefinition["apply"] = Effect.fn( "applyThreadActivitiesProjection", - )(function* (event, _attachmentSideEffects) { + )(function* (event, attachmentSideEffects) { switch (event.type) { case "thread.created": yield* projectionThreadActivityRepository.deleteByThreadId({ @@ -1241,6 +1221,7 @@ const makeOrchestrationProjectionPipeline = Effect.fn("makeOrchestrationProjecti yield* Effect.forEach(keptRows, projectionThreadActivityRepository.upsert, { concurrency: 1, }).pipe(Effect.asVoid); + attachmentSideEffects.prunedThreadRelativePaths.set(event.payload.threadId, new Set()); return; } @@ -1983,70 +1964,296 @@ const makeOrchestrationProjectionPipeline = Effect.fn("makeOrchestrationProjecti }, ]; - const applyAttachmentSideEffects = Effect.fn("applyAttachmentSideEffects")( - function* (event: OrchestrationEvent, sideEffects: AttachmentSideEffects) { - if ( - sideEffects.deletedThreadIds.size === 0 && - sideEffects.prunedThreadRelativePaths.size === 0 - ) { - return; - } + const attachmentsRootDir = serverConfig.attachmentsDir; - const deletedThreadIds = new Set(); - for (const threadId of sideEffects.deletedThreadIds) { - const recreatedLater = yield* eventStore.hasEventAfter({ - aggregateKind: "thread", - aggregateId: ThreadId.make(threadId), - type: "thread.created", - sequenceExclusive: event.sequence, - }); - if (!recreatedLater) { - deletedThreadIds.add(threadId); + // File failures are logged and reported as false so the remaining files still run. + const removeAttachmentFile = (relativePath: string) => + fileSystem.remove(path.join(attachmentsRootDir, relativePath), { force: true }).pipe( + Effect.as(true), + Effect.catch((cause) => + Effect.logWarning("failed to remove attachment file", { relativePath, cause }).pipe( + Effect.as(false), + ), + ), + ); + + const pruneAttachmentFile = (relativePath: string) => + fileSystem.stat(path.join(attachmentsRootDir, relativePath)).pipe( + Effect.flatMap((info) => + info.type === "File" ? removeAttachmentFile(relativePath) : Effect.succeed(true), + ), + Effect.catchTags({ + PlatformError: (cause) => + cause.reason._tag === "NotFound" + ? Effect.succeed(true) + : Effect.logWarning("failed to inspect attachment file", { + relativePath, + cause, + }).pipe(Effect.as(false)), + }), + ); + + // Read after every projector has applied the event, so later references are included. + const readRetainedThreadAttachmentPaths = Effect.fn("readRetainedThreadAttachmentPaths")( + function* (threadId: ThreadId) { + const messages = yield* projectionThreadMessageRepository.listByThreadId({ threadId }); + const retainedPaths = collectThreadAttachmentRelativePaths(threadId, messages); + const answers = yield* projectionThreadActivityRepository.listByThreadId({ + threadId, + activityKinds: ["user-input.answer-submitted"], + }); + for (const activity of answers) { + const payload = decodeQuestionAttachmentAnswer(activity.payload); + if (Option.isNone(payload)) continue; + for (const attachment of Object.values(payload.value.attachmentsByQuestionId).flat()) { + const relativePath = attachmentRelativePath(attachment); + if (relativePath) retainedPaths.add(relativePath); } } + return retainedPaths; + }, + ); + + // Transferred recordings and saved snapshots live outside the attachments directory, so + // revert pruning never sees them; only deleting the thread removes them. + const removeThreadBrowserArtifacts = (threadId: string) => { + const directory = resolveThreadBrowserArtifactsDir({ + browserArtifactsDir: serverConfig.browserArtifactsDir, + threadId, + }); + if (directory === null) { + return Effect.succeed(true); + } + return fileSystem.remove(directory, { recursive: true, force: true }).pipe( + Effect.as(true), + Effect.catch((cause) => + Effect.logWarning("failed to remove thread browser artifacts", { threadId, cause }).pipe( + Effect.as(false), + ), + ), + ); + }; + + const cleanupThreadAttachmentFiles = Effect.fn("cleanupThreadAttachmentFiles")(function* ( + target: AttachmentCleanupTarget, + files: ReadonlyArray, + hasBrowserArtifacts: boolean, + ) { + const threadId = ThreadId.make(target.threadId); + if (target.deletedAtSequence !== null) { + // A draft retry can re-create the id; its files then belong to the later incarnation. + const recreatedLater = yield* eventStore.hasEventAfter({ + aggregateKind: "thread", + aggregateId: threadId, + type: "thread.created", + sequenceExclusive: target.deletedAtSequence, + }); + if (!recreatedLater) { + const removed = yield* Effect.forEach(files, removeAttachmentFile, { concurrency: 1 }); + const artifactsRemoved = hasBrowserArtifacts + ? yield* removeThreadBrowserArtifacts(target.threadId) + : true; + return artifactsRemoved && removed.every(Boolean); + } + } + if (!target.pruned) { + return true; + } + const retainedPaths = yield* readRetainedThreadAttachmentPaths(threadId); + const pruned = yield* Effect.forEach( + files.filter((file) => !retainedPaths.has(file)), + pruneAttachmentFile, + { concurrency: 1 }, + ); + return pruned.every(Boolean); + }); - // Later events in the same transaction can add attachment references. - const prunedThreadRelativePaths = new Map>(); - for (const threadId of sideEffects.prunedThreadRelativePaths.keys()) { - const messages = yield* projectionThreadMessageRepository.listByThreadId({ - threadId: ThreadId.make(threadId), + /** + * Lists the attachments directory once (and the browser artifacts directory once when a + * target deletes a thread) and cleans each target's files. Threads without files cost no + * reads, unless the artifacts listing failed, in which case every deleted thread's folder is + * removed directly. Returns false when any thread or file failed (each is logged); fails only + * when the attachments directory cannot be listed. + */ + const cleanupAttachments = Effect.fn("cleanupAttachments")(function* ( + targets: ReadonlyArray, + ) { + const safeTargets: Array<{ target: AttachmentCleanupTarget; threadSegment: string }> = []; + for (const target of targets) { + const threadSegment = toSafeThreadAttachmentSegment(target.threadId); + if (threadSegment) { + safeTargets.push({ target, threadSegment }); + } else { + yield* Effect.logWarning("skipping attachment cleanup for unsafe thread id", { + threadId: target.threadId, }); - prunedThreadRelativePaths.set( - threadId, - collectThreadAttachmentRelativePaths(threadId, messages), - ); } + } + if (safeTargets.length === 0) { + return true; + } + const entries = yield* fileSystem + .readDirectory(attachmentsRootDir, { recursive: false }) + .pipe( + Effect.catchTags({ + PlatformError: (cause) => + cause.reason._tag === "NotFound" + ? Effect.succeed>([]) + : Effect.fail(cause), + }), + ); + const filesBySegment = groupAttachmentFilesByThreadSegment(entries); + let complete = true; + // Null when the listing failed: any deleted thread may then own a folder, so each one is + // removed directly (a no-op when absent) instead of being skipped for good. + let browserArtifactSegments: Set | null = new Set(); + if (safeTargets.some(({ target }) => target.deletedAtSequence !== null)) { + const artifactEntries = yield* fileSystem + .readDirectory(serverConfig.browserArtifactsDir, { recursive: false }) + .pipe( + Effect.catchTags({ + PlatformError: (cause) => + cause.reason._tag === "NotFound" + ? Effect.succeed | null>([]) + : Effect.logWarning("failed to list browser artifacts", { cause }).pipe( + Effect.as | null>(null), + ), + }), + ); + browserArtifactSegments = + artifactEntries === null + ? null + : new Set(artifactEntries.map((entry) => entry.replace(/^[/\\]+/, ""))); + } + for (const { target, threadSegment } of safeTargets) { + const files = filesBySegment.get(threadSegment) ?? []; + const hasBrowserArtifacts = + target.deletedAtSequence !== null && + (browserArtifactSegments === null || browserArtifactSegments.has(threadSegment)); + if (files.length === 0 && !hasBrowserArtifacts) continue; + const cleaned = yield* cleanupThreadAttachmentFiles( + target, + files, + hasBrowserArtifacts, + ).pipe( + Effect.catch((cause) => + Effect.logWarning("failed to clean thread attachments", { + threadId: target.threadId, + cause, + }).pipe(Effect.as(false)), + ), + ); + complete = complete && cleaned; + } + return complete; + }); - yield* runAttachmentSideEffects({ deletedThreadIds, prunedThreadRelativePaths }); - }, - Effect.provideService(FileSystem.FileSystem, fileSystem), - Effect.provideService(Path.Path, path), - Effect.provideService(ServerConfig, serverConfig), - (effect, event) => - effect.pipe( + // Cleanup has finished for every event up to this cursor. Live projection writes it with the + // projector cursors, so it trails the head by the command whose cleanup has not run yet. It is + // null before bootstrap and after a failed cleanup, which leaves the persisted cursor behind + // that event for the next bootstrap to retry. + let cleanupDone: ProjectionState | null = null; + + const applyAttachmentSideEffects = Effect.fn("applyAttachmentSideEffects")(function* ( + event: OrchestrationEvent, + sideEffects: AttachmentSideEffects, + ) { + const targets = attachmentCleanupTargets(event, sideEffects); + const complete = + targets.length === 0 || + (yield* cleanupAttachments(targets).pipe( Effect.catch((cause) => Effect.logWarning("failed to apply projected attachment side-effects", { sequence: event.sequence, eventType: event.type, cause, - }), + }).pipe(Effect.as(false)), ), - ), - ); + )); + cleanupDone = + complete && cleanupDone !== null + ? { + projector: ATTACHMENT_CLEANUP_CURSOR, + lastAppliedSequence: event.sequence, + updatedAt: event.occurredAt, + } + : null; + }); - const applyProjectorForEvent = Effect.fn("applyProjectorForEvent")(function* ( - projector: ProjectorDefinition, - event: OrchestrationEvent, - attachmentSideEffects: AttachmentSideEffects, + // Selects only what cleanup needs, so no payload is decoded and old payloads cannot fail it. + const readAttachmentCleanupEvents = Effect.fn("readAttachmentCleanupEvents")(function* ( + sequenceExclusive: number, + sequenceInclusive: number, ) { - yield* projector.apply(event, attachmentSideEffects); - yield* projectionStateRepository.upsert({ - projector: projector.name, - lastAppliedSequence: event.sequence, - updatedAt: event.occurredAt, - }); + const rows = yield* sql` + SELECT sequence, event_type AS "type", stream_id AS "threadId" + FROM orchestration_events + WHERE sequence > ${sequenceExclusive} + AND sequence <= ${sequenceInclusive} + AND aggregate_kind = 'thread' + AND event_type IN ('thread.reverted', 'thread.deleted') + ORDER BY sequence ASC + `; + const events: Array = []; + for (const row of rows) { + const decoded = decodeAttachmentCleanupEventRow(row); + if (Option.isSome(decoded)) { + events.push(decoded.value); + } else { + yield* Effect.logWarning("skipping attachment cleanup for an unreadable event", { row }); + } + } + return events; }); + /** + * Cleans files for reverts and deletes past the cursor once every projector has caught up, + * then moves the cursor to the projector head. File failures here are logged and not retried + * again, so one persistent failure cannot pin the cursor. Returns the cursor live cleanup + * continues from, or null when cleanup could not run and the next bootstrap should retry. + */ + const cleanupAttachmentBacklog = Effect.fn("cleanupAttachmentBacklog")( + function* (boundary: ProjectionState) { + const head = lowestProjectorCursor(yield* projectionStateRepository.listAll()); + if (!head || head.lastAppliedSequence <= boundary.lastAppliedSequence) { + return boundary; + } + const targetsByThread = new Map(); + for (const event of yield* readAttachmentCleanupEvents( + boundary.lastAppliedSequence, + head.lastAppliedSequence, + )) { + const current = targetsByThread.get(event.threadId) ?? { + threadId: event.threadId, + deletedAtSequence: null, + pruned: false, + }; + targetsByThread.set( + event.threadId, + event.type === "thread.deleted" + ? { ...current, deletedAtSequence: event.sequence } + : { ...current, pruned: true }, + ); + } + yield* cleanupAttachments([...targetsByThread.values()]); + const cursor = { + projector: ATTACHMENT_CLEANUP_CURSOR, + lastAppliedSequence: head.lastAppliedSequence, + updatedAt: head.updatedAt, + }; + yield* projectionStateRepository.upsert(cursor); + return cursor; + }, + (effect) => + effect.pipe( + Effect.catch((cause) => + Effect.logWarning("attachment cleanup did not run; the next start retries it", { + cause, + }).pipe(Effect.as(null)), + ), + ), + ); + const runProjectorForEvent = Effect.fn("runProjectorForEvent")(function* ( projector: ProjectorDefinition, event: OrchestrationEvent, @@ -2056,8 +2263,16 @@ const makeOrchestrationProjectionPipeline = Effect.fn("makeOrchestrationProjecti prunedThreadRelativePaths: new Map>(), }; - yield* sql.withTransaction(applyProjectorForEvent(projector, event, attachmentSideEffects)); - yield* applyAttachmentSideEffects(event, attachmentSideEffects); + yield* sql.withTransaction( + Effect.gen(function* () { + yield* projector.apply(event, attachmentSideEffects); + yield* projectionStateRepository.upsert({ + projector: projector.name, + lastAppliedSequence: event.sequence, + updatedAt: event.occurredAt, + }); + }), + ); }); const bootstrapProjector = (projector: ProjectorDefinition) => @@ -2092,22 +2307,21 @@ const makeOrchestrationProjectionPipeline = Effect.fn("makeOrchestrationProjecti { concurrency: 1, discard: true }, ); // Runtime projectors commit together. Bootstrap still advances each cursor separately. - yield* projectionStateRepository.upsertMany( - projectors.map((projector) => ({ + // The cleanup cursor rides in the same statement, at the last finished cleanup. + yield* projectionStateRepository.upsertMany([ + ...projectors.map((projector) => ({ projector: projector.name, lastAppliedSequence: event.sequence, updatedAt: event.occurredAt, })), - ); + ...(cleanupDone === null ? [] : [cleanupDone]), + ]); }), ); // Return the cleanup effect so the caller runs it after the outer transaction commits. // @effect-diagnostics-next-line returnEffectInGen:off return applyAttachmentSideEffects(event, attachmentSideEffects); }, - Effect.provideService(FileSystem.FileSystem, fileSystem), - Effect.provideService(Path.Path, path), - Effect.provideService(ServerConfig, serverConfig), Effect.catchTag("SqlError", (sqlError) => Effect.fail(toPersistenceSqlError("ProjectionPipeline.projectEvent:query")(sqlError)), ), @@ -2120,14 +2334,31 @@ const makeOrchestrationProjectionPipeline = Effect.fn("makeOrchestrationProjecti yield* cleanup; }); - const bootstrap: OrchestrationProjectionPipelineShape["bootstrap"] = Effect.forEach( - projectors, - bootstrapProjector, - { concurrency: 1 }, - ).pipe( - Effect.provideService(FileSystem.FileSystem, fileSystem), - Effect.provideService(Path.Path, path), - Effect.provideService(ServerConfig, serverConfig), + const bootstrap: OrchestrationProjectionPipelineShape["bootstrap"] = Effect.gen(function* () { + const states = yield* projectionStateRepository.listAll(); + const cleanupState = states.find((state) => state.projector === ATTACHMENT_CLEANUP_CURSOR); + const projectorFloor = lowestProjectorCursor(states, ATTACHMENT_CLEANUP_PROJECTOR_NAMES); + const projectorStart = projectorFloor?.lastAppliedSequence ?? 0; + // Projector replay cleaned files as it went before cleanup had its own cursor, so a + // database without one starts where its projectors resume. + const boundary: ProjectionState = + cleanupState && cleanupState.lastAppliedSequence <= projectorStart + ? cleanupState + : { + projector: ATTACHMENT_CLEANUP_CURSOR, + lastAppliedSequence: projectorStart, + updatedAt: + cleanupState?.updatedAt ?? projectorFloor?.updatedAt ?? "1970-01-01T00:00:00.000Z", + }; + if (boundary !== cleanupState) { + // Persist this boundary before replay: a reset projector can encounter an old + // revert, then fail after other projectors have committed past that event. + yield* projectionStateRepository.upsert(boundary); + } + cleanupDone = null; + yield* Effect.forEach(projectors, bootstrapProjector, { concurrency: 1, discard: true }); + cleanupDone = yield* cleanupAttachmentBacklog(boundary); + }).pipe( Effect.asVoid, Effect.tap(() => Effect.logDebug("orchestration projection pipeline bootstrapped").pipe( diff --git a/apps/server/src/orchestration/Layers/ProviderCommandReactor.ts b/apps/server/src/orchestration/Layers/ProviderCommandReactor.ts index 4852281cb..a746b5b0e 100644 --- a/apps/server/src/orchestration/Layers/ProviderCommandReactor.ts +++ b/apps/server/src/orchestration/Layers/ProviderCommandReactor.ts @@ -2171,6 +2171,9 @@ const make = Effect.gen(function* () { threadId: event.payload.threadId, requestId: event.payload.requestId, answers: event.payload.answers, + ...(event.payload.attachmentsByQuestionId + ? { attachmentsByQuestionId: event.payload.attachmentsByQuestionId } + : {}), }) .pipe( Effect.catchCause((cause) => diff --git a/apps/server/src/orchestration/Normalizer.attachments.test.ts b/apps/server/src/orchestration/Normalizer.attachments.test.ts index 7385b6531..982dc16db 100644 --- a/apps/server/src/orchestration/Normalizer.attachments.test.ts +++ b/apps/server/src/orchestration/Normalizer.attachments.test.ts @@ -5,13 +5,15 @@ import * as NodePath from "node:path"; import * as NodeServices from "@effect/platform-node/NodeServices"; import { describe, expect, it } from "@effect/vitest"; import { - type ClientOrchestrationCommand, + ClientOrchestrationCommand, CommandId, + ApprovalRequestId, MessageId, ThreadId, } from "@t3tools/contracts"; import * as Effect from "effect/Effect"; import * as Layer from "effect/Layer"; +import * as Schema from "effect/Schema"; import * as ServerConfig from "../config.ts"; import * as WorkspacePaths from "../workspace/WorkspacePaths.ts"; @@ -23,6 +25,7 @@ const testLayer = Layer.mergeAll( ).pipe(Layer.provideMerge(NodeServices.layer)); const attachmentUuid = "00000000-0000-4000-8000-0000000000aa"; +const isClientCommand = Schema.is(ClientOrchestrationCommand); function turnStartCommand(input: { readonly threadId?: string; @@ -358,3 +361,201 @@ describe("normalizeDispatchCommand attachments", () => { }).pipe(Effect.provide(testLayer)), ); }); + +describe("question attachments", () => { + it.effect("enforces the total response limit and claims duplicate filenames independently", () => + Effect.gen(function* () { + const config = yield* ServerConfig.ServerConfig; + const id = `pending-${attachmentUuid}-txt`; + NodeFS.writeFileSync(NodePath.join(config.attachmentsDir, `${id}.txt`), "report"); + const attachment = { + type: "file" as const, + id, + name: 'notes "final" ü.txt', + mimeType: "text/plain", + sizeBytes: 6, + }; + const command: ClientOrchestrationCommand = { + type: "thread.user-input.respond", + commandId: CommandId.make("answer-cap"), + threadId: ThreadId.make("thread-1"), + requestId: ApprovalRequestId.make("request-cap"), + answers: { first: "", second: "" }, + createdAt: "2026-08-01T00:00:00.000Z", + attachmentsByQuestionId: { + first: Array.from({ length: 4 }, () => attachment), + second: Array.from({ length: 5 }, () => attachment), + }, + }; + const failure = yield* normalizeDispatchCommand(command).pipe(Effect.flip); + expect(failure.message).toContain("up to 8"); + expect(NodeFS.readdirSync(config.attachmentsDir)).toEqual([`${id}.txt`]); + const accepted = { + ...command, + attachmentsByQuestionId: { + ...command.attachmentsByQuestionId, + second: Array.from({ length: 4 }, () => attachment), + }, + }; + const normalized = yield* normalizeDispatchCommand(accepted); + if (normalized.type !== "thread.user-input.respond") throw new Error("Wrong command"); + const attachments = Object.values(normalized.attachmentsByQuestionId!).flat(); + expect(attachments).toHaveLength(8); + expect(new Set(attachments.map((item) => item.id)).size).toBe(8); + for (const item of attachments) { + expect(item.name).toBe(attachment.name); + expect( + NodeFS.readFileSync(NodePath.join(config.attachmentsDir, `${item.id}.txt`), "utf8"), + ).toBe("report"); + } + yield* cleanupFailedUploadedAttachments(accepted, normalized); + expect(NodeFS.readdirSync(config.attachmentsDir)).toEqual([`${id}.txt`]); + }).pipe(Effect.provide(testLayer)), + ); + it("requires uploaded metadata for question images, including pasted images", () => { + expect( + isClientCommand({ + type: "thread.user-input.respond", + commandId: "answer", + threadId: "thread-1", + requestId: "request", + answers: { q: "" }, + createdAt: "2026-08-01T00:00:00.000Z", + attachmentsByQuestionId: { + q: [ + { + type: "image", + name: "image.png", + mimeType: "image/png", + sizeBytes: 6, + dataUrl: "data:image/png;base64,cGl4ZWxz", + }, + ], + }, + }), + ).toBe(false); + }); + + it.effect("preserves a __proto__ question key and cleans up its claimed files", () => + Effect.gen(function* () { + const config = yield* ServerConfig.ServerConfig; + const id = `pending-${attachmentUuid}`; + NodeFS.writeFileSync(NodePath.join(config.attachmentsDir, `${id}.png`), "pixels"); + const command: ClientOrchestrationCommand = { + type: "thread.user-input.respond", + commandId: CommandId.make("answer"), + threadId: ThreadId.make("thread-1"), + requestId: ApprovalRequestId.make("request"), + answers: { ["__proto__"]: "" }, + createdAt: "2026-08-01T00:00:00.000Z", + attachmentsByQuestionId: { + ["__proto__"]: [ + { type: "image", id, name: "image.png", mimeType: "image/png", sizeBytes: 6 }, + ], + }, + }; + const normalized = yield* normalizeDispatchCommand(command); + if (normalized.type !== "thread.user-input.respond") throw new Error("Wrong command"); + expect(Object.keys(normalized.attachmentsByQuestionId!)).toEqual(["__proto__"]); + const attachment = normalized.attachmentsByQuestionId!["__proto__"]![0]!; + const claimedPath = NodePath.join(config.attachmentsDir, `${attachment.id}.png`); + expect(NodeFS.existsSync(claimedPath)).toBe(true); + yield* cleanupFailedUploadedAttachments(command, normalized); + expect(NodeFS.existsSync(claimedPath)).toBe(false); + }).pipe(Effect.provide(testLayer)), + ); + it.effect( + "claims images and files by question, preserves answers, and cleans up failed dispatches", + () => + Effect.gen(function* () { + const config = yield* ServerConfig.ServerConfig; + const imageId = `pending-${attachmentUuid}`; + const fileId = `pending-${attachmentUuid}-txt`; + NodeFS.writeFileSync(NodePath.join(config.attachmentsDir, `${imageId}.png`), "pixels"); + NodeFS.writeFileSync(NodePath.join(config.attachmentsDir, `${fileId}.txt`), "report"); + const command: ClientOrchestrationCommand = { + type: "thread.user-input.respond", + commandId: CommandId.make("answer"), + threadId: ThreadId.make("thread-1"), + requestId: ApprovalRequestId.make("request"), + answers: { q1: ["Selected option"], q2: "" }, + createdAt: "2026-08-01T00:00:00.000Z", + attachmentsByQuestionId: { + q1: [ + { + type: "image", + id: imageId, + name: "image.png", + mimeType: "image/png", + sizeBytes: 6, + }, + ], + q2: [ + { + type: "file", + id: fileId, + name: "report.txt", + mimeType: "text/plain", + sizeBytes: 6, + }, + ], + }, + }; + const normalized = yield* normalizeDispatchCommand(command); + if (normalized.type !== "thread.user-input.respond") throw new Error("Wrong command"); + expect(normalized.answers).toEqual(command.answers); + const image = normalized.attachmentsByQuestionId!.q1![0]!; + const file = normalized.attachmentsByQuestionId!.q2![0]!; + expect( + NodeFS.readFileSync(NodePath.join(config.attachmentsDir, `${image.id}.png`), "utf8"), + ).toBe("pixels"); + expect( + NodeFS.readFileSync(NodePath.join(config.attachmentsDir, `${file.id}.txt`), "utf8"), + ).toBe("report"); + yield* cleanupFailedUploadedAttachments(command, normalized); + expect(NodeFS.existsSync(NodePath.join(config.attachmentsDir, `${image.id}.png`))).toBe( + false, + ); + expect(NodeFS.existsSync(NodePath.join(config.attachmentsDir, `${file.id}.txt`))).toBe( + false, + ); + expect(NodeFS.existsSync(NodePath.join(config.attachmentsDir, `${imageId}.png`))).toBe( + true, + ); + const retry = yield* normalizeDispatchCommand(command); + expect(retry.type).toBe("thread.user-input.respond"); + }).pipe(Effect.provide(testLayer)), + ); + + it.effect("removes all claimed copies if a later question upload is missing", () => + Effect.gen(function* () { + const config = yield* ServerConfig.ServerConfig; + const id = `pending-${attachmentUuid}`; + NodeFS.writeFileSync(NodePath.join(config.attachmentsDir, `${id}.png`), "pixels"); + const result = yield* normalizeDispatchCommand({ + type: "thread.user-input.respond", + commandId: CommandId.make("answer"), + threadId: ThreadId.make("thread-1"), + requestId: ApprovalRequestId.make("request"), + answers: { q1: "", q2: "" }, + createdAt: "2026-08-01T00:00:00.000Z", + attachmentsByQuestionId: { + q1: [{ type: "image", id, name: "image.png", mimeType: "image/png", sizeBytes: 6 }], + q2: [ + { + type: "file", + id: `${id}-txt`, + name: "missing.txt", + mimeType: "text/plain", + sizeBytes: 6, + }, + ], + }, + }).pipe(Effect.result); + expect(result._tag).toBe("Failure"); + expect( + NodeFS.readdirSync(config.attachmentsDir).filter((name) => name.startsWith("thread-1-")), + ).toEqual([]); + }).pipe(Effect.provide(testLayer)), + ); +}); diff --git a/apps/server/src/orchestration/Normalizer.ts b/apps/server/src/orchestration/Normalizer.ts index 6a9dc9f31..7ff32ade0 100644 --- a/apps/server/src/orchestration/Normalizer.ts +++ b/apps/server/src/orchestration/Normalizer.ts @@ -4,6 +4,8 @@ import * as FileSystem from "effect/FileSystem"; import * as Path from "effect/Path"; import { type ClientOrchestrationCommand, + type UserInputAttachments, + PROVIDER_SEND_TURN_MAX_ATTACHMENTS, type IsoDateTime, type OrchestrationCommand, OrchestrationDispatchCommandError, @@ -131,14 +133,27 @@ export const normalizeDispatchCommand = (command: ClientOrchestrationCommand) => if ( canonicalCommand.type !== "thread.turn.start" && - canonicalCommand.type !== "thread.input-queue.follow-up" + canonicalCommand.type !== "thread.input-queue.follow-up" && + canonicalCommand.type !== "thread.user-input.respond" ) { return canonicalCommand as OrchestrationCommand; } + const attachments = + canonicalCommand.type === "thread.user-input.respond" + ? Object.values(canonicalCommand.attachmentsByQuestionId ?? {}).flat() + : canonicalCommand.message.attachments; + if ( + canonicalCommand.type === "thread.user-input.respond" && + attachments.length > PROVIDER_SEND_TURN_MAX_ATTACHMENTS + ) { + return yield* new OrchestrationDispatchCommandError({ + message: `You can attach up to ${PROVIDER_SEND_TURN_MAX_ATTACHMENTS} files per question response.`, + }); + } const claimedAttachmentPaths: string[] = []; const normalizedAttachments = yield* Effect.forEach( - canonicalCommand.message.attachments, + attachments, (attachment) => Effect.gen(function* () { if (!("dataUrl" in attachment)) { @@ -262,6 +277,25 @@ export const normalizeDispatchCommand = (command: ClientOrchestrationCommand) => { concurrency: 1 }, ).pipe(Effect.tapError(() => removeClaimedAttachmentPaths(claimedAttachmentPaths))); + if (canonicalCommand.type === "thread.user-input.respond") { + let index = 0; + const attachmentsByQuestionId = Object.fromEntries( + Object.entries(canonicalCommand.attachmentsByQuestionId ?? {}).map( + ([questionId, original]) => { + const claimed = normalizedAttachments.slice( + index, + index + original.length, + ) as UserInputAttachments[string]; + index += original.length; + return [questionId, claimed]; + }, + ), + ); + return { + ...canonicalCommand, + ...(attachments.length > 0 ? { attachmentsByQuestionId } : {}), + }; + } return { ...canonicalCommand, message: { @@ -274,22 +308,31 @@ export const normalizeDispatchCommand = (command: ClientOrchestrationCommand) => export const cleanupFailedUploadedAttachments = Effect.fn( "Normalizer.cleanupFailedUploadedAttachments", )(function* (command: ClientOrchestrationCommand, normalizedCommand: OrchestrationCommand) { - // Both message-carrying commands claim uploaded attachments in - // normalizeDispatchCommand, so both have copies to release when the dispatch - // fails. thread.input-queue.follow-up is Pylon-only; upstream has no such - // command and only guards thread.turn.start. - const carriesAttachments = - (command.type === "thread.turn.start" && normalizedCommand.type === "thread.turn.start") || - (command.type === "thread.input-queue.follow-up" && - normalizedCommand.type === "thread.input-queue.follow-up"); - if (!carriesAttachments) { - return; - } + // Every attachment-carrying command claims uploaded attachments in + // normalizeDispatchCommand, so each has copies to release when the dispatch + // fails. thread.input-queue.follow-up is Pylon-only; upstream guards only + // thread.turn.start and thread.user-input.respond. + const originalAttachments = + command.type === "thread.turn.start" || command.type === "thread.input-queue.follow-up" + ? command.message.attachments + : command.type === "thread.user-input.respond" + ? Object.values(command.attachmentsByQuestionId ?? {}).flat() + : []; + const normalizedAttachments = + command.type !== normalizedCommand.type + ? [] + : normalizedCommand.type === "thread.turn.start" || + normalizedCommand.type === "thread.input-queue.follow-up" + ? normalizedCommand.message.attachments + : normalizedCommand.type === "thread.user-input.respond" + ? Object.values(normalizedCommand.attachmentsByQuestionId ?? {}).flat() + : []; + if (normalizedAttachments.length === 0) return; const serverConfig = yield* ServerConfig; const claimedPaths: string[] = []; - for (const [index, attachment] of normalizedCommand.message.attachments.entries()) { - const original = command.message.attachments[index]; + for (const [index, attachment] of normalizedAttachments.entries()) { + const original = originalAttachments[index]; if ( !original || "dataUrl" in original || diff --git a/apps/server/src/orchestration/decider.questionAttachments.test.ts b/apps/server/src/orchestration/decider.questionAttachments.test.ts new file mode 100644 index 000000000..6c3cdc62e --- /dev/null +++ b/apps/server/src/orchestration/decider.questionAttachments.test.ts @@ -0,0 +1,130 @@ +import { + ApprovalRequestId, + EventId, + CommandId, + ProjectId, + ProviderInstanceId, + ThreadId, + type OrchestrationReadModel, +} from "@t3tools/contracts"; +import * as NodeServices from "@effect/platform-node/NodeServices"; +import { expect, it } from "@effect/vitest"; +import * as Effect from "effect/Effect"; + +import { decideOrchestrationCommand } from "./decider.ts"; + +const UPDATED_AT = "2026-01-01T00:00:00.000Z"; + +const readModel: OrchestrationReadModel = { + snapshotSequence: 0, + projects: [], + threads: [ + { + id: ThreadId.make("thread-1"), + projectId: ProjectId.make("project-1"), + title: "Manual title", + modelSelection: { instanceId: ProviderInstanceId.make("codex"), model: "gpt-5.4" }, + runtimeMode: "full-access", + interactionMode: "default", + branch: null, + worktreePath: null, + latestTurn: null, + createdAt: UPDATED_AT, + updatedAt: UPDATED_AT, + archivedAt: null, + settledOverride: null, + settledAt: null, + snoozedUntil: null, + snoozedAt: null, + deletedAt: null, + messages: [], + proposedPlans: [], + activities: [], + checkpoints: [], + session: null, + }, + ], + updatedAt: UPDATED_AT, +}; + +const requestId = ApprovalRequestId.make("question-request"); +const command = { + type: "thread.user-input.respond" as const, + commandId: CommandId.make("answer"), + threadId: ThreadId.make("thread-1"), + requestId, + answers: { q: "" }, + createdAt: UPDATED_AT, + attachmentsByQuestionId: { + q: [ + { + type: "file" as const, + id: "thread-1-00000000-0000-4000-8000-0000000000aa-txt", + name: "spec.txt", + mimeType: "text/plain", + sizeBytes: 4, + }, + ], + }, +}; +const request = { + id: EventId.make("question"), + kind: "user-input.requested", + summary: "Question", + tone: "info" as const, + turnId: null, + createdAt: UPDATED_AT, + payload: { + requestId, + questions: [ + { id: "q", header: "Spec", question: "Provide a spec", options: [], allowCustomAnswer: true }, + ], + }, +}; +it.layer(NodeServices.layer)("question attachment answers", (it) => { + it.effect("persists the original answer with its attachment and emits a provider response", () => + Effect.gen(function* () { + const result = yield* decideOrchestrationCommand({ + readModel, + command, + userInputActivity: request, + }); + const events = Array.isArray(result) ? result : [result]; + expect(events.map((event) => event.type)).toEqual([ + "thread.activity-appended", + "thread.user-input-response-requested", + ]); + expect(events[0]?.payload).toMatchObject({ + activity: { + kind: "user-input.answer-submitted", + payload: { answers: { q: "" }, attachmentsByQuestionId: command.attachmentsByQuestionId }, + }, + }); + expect(events[1]?.payload).toMatchObject({ + answers: { q: "" }, + attachmentsByQuestionId: command.attachmentsByQuestionId, + }); + }), + ); + it.effect("rejects attachments for a resolved or unknown request", () => + Effect.gen(function* () { + const result = yield* decideOrchestrationCommand({ readModel, command }).pipe(Effect.result); + expect(result._tag).toBe("Failure"); + }), + ); + it.effect("rejects unknown question IDs and predefined-choice-only protocols", () => + Effect.gen(function* () { + for (const question of [ + { ...request.payload.questions[0], id: "other" }, + { ...request.payload.questions[0], allowCustomAnswer: false }, + ]) { + const result = yield* decideOrchestrationCommand({ + readModel, + command, + userInputActivity: { ...request, payload: { ...request.payload, questions: [question] } }, + }).pipe(Effect.result); + expect(result._tag).toBe("Failure"); + } + }), + ); +}); diff --git a/apps/server/src/orchestration/decider.ts b/apps/server/src/orchestration/decider.ts index 89857b6d1..97a14d348 100644 --- a/apps/server/src/orchestration/decider.ts +++ b/apps/server/src/orchestration/decider.ts @@ -1600,6 +1600,35 @@ export const decideOrchestrationCommand = Effect.fn("decideOrchestrationCommand" threadId: command.threadId, }); const request = userInputActivity; + const attachments = Object.values(command.attachmentsByQuestionId ?? {}).flat(); + let questionTextById: Record = {}; + if (attachments.length > 0) { + const payload = + request?.kind === "user-input.requested" + ? decodeUserInputRequestedPayload(request.payload) + : Option.none(); + if (Option.isNone(payload)) { + return yield* new OrchestrationCommandInvariantError({ + commandType: command.type, + detail: + request?.kind === "user-input.resolved" + ? "This question has already been answered." + : "This question is no longer pending.", + }); + } + questionTextById = Object.fromEntries( + payload.value.questions.map((question) => [question.id, question.question]), + ); + for (const questionId of Object.keys(command.attachmentsByQuestionId ?? {})) { + const question = payload.value.questions.find((question) => question.id === questionId); + if (!question || question.allowCustomAnswer === false) { + return yield* new OrchestrationCommandInvariantError({ + commandType: command.type, + detail: "This question does not accept file references.", + }); + } + } + } if ( request && Predicate.isObject(request.payload) && @@ -1615,13 +1644,22 @@ export const decideOrchestrationCommand = Effect.fn("decideOrchestrationCommand" const replies: string[] = []; for (const question of payload.value.questions) { const answer = command.answers[question.id]; - if (typeof answer !== "string" || answer.trim().length === 0) { + if ( + typeof answer !== "string" || + (answer.trim().length === 0 && !command.attachmentsByQuestionId?.[question.id]?.length) + ) { return yield* new OrchestrationCommandInvariantError({ commandType: command.type, detail: "Answer each question before sending.", }); } - replies.push(`${question.question}\n${answer.trim()}`); + const questionAttachments = command.attachmentsByQuestionId?.[question.id] ?? []; + const attachmentLabels = questionAttachments + .map((attachment) => `Attached file: ${attachment.name} (${attachment.id})`) + .join("\n"); + replies.push( + [`${question.question}\n${answer.trim()}`, attachmentLabels].filter(Boolean).join("\n"), + ); } // Commit the answer and its message together. The normal turn path // steers a running agent or resumes an idle session. @@ -1644,6 +1682,9 @@ export const decideOrchestrationCommand = Effect.fn("decideOrchestrationCommand" requestId: command.requestId, responseMode: "message", answers: command.answers, + ...(command.attachmentsByQuestionId + ? { attachmentsByQuestionId: command.attachmentsByQuestionId } + : {}), }, }, }, @@ -1658,30 +1699,57 @@ export const decideOrchestrationCommand = Effect.fn("decideOrchestrationCommand" messageId: MessageId.make(`async-answer:${command.requestId}`), role: "user", text: replies.join("\n\n"), - attachments: [], + attachments, }, }, ], }); } - return { + const responseEvent = { ...(yield* withEventBase({ aggregateKind: "thread", aggregateId: command.threadId, occurredAt: command.createdAt, commandId: command.commandId, - metadata: { - requestId: command.requestId, - }, + metadata: { requestId: command.requestId }, })), - type: "thread.user-input-response-requested", + type: "thread.user-input-response-requested" as const, payload: { threadId: command.threadId, requestId: command.requestId, answers: command.answers, + ...(command.attachmentsByQuestionId + ? { attachmentsByQuestionId: command.attachmentsByQuestionId } + : {}), createdAt: command.createdAt, }, }; + if (attachments.length === 0) return responseEvent; + const historyEvent = yield* decideOrchestrationCommand({ + readModel, + command: { + type: "thread.activity.append", + commandId: command.commandId, + threadId: command.threadId, + createdAt: command.createdAt, + activity: { + id: EventId.make(`question-answer:${command.commandId}`), + kind: "user-input.answer-submitted", + summary: "Question answer submitted", + tone: "info", + turnId: request?.turnId ?? null, + createdAt: command.createdAt, + payload: { + requestId: command.requestId, + answers: command.answers, + questionTextById, + attachmentsByQuestionId: command.attachmentsByQuestionId, + detail: attachments.map((attachment) => attachment.name).join("\n"), + }, + }, + }, + }); + return [...(Array.isArray(historyEvent) ? historyEvent : [historyEvent]), responseEvent]; } case "thread.user-input.dismiss": { diff --git a/apps/server/src/persistence/Layers/ProjectionState.ts b/apps/server/src/persistence/Layers/ProjectionState.ts index a6fb7f3be..383ab7d9f 100644 --- a/apps/server/src/persistence/Layers/ProjectionState.ts +++ b/apps/server/src/persistence/Layers/ProjectionState.ts @@ -1,4 +1,3 @@ -import { NonNegativeInt } from "@t3tools/contracts"; import * as SqlClient from "effect/unstable/sql/SqlClient"; import * as SqlSchema from "effect/unstable/sql/SqlSchema"; import * as Effect from "effect/Effect"; @@ -14,10 +13,6 @@ import { ProjectionState, } from "../Services/ProjectionState.ts"; -const MinLastAppliedSequenceRowSchema = Schema.Struct({ - minLastAppliedSequence: Schema.NullOr(NonNegativeInt), -}); - const makeProjectionStateRepository = Effect.gen(function* () { const sql = yield* SqlClient.SqlClient; @@ -90,17 +85,6 @@ const makeProjectionStateRepository = Effect.gen(function* () { `, }); - const readMinLastAppliedSequence = SqlSchema.findOne({ - Request: Schema.Void, - Result: MinLastAppliedSequenceRowSchema, - execute: () => - sql` - SELECT - MIN(last_applied_sequence) AS "minLastAppliedSequence" - FROM projection_state - `, - }); - const upsert: ProjectionStateRepositoryShape["upsert"] = (row) => upsertProjectionStateRow(row).pipe( Effect.mapError(toPersistenceSqlError("ProjectionStateRepository.upsert:query")), @@ -121,20 +105,11 @@ const makeProjectionStateRepository = Effect.gen(function* () { Effect.mapError(toPersistenceSqlError("ProjectionStateRepository.listAll:query")), ); - const minLastAppliedSequence: ProjectionStateRepositoryShape["minLastAppliedSequence"] = () => - readMinLastAppliedSequence(undefined).pipe( - Effect.mapError( - toPersistenceSqlError("ProjectionStateRepository.minLastAppliedSequence:query"), - ), - Effect.map((row) => row.minLastAppliedSequence), - ); - return { upsert, upsertMany, getByProjector, listAll, - minLastAppliedSequence, } satisfies ProjectionStateRepositoryShape; }); diff --git a/apps/server/src/persistence/Services/ProjectionState.ts b/apps/server/src/persistence/Services/ProjectionState.ts index 9372acdee..219765efa 100644 --- a/apps/server/src/persistence/Services/ProjectionState.ts +++ b/apps/server/src/persistence/Services/ProjectionState.ts @@ -50,16 +50,9 @@ export interface ProjectionStateRepositoryShape { ) => Effect.Effect, ProjectionRepositoryError>; /** - * List all projector cursor rows. + * List all cursor rows, including non-projector cursors such as attachment cleanup. */ readonly listAll: () => Effect.Effect, ProjectionRepositoryError>; - - /** - * Read the minimum applied sequence across all projectors. - * - * Returns `null` when no projector state rows exist. - */ - readonly minLastAppliedSequence: () => Effect.Effect; } /** diff --git a/apps/server/src/provider/Layers/ProviderService.ts b/apps/server/src/provider/Layers/ProviderService.ts index 55777367a..e6c3ea0a0 100644 --- a/apps/server/src/provider/Layers/ProviderService.ts +++ b/apps/server/src/provider/Layers/ProviderService.ts @@ -72,6 +72,7 @@ import * as SchemaIssue from "effect/SchemaIssue"; import * as Stream from "effect/Stream"; import * as SynchronizedRef from "effect/SynchronizedRef"; +import { appendUserInputAttachmentPaths } from "../userInputAttachments.ts"; import { resolveAttachmentPath } from "../../attachmentStore.ts"; import * as ServerConfig from "../../config.ts"; import { @@ -2647,7 +2648,11 @@ const makeProviderService = Effect.fn("makeProviderService")(function* ( "provider.thread_id": input.threadId, "provider.request_id": input.requestId, }); - yield* routed.adapter.respondToUserInput(routed.threadId, input.requestId, input.answers); + const answers = yield* appendUserInputAttachmentPaths({ + ...input, + attachmentsDir: serverConfig.attachmentsDir, + }).pipe(Effect.provideService(FileSystem.FileSystem, fileSystem)); + yield* routed.adapter.respondToUserInput(routed.threadId, input.requestId, answers); }).pipe( withMetrics({ counter: providerTurnsTotal, diff --git a/apps/server/src/provider/userInputAttachments.test.ts b/apps/server/src/provider/userInputAttachments.test.ts new file mode 100644 index 000000000..b9b6764ab --- /dev/null +++ b/apps/server/src/provider/userInputAttachments.test.ts @@ -0,0 +1,61 @@ +// @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 * as Effect from "effect/Effect"; +import * as Layer from "effect/Layer"; +import * as ServerConfig from "../config.ts"; +import { appendUserInputAttachmentPaths } from "./userInputAttachments.ts"; + +const layer = ServerConfig.layerTest(process.cwd(), { prefix: "t3-answer-paths-" }).pipe( + Layer.provideMerge(NodeServices.layer), +); +const attachment = { + type: "file" as const, + id: "thread-1-00000000-0000-4000-8000-0000000000aa-txt", + name: 'spec "final".txt', + mimeType: "text/plain", + sizeBytes: 4, +}; +describe("question answer paths", () => { + it.effect("keeps selected values intact and appends an actual readable server path", () => + Effect.gen(function* () { + const config = yield* ServerConfig.ServerConfig; + const path = NodePath.join(config.attachmentsDir, `${attachment.id}.txt`); + NodeFS.writeFileSync(path, "spec"); + const original = { q: ["First", "Second"], other: "No file" }; + const answers = yield* appendUserInputAttachmentPaths({ + answers: original, + attachmentsDir: config.attachmentsDir, + attachmentsByQuestionId: { q: [attachment] }, + }); + expect(answers.q).toEqual([ + "First", + "Second", + `Attached file "spec \\"final\\".txt": "${path}"`, + ]); + expect(answers.other).toBe("No file"); + expect(original.q).toEqual(["First", "Second"]); + expect(NodeFS.readFileSync(path, "utf8")).toBe("spec"); + const specialKey = yield* appendUserInputAttachmentPaths({ + answers: {}, + attachmentsDir: config.attachmentsDir, + attachmentsByQuestionId: { ["__proto__"]: [attachment] }, + }); + expect(Object.keys(specialKey)).toEqual(["__proto__"]); + expect(specialKey["__proto__"]).toContain(path); + }).pipe(Effect.provide(layer)), + ); + it.effect("does not send an unavailable attachment path", () => + Effect.gen(function* () { + const config = yield* ServerConfig.ServerConfig; + const result = yield* appendUserInputAttachmentPaths({ + answers: { q: "" }, + attachmentsDir: config.attachmentsDir, + attachmentsByQuestionId: { q: [attachment] }, + }).pipe(Effect.result); + expect(result._tag).toBe("Failure"); + }).pipe(Effect.provide(layer)), + ); +}); diff --git a/apps/server/src/provider/userInputAttachments.ts b/apps/server/src/provider/userInputAttachments.ts new file mode 100644 index 000000000..cb210951f --- /dev/null +++ b/apps/server/src/provider/userInputAttachments.ts @@ -0,0 +1,59 @@ +import type { ProviderUserInputAnswers, UserInputAttachments } from "@t3tools/contracts"; +import * as Schema from "effect/Schema"; +import * as Effect from "effect/Effect"; +import * as FileSystem from "effect/FileSystem"; +import { resolveAttachmentPath } from "../attachmentStore.ts"; +import { ProviderValidationError } from "./Errors.ts"; + +const quoteReference = Schema.encodeSync(Schema.fromJsonString(Schema.String)); + +/** Keep provider answer protocols unchanged; paths refer to files on the provider's server. */ +export const appendUserInputAttachmentPaths = Effect.fn("appendUserInputAttachmentPaths")( + function* (input: { + answers: ProviderUserInputAnswers; + attachmentsByQuestionId?: UserInputAttachments | undefined; + attachmentsDir: string; + }) { + const answers = new Map(Object.entries(input.answers)); + const fs = yield* FileSystem.FileSystem; + for (const [questionId, attachments] of Object.entries(input.attachmentsByQuestionId ?? {})) { + if (attachments.length === 0) continue; + const references: string[] = []; + for (const attachment of attachments) { + const path = resolveAttachmentPath({ attachmentsDir: input.attachmentsDir, attachment }); + if ( + !path || + !(yield* fs.exists(path).pipe( + Effect.mapError( + (cause) => + new ProviderValidationError({ + operation: "respondToUserInput", + issue: `Could not access attachment '${attachment.name}'.`, + cause, + }), + ), + )) + ) { + return yield* new ProviderValidationError({ + operation: "respondToUserInput", + issue: `Attachment '${attachment.name}' is no longer available. Attach it again.`, + }); + } + references.push( + `Attached ${attachment.type} ${quoteReference(attachment.name)}: ${quoteReference(path)}`, + ); + } + const answer = answers.get(questionId); + const text = references.join("\n"); + answers.set( + questionId, + Array.isArray(answer) + ? [...answer, text] + : typeof answer === "string" && answer.length > 0 + ? `${answer}\n\n${text}` + : text, + ); + } + return Object.fromEntries(answers); + }, +); diff --git a/apps/web/src/components/ChatView.tsx b/apps/web/src/components/ChatView.tsx index cae33d299..aaa7c5121 100644 --- a/apps/web/src/components/ChatView.tsx +++ b/apps/web/src/components/ChatView.tsx @@ -6,6 +6,13 @@ import { } from "./settings/ProjectSettingsPanel.logic"; import { videoMimeType } from "@t3tools/shared/video"; import { derivePendingRequests } from "@t3tools/client-runtime/pending-requests"; +import { + questionAttachmentDraftId, + questionAttachmentDraftPrefix, + clearQuestionAttachmentDraft, + useQuestionAttachmentPreparation, +} from "../questionAttachments"; +import { useAttachmentUploadStore } from "../lib/attachmentUploadQueue"; import { type AssistantCitation, type ApprovalRequestId, @@ -311,7 +318,7 @@ import { markPromotedDraftThreadByRef, useComposerDraftStore, useEffectiveComposerModelState, - type DraftId, + DraftId, } from "../composerDraftStore"; import { appendTerminalContextsToPrompt, @@ -1744,6 +1751,7 @@ export default function ChatView(props: ChatViewProps) { null, ); const [respondingRequestIds, setRespondingRequestIds] = useState([]); + const userInputResponsesInFlight = useRef(new Set()); const [respondingUserInputRequestIds, setRespondingUserInputRequestIds] = useState< ApprovalRequestId[] >([]); @@ -2473,6 +2481,8 @@ export default function ChatView(props: ChatViewProps) { : null; const attachmentEnvironmentConfig = environmentById.get(environmentId)?.serverConfig ?? null; const attachmentUploadsCapabilityKnown = attachmentEnvironmentConfig !== null; + const supportsQuestionAttachments = + attachmentEnvironmentConfig?.environment.capabilities.questionAttachments === true; const supportsAttachmentUploads = attachmentEnvironmentConfig?.environment.capabilities.attachmentUploads === true; const advertisedFileAttachmentBytes = @@ -2907,16 +2917,120 @@ export default function ChatView(props: ChatViewProps) { [threadActivities], ); const activePendingUserInput = pendingUserInputs[0] ?? null; - const activePendingDraftAnswers = useMemo( + const activePendingRequestKey = JSON.stringify([ + environmentId, + activeThreadId, + activePendingUserInput?.requestId, + ]); + const pendingQuestionDraftKeys = useMemo( () => - activePendingUserInput - ? (pendingUserInputAnswersByRequestId[activePendingUserInput.requestId] ?? - EMPTY_PENDING_USER_INPUT_ANSWERS) - : EMPTY_PENDING_USER_INPUT_ANSWERS, - [activePendingUserInput, pendingUserInputAnswersByRequestId], + activeThreadId + ? pendingUserInputs.flatMap((request) => + request.questions.map((question) => + questionAttachmentDraftId( + environmentId, + activeThreadId, + request.requestId, + question.id, + ), + ), + ) + : [], + [activeThreadId, environmentId, pendingUserInputs], + ); + const questionComposerDrafts = useComposerDraftStore( + useShallow((state) => + Object.fromEntries( + pendingQuestionDraftKeys.map((key) => [key, state.draftsByThreadKey[key]]), + ), + ), ); + const questionUploadsBlocked = useAttachmentUploadStore( + useShallow((state) => + Object.fromEntries( + pendingQuestionDraftKeys.map((key) => { + const draft = questionComposerDrafts[key]; + const attachments = draft ? [...draft.images, ...draft.files] : []; + return [ + key, + attachments.some((attachment) => { + const upload = state.uploadsByImageId[attachment.id]; + return upload?.status !== "ready" || upload.environmentId !== environmentId; + }), + ]; + }), + ), + ), + ); + const questionPreparations = useQuestionAttachmentPreparation( + useShallow((state) => + Object.fromEntries(pendingQuestionDraftKeys.map((key) => [key, state.counts[key] ?? 0])), + ), + ); + useEffect(() => { + if (routeThreadState.status !== "live" || routeThreadState.data._tag !== "Some") return; + const questionThread = routeThreadState.data.value; + const { userInputs: currentRequests } = derivePendingRequests(questionThread.activities); + const prefix = questionAttachmentDraftPrefix(environmentId, questionThread.id); + const retained = new Set( + currentRequests.flatMap((request) => + request.questions.map((question) => + questionAttachmentDraftId( + environmentId, + questionThread.id, + request.requestId, + question.id, + ), + ), + ), + ); + const keys = new Set([ + ...Object.keys(useComposerDraftStore.getState().draftsByThreadKey), + ...Object.keys(useQuestionAttachmentPreparation.getState().counts), + ]); + for (const key of keys) { + if (key.startsWith(prefix) && !retained.has(DraftId.make(key))) + clearQuestionAttachmentDraft(DraftId.make(key)); + } + }, [environmentId, routeThreadState.data, routeThreadState.status]); + const activePendingDraftAnswers = useMemo(() => { + if (!activePendingUserInput || !activeThreadId) return EMPTY_PENDING_USER_INPUT_ANSWERS; + return Object.fromEntries( + activePendingUserInput.questions.map((question) => { + const key = questionAttachmentDraftId( + environmentId, + activeThreadId, + activePendingUserInput.requestId, + question.id, + ); + const draft = questionComposerDrafts[key]; + const attachments = draft ? [...draft.images, ...draft.files] : []; + return [ + question.id, + { + ...pendingUserInputAnswersByRequestId[activePendingRequestKey]?.[question.id], + attachmentCount: attachments.length, + attachmentsBlocked: + (attachments.length > 0 && !supportsQuestionAttachments) || + (questionPreparations[key] ?? 0) > 0 || + questionUploadsBlocked[key] === true, + }, + ]; + }), + ); + }, [ + activePendingUserInput, + activeThreadId, + environmentId, + questionComposerDrafts, + questionUploadsBlocked, + supportsQuestionAttachments, + questionPreparations, + pendingUserInputAnswersByRequestId, + activePendingRequestKey, + ]); const activePendingQuestionIndex = activePendingUserInput - ? (pendingUserInputQuestionIndexByRequestId[activePendingUserInput.requestId] ?? 0) + ? (pendingUserInputQuestionIndexByRequestId[activePendingRequestKey] ?? 0) : 0; const activePendingProgress = useMemo( () => @@ -7923,8 +8037,38 @@ export default function ChatView(props: ChatViewProps) { const onRespondToUserInput = useCallback( async (requestId: ApprovalRequestId, answers: Record) => { - if (!activeThreadId) return; - + if (!activeThreadId || !activePendingUserInput || activePendingIsResponding) return; + const responseKey = JSON.stringify([environmentId, activeThreadId, requestId]); + if (userInputResponsesInFlight.current.has(responseKey)) return; + const attachmentsByQuestionId = new Map< + string, + import("@t3tools/contracts").UserInputAttachments[string] + >(); + for (const question of activePendingUserInput.questions) { + const target = questionAttachmentDraftId( + environmentId, + activeThreadId, + requestId, + question.id, + ); + if ((useQuestionAttachmentPreparation.getState().counts[target] ?? 0) > 0) return; + const draft = useComposerDraftStore.getState().getComposerDraft(target); + const attachments = draft ? [...draft.images, ...draft.files] : []; + if (attachments.length === 0) continue; + const uploaded = getUploadedAttachments({ environmentId, images: attachments }); + if (!uploaded) { + setThreadError( + activeThreadId, + "Wait for attachments to finish uploading, or remove failed uploads.", + ); + return; + } + attachmentsByQuestionId.set( + question.id, + uploaded as import("@t3tools/contracts").UserInputAttachments[string], + ); + } + userInputResponsesInFlight.current.add(responseKey); setRespondingUserInputRequestIds((existing) => existing.includes(requestId) ? existing : [...existing, requestId], ); @@ -7934,6 +8078,9 @@ export default function ChatView(props: ChatViewProps) { threadId: activeThreadId, requestId, answers, + ...(attachmentsByQuestionId.size > 0 + ? { attachmentsByQuestionId: Object.fromEntries(attachmentsByQuestionId) } + : {}), }, }); if (result._tag === "Failure" && !isAtomCommandInterrupted(result)) { @@ -7943,10 +8090,18 @@ export default function ChatView(props: ChatViewProps) { error instanceof Error ? error.message : "Failed to submit user input.", ); } + userInputResponsesInFlight.current.delete(responseKey); setRespondingUserInputRequestIds((existing) => existing.filter((id) => id !== requestId)); return result; }, - [activeThreadId, environmentId, respondToThreadUserInput, setThreadError], + [ + activeThreadId, + activePendingUserInput, + activePendingIsResponding, + environmentId, + respondToThreadUserInput, + setThreadError, + ], ); const onRespondToSessionInteraction = useCallback( @@ -8041,10 +8196,10 @@ export default function ChatView(props: ChatViewProps) { } setPendingUserInputQuestionIndexByRequestId((existing) => ({ ...existing, - [activePendingUserInput.requestId]: nextQuestionIndex, + [activePendingRequestKey]: nextQuestionIndex, })); }, - [activePendingUserInput], + [activePendingUserInput, activePendingRequestKey], ); const onSelectActivePendingUserInputOption = useCallback( @@ -8064,11 +8219,11 @@ export default function ChatView(props: ChatViewProps) { return { ...existing, - [activePendingUserInput.requestId]: { - ...existing[activePendingUserInput.requestId], + [activePendingRequestKey]: { + ...existing[activePendingRequestKey], [questionId]: togglePendingUserInputOptionSelection( question, - existing[activePendingUserInput.requestId]?.[questionId], + existing[activePendingRequestKey]?.[questionId], optionLabel, ), }, @@ -8077,7 +8232,12 @@ export default function ChatView(props: ChatViewProps) { promptRef.current = ""; composerRef.current?.resetCursorState({ cursor: 0 }); }, - [activePendingProgress?.activeQuestion, activePendingUserInput, composerRef], + [ + activePendingProgress?.activeQuestion, + activePendingUserInput, + activePendingRequestKey, + composerRef, + ], ); const onChangeActivePendingUserInputCustomAnswer = useCallback( @@ -8098,10 +8258,10 @@ export default function ChatView(props: ChatViewProps) { promptRef.current = value; setPendingUserInputAnswersByRequestId((existing) => ({ ...existing, - [activePendingUserInput.requestId]: { - ...existing[activePendingUserInput.requestId], + [activePendingRequestKey]: { + ...existing[activePendingRequestKey], [questionId]: setPendingUserInputCustomAnswer( - existing[activePendingUserInput.requestId]?.[questionId], + existing[activePendingRequestKey]?.[questionId], value, ), }, @@ -8115,11 +8275,16 @@ export default function ChatView(props: ChatViewProps) { composerRef.current?.focusAt(nextCursor); } }, - [activePendingUserInput, composerRef], + [activePendingUserInput, activePendingRequestKey, composerRef], ); const onAdvanceActivePendingUserInput = useCallback(() => { - if (!activePendingUserInput || !activePendingProgress) { + if ( + !activePendingUserInput || + !activePendingProgress || + !activePendingProgress.canAdvance || + activePendingIsResponding + ) { return; } if (activePendingProgress.isLastQuestion) { @@ -8133,6 +8298,7 @@ export default function ChatView(props: ChatViewProps) { activePendingProgress, activePendingResolvedAnswers, activePendingUserInput, + activePendingIsResponding, onRespondToUserInput, setActivePendingUserInputQuestionIndex, ]); @@ -9415,6 +9581,7 @@ export default function ChatView(props: ChatViewProps) { environmentId={environmentId} attachmentUploadsCapabilityKnown={attachmentUploadsCapabilityKnown} supportsAttachmentUploads={supportsAttachmentUploads} + supportsQuestionAttachments={supportsQuestionAttachments} maxFileAttachmentBytes={maxFileAttachmentBytes} routeKind={routeKind} routeThreadRef={routeThreadRef} diff --git a/apps/web/src/components/chat/ChatComposer.tsx b/apps/web/src/components/chat/ChatComposer.tsx index f8327bd77..f6a263740 100644 --- a/apps/web/src/components/chat/ChatComposer.tsx +++ b/apps/web/src/components/chat/ChatComposer.tsx @@ -1,6 +1,11 @@ import { observeResponsiveBreakpointFade, usePanelAnimationSettings } from "../../panelAnimations"; import { measureRestingComposerControls } from "./restingComposerControlsMeasurement"; import { RefreshIcon } from "~/components/ui/refresh-icon"; +import { + questionAttachmentDraftId, + useQuestionAttachmentPreparation, + changeQuestionAttachmentPreparation, +} from "../../questionAttachments"; import type { ApprovalRequestId, AssistantCitation, @@ -1316,6 +1321,7 @@ export interface ChatComposerProps { environmentId: EnvironmentId; attachmentUploadsCapabilityKnown: boolean; supportsAttachmentUploads: boolean; + supportsQuestionAttachments: boolean; maxFileAttachmentBytes: number | null; routeKind: "server" | "draft"; routeThreadRef: ScopedThreadRef; @@ -1504,6 +1510,7 @@ export const ChatComposer = memo(function ChatComposer(props: ChatComposerProps) environmentId, attachmentUploadsCapabilityKnown, supportsAttachmentUploads, + supportsQuestionAttachments, maxFileAttachmentBytes, routeKind, routeThreadRef, @@ -1622,9 +1629,24 @@ export const ChatComposer = memo(function ChatComposer(props: ChatComposerProps) // happened while they awaited. const composerDraftTargetKeyRef = useRef(""); composerDraftTargetKeyRef.current = composerTargetKey(composerDraftTarget); + // A pending question keeps its attachments in its own draft, so they never + // mix with the thread's normal prompt draft. + const questionAttachmentTarget = + pendingUserInputs[0] && activePendingProgress?.activeQuestion && activeThreadId + ? questionAttachmentDraftId( + environmentId, + activeThreadId, + pendingUserInputs[0].requestId, + activePendingProgress.activeQuestion.id, + ) + : null; + const attachmentDraftTarget = questionAttachmentTarget ?? composerDraftTarget; + const attachmentDraft = useComposerThreadDraft(attachmentDraftTarget); + const attachmentTargetKey = composerTargetKey(attachmentDraftTarget); + const questionPreparations = useQuestionAttachmentPreparation((state) => state.counts); const prompt = composerDraft.prompt; - const composerImages = composerDraft.images; - const composerFiles = composerDraft.files; + const composerImages = attachmentDraft.images; + const composerFiles = attachmentDraft.files; const composerVideos = composerFiles.filter((file) => isPreviewableComposerVideo(file, environmentId), ); @@ -1641,7 +1663,7 @@ export const ChatComposer = memo(function ChatComposer(props: ChatComposerProps) ); return composerImages.filter((image) => !previewAnnotationIds.has(image.id)); }, [composerImages, composerPreviewAnnotations]); - const nonPersistedComposerImageIds = composerDraft.nonPersistedImageIds; + const nonPersistedComposerImageIds = attachmentDraft.nonPersistedImageIds; const uploadsByImageId = useAttachmentUploadStore((state) => state.uploadsByImageId); const needsReattachFileCount = composerFiles.filter(composerFileNeedsReattach).length; const fileStagingLimit = fileAttachmentStagingLimit({ @@ -1656,6 +1678,11 @@ export const ChatComposer = memo(function ChatComposer(props: ChatComposerProps) maxFileAttachmentBytes, }); const attachmentBlockReason = + (questionAttachmentTarget && + !supportsQuestionAttachments && + (composerImages.length > 0 || composerFiles.length > 0) + ? "Update this server to send files with question answers" + : null) ?? fileCapabilityBlockReason ?? (supportsAttachmentUploads ? needsReattachFileCount > 0 @@ -1668,11 +1695,9 @@ export const ChatComposer = memo(function ChatComposer(props: ChatComposerProps) environmentId, }) : null); - const baseSendDisabledReason = - externalSendDisabledReason ?? (activePendingProgress ? null : attachmentBlockReason); + const baseSendDisabledReason = externalSendDisabledReason ?? attachmentBlockReason; const setComposerDraftPrompt = useComposerDraftStore((store) => store.setPrompt); - const addComposerDraftImage = useComposerDraftStore((store) => store.addImage); const addComposerDraftImages = useComposerDraftStore((store) => store.addImages); const removeComposerDraftImage = useComposerDraftStore((store) => store.removeImage); const addComposerDraftFiles = useComposerDraftStore((store) => store.addFiles); @@ -1740,11 +1765,15 @@ export const ChatComposer = memo(function ChatComposer(props: ChatComposerProps) if (attachment.type === "file" && composerFileNeedsReattach(attachment)) { continue; } - startAttachmentUpload({ environmentId, image: attachment, draftTarget: composerDraftTarget }); + startAttachmentUpload({ + environmentId, + image: attachment, + draftTarget: attachmentDraftTarget, + }); } }, [ attachmentUploadsCapabilityKnown, - composerDraftTarget, + attachmentDraftTarget, composerFiles, composerImages, environmentId, @@ -1765,7 +1794,7 @@ export const ChatComposer = memo(function ChatComposer(props: ChatComposerProps) const upload = uploadsByImageId[file.id]; if (upload?.status === "ready" && upload.environmentId === environmentId) { setComposerDraftFileUpload( - composerDraftTarget, + attachmentDraftTarget, file.id, environmentId, upload.attachmentId, @@ -1774,7 +1803,7 @@ export const ChatComposer = memo(function ChatComposer(props: ChatComposerProps) } }, [ attachmentUploadsCapabilityKnown, - composerDraftTarget, + attachmentDraftTarget, composerFiles, environmentId, maxFileAttachmentBytes, @@ -2727,7 +2756,7 @@ export const ChatComposer = memo(function ChatComposer(props: ChatComposerProps) * and checked before sending or compacting so an image cannot move into * the next draft. */ - const pendingImageCompressionsRef = useRef>(new Map()); + const pendingImageCompressionsRef = useRef>(new Map()); // ------------------------------------------------------------------ // Derived: composer send state @@ -2922,6 +2951,11 @@ export const ChatComposer = memo(function ChatComposer(props: ChatComposerProps) (!isComposerCollapsedMobile && showPlanFollowUpPrompt && activeProposedPlan !== null); const showCollapsedMobilePromptRow = isComposerCollapsedMobile && !isComposerApprovalState && pendingUserInputs.length === 0; + const showComposerAttachAction = + fileStagingLimit !== null && + (!activePendingProgress || + (supportsQuestionAttachments && + activePendingProgress.activeQuestion?.allowCustomAnswer !== false)); const composerFooterHasWideActions = showPlanFollowUpPrompt || activePendingProgress !== null; const composerFooterActionLayoutKey = useMemo(() => { if (activePendingProgress) { @@ -3013,12 +3047,22 @@ export const ChatComposer = memo(function ChatComposer(props: ChatComposerProps) ? { questionIndex: activePendingProgress.questionIndex, isLastQuestion: activePendingProgress.isLastQuestion, - canAdvance: activePendingProgress.canAdvance, + canAdvance: + activePendingProgress.canAdvance && + !attachmentBlockReason && + !(questionPreparations[attachmentTargetKey] ?? 0), isResponding: activePendingIsResponding, isComplete: Boolean(activePendingResolvedAnswers), } : null, - [activePendingIsResponding, activePendingProgress, activePendingResolvedAnswers], + [ + activePendingIsResponding, + activePendingProgress, + activePendingResolvedAnswers, + attachmentBlockReason, + questionPreparations, + attachmentTargetKey, + ], ); const collapsedComposerPrimaryActionDisabled = phase === "running" || @@ -3053,35 +3097,42 @@ export const ChatComposer = memo(function ChatComposer(props: ChatComposerProps) const addComposerImage = useCallback( (image: ComposerImageAttachment) => { - addComposerDraftImage(composerDraftTarget, image); + addComposerDraftImages(attachmentDraftTarget, [image]); }, - [composerDraftTarget, addComposerDraftImage], + [attachmentDraftTarget, addComposerDraftImages], ); const addComposerImagesToDraft = useCallback( (images: ComposerImageAttachment[]) => { - addComposerDraftImages(composerDraftTarget, images); + addComposerDraftImages(attachmentDraftTarget, images); }, - [composerDraftTarget, addComposerDraftImages], + [attachmentDraftTarget, addComposerDraftImages], ); const addComposerFilesToDraft = useCallback( (files: ComposerFileAttachment[]) => { - addComposerDraftFiles(composerDraftTarget, files); + addComposerDraftFiles(attachmentDraftTarget, files); }, - [addComposerDraftFiles, composerDraftTarget], + [addComposerDraftFiles, attachmentDraftTarget], ); const removeComposerImageFromDraft = useCallback( (imageId: string) => { + if (questionAttachmentTarget && activePendingIsResponding) return; releaseAttachmentUpload(imageId); - removeComposerDraftImage(composerDraftTarget, imageId); + removeComposerDraftImage(attachmentDraftTarget, imageId); }, - [composerDraftTarget, removeComposerDraftImage], + [ + attachmentDraftTarget, + questionAttachmentTarget, + activePendingIsResponding, + removeComposerDraftImage, + ], ); const removeComposerFileFromDraft = useCallback( (fileId: string) => { + if (questionAttachmentTarget && activePendingIsResponding) return; // Release by the draft attachment, not the bare queue key: a hydrated // file's upload lives server-side under its persisted attachment id. const file = composerFilesRef.current.find((candidate) => candidate.id === fileId); @@ -3090,9 +3141,15 @@ export const ChatComposer = memo(function ChatComposer(props: ChatComposerProps) } else { releaseAttachmentUpload(fileId); } - removeComposerDraftFile(composerDraftTarget, fileId); + removeComposerDraftFile(attachmentDraftTarget, fileId); }, - [composerDraftTarget, composerFilesRef, removeComposerDraftFile], + [ + attachmentDraftTarget, + questionAttachmentTarget, + activePendingIsResponding, + composerFilesRef, + removeComposerDraftFile, + ], ); const removeComposerTerminalContextFromDraft = useCallback( @@ -3311,11 +3368,11 @@ export const ChatComposer = memo(function ChatComposer(props: ChatComposerProps) let cancelled = false; void (async () => { if (composerImages.length === 0) { - clearComposerDraftPersistedAttachments(composerDraftTarget); + clearComposerDraftPersistedAttachments(attachmentDraftTarget); return; } const getPersistedAttachmentsForThread = () => - getComposerDraft(composerDraftTarget)?.persistedAttachments ?? []; + getComposerDraft(attachmentDraftTarget)?.persistedAttachments ?? []; try { const currentPersistedAttachments = getPersistedAttachmentsForThread(); const existingPersistedById = new Map( @@ -3343,7 +3400,7 @@ export const ChatComposer = memo(function ChatComposer(props: ChatComposerProps) ); const serialized = Array.from(stagedAttachmentById.values()); if (cancelled) return; - syncComposerDraftPersistedAttachments(composerDraftTarget, serialized); + syncComposerDraftPersistedAttachments(attachmentDraftTarget, serialized); } catch { const currentImageIds = new Set(composerImages.map((image) => image.id)); const fallbackPersistedAttachments = getPersistedAttachmentsForThread(); @@ -3358,14 +3415,14 @@ export const ChatComposer = memo(function ChatComposer(props: ChatComposerProps) fallbackPersistedIdSet.has(attachment.id), ); if (cancelled) return; - syncComposerDraftPersistedAttachments(composerDraftTarget, fallbackAttachments); + syncComposerDraftPersistedAttachments(attachmentDraftTarget, fallbackAttachments); } })(); return () => { cancelled = true; }; }, [ - composerDraftTarget, + attachmentDraftTarget, clearComposerDraftPersistedAttachments, composerImages, getComposerDraft, @@ -3705,7 +3762,10 @@ export const ChatComposer = memo(function ChatComposer(props: ChatComposerProps) // image: the turn snapshot wouldn't include it, and it would surface // in the *next* draft instead. Only oversized images hit this — small // files clear the pending counter within a microtask. - if (activeThreadId && (pendingImageCompressionsRef.current.get(activeThreadId) ?? 0) > 0) { + if ( + activeThreadId && + (pendingImageCompressionsRef.current.get(attachmentTargetKey) ?? 0) > 0 + ) { event?.preventDefault(); toastManager.add({ type: "info", @@ -3735,6 +3795,7 @@ export const ChatComposer = memo(function ChatComposer(props: ChatComposerProps) [ activeThreadId, activePendingProgress, + attachmentTargetKey, blurMobileComposerAfterSend, isSendDisabled, providerTurnUnavailable, @@ -3774,7 +3835,7 @@ export const ChatComposer = memo(function ChatComposer(props: ChatComposerProps) // The compact buttons cannot see the compression counter (it lives in // a ref), so they render enabled during a paste; toast instead of // silently ignoring the click. - if ((pendingImageCompressionsRef.current.get(activeThreadId) ?? 0) > 0) { + if ((pendingImageCompressionsRef.current.get(attachmentTargetKey) ?? 0) > 0) { toastManager.add({ type: "info", title: "Still compressing a pasted image.", @@ -3797,6 +3858,7 @@ export const ChatComposer = memo(function ChatComposer(props: ChatComposerProps) }, [ activePendingApproval, activeThreadId, + attachmentTargetKey, compactCommandAvailable, compactDisabled, composerDraftTarget, @@ -5201,10 +5263,15 @@ export const ChatComposer = memo(function ChatComposer(props: ChatComposerProps) // ------------------------------------------------------------------ const addComposerAttachments = async (files: File[]) => { if (!activeThreadId || files.length === 0) return; - if (pendingUserInputs.length > 0) { + if ( + pendingUserInputs.length > 0 && + (!supportsQuestionAttachments || + activePendingProgress?.activeQuestion?.allowCustomAnswer === false || + activePendingIsResponding) + ) { toastManager.add({ type: "error", - title: "Attach files after answering pending questions.", + title: "This question cannot accept attachments.", }); return; } @@ -5216,9 +5283,32 @@ export const ChatComposer = memo(function ChatComposer(props: ChatComposerProps) // Validation happens synchronously so concurrent pastes see each other: // accepted files reserve their attachment slots (via the pending counter) // before the first await, keeping the total under the limit. - const pendingCount = pendingImageCompressionsRef.current.get(threadId) ?? 0; + const pendingCount = pendingImageCompressionsRef.current.get(attachmentTargetKey) ?? 0; + // The attachment limit covers one whole set of answers, not each question. + const otherQuestionAttachments = + questionAttachmentTarget && pendingUserInputs[0] + ? pendingUserInputs[0].questions.reduce((count, question) => { + const target = questionAttachmentDraftId( + environmentId, + threadId, + pendingUserInputs[0]!.requestId, + question.id, + ); + if (target === questionAttachmentTarget) return count; + const draft = getComposerDraft(target); + return ( + count + + (draft?.images.length ?? 0) + + (draft?.files.length ?? 0) + + (useQuestionAttachmentPreparation.getState().counts[target] ?? 0) + ); + }, 0) + : 0; let reservedCount = - composerImagesRef.current.length + composerFilesRef.current.length + pendingCount; + composerImagesRef.current.length + + composerFilesRef.current.length + + pendingCount + + otherQuestionAttachments; // A pick that matches a needs-reattach marker replaces it in the draft, so // it must not consume a slot; a draft full of markers would otherwise hit // the capacity error before the replacement path could run. @@ -5301,7 +5391,12 @@ export const ChatComposer = memo(function ChatComposer(props: ChatComposerProps) } if (acceptedImages.length === 0) return; - pendingImageCompressionsRef.current.set(threadId, pendingCount + acceptedImages.length); + pendingImageCompressionsRef.current.set( + attachmentTargetKey, + pendingCount + acceptedImages.length, + ); + if (questionAttachmentTarget) + changeQuestionAttachmentPreparation(questionAttachmentTarget, acceptedImages.length); try { const nextImages: ComposerImageAttachment[] = []; let compressionError: string | null = null; @@ -5331,6 +5426,15 @@ export const ChatComposer = memo(function ChatComposer(props: ChatComposerProps) file: attachmentFile, }); } + // The question was answered or dismissed while compressing: its draft is + // gone, so drop the images instead of reviving it. + if ( + questionAttachmentTarget && + !useQuestionAttachmentPreparation.getState().counts[questionAttachmentTarget] + ) { + for (const image of nextImages) URL.revokeObjectURL(image.previewUrl); + return; + } if (nextImages.length === 1 && nextImages[0]) { addComposerImage(nextImages[0]); } else if (nextImages.length > 1) { @@ -5344,12 +5448,14 @@ export const ChatComposer = memo(function ChatComposer(props: ChatComposerProps) setThreadError(threadId, compressionError); } } finally { + if (questionAttachmentTarget) + changeQuestionAttachmentPreparation(questionAttachmentTarget, -acceptedImages.length); const remaining = - (pendingImageCompressionsRef.current.get(threadId) ?? 0) - acceptedImages.length; + (pendingImageCompressionsRef.current.get(attachmentTargetKey) ?? 0) - acceptedImages.length; if (remaining > 0) { - pendingImageCompressionsRef.current.set(threadId, remaining); + pendingImageCompressionsRef.current.set(attachmentTargetKey, remaining); } else { - pendingImageCompressionsRef.current.delete(threadId); + pendingImageCompressionsRef.current.delete(attachmentTargetKey); } } }; @@ -6154,7 +6260,7 @@ export const ChatComposer = memo(function ChatComposer(props: ChatComposerProps) retryAttachmentUpload({ environmentId, image, - draftTarget: composerDraftTarget, + draftTarget: attachmentDraftTarget, }), } : {})} @@ -6198,7 +6304,6 @@ export const ChatComposer = memo(function ChatComposer(props: ChatComposerProps) {!isComposerCollapsedMobile && !isComposerApprovalState && - pendingUserInputs.length === 0 && (composerVideos.length > 0 || (!isComposerResting && standaloneComposerImages.length > 0)) && (
@@ -6277,7 +6382,7 @@ export const ChatComposer = memo(function ChatComposer(props: ChatComposerProps) retryAttachmentUpload({ environmentId, image, - draftTarget: composerDraftTarget, + draftTarget: attachmentDraftTarget, }) } aria-label={`Retry upload for ${image.name}`} @@ -6363,7 +6468,7 @@ export const ChatComposer = memo(function ChatComposer(props: ChatComposerProps) retryAttachmentUpload({ environmentId, image: file, - draftTarget: composerDraftTarget, + draftTarget: attachmentDraftTarget, }) } aria-label={`Retry upload for ${file.name}`} @@ -6397,7 +6502,6 @@ export const ChatComposer = memo(function ChatComposer(props: ChatComposerProps) {!isComposerCollapsedMobile && !isComposerApprovalState && - pendingUserInputs.length === 0 && composerOtherFiles.length > 0 && (
{composerOtherFiles.map((file) => { @@ -6440,7 +6544,7 @@ export const ChatComposer = memo(function ChatComposer(props: ChatComposerProps) retryAttachmentUpload({ environmentId, image: file, - draftTarget: composerDraftTarget, + draftTarget: attachmentDraftTarget, }) } aria-label={`Retry upload for ${file.name}`} @@ -6479,7 +6583,7 @@ export const ChatComposer = memo(function ChatComposer(props: ChatComposerProps) ((settings.contextWindowIndicatorEnabled && activeContextWindow) || reserveContextWindowMeter ? "pr-28" - : fileStagingLimit !== null + : showComposerAttachAction ? "pr-20" : "pr-12"), )} @@ -6540,7 +6644,8 @@ export const ChatComposer = memo(function ChatComposer(props: ChatComposerProps) isConnecting || isComposerApprovalState || isChoiceOnlyPendingQuestion || - projectSelectionRequired + projectSelectionRequired || + activePendingIsResponding } /> {isComposerResting ? collapsedComposerImagePreviews : null} @@ -6626,7 +6731,7 @@ export const ChatComposer = memo(function ChatComposer(props: ChatComposerProps) } className="flex shrink-0 flex-nowrap items-center justify-end gap-2" > - {fileStagingLimit !== null && pendingUserInputs.length === 0 ? ( + {showComposerAttachAction ? ( <> { expect(markup).toContain('aria-label="Next turn"'); }); + // Expanding history uses this suite's existing test renderer, deprecated in + // React 19. Migrate these interaction tests together when a DOM test setup is added. + it.each([{}, { text: "Text-only answer", file: "Answer with a file" }])( + "renders attachment-only question history alongside text answers: %j", + async (answers) => { + vi.stubGlobal("IS_REACT_ACT_ENVIRONMENT", true); + vi.stubGlobal("requestAnimationFrame", () => 0); + vi.stubGlobal("cancelAnimationFrame", () => {}); + let renderer: ReactTestRenderer | undefined; + try { + await act(() => { + renderer = create( + , + ); + }); + const toggle = renderer!.root.findByProps({ "aria-expanded": false }); + await act(() => toggle.props.onClick()); + const markup = JSON.stringify(renderer!.toJSON()); + expect(markup.match(/Provide a spec/g)).toHaveLength(1); + expect(markup.match(/spec\.txt/g)).toHaveLength(1); + expect(markup).toContain("Provide a screenshot"); + expect(markup).toContain("shot.png"); + for (const answer of Object.values(answers)) expect(markup).toContain(answer); + } finally { + await act(() => renderer?.unmount()); + } + }, + ); + it.each([ { toolLifecycleStatus: "inProgress", isAtEnd: true }, { toolLifecycleStatus: "inProgress", isAtEnd: false }, diff --git a/apps/web/src/components/chat/MessagesTimeline.tsx b/apps/web/src/components/chat/MessagesTimeline.tsx index 3bd90ac20..a4b66d98a 100644 --- a/apps/web/src/components/chat/MessagesTimeline.tsx +++ b/apps/web/src/components/chat/MessagesTimeline.tsx @@ -3311,7 +3311,9 @@ const PlainWorkEntryRow = memo(function PlainWorkEntryRow(props: { showWarningIndicator || showDestructiveRowStyle ? undefined : (workEntry.toolIcon ?? workEntry.toolSource?.icon); - const previewText = displayLabel ?? workEntryDisplayLabel(workEntry, workspaceRoot); + const previewText = workEntry.questionAnswer + ? "Question answer submitted" + : (displayLabel ?? workEntryDisplayLabel(workEntry, workspaceRoot)); const displayText = !toolPresentation && expanded && workEntry.command?.trim() ? "Command" : previewText; const viewedImagePath = workEntryViewedImagePath(workEntry); @@ -3497,6 +3499,9 @@ const PlainWorkEntryRow = memo(function PlainWorkEntryRow(props: {
+ {workEntry.questionAnswer ? ( + + ) : null} {expanded && canExpand && (expandedBody || viewedImage) ? (
); } + +function QuestionAnswerHistory({ + answer, +}: { + answer: import("@t3tools/contracts").UserInputAttachmentAnswerPayload; +}) { + const { activeThreadEnvironmentId } = use(TimelineRowCtx); + const attachments = useMemo(() => Object.values(answer.attachmentsByQuestionId).flat(), [answer]); + const resources = useMemo( + () => + attachments.map((attachment) => ({ + _tag: "attachment" as const, + attachmentId: attachment.id, + })), + [attachments], + ); + const urls = useAssetUrls(activeThreadEnvironmentId, resources); + return ( +
+ {[ + ...new Set([ + ...Object.keys(answer.answers), + ...Object.keys(answer.attachmentsByQuestionId), + ]), + ].map((questionId) => ( +
+ {answer.questionTextById?.[questionId] ? ( +

{answer.questionTextById[questionId]}

+ ) : null} +

+ {[answer.answers[questionId]] + .flat() + .filter((value): value is string => typeof value === "string") + .join(", ")} +

+
+ {(answer.attachmentsByQuestionId[questionId] ?? []).map((attachment) => { + const url = urls[attachments.indexOf(attachment)]; + return ( + + {attachment.type === "image" && url ? ( + {attachment.name} + ) : ( + attachment.name + )} + + ); + })} +
+
+ ))} +
+ ); +} diff --git a/apps/web/src/lib/attachmentUploadQueue.test.ts b/apps/web/src/lib/attachmentUploadQueue.test.ts index d7b30e452..a4d8c5ea0 100644 --- a/apps/web/src/lib/attachmentUploadQueue.test.ts +++ b/apps/web/src/lib/attachmentUploadQueue.test.ts @@ -325,6 +325,32 @@ describe("attachmentUploadQueue", () => { } }); + it("persists a retried question upload without a mounted composer", async () => { + const draftId = DraftId.make("question-retry-upload"); + const file = makeFile("question-retry"); + const store = useComposerDraftStore.getState(); + store.addFiles(draftId, [file]); + try { + startAttachmentUpload({ environmentId: firstEnvironment, image: file, draftTarget: draftId }); + await Promise.resolve(); + let settled = awaitAttachmentUploads([file.id]); + TestXmlHttpRequest.requests[0]!.complete(500); + await settled; + expect(store.getComposerDraft(draftId)?.files[0]?.uploadedAttachmentId).toBeUndefined(); + retryAttachmentUpload({ environmentId: firstEnvironment, image: file, draftTarget: draftId }); + await Promise.resolve(); + settled = awaitAttachmentUploads([file.id]); + TestXmlHttpRequest.requests[1]!.complete(); + await settled; + expect(store.getComposerDraft(draftId)?.files[0]).toMatchObject({ + uploadedAttachmentId: "pending-environment-1-question-retry.pdf", + uploadEnvironmentId: firstEnvironment, + }); + } finally { + store.clearComposerContent(draftId); + } + }); + it("verifies an uploaded file reference before restoring it", async () => { const file: ComposerFileAttachment = { ...makeFile("restored"), diff --git a/apps/web/src/pendingUserInput.test.ts b/apps/web/src/pendingUserInput.test.ts index 7ccc59ff1..f2f093a5c 100644 --- a/apps/web/src/pendingUserInput.test.ts +++ b/apps/web/src/pendingUserInput.test.ts @@ -302,3 +302,22 @@ describe("pending user input question progress", () => { }); }); }); + +it("accepts attachment-only answers after every upload finishes", () => { + const questions = [ + { id: "spec", header: "Spec", question: "Provide a spec", options: [], multiSelect: false }, + ]; + expect(buildPendingUserInputAnswers(questions, { spec: { attachmentCount: 1 } })).toEqual({ + spec: "", + }); + expect( + buildPendingUserInputAnswers(questions, { + spec: { attachmentCount: 1, attachmentsBlocked: true }, + }), + ).toBeNull(); + expect( + buildPendingUserInputAnswers([{ ...questions[0]!, allowCustomAnswer: false }], { + spec: { attachmentCount: 1 }, + }), + ).toBeNull(); +}); diff --git a/apps/web/src/pendingUserInput.ts b/apps/web/src/pendingUserInput.ts index a10e9d716..5ac004649 100644 --- a/apps/web/src/pendingUserInput.ts +++ b/apps/web/src/pendingUserInput.ts @@ -3,6 +3,8 @@ import type { UserInputQuestion } from "@t3tools/contracts"; export interface PendingUserInputDraftAnswer { selectedOptionValues?: string[]; customAnswer?: string; + attachmentCount?: number; + attachmentsBlocked?: boolean; } export interface PendingUserInputProgress { @@ -41,6 +43,7 @@ export function resolvePendingUserInputAnswer( question: UserInputQuestion, draft: PendingUserInputDraftAnswer | undefined, ): string | string[] | null { + if (draft?.attachmentsBlocked) return null; const customAnswer = question.allowCustomAnswer === false ? null : normalizeDraftAnswer(draft?.customAnswer); if (customAnswer) { @@ -51,10 +54,17 @@ export function resolvePendingUserInputAnswer( (value) => question.options.some((option) => (option.value ?? option.label) === value), ); if (question.multiSelect) { - return selectedOptionValues.length > 0 ? selectedOptionValues : null; + return selectedOptionValues.length > 0 + ? selectedOptionValues + : question.allowCustomAnswer !== false && (draft?.attachmentCount ?? 0) > 0 + ? "" + : null; } - return selectedOptionValues[0] ?? null; + return ( + selectedOptionValues[0] ?? + (question.allowCustomAnswer !== false && (draft?.attachmentCount ?? 0) > 0 ? "" : null) + ); } export function setPendingUserInputCustomAnswer( diff --git a/apps/web/src/questionAttachments.test.ts b/apps/web/src/questionAttachments.test.ts new file mode 100644 index 000000000..46fbe0260 --- /dev/null +++ b/apps/web/src/questionAttachments.test.ts @@ -0,0 +1,100 @@ +import { ApprovalRequestId, EnvironmentId, ThreadId } from "@t3tools/contracts"; +import { beforeEach, expect, it, vi } from "vite-plus/test"; +import { useComposerDraftStore } from "./composerDraftStore"; +import { + questionAttachmentDraftId, + questionAttachmentDraftPrefix, + changeQuestionAttachmentPreparation, + clearQuestionAttachmentDraft, + useQuestionAttachmentPreparation, +} from "./questionAttachments"; + +const release = vi.hoisted(() => vi.fn()); +vi.mock("./lib/attachmentUploadQueue", () => ({ releaseDraftAttachments: release })); +const environmentId = EnvironmentId.make("environment-1"); +const threadId = ThreadId.make("thread-1"); +const requestId = ApprovalRequestId.make("request-1"); +beforeEach(() => { + useComposerDraftStore.setState({ draftsByThreadKey: {}, draftThreadsByThreadKey: {} }); + useQuestionAttachmentPreparation.setState({ counts: {} }); + release.mockClear(); +}); +it("keeps question files separate from the normal draft and other questions", () => { + const revoke = vi.spyOn(URL, "revokeObjectURL"); + const store = useComposerDraftStore.getState(); + const first = questionAttachmentDraftId(environmentId, threadId, requestId, "first"); + const second = questionAttachmentDraftId(environmentId, threadId, requestId, "second"); + store.setPrompt({ environmentId, threadId }, "Unsent prompt"); + const image = { + type: "image" as const, + id: "image-1", + name: "image.png", + mimeType: "image/png", + sizeBytes: 5, + previewUrl: "blob:question-image", + file: new File(["image"], "image.png", { type: "image/png" }), + }; + store.addImages(first, [image]); + store.addFiles(second, [ + { + type: "file", + id: "file-1", + name: "spec.txt", + mimeType: "text/plain", + sizeBytes: 4, + file: new File(["spec"], "spec.txt", { type: "text/plain" }), + }, + ]); + changeQuestionAttachmentPreparation(first, 1); + expect(store.getComposerDraft(first)?.images).toHaveLength(1); + expect(store.getComposerDraft(second)?.files).toHaveLength(1); + clearQuestionAttachmentDraft(first); + expect(store.getComposerDraft(first)).toBeNull(); + expect(store.getComposerDraft(second)?.files).toHaveLength(1); + expect(store.getComposerDraft({ environmentId, threadId })?.prompt).toBe("Unsent prompt"); + expect(useQuestionAttachmentPreparation.getState().counts[first]).toBeUndefined(); + expect(release).toHaveBeenCalledWith([image]); + expect(revoke).toHaveBeenCalledWith(image.previewUrl); + revoke.mockRestore(); +}); +it("scopes provider request ids to their environment and thread", () => { + const keys = [ + questionAttachmentDraftId(environmentId, threadId, requestId, "q"), + questionAttachmentDraftId(EnvironmentId.make("environment-2"), threadId, requestId, "q"), + questionAttachmentDraftId(environmentId, ThreadId.make("thread-2"), requestId, "q"), + ]; + expect(new Set(keys).size).toBe(3); +}); +it("does not match drafts belonging to threads with a shared id prefix", () => { + const prefix = questionAttachmentDraftPrefix(environmentId, threadId); + expect( + questionAttachmentDraftId(environmentId, threadId, requestId, "q").startsWith(prefix), + ).toBe(true); + for (const suffix of ["-extra", ":extra", "/extra", "%extra"]) { + expect( + questionAttachmentDraftId( + environmentId, + ThreadId.make(`${threadId}${suffix}`), + requestId, + "q", + ).startsWith(prefix), + ).toBe(false); + } +}); + +it("does not clear another environment whose id contains a question prefix", () => { + const otherEnvironment = EnvironmentId.make( + `${environmentId}:question-${encodeURIComponent(JSON.stringify(threadId))}-nested`, + ); + const otherKey = questionAttachmentDraftId(otherEnvironment, threadId, requestId, "q"); + const ownKey = questionAttachmentDraftId(environmentId, threadId, requestId, "q"); + const store = useComposerDraftStore.getState(); + store.setPrompt(otherKey, "Keep this answer"); + store.setPrompt(ownKey, "Discard this answer"); + const prefix = questionAttachmentDraftPrefix(environmentId, threadId); + for (const key of [ownKey, otherKey]) { + if (key.startsWith(prefix)) clearQuestionAttachmentDraft(key); + } + expect(store.getComposerDraft(ownKey)).toBeNull(); + expect(store.getComposerDraft(otherKey)?.prompt).toBe("Keep this answer"); +}); diff --git a/apps/web/src/questionAttachments.ts b/apps/web/src/questionAttachments.ts new file mode 100644 index 000000000..92033d05d --- /dev/null +++ b/apps/web/src/questionAttachments.ts @@ -0,0 +1,53 @@ +import type { ApprovalRequestId, EnvironmentId, ThreadId } from "@t3tools/contracts"; +import { create } from "zustand"; +import { DraftId, useComposerDraftStore } from "./composerDraftStore"; +import { releaseDraftAttachments } from "./lib/attachmentUploadQueue"; + +export function questionAttachmentDraftPrefix( + environmentId: EnvironmentId, + threadId: ThreadId, +): string { + return `${encodeURIComponent(JSON.stringify(environmentId))}:question-${encodeURIComponent(JSON.stringify(threadId))}-`; +} + +export function questionAttachmentDraftId( + environmentId: EnvironmentId, + threadId: ThreadId, + requestId: ApprovalRequestId, + questionId: string, +): DraftId { + return DraftId.make( + `${questionAttachmentDraftPrefix(environmentId, threadId)}${encodeURIComponent(JSON.stringify([requestId, questionId]))}`, + ); +} + +export const useQuestionAttachmentPreparation = create<{ counts: Record }>(() => ({ + counts: {}, +})); + +export function changeQuestionAttachmentPreparation(key: DraftId, delta: number): void { + useQuestionAttachmentPreparation.setState((state) => + delta < 0 && !(key in state.counts) + ? state + : { + counts: { ...state.counts, [key]: Math.max(0, (state.counts[key] ?? 0) + delta) }, + }, + ); +} + +export function clearQuestionAttachmentDraft(key: DraftId): void { + const store = useComposerDraftStore.getState(); + const draft = store.getComposerDraft(key); + if (draft) { + releaseDraftAttachments([...draft.images, ...draft.files]); + for (const image of draft.images) { + if (image.previewUrl.startsWith("blob:")) URL.revokeObjectURL(image.previewUrl); + } + } + store.clearComposerContent(key); + useQuestionAttachmentPreparation.setState(({ counts }) => { + const next = { ...counts }; + delete next[key]; + return { counts: next }; + }); +} diff --git a/apps/web/src/session-logic.ts b/apps/web/src/session-logic.ts index 2ecfcc5d7..07d3d68e9 100644 --- a/apps/web/src/session-logic.ts +++ b/apps/web/src/session-logic.ts @@ -2,7 +2,9 @@ import { requestKindFromRequestType, type PendingApproval, } from "@t3tools/client-runtime/pending-requests"; +import { UserInputAttachmentAnswerPayload } from "@t3tools/contracts"; import * as Option from "effect/Option"; +import * as Schema from "effect/Schema"; import * as Arr from "effect/Array"; import { shallow } from "zustand/vanilla/shallow"; import { @@ -98,6 +100,7 @@ export { } from "@t3tools/client-runtime/work-log/presentation"; export interface WorkLogEntry { + questionAnswer?: UserInputAttachmentAnswerPayload; id: string; createdAt: string; turnId?: TurnId | null; @@ -606,6 +609,8 @@ function isPlanBoundaryToolActivity(activity: OrchestrationThreadActivity): bool return typeof payload?.detail === "string" && payload.detail.startsWith("ExitPlanMode:"); } +const decodeQuestionAttachmentAnswer = Schema.decodeUnknownOption(UserInputAttachmentAnswerPayload); + function toDerivedWorkLogEntry(activity: OrchestrationThreadActivity): DerivedWorkLogEntry { const cachedEntry = derivedWorkLogEntryByActivity.get(activity); if (cachedEntry) { @@ -657,6 +662,10 @@ function toDerivedWorkLogEntry(activity: OrchestrationThreadActivity): DerivedWo : activity.tone, sourceActivityKind: activity.kind, }; + if (activity.kind === "user-input.answer-submitted") { + const answer = decodeQuestionAttachmentAnswer(payload); + if (Option.isSome(answer)) entry.questionAnswer = answer.value; + } const itemType = extractWorkLogItemType(payload); const requestKind = extractWorkLogRequestKind(payload); const viewedImagePath = asTrimmedString(asRecord(payload?.data)?.imagePath); diff --git a/docs/README.md b/docs/README.md index 960609d47..e072d664d 100644 --- a/docs/README.md +++ b/docs/README.md @@ -17,6 +17,7 @@ - [Keeping app and server in sync](./user/updating.md) - [Source control integrations](./user/source-control.md) - [Revert a conversation](./user/conversation-rollback.md) +- [Files in question answers](./user/question-attachments.md) - [Background service (Linux)](./user/background-service.md) - Providers: [Codex](./user/providers-codex.md) · [Claude](./user/providers-claude.md) · [OpenCode](./user/providers-opencode.md) · [Prime Agent](./user/providers-prime-agent.md) diff --git a/docs/internals/overview.md b/docs/internals/overview.md index 987a8db48..2438a9ca2 100644 --- a/docs/internals/overview.md +++ b/docs/internals/overview.md @@ -80,6 +80,33 @@ Because persistence and projection share a transaction, the read model cannot du the event log. On dispatch failure the engine rereads persisted events past the starting sequence and reconciles. +### Attachment cleanup cursor + +Attachment files are not event-sourced, so reverts and deletes clean them after commit. Cleanup +records its progress in its own `projection.attachment-cleanup` row in `projection_state`, meaning +every event at or below it has been cleaned (`ProjectionPipeline.ts`): + +- **Live.** The cursor is written in the same statement as the projector cursors, at the last event + whose cleanup finished. Cleanup runs after commit, so it trails the head by the current command. + A failed cleanup stops the cursor until the next bootstrap. A transaction that rolls back never + ran its cleanup, so it leaves no gap. +- **Bootstrap.** After every projector has caught up, so message and `user-input.answer-submitted` + references are current, bootstrap selects only the `thread.reverted` and `thread.deleted` rows + past the cursor, without decoding payloads. It lists the attachments directory once, and the + browser artifacts directory once when a delete is in range (transferred recordings and saved + snapshots live in `browser-artifacts//`, which only deleting the thread removes). Threads + with neither kind of file are skipped; if the artifacts listing fails, each deleted thread's + folder is removed directly instead. It then moves the cursor to the projector head. That pass is + the single retry: a file that still cannot be removed is logged and left behind rather than + holding the cursor. A database without the row starts at the lowest cursor of the projectors + that record cleanup (threads, messages, activities), because their replay cleaned files before + this cursor existed; adding an unrelated projector does not move it. Only a failure to list the + attachments directory or read the log leaves the cursor for the next start. + +`projection_state` can therefore hold rows that are not projectors. The snapshot sequence comes only +from the required projectors (`computeSnapshotSequence` in `ProjectionSnapshotQuery.ts`), and code +reading the table must filter by projector name rather than taking a minimum over every row. + Command and event names live in [`orchestration.ts`][contracts]. Some commands are client dispatchable (`thread.create`, `thread.turn.start`, `thread.approval.respond`); others are internal and produced only by server-side reactors (`thread.message.assistant.delta`, diff --git a/docs/user/question-attachments.md b/docs/user/question-attachments.md new file mode 100644 index 000000000..c0009faab --- /dev/null +++ b/docs/user/question-attachments.md @@ -0,0 +1,9 @@ +# Files in question answers + +When an agent asks a question that accepts a custom answer, use **Attach files** or paste an image into the answer field. On mobile, use the attachment button to choose photos or files. You can send a file by itself, with a selected option, or with a typed answer. + +Each question keeps its own attachments as you move between questions. Your normal prompt draft stays separate. Wait for uploads to finish before submitting; retry or remove any failed upload. A failed response keeps the draft available to try again. + +Files upload to the environment running the thread, including remote environments. The agent receives paths to those saved files with your answer and can open them with its available tools. Questions that only accept predefined choices do not offer attachments, so Antigravity's questions never do. Prime Agent does not ask this kind of question. Older servers need updating before this option appears. + +Submitted attachments remain in the thread history. Reverting a thread removes attachments belonging to discarded answers; deleting the thread uses the same file cleanup as other attachments. You can attach up to eight files across one set of answers, using the same file size limits as the normal composer. diff --git a/packages/contracts/src/environment.ts b/packages/contracts/src/environment.ts index c70ee6e33..915a0197e 100644 --- a/packages/contracts/src/environment.ts +++ b/packages/contracts/src/environment.ts @@ -80,6 +80,8 @@ export const ExecutionEnvironmentCapabilities = Schema.Struct({ connectionProbe: Schema.optionalKey(Schema.Boolean), /** Missing on older servers, which still accept inline image attachments. */ attachmentUploads: Schema.optionalKey(Schema.Boolean), + /** Uploaded files may accompany question answers. */ + questionAttachments: Schema.optionalKey(Schema.Boolean), /** Missing on servers that only accept image attachments. */ fileAttachments: Schema.optionalKey( Schema.Struct({ diff --git a/packages/contracts/src/orchestration.ts b/packages/contracts/src/orchestration.ts index 35a4e9d75..d81b5aa41 100644 --- a/packages/contracts/src/orchestration.ts +++ b/packages/contracts/src/orchestration.ts @@ -246,6 +246,22 @@ export const ChatAttachment = Schema.Union([ ChatUnknownAttachment, ]); export type ChatAttachment = typeof ChatAttachment.Type; + +export const UserInputAttachments = Schema.Record( + Schema.String, + Schema.Array(Schema.Union([ChatImageAttachment, ChatFileAttachment])).pipe( + Schema.check(Schema.isMaxLength(PROVIDER_SEND_TURN_MAX_ATTACHMENTS)), + ), +); +export type UserInputAttachments = typeof UserInputAttachments.Type; + +export const UserInputAttachmentAnswerPayload = Schema.Struct({ + requestId: ApprovalRequestId, + questionTextById: Schema.optional(Schema.Record(Schema.String, Schema.String)), + answers: ProviderUserInputAnswers, + attachmentsByQuestionId: UserInputAttachments, +}); +export type UserInputAttachmentAnswerPayload = typeof UserInputAttachmentAnswerPayload.Type; const UploadChatAttachment = Schema.Union([UploadChatImageAttachment]); export type UploadChatAttachment = typeof UploadChatAttachment.Type; @@ -1299,6 +1315,7 @@ const ThreadUserInputRespondCommand = Schema.Struct({ threadId: ThreadId, requestId: ApprovalRequestId, answers: ProviderUserInputAnswers, + attachmentsByQuestionId: Schema.optional(UserInputAttachments), createdAt: IsoDateTime, }); @@ -1856,6 +1873,7 @@ const ThreadUserInputResponseRequestedPayload = Schema.Struct({ threadId: ThreadId, requestId: ApprovalRequestId, answers: ProviderUserInputAnswers, + attachmentsByQuestionId: Schema.optional(UserInputAttachments), createdAt: IsoDateTime, }); diff --git a/packages/contracts/src/provider.ts b/packages/contracts/src/provider.ts index add2c6bfa..192c8492f 100644 --- a/packages/contracts/src/provider.ts +++ b/packages/contracts/src/provider.ts @@ -23,6 +23,7 @@ import { ProviderRequestKind, ProviderSandboxMode, ProviderUserInputAnswers, + UserInputAttachments, RuntimeMode, } from "./orchestration.ts"; import { ProviderInstanceId, ProviderDriverKind } from "./providerInstance.ts"; @@ -544,6 +545,7 @@ export const ProviderRespondToUserInputInput = Schema.Struct({ threadId: ThreadId, requestId: ApprovalRequestId, answers: ProviderUserInputAnswers, + attachmentsByQuestionId: Schema.optional(UserInputAttachments), }); export type ProviderRespondToUserInputInput = typeof ProviderRespondToUserInputInput.Type; diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 5596e7d8c..b61065f1b 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -466,6 +466,9 @@ importers: '@types/react': specifier: ~19.2.0 version: 19.2.16 + '@types/react-dom': + specifier: ~19.2.3 + version: 19.2.3(@types/react@19.2.16) babel-preset-expo: specifier: ~57.0.9 version: 57.0.9(@babel/core@7.29.7)(@babel/runtime@7.29.7)(expo-widgets@57.0.15)(expo@57.0.18)(react-refresh@0.14.2)