diff --git a/doc/DEVELOPING.md b/doc/DEVELOPING.md index 0c2df84b6fc..27d934c879a 100644 --- a/doc/DEVELOPING.md +++ b/doc/DEVELOPING.md @@ -582,6 +582,50 @@ appears, followed briefly by **Back online**. Recovery does not reload the browser. Authorization failures still require sign-in or an explicit retry. Storybook **App / Connection recovery** shows the startup screens and banner. +Writes keep the user's text through an outage: + +- Comments, new tasks, and board sends store the request and its key + (`clientRequestId` or `idempotencyKey`) before they send + (`ui/src/lib/pending-send.ts`, `ui/src/hooks/useDurableSubmit.ts`). After a + transient failure or a lost response, the composer shows **Sending when + reconnected…** with **Resend now** and **Cancel**, and resends with the same + key on recovery. The server dedupes by that key. Restored board publications + wait for an explicit retry. Restored plain comments and keyed requests resend + after their initial backoff. A combined comment that also reassigns, reopens, + or changes run settings waits for a matching server receipt or Cancel: comment + deduplication alone cannot safely replay those task changes. Cancel returns + the text for a fresh, intentional send. A new task restored without its staged + files is not sent; the dialog asks for the files again. Restored subtasks keep + their saved parent and goal defaults even when reopened from the normal New + Task action. Successful staged-file + uploads are identified by content digest as well as metadata, so distinct files + with identical names, sizes and timestamps are both uploaded. Without browser + hashing support, uploads proceed without receipt-based deduplication. +- Document and inline-editor autosave (`ui/src/hooks/useDurableAutosave.ts`) + keeps unsaved edits in browser storage, shows **Will save when reconnected**, + and saves the newest draft for each task or document on recovery. A `409` + whose server copy equals the draft counts as saved. A temporary failure of + that verification stays pending. Exhausted attempts show a manual retry action. + Admitted edits can finish after route navigation unmounts the editor. That + work waits for recovery events while offline, uses bounded backoff while + online, and verifies the current account before each retry. The latest admitted + save is shared by account and target across editor mounts, so a superseded + detached retry cannot overwrite a newer edit after reopening the same task. + Replayable task-field updates and interaction resolutions bind the original + account, account generation and operation at dispatch. A sign-out or account + change cancels that intent before it can send, including after waiting behind + another write; callbacks cannot apply an old result to the next account. + Stored document drafts and mounted editors belong to the signed-in user. + Storage failure shows a warning to keep the tab open or copy the text. +- Issue field updates go through `useUpdateIssueMutation`: optimistic, rolled + back with readable copy on failure, and replayed on reconnect for absolute + fields. Updates run in order, so an older update that retries cannot + overwrite a newer one. +- Uploads and other non-idempotent requests never resend on their own; they + keep the input and offer **Retry**. + +The banner counts every write that waits for the connection. + To reproduce a deploy blip in a dev or QA build, set the outage simulator in the browser console and remove it to end the outage: @@ -3399,6 +3443,64 @@ which reinstalls dependencies when `node_modules` is missing. Repositories witho a provision command rely on the agent's own install step, as they do for a new worktree. +### Browser write recovery + +Use `useDurableAutosave` for replayable field updates and `useDurableSubmit` for +user submissions that have an idempotent server contract. Bind queued jobs to +the original task and save callback. A task switch must not retarget pending work. +Keep the original creation key when a restored draft needs its attachments again; +attachment selection order does not create a new request. Document and decision +writes with an uncertain response verify the saved result before reporting failure. +Verification reads can themselves need retry. Conflicts and permanent failures +must reach the caller. Once automatic attempts stop, show **Resend now** instead +of promising another automatic attempt. See the hook, document, task creation and +connected-task banner tests for these contracts. + +Inline editors preserve typing through the first owner lookup and remount after +a known draft owner changes. Connected-channel sends use +account-scoped storage and publication queries; retries and receipts retain the +original account generation. Returning to that account restores its saved send +for an explicit same-key retry. Earlier unowned channel receipts are preserved +without revealing or replaying their text. They require inspecting Activity +before a new send because their original sender cannot be established safely. +After checking Activity, a person can explicitly discard the unowned browser +record to start a fresh update. This never replays the old text or cancels an +already accepted publication; storage failures keep the hold in place. + +Task-creation drafts and recovery state are account-scoped. A known account +change remounts the dialog, clears staged in-memory files and stops old creates +before any subsequent upload. An interrupted account-bound request stays pending +under its original key; it is never relabelled as a provider refusal. Legacy +unowned form drafts remain in browser storage but are not restored into another +account. Storybook fixtures seed the same account-specific keys. + +Successful pending-record writes have the same readable size range as browser +storage. In particular, a large task description cannot silently drop its +creation key on reload. Reselected text documents recover their original +file-to-document-key mapping before fingerprint comparison and upload. +Document editors remount on subject changes, and a message attempt settles only +once when a server receipt races its HTTP retry. Startup autosaves bind to the +first resolved account; a later known-account change still cancels old work. +Account-bound status changes and decisions also wait for the first lookup rather +than rejecting an action made while it loads. Waiting work starts a bounded retry +of a failed session lookup; exhaustion reaches the original error callback instead +of leaving the operation pending. Removing that session scope cancels its lookup +work. Account cancellation after an uncertain POST keeps the original request key. +Comment retries check the original +account generation at the click and again immediately before their POST, including +after optimistic cache cancellation or conversation creation. A retained accepted +comment returns its existing receipt before the route can reopen a completed task. +`server/src/services/issue-comment-receipt.ts` completes reference and activity +records for keyed board comments. It locks the same-company task before its +comment, matching task and company deletion, comment removal and tombstoning. It repairs references and +inserts a missing activity record in one +transaction, and publishes after commit. The task uses `FOR NO KEY UPDATE` so +concurrent comments can reference each other without blocking foreign-key checks. Both the initial send and a retained +receipt use this owner so concurrent completion cannot duplicate the activity. +Large comment request records use the same readable range as successful writes, +including JSON-escaped bodies. Account changes retire only the original optimistic +echo and cannot apply its rollback to the next account. + ### Browser realtime connection recovery If a browser cannot construct a WebSocket, the live-update and run transcript diff --git a/maps/agent-access-map/recovery.md b/maps/agent-access-map/recovery.md index 0c8b3758edd..48329cb29db 100644 --- a/maps/agent-access-map/recovery.md +++ b/maps/agent-access-map/recovery.md @@ -514,6 +514,12 @@ Gaps: Legacy/native/channel runtimes separately qualify; preserve original undis Capability ID: `recovery/reconnect`. +Browser autosave and resend preserve the existing actor, task and idempotency +contracts. They grant no new agent access. Retrying a document or decision still +requires its normal authorized operation; an uncertain outcome requires saved-result +verification rather than an unguarded repeat. + + Review: `operation-reviewed`. Browser transport recovery refetches persisted state without implying runner recovery. diff --git a/maps/agent-access-map/tasks.md b/maps/agent-access-map/tasks.md index 167a5d71ebd..859c5304653 100644 --- a/maps/agent-access-map/tasks.md +++ b/maps/agent-access-map/tasks.md @@ -274,7 +274,7 @@ API: `present` — POST /api/issues/{id}/comments; PATCH task with comment. Agent access: `partial` — REST/call_api, CLI issue comment/update--comment; native report_progress plain CURRENT-task body+stable key only. answer_status_question is declared but not production-admitted. -Policy: issue:comment/responsible-user/influence ceilings may allow peer/direct-parent reporting without disposition authority. body nonempty, clientRequestId UUID, maximum 20 unique attachment IDs. Agent onBehalfOfUserId rejected; presentation/metadata board-only; authorType cannot gain board wake. Board paused task returns 409. Board Agent Chat additionally flag/owner/clientRequestId. Pending/running retries preserved by ordinary board comments; explicit control separate. +Policy: issue:comment/responsible-user/influence ceilings may allow peer/direct-parent reporting without disposition authority. body nonempty, clientRequestId UUID, maximum 20 unique attachment IDs. Agent onBehalfOfUserId rejected; presentation/metadata board-only; authorType cannot gain board wake. A new board comment on a paused task returns 409. An ordinary board comment whose same-task, same-user request key was already accepted repairs its reference/activity records and returns that receipt before task mutations; different text conflicts without reopening work. Completion serializes on the same-company task/comment and does not duplicate the activity. Board Agent Chat additionally flag/owner/clientRequestId. Pending/running retries preserved by ordinary board comments; explicit control separate. Evidence: `source-only` — reviewed at `3bbc2104b65caf9f7763938a01260a5673e12f7a`; [issues.ts](../../server/src/routes/issues.ts), [issue.ts](../../packages/shared/src/validators/issue.ts), [paperclip-runner-tool-authority.ts](../../server/src/services/native-runtime/paperclip-runner-tool-authority.ts), [issue.ts](../../cli/src/commands/client/issue.ts), [issue-comment-attribution-audit-routes.test.ts](../../server/src/__tests__/issue-comment-attribution-audit-routes.test.ts), [issue-comment-reopen-routes.test.ts](../../server/src/__tests__/issue-comment-reopen-routes.test.ts), [cross-issue-influence-limit-postgres.test.ts](../../server/src/__tests__/cross-issue-influence-limit-postgres.test.ts). No live execution claimed. diff --git a/maps/feature-map/recovery.md b/maps/feature-map/recovery.md index fa75b68c24d..6969e047fce 100644 --- a/maps/feature-map/recovery.md +++ b/maps/feature-map/recovery.md @@ -58,6 +58,44 @@ Agent access: [companion](../agent-access-map/recovery.md). ## How to get to it (user POV) +For `recovery/reconnect`, also interrupt a write response after the server accepts +it. Keep a title draft bound to its original task while navigating. Restore a +new-task draft with its files selected in another order and require the same +creation key and one task. Verify lost document-create and decision responses, +including an interrupted verification read. Once the bounded retry budget ends, +require a visible **Resend now** action. Use +[durable autosave tests](../../ui/src/hooks/useDurableAutosave.test.tsx) and +[task creation tests](../../ui/src/components/NewIssueDialog.test.tsx) for the +controlled source cases; repeat the real browser journey for acceptance. +Switch accounts while an inline edit is open or a channel send waits to retry: +the next account must see neither private text nor a retry from the first account. +Returning to the original account restores its retained channel request with the +same key for explicit retry. Unowned legacy receipts stay held for Activity +inspection until a person explicitly discards their browser record; discard does +not replay the text or cancel an accepted update. Typing before the first session +lookup survives that initial lookup and is backed up under the resolved owner. Component evidence lives in the +[inline editor tests](../../ui/src/components/InlineEditor.test.tsx) and +[connected-channel tests](../../ui/src/components/chat/ExternallyConnectedTaskBanner.test.tsx); +these controlled session changes do not establish hosted authentication acceptance. +Also switch accounts during a restored task create and while its files are +uploading: later uploads stop, the next account sees a fresh form, and the +original request key remains owned by the first account. Reselect colliding +text-document filenames in reverse order and verify their original keys and +contents. Switch between cached document subjects without copying either draft. +A receipt arriving before an in-flight resend must preserve the next message. +The startup browser journey in +[disconnect recovery](../../tests/e2e/disconnect-write-recovery.spec.ts) holds the +first session response while a title save waits, simulates an offline event with +API failures while keeping static assets reachable, then verifies the real saved +server title after account resolution and reload. The accepted-comment journey +then completes and assigns a task before repeating its saved comment request; +the original receipt must return once, its activity must appear once, and the task must stay Done. Reusing that +key with different text must return a conflict without changing the task. This +uses a disposable process agent and makes no provider call. A separate healthy-API +startup journey fails the session endpoint with JSON 500 responses, then restores +it while an edit waits; the same edit must save and survive reload without a +network reconnect or another user action. + ### `source-task` Open the affected task from Tasks, Inbox/Blocked, search, or a direct task link. diff --git a/server/src/__tests__/issue-comment-receipt-postgres.test.ts b/server/src/__tests__/issue-comment-receipt-postgres.test.ts new file mode 100644 index 00000000000..b3b59afe796 --- /dev/null +++ b/server/src/__tests__/issue-comment-receipt-postgres.test.ts @@ -0,0 +1,232 @@ +import { randomUUID } from "node:crypto"; +import { and, eq, sql } from "drizzle-orm"; +import { afterAll, beforeAll, describe, expect, it, vi } from "vitest"; +import { activityLog, companies, createDb, issueComments, issueReferenceMentions, issues } from "@paperclipai/db"; +import { completeIssueCommentReceipt } from "../services/issue-comment-receipt.js"; +import { issueReferenceService } from "../services/issue-references.js"; +import { issueService } from "../services/issues.js"; +import { deleteCompany } from "../services/company-deletion.js"; +import { getEmbeddedPostgresTestSupport, startEmbeddedPostgresTestDatabase } from "./helpers/embedded-postgres.js"; + +const support = await getEmbeddedPostgresTestSupport(); +const describePostgres = support.supported ? describe : describe.skip; + +describePostgres("accepted comment finalization", () => { + let db: ReturnType; + let database: Awaited>; + beforeAll(async () => { + database = await startEmbeddedPostgresTestDatabase("paperclip-comment-receipt-"); + db = createDb(database.connectionString); + }, 30_000); + afterAll(async () => { + await db?.$client.end({ timeout: 5 }); + await database?.cleanup(); + }); + + it("rolls back partial references and repairs them once across concurrent retries", async () => { + const companyId = randomUUID(), issueId = randomUUID(), targetId = randomUUID(), commentId = randomUUID(); + const prefix = `RCP${companyId.slice(0, 6).toUpperCase()}`; + await db.insert(companies).values({ id: companyId, name: "Comment receipt recovery", issuePrefix: prefix }); + await db.insert(issues).values([ + { id: issueId, companyId, title: "Completed task", status: "done", identifier: `${prefix}-1`, issueNumber: 1 }, + { id: targetId, companyId, title: "Referenced task", identifier: `${prefix}-2`, issueNumber: 2 }, + ]); + await db.insert(issueComments).values({ + id: commentId, companyId, issueId, authorType: "user", authorUserId: "local-board", + clientRequestId: randomUUID(), body: `See ${prefix}-2`, + }); + const refs = issueReferenceService(db); + let fail = true; + const interrupted = { + ...refs, + syncComment: async (...args: Parameters) => { + await refs.syncComment(...args); + if (fail) throw new Error("Reference write interrupted before activity"); + }, + }; + const input = { + companyId, issueId, commentId, + activity: () => ({ companyId, actorType: "user" as const, actorId: "local-board", action: "issue.comment_added", + entityType: "issue", entityId: issueId, details: { commentId } }), + }; + await expect(completeIssueCommentReceipt(db, interrupted, input)).rejects.toThrow("Reference write interrupted"); + expect(await db.select().from(issueReferenceMentions).where(eq(issueReferenceMentions.sourceRecordId, commentId))).toHaveLength(0); + expect(await db.select().from(activityLog).where(eq(activityLog.companyId, companyId))).toHaveLength(0); + fail = false; + await Promise.all(Array.from({ length: 3 }, () => completeIssueCommentReceipt(db, interrupted, input))); + expect(await db.select().from(issueReferenceMentions).where(and( + eq(issueReferenceMentions.sourceRecordId, commentId), eq(issueReferenceMentions.targetIssueId, targetId), + ))).toHaveLength(1); + expect(await db.select().from(activityLog).where(eq(activityLog.companyId, companyId))).toHaveLength(1); + await db.delete(issueReferenceMentions).where(eq(issueReferenceMentions.sourceRecordId, commentId)); + await completeIssueCommentReceipt(db, refs, input); + expect(await db.select().from(issueReferenceMentions).where(eq(issueReferenceMentions.sourceRecordId, commentId))).toHaveLength(1); + expect(await db.select().from(activityLog).where(eq(activityLog.companyId, companyId))).toHaveLength(1); + expect((await db.select().from(issues).where(eq(issues.id, issueId)))[0].status).toBe("done"); + await expect(completeIssueCommentReceipt(db, refs, { ...input, companyId: randomUUID() })).rejects.toMatchObject({ status: 404 }); + expect(await db.select().from(activityLog).where(eq(activityLog.companyId, companyId))).toHaveLength(1); + }, 30_000); + it("completes concurrent cross-task references without a deadlock", async () => { + const companyId = randomUUID(), firstId = randomUUID(), secondId = randomUUID(); + const firstCommentId = randomUUID(), secondCommentId = randomUUID(); + const prefix = `RCP${companyId.slice(0, 6).toUpperCase()}`; + await db.insert(companies).values({ id: companyId, name: "Cross-task comment recovery", issuePrefix: prefix }); + await db.insert(issues).values([ + { id: firstId, companyId, title: "First task", status: "done", identifier: `${prefix}-1`, issueNumber: 1 }, + { id: secondId, companyId, title: "Second task", status: "done", identifier: `${prefix}-2`, issueNumber: 2 }, + ]); + await db.insert(issueComments).values([ + { id: firstCommentId, companyId, issueId: firstId, authorUserId: "local-board", body: `See ${prefix}-2` }, + { id: secondCommentId, companyId, issueId: secondId, authorUserId: "local-board", body: `See ${prefix}-1` }, + ]); + const refs = issueReferenceService(db); + let arrived = 0; + let release!: () => void; + const bothLocked = new Promise(resolve => { release = resolve; }); + const synchronized = { + ...refs, + syncComment: async (...args: Parameters) => { + if (++arrived === 2) release(); + await bothLocked; + await refs.syncComment(...args); + }, + }; + const results = await Promise.allSettled([[firstId, firstCommentId], [secondId, secondCommentId]].map(([issueId, commentId]) => + completeIssueCommentReceipt(db, synchronized, { companyId, issueId, commentId, + activity: () => ({ companyId, actorType: "user", actorId: "local-board", action: "issue.comment_added", + entityType: "issue", entityId: issueId, details: { commentId } }), + }))); + expect(results.map(result => result.status)).toEqual(["fulfilled", "fulfilled"]); + expect(await db.select().from(issueReferenceMentions).where(eq(issueReferenceMentions.companyId, companyId))).toHaveLength(2); + expect(await db.select().from(activityLog).where(eq(activityLog.companyId, companyId))).toHaveLength(2); + expect((await db.select().from(issues).where(eq(issues.companyId, companyId))).every(issue => issue.status === "done")).toBe(true); + }, 30_000); + + it.each(["remove", "tombstone"] as const)("serializes completion with a real %s without a deadlock", async (kind) => { + const companyId = randomUUID(), issueId = randomUUID(), commentId = randomUUID(); + await db.insert(companies).values({ id: companyId, name: "Comment deletion race", issuePrefix: `RCP${companyId.slice(0, 6).toUpperCase()}` }); + await db.insert(issues).values({ id: issueId, companyId, title: "Completed task", status: "done" }); + await db.insert(issueComments).values({ id: commentId, companyId, issueId, authorUserId: "local-board", body: "Saved comment" }); + await issueService(db).removeComment(randomUUID()); + const monitored = (started: (pid: number) => void) => new Proxy(db, { + get(target, property) { + if (property === "transaction") return (callback: (tx: any) => Promise) => target.transaction(async tx => { + const [row] = await tx.execute(sql`select pg_backend_pid() as pid`); + started(Number(row.pid)); + return callback(tx); + }); + const value = Reflect.get(target, property, target); + return typeof value === "function" ? value.bind(target) : value; + }, + }); + const waitForLock = async (pid: number) => vi.waitFor(async () => { + const [state] = await db.execute(sql`select wait_event_type from pg_stat_activity where pid=${pid}`); + expect(state?.wait_event_type).toBe("Lock"); + }, { timeout: 5_000, interval: 10 }); + let release!: () => void, held!: () => void; + const gate = new Promise(resolve => { release = resolve; }); + const ready = new Promise(resolve => { held = resolve; }); + const holding = db.transaction(async tx => { + await tx.select().from(issueComments).where(eq(issueComments.id, commentId)).for("update"); + held(); + await gate; + }); + const operations: Promise[] = [holding]; + try { + await ready; + let deleteStarted!: (pid: number) => void; + const deletePid = new Promise(resolve => { deleteStarted = resolve; }); + const deletingService = issueService(monitored(deleteStarted)); + const deleting = (kind === "remove" ? deletingService.removeComment(commentId) + : deletingService.tombstoneComment(commentId, { actorType: "user", userId: "local-board" })).catch(error => error); + operations.push(deleting); + await waitForLock(await deletePid); + let completeStarted!: (pid: number) => void; + const completePid = new Promise(resolve => { completeStarted = resolve; }); + const completing = completeIssueCommentReceipt(monitored(completeStarted), issueReferenceService(db), { + companyId, issueId, commentId, + activity: () => ({ companyId, actorType: "user", actorId: "local-board", action: "issue.comment_added", + entityType: "issue", entityId: issueId, details: { commentId } }), + }).catch(error => error); + operations.push(completing); + await waitForLock(await completePid); + release(); + await holding; + expect(await deleting).toMatchObject({ id: commentId }); + expect(await completing).toMatchObject({ status: 404 }); + expect(await db.select().from(activityLog).where(eq(activityLog.companyId, companyId))).toHaveLength(0); + expect((await db.select().from(issues).where(eq(issues.id, issueId)))[0].status).toBe("done"); + } finally { + release(); + await Promise.allSettled(operations); + } + }, 30_000); + + it.each(["task", "company"] as const)("serializes receipt completion with actual %s deletion", async kind => { + const companyId = randomUUID(), issueId = randomUUID(), commentId = randomUUID(); + await db.insert(companies).values({ id: companyId, name: "Task deletion race", issuePrefix: `RCP${companyId.slice(0, 6).toUpperCase()}` }); + await db.insert(issues).values({ id: issueId, companyId, title: "Completed task", status: "done" }); + await db.insert(issueComments).values({ id: commentId, companyId, issueId, authorUserId: "local-board", body: "Saved comment" }); + let release!: () => void, held!: () => void, receiptStarted!: (pid: number) => void; + const gate = new Promise(resolve => { release = resolve; }); + const ready = new Promise(resolve => { held = resolve; }); + const receiptPid = new Promise(resolve => { receiptStarted = resolve; }); + const wrap = (deleting: boolean) => new Proxy(db, { + get(target, property) { + if (property === "transaction") return (callback: (tx: any) => Promise) => target.transaction(async tx => { + if (!deleting) { + const [row] = await tx.execute(sql`select pg_backend_pid() as pid`); + receiptStarted(Number(row.pid)); + } + return callback(new Proxy(tx, { + get(transaction, method) { + // Delay the real task DELETE, preserving each service's normal + // earlier locks (including company deletion's child-row locks). + if (deleting && method === "delete") return (table: unknown) => { + const builder = transaction.delete(table as typeof issues); + if (table !== issues) return builder; + return { where: (...args: Parameters) => { + const query = builder.where(...args); + const wait = async () => { held(); await gate; }; + return kind === "task" + ? { returning: async () => { await wait(); return query.returning(); } } + : (async () => { await wait(); return query; })(); + } }; + }; + const value = Reflect.get(transaction, method, transaction); + return typeof value === "function" ? value.bind(transaction) : value; + }, + })); + }); + const value = Reflect.get(target, property, target); + return typeof value === "function" ? value.bind(target) : value; + }, + }); + const deleting = (kind === "task" ? issueService(wrap(true)).remove(issueId) + : deleteCompany(wrap(true), companyId)).catch(error => error); + let completing: Promise | undefined; + try { + await ready; + completing = completeIssueCommentReceipt(wrap(false), issueReferenceService(db), { + companyId, issueId, commentId, + activity: () => ({ companyId, actorType: "user", actorId: "local-board", action: "issue.comment_added", + entityType: "issue", entityId: issueId, details: { commentId } }), + }).catch(error => error); + const pid = await receiptPid; + await vi.waitFor(async () => { + const [state] = await db.execute(sql`select wait_event_type from pg_stat_activity where pid=${pid}`); + expect(state?.wait_event_type).toBe("Lock"); + }, { timeout: 5_000, interval: 10 }); + release(); + expect(await deleting).toMatchObject({ id: kind === "task" ? issueId : companyId }); + expect(await completing).toMatchObject({ status: 404 }); + expect(await db.select().from(issues).where(eq(issues.id, issueId))).toHaveLength(0); + expect(await db.select().from(issueComments).where(eq(issueComments.id, commentId))).toHaveLength(0); + expect(await db.select().from(activityLog).where(eq(activityLog.companyId, companyId))).toHaveLength(0); + if (kind === "company") expect(await db.select().from(companies).where(eq(companies.id, companyId))).toHaveLength(0); + } finally { + release(); + await Promise.allSettled([deleting, completing]); + } + }, 30_000); +}); diff --git a/server/src/__tests__/issue-comment-reopen-routes.test.ts b/server/src/__tests__/issue-comment-reopen-routes.test.ts index 28c6900a190..ffcfad3ca2e 100644 --- a/server/src/__tests__/issue-comment-reopen-routes.test.ts +++ b/server/src/__tests__/issue-comment-reopen-routes.test.ts @@ -49,17 +49,20 @@ const mockAgentService = vi.hoisted(() => ({ })); const mockLogActivity = vi.hoisted(() => vi.fn(async () => undefined)); +const mockSyncCommentReferences = vi.hoisted(() => vi.fn(async () => undefined)); const mockTxInsertValues = vi.hoisted(() => vi.fn(async () => undefined)); const mockTxInsert = vi.hoisted(() => vi.fn(() => ({ values: mockTxInsertValues })), ); const mockTx = vi.hoisted(() => ({ + select: () => mockDbSelect(), insert: mockTxInsert, })); const mockDbSelectOrderBy = vi.hoisted(() => vi.fn(async () => [])); const mockDbSelectWhere = vi.hoisted(() => vi.fn(() => ({ orderBy: mockDbSelectOrderBy, + for: async () => [{ id: "comment-1" }], then: ( onFulfilled: (rows: unknown[]) => unknown, onRejected?: (reason: unknown) => unknown, @@ -203,7 +206,7 @@ vi.mock("../services/index.js", () => ({ }), emptySummary: () => ({ outbound: [], inbound: [] }), listIssueReferenceSummary: async () => ({ outbound: [], inbound: [] }), - syncComment: async () => undefined, + syncComment: mockSyncCommentReferences, syncDocument: async () => undefined, syncIssue: async () => undefined, }), @@ -397,6 +400,7 @@ describe("issue comment reopen routes", () => { mockAgentService.list.mockReset(); mockAgentService.resolveByReference.mockReset(); mockLogActivity.mockReset(); + mockSyncCommentReferences.mockReset().mockResolvedValue(undefined); mockFeedbackService.listIssueVotesForUser.mockReset(); mockFeedbackService.saveIssueVote.mockReset(); mockInstanceSettingsService.get.mockReset(); @@ -422,6 +426,7 @@ describe("issue comment reopen routes", () => { mockDbSelectOrderBy.mockResolvedValue([]); mockDbSelectWhere.mockImplementation(() => ({ orderBy: mockDbSelectOrderBy, + for: async () => [{ id: "comment-1" }], then: ( onFulfilled: (rows: unknown[]) => unknown, onRejected?: (reason: unknown) => unknown, @@ -573,6 +578,41 @@ describe("issue comment reopen routes", () => { ); }); + it("finishes missing reference and activity writes for an accepted comment", async () => { + const issue = makeIssue("done"); + mockIssueService.getById.mockResolvedValue(issue); + const selectFrom = mockDbSelectFrom.getMockImplementation()!; + mockDbSelectFrom.mockImplementation((table) => getTableName(table as Parameters[0]) === "issue_comments" + ? { where: vi.fn(() => Object.assign(Promise.resolve([{ id: "comment-1" }]), { for: async () => [{ id: "comment-1" }] })) } as any : selectFrom(table)); + const response = await request(await installActor(createApp())).post(`/api/issues/${issue.id}/comments`) + .send({ body: "hello", clientRequestId: "77777777-7777-4777-8777-777777777777" }); + expect(response.status).toBe(201); + expect(mockSyncCommentReferences).toHaveBeenCalledWith("comment-1", expect.anything()); + expect(mockLogActivity).toHaveBeenCalledWith(expect.anything(), expect.objectContaining({ + action: "issue.comment_added", details: expect.objectContaining({ commentId: "comment-1" }), + }), expect.anything()); + expect(mockIssueService.update).not.toHaveBeenCalled(); + expect(mockHeartbeatService.wakeup).not.toHaveBeenCalled(); + }); + + it.each([false, true])("returns a retained comment before reopening completed work (changed body=%s)", async (changed) => { + const issue = makeIssue("done"); + const requestId = "77777777-7777-4777-8777-777777777777"; + mockIssueService.getById.mockResolvedValue(issue); + const selectFrom = mockDbSelectFrom.getMockImplementation()!; + mockDbSelectFrom.mockImplementation((table) => getTableName(table as Parameters[0]) === "issue_comments" + ? { where: vi.fn(() => Object.assign(Promise.resolve([{ id: "comment-1" }]), { for: async () => [{ id: "comment-1" }] })) } as any : getTableName(table as Parameters[0]) === "activity_log" + ? { where: async () => [{ id: "activity-1" }] } as any : selectFrom(table)); + if (changed) mockIssueService.addComment.mockRejectedValue(new HttpError(409, "Message request ID was already used for different content")); + const response = await request(await installActor(createApp())).post(`/api/issues/${issue.id}/comments`) + .send({ body: changed ? "Changed body" : "hello", clientRequestId: requestId }); + expect(response.status).toBe(changed ? 409 : 201); + if (!changed) expect(response.body.id).toBe("comment-1"); + expect(mockIssueService.update).not.toHaveBeenCalled(); + expect(mockLogActivity).not.toHaveBeenCalled(); + expect(mockHeartbeatService.wakeup).not.toHaveBeenCalled(); + }); + it("treats reopen=true as a no-op when the issue is already open", async () => { mockIssueService.getById.mockResolvedValue(makeIssue("todo")); mockIssueService.update.mockImplementation( diff --git a/server/src/__tests__/issue-update-comment-wakeup-routes.test.ts b/server/src/__tests__/issue-update-comment-wakeup-routes.test.ts index 842159d91bf..81108a04abf 100644 --- a/server/src/__tests__/issue-update-comment-wakeup-routes.test.ts +++ b/server/src/__tests__/issue-update-comment-wakeup-routes.test.ts @@ -2,6 +2,11 @@ import express from "express"; import request from "supertest"; import { beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; +vi.mock("../services/activity-log.js", () => ({ + logActivity: vi.fn(async () => undefined), + publishActivity: vi.fn(), +})); + vi.mock("../services/fast-responses.js", () => ({ enqueueFastResponse: vi.fn(async () => undefined), })); @@ -222,7 +227,7 @@ async function loadAppModules() { } async function createApp(transaction: (callback: (tx: Record) => Promise) => Promise = - async (callback) => callback({})) { + async (callback) => callback({ select: () => ({ from: () => ({ where: () => Object.assign(Promise.resolve([]), { for: async () => [{ id: "comment-3" }] }) }) }) } as any)) { const [{ errorHandler }, { issueRoutes }] = await loadAppModules(); const app = express(); app.use(express.json()); @@ -239,6 +244,7 @@ async function createApp(transaction: (callback: (tx: Record) => }); app.use("/api", issueRoutes({ transaction, + select: () => ({ from: () => ({ where: async () => [] }) }), } as any, {} as any)); app.use(errorHandler); return app; diff --git a/server/src/routes/issues.ts b/server/src/routes/issues.ts index 7cf93d0e9f2..55649ef9f81 100644 --- a/server/src/routes/issues.ts +++ b/server/src/routes/issues.ts @@ -1,3 +1,4 @@ +import { completeIssueCommentReceipt } from "../services/issue-comment-receipt.js"; import { taskInterruptionPresentation, taskInterruptionPresentations } from "../services/task-interruption-presentation.js"; import { parseObject } from "../adapters/utils.js"; import { prepareRestrictedWorkspaceRetry } from "../services/restricted-workspace-retry.js"; @@ -18468,7 +18469,6 @@ export function issueRoutes( issue, ); if (!commentAccessDecision) return; - if (!(await assertBoardCommentNotPaused(req, res, issue))) return; const commentAuthorizationReason = issueWriteAuthorizationReason( req, commentAccessDecision, @@ -18480,6 +18480,42 @@ export function issueRoutes( }) ) return; + if (req.actor.type === "board" && req.actor.userId && req.body.clientRequestId) { + const [accepted] = await db.select({ id: issueComments.id }).from(issueComments).where(and( + eq(issueComments.companyId, issue.companyId), eq(issueComments.issueId, issue.id), + eq(issueComments.authorUserId, req.actor.userId), eq(issueComments.clientRequestId, req.body.clientRequestId), + )); + if (accepted) { + const receipt = await svc.addComment(issue.id, req.body.body, { userId: req.actor.userId }, { + clientRequestId: req.body.clientRequestId, attachmentIds: req.body.attachmentIds, + authorType: "user", + }); + const userId = req.actor.userId; + await completeIssueCommentReceipt(db, issueReferencesSvc, { + companyId: issue.companyId, issueId: issue.id, commentId: receipt.id, + activity: diff => ({ + companyId: issue.companyId, actorType: "user", actorId: userId, + runId: receipt.createdByRunId, + responsibleUserIdOverride: receipt.onBehalfOfUserId ?? userId, + action: "issue.comment_added", entityType: "issue", entityId: issue.id, + details: { + commentId: receipt.id, bodySnippet: receipt.body.slice(0, 120), + identifier: issue.identifier, issueTitle: issue.title, + authorizationReason: commentAuthorizationReason, source: "comment_receipt_recovery", + ...summarizeIssueReferenceActivityDetails({ + addedReferencedIssues: diff.addedReferencedIssues.map(summarizeIssueRelationForActivity), + removedReferencedIssues: diff.removedReferencedIssues.map(summarizeIssueRelationForActivity), + currentReferencedIssues: diff.currentReferencedIssues.map(summarizeIssueRelationForActivity), + }), + }, + }), + }); + await externalObjectsSvc.syncCommentSafely(receipt.id); + res.status(201).json(receipt); + return; + } + } + if (!(await assertBoardCommentNotPaused(req, res, issue))) return; const closedExecutionWorkspace = await getClosedIssueExecutionWorkspace(issue); @@ -18969,72 +19005,8 @@ export function issueRoutes( .catch(() => logger.warn({ issueId: currentIssue.id }, "fast response admission unavailable")); } - await issueReferencesSvc.syncComment(comment.id); - await externalObjectsSvc.syncCommentSafely(comment.id); - if ( - currentIssue.assigneeAgentId && - // Slack-linked messages need the normal attributed wake boundary so - // the accepted requester and return-thread receipt travel with the run. - !currentIssue.externalConversationState && - !( - actor.actorType === "agent" && - actor.actorId === currentIssue.assigneeAgentId - ) - ) { - const goalProjection = await runnerGoals.projection( - currentIssue.companyId, - currentIssue.id, - currentIssue.assigneeAgentId, - ); - if ( - goalProjection?.goal?.status === "active" && - goalProjection.workingNow - ) { - const steer = queueLiveRunnerPrpCommand({ - companyId: currentIssue.companyId, - issueId: currentIssue.id, - agentId: currentIssue.assigneeAgentId, - type: "turn.steer", - payload: { text: comment.body }, - commandId: `goal_comment_${comment.id}`, - }); - if (steer) { - try { - await steer.completion; - goalCommentSteered = true; - } catch (err) { - logger.warn( - { - err, - issueId: currentIssue.id, - commentId: comment.id, - runId: steer.runId, - }, - "failed to steer an active session goal; falling back to a boundary wake", - ); - } - } - } - } - const commentReferenceSummaryAfter = - await issueReferencesSvc.listIssueReferenceSummary(currentIssue.id); - const commentReferenceDiff = issueReferencesSvc.diffIssueReferenceSummary( - commentReferenceSummaryBefore, - commentReferenceSummaryAfter, - ); - - if (actor.runId) { - await heartbeat - .reportRunActivity(actor.runId) - .catch((err) => - logger.warn( - { err, runId: actor.runId }, - "failed to clear detached run warning after issue comment", - ), - ); - } - - await logActivity(db, { + const keyedComment = req.actor.type === "board" && Boolean(req.actor.userId && req.body.clientRequestId); + const buildCommentActivity = (commentReferenceDiff: ReturnType) => ({ companyId: currentIssue.companyId, actorType: actor.actorType, actorId: actor.actorId, @@ -19091,6 +19063,78 @@ export function issueRoutes( }), }, }); + if (keyedComment) await completeIssueCommentReceipt(db, issueReferencesSvc, { + companyId: currentIssue.companyId, issueId: currentIssue.id, commentId: comment.id, + referenceSummaryBefore: commentReferenceSummaryBefore, activity: buildCommentActivity, + }); + else await issueReferencesSvc.syncComment(comment.id); + await externalObjectsSvc.syncCommentSafely(comment.id); + if ( + currentIssue.assigneeAgentId && + // Slack-linked messages need the normal attributed wake boundary so + // the accepted requester and return-thread receipt travel with the run. + !currentIssue.externalConversationState && + !( + actor.actorType === "agent" && + actor.actorId === currentIssue.assigneeAgentId + ) + ) { + const goalProjection = await runnerGoals.projection( + currentIssue.companyId, + currentIssue.id, + currentIssue.assigneeAgentId, + ); + if ( + goalProjection?.goal?.status === "active" && + goalProjection.workingNow + ) { + const steer = queueLiveRunnerPrpCommand({ + companyId: currentIssue.companyId, + issueId: currentIssue.id, + agentId: currentIssue.assigneeAgentId, + type: "turn.steer", + payload: { text: comment.body }, + commandId: `goal_comment_${comment.id}`, + }); + if (steer) { + try { + await steer.completion; + goalCommentSteered = true; + } catch (err) { + logger.warn( + { + err, + issueId: currentIssue.id, + commentId: comment.id, + runId: steer.runId, + }, + "failed to steer an active session goal; falling back to a boundary wake", + ); + } + } + } + } + + if (actor.runId) { + await heartbeat + .reportRunActivity(actor.runId) + .catch((err) => + logger.warn( + { err, runId: actor.runId }, + "failed to clear detached run warning after issue comment", + ), + ); + } + + if (!keyedComment) { + const commentReferenceSummaryAfter = + await issueReferencesSvc.listIssueReferenceSummary(currentIssue.id); + const commentReferenceDiff = issueReferencesSvc.diffIssueReferenceSummary( + commentReferenceSummaryBefore, + commentReferenceSummaryAfter, + ); + await logActivity(db, buildCommentActivity(commentReferenceDiff)); + } const expiredInteractions = await issueThreadInteractionService( db, diff --git a/server/src/services/company-deletion.ts b/server/src/services/company-deletion.ts index 10c01ed86e0..819f2745abb 100644 --- a/server/src/services/company-deletion.ts +++ b/server/src/services/company-deletion.ts @@ -100,6 +100,9 @@ export async function deleteCompany(db: Db, id: string) { await tx.delete(agentWakeupRequests).where(eq(agentWakeupRequests.companyId, id)); await tx.delete(agentApiKeys).where(eq(agentApiKeys.companyId, id)); await tx.delete(agentRuntimeState).where(eq(agentRuntimeState.companyId, id)); + // Match receipt completion and comment deletion: task before comment. + await tx.select({ id: issues.id }).from(issues) + .where(eq(issues.companyId, id)).orderBy(issues.id).for("no key update"); await tx.delete(issueComments).where(eq(issueComments.companyId, id)); await tx.delete(approvalComments).where(eq(approvalComments.companyId, id)); await notifyCompanyRetentionWork(tx, id); diff --git a/server/src/services/issue-comment-receipt.ts b/server/src/services/issue-comment-receipt.ts new file mode 100644 index 00000000000..416dc596a82 --- /dev/null +++ b/server/src/services/issue-comment-receipt.ts @@ -0,0 +1,52 @@ +import { and, eq, sql } from "drizzle-orm"; +import { activityLog, issueComments, issues, type Db } from "@paperclipai/db"; +import type { IssueRelatedWorkSummary } from "@paperclipai/shared"; +import { notFound } from "../errors.js"; +import { logActivity, publishActivity, type ActivityPublication, type LogActivityInput } from "./activity-log.js"; +import type { issueReferenceService } from "./issue-references.js"; + +type CommentReferences = Pick, + "syncComment" | "listIssueReferenceSummary" | "diffIssueReferenceSummary">; +type ReferenceDiff = ReturnType; + +export async function completeIssueCommentReceipt(db: Db, references: CommentReferences, input: { + companyId: string; + issueId: string; + commentId: string; + referenceSummaryBefore?: IssueRelatedWorkSummary; + activity: (diff: ReferenceDiff) => LogActivityInput; +}) { + const publications: ActivityPublication[] = []; + await db.transaction(async tx => { + const connection = tx as unknown as Db; + // Match task deletion and comment mutation: lock the task before its + // comment. NO KEY UPDATE still permits references from other tasks. + const [issue] = await tx.select({ id: issues.id }).from(issues).where(and( + eq(issues.id, input.issueId), eq(issues.companyId, input.companyId), + )).for("no key update"); + if (!issue) throw notFound("Issue not found"); + const [comment] = await tx.select({ id: issueComments.id, deletedAt: issueComments.deletedAt }).from(issueComments).where(and( + eq(issueComments.id, input.commentId), eq(issueComments.companyId, input.companyId), + eq(issueComments.issueId, input.issueId), + )).for("update"); + if (!comment || comment.deletedAt) throw notFound("Issue comment not found"); + const before = input.referenceSummaryBefore ?? await references.listIssueReferenceSummary(input.issueId, connection); + await references.syncComment(input.commentId, connection); + const after = await references.listIssueReferenceSummary(input.issueId, connection); + const diff = references.diffIssueReferenceSummary(before, after); + const [existing] = await tx.select({ id: activityLog.id }).from(activityLog).where(and( + eq(activityLog.companyId, input.companyId), eq(activityLog.entityType, "issue"), + eq(activityLog.entityId, input.issueId), eq(activityLog.action, "issue.comment_added"), + sql`${activityLog.details}->>'commentId' = ${input.commentId}`, + )); + if (!existing) { + const activity = input.activity(diff); + await logActivity(connection, { + ...activity, companyId: input.companyId, action: "issue.comment_added", + entityType: "issue", entityId: input.issueId, + details: { ...activity.details, commentId: input.commentId }, + }, publications); + } + }); + for (const publication of publications) publishActivity(publication); +} diff --git a/server/src/services/issues.ts b/server/src/services/issues.ts index e36f1d20a28..aa550c775a3 100644 --- a/server/src/services/issues.ts +++ b/server/src/services/issues.ts @@ -6750,6 +6750,17 @@ export async function readIssueCommentRunLogText(run: { return content; } +/** Share task-before-comment lock order with receipt completion and task deletion. */ +async function lockIssueForCommentMutation(tx: DbTransaction, commentId: string) { + const [comment] = await tx.select({ issueId: issueComments.issueId, companyId: issueComments.companyId }) + .from(issueComments).where(eq(issueComments.id, commentId)); + if (!comment) return false; + const [issue] = await tx.select({ id: issues.id }).from(issues).where(and( + eq(issues.id, comment.issueId), eq(issues.companyId, comment.companyId), + )).for("no key update"); + return Boolean(issue); +} + export function issueService(db: Db) { const instanceSettings = instanceSettingsService(db); const treeControlSvc = issueTreeControlService(db); @@ -12569,6 +12580,7 @@ export function issueService(db: Db) { }; return db.transaction(async (tx) => { + if (!(await lockIssueForCommentMutation(tx, commentId))) return null; const [comment] = await tx .delete(issueComments) .where(eq(issueComments.id, commentId)) @@ -12602,6 +12614,7 @@ export function issueService(db: Db) { }; return db.transaction(async (tx) => { + if (!(await lockIssueForCommentMutation(tx, commentId))) return null; const now = new Date(); const [comment] = await tx .update(issueComments) diff --git a/tests/e2e/chat-adapters-ui-messaging.spec.ts b/tests/e2e/chat-adapters-ui-messaging.spec.ts index 17208596a08..cb752e0c066 100644 --- a/tests/e2e/chat-adapters-ui-messaging.spec.ts +++ b/tests/e2e/chat-adapters-ui-messaging.spec.ts @@ -580,6 +580,12 @@ test.describe("Board send delivery refresh", () => { const sends: Record[] = []; let attachmentId = ""; let privateComment: { id: string } | undefined; + // The lost send resends on its own; hold that resend until the waiting + // state has been checked. + let releaseResend!: () => void; + const resendReleased = new Promise((resolve) => { + releaseResend = resolve; + }); await page.route( `**/api/chat-endpoints/${endpointId}/conversations/${conversationId}/publications`, async (route) => { @@ -598,7 +604,8 @@ test.describe("Board send delivery refresh", () => { ); return route.abort("connectionreset"); } - if (sends.length === 2) + if (sends.length === 2) { + await resendReleased; return fulfill( route, { @@ -613,6 +620,7 @@ test.describe("Board send delivery refresh", () => { }, 409, ); + } return fulfill( route, { @@ -653,15 +661,15 @@ test.describe("Board send delivery refresh", () => { .getByRole("button", { name: "Send to channel", exact: true }) .last() .click(); + // A lost response waits and resends on its own with the same identity. await expect( - banner.getByText("Delivery result not confirmed", { exact: true }), + banner.getByText("Sending when reconnected…", { exact: true }), ).toBeVisible(); await expect( banner.getByRole("textbox", { name: "Board update" }), ).toBeDisabled(); - await banner - .getByRole("button", { name: "Retry safely", exact: true }) - .click(); + await expect.poll(() => sends.length).toBe(2); + releaseResend(); await expect( banner.getByText("Update was not sent", { exact: true }), ).toBeVisible(); @@ -735,7 +743,11 @@ test.describe("Board send delivery refresh", () => { const statusReads: string[] = []; let terminal = false; const publicationsPath = `/api/chat-endpoints/${endpointId}/conversations/${conversationId}/publications`; - const storageKey = `paperclip:board-send:v1:${JSON.stringify([seed.companyId, issue.id, endpointId, conversationId])}`; + const session = await json<{ user: { id: string } }>( + await request.get("/api/auth/get-session"), + "read receipt owner", + ); + const storageKey = `paperclip:board-send:v2:${JSON.stringify([session.user.id, seed.companyId, issue.id, endpointId, conversationId])}`; await page.route("**/api/instance/settings/experimental", (route) => fulfill(route, { enableChatConnectors: true }), ); @@ -1028,6 +1040,10 @@ test.describe("Board send delivery refresh", () => { let canonicalAttachmentReadsAfterSend = 0; let canonicalCommentReadsAfterSend = 0; let status = "streaming"; + let releaseResend!: () => void; + const resendReleased = new Promise((resolve) => { + releaseResend = resolve; + }); await page.route("**/api/instance/settings/experimental", (route) => fulfill(route, { enableChatConnectors: true }), ); @@ -1132,6 +1148,13 @@ test.describe("Board send delivery refresh", () => { await route.abort("failed"); return; } + if (outcome === "response_lost" && sends === 2) { + // The automatic resend is still in flight when the page reloads; + // the browser drops it. + await resendReleased; + await route.abort("failed").catch(() => {}); + return; + } await fulfill( route, { id: publicationId, state: "streaming", attempts: 1 }, @@ -1182,21 +1205,30 @@ test.describe("Board send delivery refresh", () => { ).toHaveCount(0); }; if (outcome === "response_lost") { + // The lost send resends on its own with the same identity. + await expect( + page.getByText("Sending when reconnected…", { exact: true }), + ).toBeVisible(); + await expect.poll(() => submittedPayloads.length).toBe(2); + expect(submittedPayloads[1]).toEqual(submittedPayloads[0]); + // A send restored after reload has an unknown outcome and waits for + // an explicit, same-identity retry. + await page.reload(); + releaseResend(); await expect( page.getByText("Delivery result not confirmed", { exact: true }), ).toBeVisible(); - await page.reload(); await expect( page.getByRole("textbox", { name: "Board update", exact: true }), ).toBeDisabled(); await expectRetainedFiles(); - expect(sends).toBe(1); + expect(sends).toBe(2); expect(reads).toBe(0); await page .getByRole("button", { name: "Retry safely", exact: true }) .click(); - await expect.poll(() => submittedPayloads.length).toBe(2); - expect(submittedPayloads[1]).toEqual(submittedPayloads[0]); + await expect.poll(() => submittedPayloads.length).toBe(3); + expect(submittedPayloads[2]).toEqual(submittedPayloads[0]); } else { await expect( page.getByText("Publishing to channel", { exact: true }).first(), @@ -1258,7 +1290,10 @@ test.describe("Board send delivery refresh", () => { page.getByRole("textbox", { name: "Board update", exact: true }), ).toHaveCount(0, { timeout: 8_000 }); } - expect(sends).toBe(outcome === "response_lost" ? 2 : 1); + // response_lost: the lost send, its automatic resend, and the explicit + // retry after reload, all with one identity. + const expectedSends = outcome === "response_lost" ? 3 : 1; + expect(sends).toBe(expectedSends); expect(canonicalCommentReadsAfterSend).toBeGreaterThan(0); expect(canonicalAttachmentReadsAfterSend).toBeGreaterThan(0); await expect( @@ -1280,7 +1315,7 @@ test.describe("Board send delivery refresh", () => { await expect( newFiles.getByRole("checkbox", { name: "internal-only.txt" }), ).toBeEnabled(); - expect(sends).toBe(outcome === "response_lost" ? 2 : 1); + expect(sends).toBe(expectedSends); }); } }); diff --git a/tests/e2e/disconnect-write-recovery.spec.ts b/tests/e2e/disconnect-write-recovery.spec.ts new file mode 100644 index 00000000000..e533361e775 --- /dev/null +++ b/tests/e2e/disconnect-write-recovery.spec.ts @@ -0,0 +1,206 @@ +import { test, expect, type APIResponse } from "@playwright/test"; + +async function json(response: APIResponse) { + const body = await response.text(); + expect(response.ok(), `${response.url()}: ${response.status()} ${body}`).toBe(true); + return JSON.parse(body); +} + +test("a task title survives an outage and saves after server recovery", async ({ page, request }) => { + const company = await json(await request.post("/api/companies", { data: { name: `Write recovery ${Date.now()}` } })); + try { + const issue = await json(await request.post(`/api/companies/${company.id}/issues`, { + data: { title: "Original task title", status: "backlog" }, + })); + const other = await json(await request.post(`/api/companies/${company.id}/issues`, { + data: { title: "Another task stays unchanged", status: "backlog" }, + })); + await page.goto(`/${company.issuePrefix}/issues/${issue.identifier}`); + const header = page.getByTestId("issue-detail-header"); + await expect(header.getByRole("heading", { name: issue.title, exact: true })).toBeVisible(); + + let unavailable = true; + let failedWrites = 0; + await page.route("**/api/**", async route => { + if (!unavailable) return route.continue(); + if (route.request().method() === "PATCH" && [`/api/issues/${issue.id}`, `/api/issues/${issue.identifier}`].includes(new URL(route.request().url()).pathname)) { + failedWrites += 1; + } + if (!failedWrites) return route.continue(); + await route.fulfill({ status: 503, contentType: "application/json", body: JSON.stringify({ error: "tenant_app_unavailable" }) }); + }); + await header.getByRole("heading", { name: issue.title, exact: true }).click(); + const title = header.locator("textarea"); + await title.fill("A title retained through the outage"); + await title.press("Enter"); + await expect.poll(() => failedWrites).toBeGreaterThan(0); + await expect(page.locator('[data-connection-status="reconnecting"]')).toBeVisible(); + expect((await json(await request.get(`/api/issues/${issue.id}`))).title).toBe(issue.title); + await expect(header).toBeVisible(); + + unavailable = false; + await expect.poll(async () => (await json(await request.get(`/api/issues/${issue.id}`))).title, + { timeout: 30_000 }).toBe("A title retained through the outage"); + expect((await json(await request.get(`/api/issues/${other.id}`))).title).toBe(other.title); + await page.reload(); + await expect(header.getByRole("heading", { name: "A title retained through the outage", exact: true })).toBeVisible(); + expect((await json(await request.get(`/api/issues/${issue.id}`))).status).toBe("backlog"); + } finally { + await request.patch(`/api/companies/${company.id}`, { data: { status: "archived" } }); + } +}); + +test("two cached tasks keep separate title saves while the browser is offline", async ({ page, request }) => { + test.setTimeout(120_000); + const company = await json(await request.post("/api/companies", { data: { name: `Queued writes ${Date.now()}` } })); + try { + const first = await json(await request.post(`/api/companies/${company.id}/issues`, { data: { title: "First queued task", status: "todo" } })); + const second = await json(await request.post(`/api/companies/${company.id}/issues`, { data: { title: "Second queued task", status: "todo" } })); + await page.goto(`/${company.issuePrefix}/issues/${second.id}`); + await expect(page.getByRole("heading", { name: second.title, exact: true })).toBeVisible(); + await page.getByRole("link", { name: "Tasks", exact: true }).first().click(); + await page.getByRole("link").filter({ hasText: first.title }).first().click(); + const header = page.getByTestId("issue-detail-header"); + await expect(header.getByRole("heading", { name: first.title, exact: true })).toBeVisible(); + const dismissAnnouncement = page.getByRole("button", { name: "Dismiss announcement" }); + if (await dismissAnnouncement.isVisible()) await dismissAnnouncement.click(); + const secondLink = page.getByRole("link").filter({ hasText: second.title }).first(); + await expect(secondLink).toBeVisible(); + await page.context().setOffline(true); + await expect(page.locator('[data-connection-status="offline"]')).toBeVisible(); + await header.getByRole("heading", { name: first.title, exact: true }).click(); + await header.locator("textarea").fill("First title retained offline"); + await header.locator("textarea").press("Enter"); + await secondLink.focus(); + await secondLink.press("Enter", { timeout: 5_000 }); + await expect(page).toHaveURL(new RegExp(`${second.id}$`)); + await expect(header.getByRole("heading", { name: second.title, exact: true })).toBeVisible(); + await header.getByRole("heading", { name: second.title, exact: true }).click(); + await expect(header.locator("textarea")).toHaveValue(second.title); + await header.locator("textarea").fill("Second title retained offline"); + await header.locator("textarea").press("Enter"); + expect((await json(await request.get(`/api/issues/${first.id}`))).title).toBe(first.title); + expect((await json(await request.get(`/api/issues/${second.id}`))).title).toBe(second.title); + await page.context().setOffline(false); + await expect.poll(async () => (await json(await request.get(`/api/issues/${first.id}`))).title, { timeout: 30_000 }).toBe("First title retained offline"); + await expect.poll(async () => (await json(await request.get(`/api/issues/${second.id}`))).title, { timeout: 30_000 }).toBe("Second title retained offline"); + await page.reload(); + await expect(header.getByRole("heading", { name: "Second title retained offline", exact: true })).toBeVisible(); + } finally { + await page.context().setOffline(false).catch(() => {}); + await request.patch(`/api/companies/${company.id}`, { data: { status: "archived" } }); + } +}); + +test("a queued startup save survives the first session lookup during an API outage", async ({ page, request }) => { + const company = await json(await request.post("/api/companies", { data: { name: `Startup recovery ${Date.now()}` } })); + const session = await json(await request.get("/api/auth/get-session")); + let releaseSession!: () => void; + const sessionReady = new Promise(resolve => { releaseSession = resolve; }); + const savedTitle = "Saved through first session lookup"; + let apiUnavailable = false; + try { + const issue = await json(await request.post(`/api/companies/${company.id}/issues`, { + data: { title: "Startup original title", status: "backlog" }, + })); + await page.route("**/api/**", async route => { + const path = new URL(route.request().url()).pathname; + if (apiUnavailable && (path === "/api/health" || (route.request().method() === "PATCH" && path.startsWith("/api/issues/")))) { + return route.fulfill({ status: 503, contentType: "application/json", body: JSON.stringify({ error: "tenant_app_unavailable" }) }); + } + await route.continue(); + }); + await page.route("**/api/auth/get-session", async route => { + await sessionReady; + await route.fulfill({ status: 200, contentType: "application/json", body: JSON.stringify(session) }); + }); + await page.goto(`/${company.issuePrefix}/issues/${issue.identifier}`); + const header = page.getByTestId("issue-detail-header"); + await expect(header.getByRole("heading", { name: issue.title, exact: true })).toBeVisible(); + apiUnavailable = true; + await page.evaluate(() => window.dispatchEvent(new Event("offline"))); + await expect(page.locator('[data-connection-status="offline"]')).toBeVisible(); + await header.getByRole("heading", { name: issue.title, exact: true }).click(); + await header.locator("textarea").fill(savedTitle); + await header.locator("textarea").press("Enter"); + expect((await json(await request.get(`/api/issues/${issue.id}`))).title).toBe(issue.title); + releaseSession(); + await expect.poll(() => page.evaluate(({ owner, title }) => Object.keys(localStorage).some(key => + key.startsWith(`paperclip:inline-draft:v2:${owner}:`) && JSON.parse(localStorage.getItem(key) ?? "null")?.draft === title), + { owner: session.user.id, title: savedTitle })).toBe(true); + apiUnavailable = false; + await page.evaluate(() => window.dispatchEvent(new Event("online"))); + await expect.poll(async () => (await json(await request.get(`/api/issues/${issue.id}`))).title, + { timeout: 15_000 }).toBe(savedTitle); + await page.reload(); + await expect(header.getByRole("heading", { name: savedTitle, exact: true })).toBeVisible(); + } finally { + releaseSession(); + apiUnavailable = false; + await page.evaluate(() => window.dispatchEvent(new Event("online"))).catch(() => {}); + await request.patch(`/api/companies/${company.id}`, { data: { status: "archived" } }); + } +}); + +test("replaying an accepted comment does not reopen completed work", async ({ page, request }) => { + const company = await json(await request.post("/api/companies", { data: { name: `Comment recovery ${Date.now()}` } })); + try { + const issue = await json(await request.post(`/api/companies/${company.id}/issues`, { + data: { title: "Completed work stays completed", status: "backlog", assigneeAgentId: null, assigneeUserId: null }, + })); + const body = "A comment whose response was lost"; + const clientRequestId = crypto.randomUUID(); + const first = await json(await request.post(`/api/issues/${issue.id}/comments`, { data: { body, clientRequestId } })); + const agent = await json(await request.post(`/api/companies/${company.id}/agents`, { + data: { name: "Completed comment worker", role: "engineer", adapterType: "process", runner: "legacy", + adapterConfig: { command: "/usr/bin/true" }, runtimeConfig: { heartbeat: { enabled: false } } }, + })); + await json(await request.patch(`/api/issues/${issue.id}`, { data: { status: "done", assigneeAgentId: agent.id } })); + const receipt = await json(await request.post(`/api/issues/${issue.id}/comments`, { data: { body, clientRequestId } })); + expect(receipt.id).toBe(first.id); + expect((await json(await request.get(`/api/issues/${issue.id}`))).status).toBe("done"); + const conflict = await request.post(`/api/issues/${issue.id}/comments`, { data: { body: "Different request", clientRequestId } }); + expect(conflict.status()).toBe(409); + expect((await json(await request.get(`/api/issues/${issue.id}`))).status).toBe("done"); + const comments = await json(await request.get(`/api/issues/${issue.id}/comments`)); + expect(comments.filter((comment: { clientRequestId?: string }) => comment.clientRequestId === clientRequestId)).toHaveLength(1); + const activity = await json(await request.get(`/api/issues/${issue.id}/activity`)); + expect(activity.filter((entry: { action: string; details?: { commentId?: string } }) => + entry.action === "issue.comment_added" && entry.details?.commentId === first.id)).toHaveLength(1); + await page.goto(`/${company.issuePrefix}/issues/${issue.id}`); + await expect(page.getByRole("heading", { name: issue.title, exact: true })).toBeVisible(); + await expect(page.getByText(body, { exact: true })).toBeVisible(); + } finally { + await request.patch(`/api/companies/${company.id}`, { data: { status: "archived" } }); + } +}); + +test("startup work retries a failed session lookup while other APIs stay healthy", async ({ page, request }) => { + const company = await json(await request.post("/api/companies", { data: { name: `Session recovery ${Date.now()}` } })); + const session = await json(await request.get("/api/auth/get-session")); + let healthy = false, saving = false, failedAfterSave = 0; + try { + const issue = await json(await request.post(`/api/companies/${company.id}/issues`, { + data: { title: "Before session recovery", status: "backlog" }, + })); + await page.route("**/api/auth/get-session", async route => { + if (healthy) return route.fulfill({ status: 200, json: session }); + if (saving) failedAfterSave += 1; + await route.fulfill({ status: 500, json: { error: "session_lookup_failed" } }); + }); + await page.goto(`/${company.issuePrefix}/issues/${issue.id}`); + const header = page.getByTestId("issue-detail-header"); + await header.getByRole("heading", { name: issue.title, exact: true }).click(); + await header.locator("textarea").fill("Saved after session recovery"); + saving = true; + await header.locator("textarea").press("Enter"); + await expect.poll(() => failedAfterSave).toBeGreaterThan(0); + healthy = true; + await expect.poll(async () => (await json(await request.get(`/api/issues/${issue.id}`))).title, + { timeout: 15_000 }).toBe("Saved after session recovery"); + await page.reload(); + await expect(header.getByRole("heading", { name: "Saved after session recovery", exact: true })).toBeVisible(); + } finally { + await request.patch(`/api/companies/${company.id}`, { data: { status: "archived" } }); + } +}); diff --git a/ui/src/api/issues.test.ts b/ui/src/api/issues.test.ts index 31a4548767d..310a0e7d6aa 100644 --- a/ui/src/api/issues.test.ts +++ b/ui/src/api/issues.test.ts @@ -11,7 +11,8 @@ vi.mock("./client", async (importOriginal) => ({ api: mockApi, })); -import { issuesApi } from "./issues"; +import type { IssueThreadInteraction } from "@paperclipai/shared"; +import { acceptedResolutionMatches, answeredResolutionMatches, issuesApi } from "./issues"; import { ApiError } from "./client"; import { CommentSubmissionUnknownError } from "../lib/comment-submit-result"; @@ -244,3 +245,100 @@ describe("issuesApi.list", () => { ); }); }); + +describe("interaction resolution replay", () => { + const alreadyResolved = () => + new ApiError("Interaction has already been resolved", 409, { + error: "Interaction has already been resolved", + code: "interaction_already_resolved", + }); + const outage = () => new ApiError("Paperclip is restarting", 503, { error: "tenant_app_unavailable" }); + + beforeEach(() => { + mockApi.get.mockReset(); + mockApi.post.mockReset(); + }); + + it("treats already-resolved after a transient failure as success when the resolution matches", async () => { + const accepted = { id: "interaction-1", status: "accepted" }; + mockApi.post.mockRejectedValueOnce(outage()).mockRejectedValueOnce(alreadyResolved()); + mockApi.get.mockResolvedValue([accepted]); + await expect(issuesApi.acceptInteraction("issue-1", "interaction-1")).rejects.toMatchObject({ status: 503 }); + await expect(issuesApi.acceptInteraction("issue-1", "interaction-1")).resolves.toEqual(accepted); + expect(mockApi.get).toHaveBeenCalledWith("/issues/issue-1/interactions"); + }); + + it("retries the verification when reading the card back also hits the outage", async () => { + const accepted = { id: "interaction-6", status: "accepted" }; + mockApi.post + .mockRejectedValueOnce(outage()) + .mockRejectedValueOnce(alreadyResolved()) + .mockRejectedValueOnce(alreadyResolved()); + mockApi.get.mockRejectedValueOnce(outage()).mockResolvedValue([accepted]); + await expect(issuesApi.acceptInteraction("issue-1", "interaction-6")).rejects.toMatchObject({ status: 503 }); + // The decision landed, but the read-back is still down: stay retryable. + await expect(issuesApi.acceptInteraction("issue-1", "interaction-6")).rejects.toMatchObject({ status: 503 }); + await expect(issuesApi.acceptInteraction("issue-1", "interaction-6")).resolves.toEqual(accepted); + }); + + it("keeps the conflict when the card was resolved differently", async () => { + mockApi.post.mockRejectedValueOnce(outage()).mockRejectedValueOnce(alreadyResolved()); + mockApi.get.mockResolvedValue([{ id: "interaction-2", status: "rejected" }]); + await expect(issuesApi.acceptInteraction("issue-1", "interaction-2")).rejects.toBeInstanceOf(ApiError); + await expect(issuesApi.acceptInteraction("issue-1", "interaction-2")).rejects.toMatchObject({ status: 409 }); + }); + + it("keeps the conflict when someone else accepted different checkbox options", async () => { + const savedByOther = { + id: "interaction-4", + kind: "request_checkbox_confirmation", + status: "accepted", + result: { version: 1, outcome: "accepted", selectedOptionIds: ["b"] }, + }; + mockApi.post.mockRejectedValueOnce(outage()).mockRejectedValueOnce(alreadyResolved()); + mockApi.get.mockResolvedValue([savedByOther]); + await expect(issuesApi.acceptInteraction("issue-1", "interaction-4", { selectedOptionIds: ["a"] })).rejects.toMatchObject({ status: 503 }); + await expect(issuesApi.acceptInteraction("issue-1", "interaction-4", { selectedOptionIds: ["a"] })).rejects.toMatchObject({ status: 409 }); + }); + + it("treats already-resolved as success when the saved checkbox options are ours", async () => { + const saved = { + id: "interaction-5", + kind: "request_checkbox_confirmation", + status: "accepted", + result: { version: 1, outcome: "accepted", selectedOptionIds: ["b", "a"] }, + }; + mockApi.post.mockRejectedValueOnce(outage()).mockRejectedValueOnce(alreadyResolved()); + mockApi.get.mockResolvedValue([saved]); + await expect(issuesApi.acceptInteraction("issue-1", "interaction-5", { selectedOptionIds: ["a", "b"] })).rejects.toMatchObject({ status: 503 }); + await expect(issuesApi.acceptInteraction("issue-1", "interaction-5", { selectedOptionIds: ["a", "b"] })).resolves.toEqual(saved); + }); + + it("matches answers and suggested-task selections exactly", () => { + const answers = [{ questionId: "q1", optionIds: ["x"], otherText: null }]; + const answered = { + kind: "ask_user_questions", + status: "answered", + result: { version: 1, answers }, + } as unknown as IssueThreadInteraction; + expect(answeredResolutionMatches(answered, answers)).toBe(true); + expect(answeredResolutionMatches(answered, [{ questionId: "q1", optionIds: ["y"] }])).toBe(false); + + const suggested = { + kind: "suggest_tasks", + status: "accepted", + payload: { tasks: [{ clientKey: "t1" }, { clientKey: "t2" }] }, + result: { version: 1, skippedClientKeys: ["t2"] }, + } as unknown as IssueThreadInteraction; + expect(acceptedResolutionMatches(suggested, { selectedClientKeys: ["t1"] })).toBe(true); + expect(acceptedResolutionMatches(suggested, { selectedClientKeys: ["t1", "t2"] })).toBe(false); + expect(acceptedResolutionMatches(suggested, undefined)).toBe(false); + }); + + it("keeps the conflict when no transient failure came first", async () => { + mockApi.post.mockRejectedValueOnce(alreadyResolved()); + mockApi.get.mockResolvedValue([{ id: "interaction-3", status: "answered" }]); + await expect(issuesApi.respondToInteraction("issue-1", "interaction-3", { answers: [] })).rejects.toMatchObject({ status: 409 }); + expect(mockApi.get).not.toHaveBeenCalled(); + }); +}); diff --git a/ui/src/api/issues.ts b/ui/src/api/issues.ts index 159c215ce7a..5a68b1abe7e 100644 --- a/ui/src/api/issues.ts +++ b/ui/src/api/issues.ts @@ -46,6 +46,8 @@ import type { } from "@paperclipai/shared"; import { api, ApiError, detachInflightGet, type RequestOptions } from "./client"; import { CommentSubmissionUnknownError } from "../lib/comment-submit-result"; +import { classifySendFailure } from "../lib/pending-send"; +import { interactionResolutionErrorCode } from "../lib/interaction-resolution-error"; function hasCommentReceipt(value: unknown): boolean { if (!value || typeof value !== "object" || Array.isArray(value)) return false; @@ -76,6 +78,104 @@ async function confirmedCommentResponse( } } +/** + * Resolutions that failed transiently in this tab. A resolution may land on + * the server and lose its response; resending it then gets + * `interaction_already_resolved`. That is our own answer, not a conflict, but + * only when the saved resolution is exactly the one we sent: someone else may + * have resolved the card meanwhile with the same status and different choices. + */ +const transientlyFailedResolutions = new Set(); + +function sameSet(left: readonly string[] | null | undefined, right: readonly string[] | null | undefined): boolean { + const a = new Set(left ?? []); + const b = new Set(right ?? []); + return a.size === b.size && [...a].every((value) => b.has(value)); +} + +function sameText(left: string | null | undefined, right: string | null | undefined): boolean { + return (left?.trim() || null) === (right?.trim() || null); +} + +type AcceptInteractionInput = { + selectedClientKeys?: string[]; + selectedOptionIds?: string[]; + rememberAction?: boolean; +}; + +/** Whether a stored accepted resolution records exactly this accept request. */ +export function acceptedResolutionMatches(current: IssueThreadInteraction, data: AcceptInteractionInput | undefined): boolean { + if (current.status !== "accepted") return false; + if (current.kind === "suggest_tasks") { + // The server skips every suggested task that was not selected. + const allKeys = current.payload.tasks.map((task) => task.clientKey); + const selected = new Set(data?.selectedClientKeys ?? allKeys); + return sameSet(current.result?.skippedClientKeys, allKeys.filter((key) => !selected.has(key))); + } + if (current.kind === "request_checkbox_confirmation") { + return sameSet(current.result?.selectedOptionIds, data?.selectedOptionIds); + } + return !data?.selectedClientKeys && !data?.selectedOptionIds; +} + +/** Whether a stored rejection records exactly this reject request. */ +export function rejectedResolutionMatches(current: IssueThreadInteraction, reason: string | undefined): boolean { + if (current.status !== "rejected") return false; + const result = (current.result ?? null) as { reason?: string | null; rejectionReason?: string | null } | null; + return sameText(current.kind === "suggest_tasks" ? result?.rejectionReason : result?.reason, reason); +} + +/** Whether stored answers record exactly these answers. */ +export function answeredResolutionMatches(current: IssueThreadInteraction, answers: AskUserQuestionsAnswer[]): boolean { + if (current.status !== "answered" || current.kind !== "ask_user_questions") return false; + const saved = current.result?.answers ?? []; + if (saved.length !== answers.length) return false; + return answers.every((answer) => { + const match = saved.find((row) => row.questionId === answer.questionId); + return Boolean(match && sameSet(match.optionIds, answer.optionIds) && sameText(match.otherText, answer.otherText)); + }); +} + +async function resolveInteractionOnce( + issueId: string, + interactionId: string, + /** Identifies this exact request; a different answer is a different resolution. */ + requestFingerprint: string, + matchesRequest: (current: IssueThreadInteraction) => boolean, + request: () => Promise, +): Promise { + const key = `${issueId}:${interactionId}:${requestFingerprint}`; + try { + const result = await request(); + transientlyFailedResolutions.delete(key); + return result; + } catch (error) { + if (classifySendFailure(error) === "pending") { + transientlyFailedResolutions.add(key); + throw error; + } + if ( + transientlyFailedResolutions.has(key) && + interactionResolutionErrorCode(error) === "interaction_already_resolved" + ) { + const current = await api + .get(`/issues/${issueId}/interactions`) + .then((rows) => rows.find((row) => row.id === interactionId) ?? null) + .catch((readError: unknown) => { + // A temporary read failure is no verdict: let it escape so the + // replay policy retries, and the next attempt verifies again. + if (classifySendFailure(readError) === "pending") throw readError; + return null; + }); + if (current && matchesRequest(current)) { + transientlyFailedResolutions.delete(key); + return current; + } + } + throw error; + } +} + export type IssueUpdateResponse = Issue & { comment?: IssueComment | null; changes: IssueChanges; @@ -482,20 +582,30 @@ export const issuesApi = { acceptInteraction: ( id: string, interactionId: string, - data?: { - selectedClientKeys?: string[]; - selectedOptionIds?: string[]; - rememberAction?: boolean; - }, + data?: AcceptInteractionInput, ) => - api.post( - `/issues/${id}/interactions/${interactionId}/accept`, - data ?? {}, + resolveInteractionOnce( + id, + interactionId, + `accept:${JSON.stringify([data?.selectedClientKeys ?? null, data?.selectedOptionIds ?? null])}`, + (current) => acceptedResolutionMatches(current, data), + () => + api.post( + `/issues/${id}/interactions/${interactionId}/accept`, + data ?? {}, + ), ), rejectInteraction: (id: string, interactionId: string, reason?: string) => - api.post( - `/issues/${id}/interactions/${interactionId}/reject`, - reason ? { reason } : {}, + resolveInteractionOnce( + id, + interactionId, + `reject:${JSON.stringify(reason ?? null)}`, + (current) => rejectedResolutionMatches(current, reason), + () => + api.post( + `/issues/${id}/interactions/${interactionId}/reject`, + reason ? { reason } : {}, + ), ), cancelInteraction: (id: string, interactionId: string, reason?: string) => api.post( @@ -517,9 +627,16 @@ export const issuesApi = { summaryMarkdown?: string | null; }, ) => - api.post( - `/issues/${id}/interactions/${interactionId}/respond`, - data, + resolveInteractionOnce( + id, + interactionId, + `respond:${JSON.stringify(data.answers)}`, + (current) => answeredResolutionMatches(current, data.answers), + () => + api.post( + `/issues/${id}/interactions/${interactionId}/respond`, + data, + ), ), submitInteractionVerdicts: ( id: string, diff --git a/ui/src/components/InlineEditor.test.tsx b/ui/src/components/InlineEditor.test.tsx index 2b1cef50abf..8261aa07588 100644 --- a/ui/src/components/InlineEditor.test.tsx +++ b/ui/src/components/InlineEditor.test.tsx @@ -2,7 +2,14 @@ import { act, forwardRef, useImperativeHandle, useRef, type ReactNode } from "react"; import { createRoot } from "react-dom/client"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { queryKeys } from "../lib/queryKeys"; + +const mockSession = vi.hoisted(() => ({ userId: "user-1", pending: null as Promise<{ user: { id: string } } | null> | null })); +vi.mock("../api/auth", () => ({ + authApi: { getSession: () => mockSession.pending ?? Promise.resolve({ user: { id: mockSession.userId } }) }, +})); vi.mock("./MarkdownEditor", () => ({ MarkdownEditor: forwardRef< @@ -32,6 +39,14 @@ vi.mock("./MarkdownBody", () => ({ import { InlineEditor, queueContainedBlurCommit } from "./InlineEditor"; +/** Render as a signed-in user: stored drafts belong to one account. */ +function signedIn(userId: string, node: ReactNode) { + mockSession.userId = userId; + const client = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + client.setQueryData(queryKeys.auth.session, { user: { id: userId } }); + return {node}; +} + /** Enter multiline edit mode by clicking the preview surface. */ function enterMultilineEdit(container: HTMLDivElement) { const preview = container.querySelector('[data-testid="multiline-md-preview"]'); @@ -111,6 +126,203 @@ describe("InlineEditor", () => { }); }); + it("keeps a single-line edit open with no unhandled rejection when the save fails", async () => { + const unhandled = vi.fn(); + window.addEventListener("unhandledrejection", unhandled); + process.on("unhandledRejection", unhandled); + const onSave = vi.fn().mockRejectedValue(new Error("Title is too long")); + const root = createRoot(container); + act(() => { + root.render(); + }); + act(() => { + container.querySelector("span")!.dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + const textarea = container.querySelector("textarea")!; + act(() => setNativeTextareaValue(textarea, "hello world")); + await act(async () => { + textarea.blur(); + await new Promise((resolve) => setTimeout(resolve, 0)); + }); + expect(onSave).toHaveBeenCalledWith("hello world"); + // The text stays in the editor instead of snapping back to the old value. + expect(container.querySelector("textarea")?.value).toBe("hello world"); + expect(unhandled).not.toHaveBeenCalled(); + window.removeEventListener("unhandledrejection", unhandled); + process.off("unhandledRejection", unhandled); + act(() => root.unmount()); + }); + + it("restores an unsaved edit and saves it when the server value has not moved", async () => { + localStorage.setItem("paperclip:inline-draft:v2:user-1:issue-1:description", JSON.stringify({ version: 1, draft: "Typed before reload", base: "Original" })); + const onSave = vi.fn().mockResolvedValue(undefined); + const root = createRoot(container); + await act(async () => { + root.render(signedIn("user-1", )); + }); + expect(container.querySelector('[data-testid="multiline-md-mock"]')?.value).toBe("Typed before reload"); + expect(onSave).toHaveBeenCalledWith("Typed before reload"); + act(() => root.unmount()); + localStorage.clear(); + }); + + it("restores the next task's stored edit without remounting the editor", async () => { + localStorage.setItem("paperclip:inline-draft:v2:user-1:issue-b:description", JSON.stringify({ version: 1, draft: "Recovered B", base: "Original B" })); + const client = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + client.setQueryData(queryKeys.auth.session, { user: { id: "user-1" } }); + const saveA = vi.fn().mockResolvedValue(undefined); + const saveB = vi.fn().mockResolvedValue(undefined); + const root = createRoot(container); + const renderTask = (id: string, onSave: typeof saveA) => root.render( + + + , + ); + try { + await act(async () => renderTask("a", saveA)); + await act(async () => renderTask("b", saveB)); + expect(container.querySelector('[data-testid="multiline-md-mock"]')?.value).toBe("Recovered B"); + expect(saveB).toHaveBeenCalledWith("Recovered B"); + expect(saveA).not.toHaveBeenCalled(); + } finally { + act(() => root.unmount()); + localStorage.clear(); + client.clear(); + } + }); + + it("restores an unsaved edit without overwriting a newer server value", async () => { + localStorage.setItem("paperclip:inline-draft:v2:user-1:issue-2:description", JSON.stringify({ version: 1, draft: "My old edit", base: "Original" })); + const onSave = vi.fn().mockResolvedValue(undefined); + const root = createRoot(container); + await act(async () => { + root.render(signedIn("user-1", )); + }); + expect(container.querySelector('[data-testid="multiline-md-mock"]')?.value).toBe("My old edit"); + expect(onSave).not.toHaveBeenCalled(); + act(() => root.unmount()); + localStorage.clear(); + }); + + it("neither shows nor saves another account's unsaved edit", async () => { + localStorage.setItem("paperclip:inline-draft:v2:user-1:issue-3:description", JSON.stringify({ version: 1, draft: "Unpublished by user 1", base: "Original" })); + const onSave = vi.fn().mockResolvedValue(undefined); + const root = createRoot(container); + await act(async () => { + root.render(signedIn("user-2", )); + }); + expect(container.textContent).not.toContain("Unpublished by user 1"); + expect(container.querySelector('[data-testid="multiline-md-mock"]')?.value ?? "").not.toBe("Unpublished by user 1"); + expect(onSave).not.toHaveBeenCalled(); + act(() => root.unmount()); + localStorage.clear(); + }); + + it.each(["user-1", "signed-out"])("keeps typing through the first session lookup (%s)", async (owner) => { + localStorage.clear(); + localStorage.setItem(`paperclip:inline-draft:v2:${owner}:initial-session`, JSON.stringify({ version: 1, draft: "Earlier saved draft", base: "Original" })); + let finish!: (session: { user: { id: string } } | null) => void; + mockSession.pending = new Promise(resolve => { finish = resolve; }); + const client = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + const onSave = vi.fn().mockResolvedValue(undefined); + const root = createRoot(container); + try { + await act(async () => root.render( + + + , + )); + act(() => container.querySelector("span")!.click()); + const input = container.querySelector("textarea")!; + act(() => setNativeTextareaValue(input, "Typing while session loads")); + await act(async () => { + finish(owner === "signed-out" ? null : { user: { id: owner } }); + await new Promise(resolve => setTimeout(resolve, 0)); + }); + expect(container.querySelector("textarea")).toBe(input); + expect(input.value).toBe("Typing while session loads"); + await vi.waitFor(async () => { + await act(async () => { await new Promise(resolve => setTimeout(resolve, 0)); }); + expect(localStorage.getItem(`paperclip:inline-draft:v2:${owner}:initial-session`)).toContain("Typing while session loads"); + }); + expect(onSave).not.toHaveBeenCalled(); + } finally { + act(() => root.unmount()); + client.clear(); + localStorage.clear(); + mockSession.pending = null; + } + }); + + it("does not restore typing cancelled before the first session lookup", async () => { + localStorage.clear(); + localStorage.setItem("paperclip:inline-draft:v2:user-1:cancel-before-session", JSON.stringify({ version: 1, draft: "Original", base: "Original" })); + let finish!: (session: { user: { id: string } }) => void; + mockSession.pending = new Promise(resolve => { finish = resolve; }); + const client = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + const root = createRoot(container); + try { + await act(async () => root.render( + + + , + )); + act(() => container.querySelector("span")!.click()); + act(() => setNativeTextareaValue(container.querySelector("textarea")!, "Cancelled private text")); + act(() => container.querySelector("textarea")!.dispatchEvent(new KeyboardEvent("keydown", { key: "Escape", bubbles: true }))); + await act(async () => { + finish({ user: { id: "user-1" } }); + await new Promise(resolve => setTimeout(resolve, 0)); + }); + expect(container.textContent).not.toContain("Cancelled private text"); + await vi.waitFor(async () => { + await act(async () => { await new Promise(resolve => setTimeout(resolve, 0)); }); + expect(localStorage.getItem("paperclip:inline-draft:v2:user-1:cancel-before-session")).toBeNull(); + }); + } finally { + act(() => root.unmount()); client.clear(); localStorage.clear(); mockSession.pending = null; + } + }); + + it.each([false, true])("isolates a mounted private edit after an account change (multiline=%s)", async (multiline) => { + mockSession.userId = "user-1"; + localStorage.clear(); + const client = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + client.setQueryData(queryKeys.auth.session, { user: { id: "user-1" } }); + const onSave = vi.fn().mockResolvedValue(undefined); + const root = createRoot(container); + try { + await act(async () => root.render( + + + , + )); + if (!multiline) act(() => container.querySelector("span")!.click()); + act(() => setNativeTextareaValue(container.querySelector("textarea")!, "Private account A draft")); + const storedA = localStorage.getItem("paperclip:inline-draft:v2:user-1:private-switch"); + expect(storedA).toContain("Private account A draft"); + mockSession.userId = "user-2"; + await act(async () => { + client.setQueryData(queryKeys.auth.session, { user: { id: "user-2" } }); + await new Promise(resolve => setTimeout(resolve, 0)); + }); + expect(container.querySelector("textarea")?.value ?? "").not.toContain("Private account A draft"); + expect(container.textContent).not.toContain("Private account A draft"); + expect(localStorage.getItem("paperclip:inline-draft:v2:user-1:private-switch")).toBe(storedA); + expect(localStorage.getItem("paperclip:inline-draft:v2:user-2:private-switch")).toBeNull(); + expect(onSave).not.toHaveBeenCalled(); + if (!multiline) act(() => container.querySelector("span")!.click()); + act(() => setNativeTextareaValue(container.querySelector("textarea")!, "Account B edit")); + expect(localStorage.getItem("paperclip:inline-draft:v2:user-2:private-switch")).toContain("Account B edit"); + expect(localStorage.getItem("paperclip:inline-draft:v2:user-2:private-switch")).not.toContain("Private account A draft"); + } finally { + act(() => root.unmount()); + client.clear(); + localStorage.clear(); + mockSession.userId = "user-1"; + } + }); + it("does not call onSave when nullable is false/omitted and the field is cleared", () => { const onSave = vi.fn().mockResolvedValue(undefined); const root = createRoot(container); diff --git a/ui/src/components/InlineEditor.tsx b/ui/src/components/InlineEditor.tsx index 52f658e1827..bf55535ec4d 100644 --- a/ui/src/components/InlineEditor.tsx +++ b/ui/src/components/InlineEditor.tsx @@ -1,8 +1,12 @@ -import { useState, useRef, useEffect, useCallback } from "react"; +import { useState, useRef, useEffect, useCallback, useId } from "react"; import { cn } from "../lib/utils"; import { MarkdownBody, type MarkdownExternalReferenceMap } from "./MarkdownBody"; import { MarkdownEditor, type MarkdownEditorRef, type MentionOption } from "./MarkdownEditor"; -import { useAutosaveIndicator } from "../hooks/useAutosaveIndicator"; +import { useDurableAutosave } from "../hooks/useDurableAutosave"; +import { useDraftOwner } from "../hooks/useDraftOwner"; +import { loadStructuredDraft, saveStructuredDraft } from "../lib/composer-draft"; + +type StoredInlineDraft = { version: 1; draft: string; base: string }; import { FoldCurtain } from "./FoldCurtain"; interface InlineEditorProps { @@ -31,6 +35,11 @@ interface InlineEditorProps { defaultEditing?: boolean; /** Notified when the multiline editor swaps between display and edit mode. */ onEditingChange?: (editing: boolean) => void; + /** + * Keep unsaved edits in browser storage under this key, so a reload or an + * outage does not lose them. A stored edit is restored and saved on mount. + */ + draftKey?: string; } /** Shared padding so display and edit modes occupy the exact same box. */ @@ -55,7 +64,15 @@ export function queueContainedBlurCommit(container: HTMLDivElement, onCommit: () }; } -export function InlineEditor({ +export function InlineEditor(props: InlineEditorProps) { + const draftOwner = useDraftOwner(); + const [scope, setScope] = useState({ owner: draftOwner, generation: 0 }); + const generation = scope.generation + (scope.owner !== undefined && scope.owner !== draftOwner ? 1 : 0); + if (scope.owner !== draftOwner) setScope({ owner: draftOwner, generation }); + return ; +} + +function OwnedInlineEditor({ value, onSave, as: Tag = "span", @@ -70,11 +87,14 @@ export function InlineEditor({ externalReferences, defaultEditing = false, onEditingChange, -}: InlineEditorProps) { + draftKey, + draftOwner, +}: InlineEditorProps & { draftOwner: string | undefined }) { const [editing, setEditing] = useState(false); const [multilineEditing, setMultilineEditing] = useState(multiline && defaultEditing); const [multilineFocused, setMultilineFocused] = useState(false); const [draft, setDraft] = useState(value); + const unownedDraft = useRef(null); const lastPropValueRef = useRef(value); const inputRef = useRef(null); const markdownRef = useRef(null); @@ -83,12 +103,46 @@ export function InlineEditor({ const pendingFocusFrameRef = useRef(null); const justEnteredEditRef = useRef(multiline && defaultEditing); const hasBeenFocusedRef = useRef(false); - const { - state: autosaveState, - markDirty, - reset, - runSave, - } = useAutosaveIndicator(); + const autosave = useDurableAutosave({ + sourceId: `inline-editor:${useId()}`, + targetKey: draftKey ?? undefined, + save: async (next) => { + await onSave(next); + }, + }); + const { markDirty, reset, run: runAutosave } = autosave; + const autosaveState = autosave.status; + const autosaveStateRef = useRef(autosaveState); + autosaveStateRef.current = autosaveState; + + // Unsaved edits are stored with the value they were based on, so a restored + // edit never silently overwrites a newer server value, and per signed-in + // account, so another account in this browser never sees or saves them. + const storageKey = draftKey && draftOwner !== undefined ? `paperclip:inline-draft:v2:${draftOwner}:${draftKey}` : null; + const clearStoredDraft = useCallback(() => { + if (!storageKey) return; + try { + localStorage.removeItem(storageKey); + } catch { + // Unavailable storage holds nothing to clear. + } + }, [storageKey]); + + // Any path that lands the edit (blur, autosave, a resend after an outage) + // updates `value`; the stored copy is then no longer needed. + useEffect(() => { + if (!storageKey) return; + const stored = loadStructuredDraft(storageKey, null); + if (stored && typeof stored.draft === "string" && stored.draft.trim() === value.trim()) clearStoredDraft(); + }, [clearStoredDraft, storageKey, value]); + + const changeDraft = useCallback((next: string) => { + setDraft(next); + if (draftOwner === undefined) unownedDraft.current = { version: 1, draft: next, base: value }; + if (!storageKey) return; + if (next.trim() === value.trim()) clearStoredDraft(); + else saveStructuredDraft(storageKey, { version: 1, draft: next, base: value } satisfies StoredInlineDraft); + }, [clearStoredDraft, draftOwner, storageKey, value]); useEffect(() => { const previousValue = lastPropValueRef.current; @@ -101,6 +155,36 @@ export function InlineEditor({ }); }, [value, multiline, multilineFocused]); + // An edit that never saved (reload, crash, outage) comes back. It saves on + // its own only when the server value is still the one it was based on; + // otherwise it waits in the editor for the user. + const restoredDraftRef = useRef(null); + useEffect(() => { + if (!storageKey || restoredDraftRef.current === storageKey) return; + restoredDraftRef.current = storageKey; + if (unownedDraft.current) { + if (draft.trim() !== value.trim()) saveStructuredDraft(storageKey, { ...unownedDraft.current, draft }); + else clearStoredDraft(); + unownedDraft.current = null; + return; + } + const stored = loadStructuredDraft(storageKey, null); + if (!stored || stored.version !== 1 || typeof stored.draft !== "string" || typeof stored.base !== "string") return; + if (stored.draft.trim() === value.trim()) { + clearStoredDraft(); + return; + } + setDraft(stored.draft); + if (multiline) { + setMultilineEditing(true); + onEditingChange?.(true); + } else { + setEditing(true); + } + if (stored.base === value) void runAutosave(stored.draft.trim()); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [storageKey]); + useEffect(() => { return () => { if (autosaveDebounceRef.current) { @@ -162,6 +246,7 @@ export function InlineEditor({ } if (!multiline || !multilineEditing) return; if (!hasBeenFocusedRef.current) return; + // Waiting or failed saves keep the editor open, so the text stays visible. if (autosaveState !== "idle") return; hasBeenFocusedRef.current = false; setMultilineEditing(false); @@ -169,37 +254,39 @@ export function InlineEditor({ }, [multiline, multilineEditing, multilineFocused, autosaveState, onEditingChange]); + // A single-line edit that waited for the connection closes once it saves. + useEffect(() => { + if (multiline || !editing || autosaveState !== "saved") return; + if (document.activeElement === inputRef.current) return; + setEditing(false); + }, [autosaveState, editing, multiline]); + + /** Save the draft if it changed. Never rejects; a failed save keeps the text. */ const commit = useCallback(async (nextValue = draft) => { const valueToSave = nextValue.trim(); const valueChanged = valueToSave !== value; const shouldSave = nullable ? valueChanged : Boolean(valueToSave && valueChanged); - if (shouldSave) { - await Promise.resolve(onSave(valueToSave)); - } else { + if (!shouldSave) { setDraft(value); + if (draftKey) clearStoredDraft(); + if (!multiline) setEditing(false); + return; } - if (!multiline) { - setEditing(false); - } - }, [draft, multiline, nullable, onSave, value]); + const result = await runAutosave(valueToSave); + // A single-line edit stays open until it is saved, so its text is not lost. + if (!multiline && result.kind === "saved") setEditing(false); + }, [clearStoredDraft, draft, draftKey, multiline, nullable, runAutosave, value]); /** Multiline blur/submit: show autosave indicator when persisting */ const finalizeMultilineBlurOrSubmit = useCallback(() => { const trimmed = draft.trim(); - if (trimmed === value) { + if (trimmed === value || (!trimmed && !nullable)) { reset(); - void commit(); - return; } - if (!trimmed && !nullable) { - reset(); - void commit(); - return; - } - void runSave(() => commit()); - }, [commit, draft, nullable, reset, runSave, value]); + void commit(); + }, [commit, draft, nullable, reset, value]); const cancelPendingBlurCommit = useCallback(() => { if (blurCommitFrameRef.current === null) return; @@ -230,6 +317,7 @@ export function InlineEditor({ } reset(); setDraft(value); + if (draftKey) clearStoredDraft(); if (multiline) { setMultilineFocused(false); setMultilineEditing(false); @@ -250,7 +338,7 @@ export function InlineEditor({ const trimmed = draft.trim(); // Nullable: empty draft can still be a real edit (clearing); only skip debounce when unchanged or empty is invalid. if (trimmed === value || (!trimmed && !nullable)) { - if (autosaveState !== "saved") { + if (autosaveStateRef.current !== "saved") { reset(); } return; @@ -260,7 +348,7 @@ export function InlineEditor({ clearTimeout(autosaveDebounceRef.current); } autosaveDebounceRef.current = setTimeout(() => { - void runSave(() => commit(trimmed)); + void commit(trimmed); }, AUTOSAVE_DEBOUNCE_MS); return () => { @@ -268,7 +356,7 @@ export function InlineEditor({ clearTimeout(autosaveDebounceRef.current); } }; - }, [autosaveState, commit, draft, markDirty, multiline, multilineFocused, nullable, reset, runSave, value]); + }, [commit, draft, markDirty, multiline, multilineFocused, nullable, reset, value]); if (multiline) { const previewValue = autosaveState === "saved" || autosaveState === "idle" ? draft : value; @@ -353,7 +441,7 @@ export function InlineEditor({ - {autosaveState === "saving" - ? "Autosaving..." - : autosaveState === "saved" - ? "Saved" - : autosaveState === "error" - ? "Could not save" - : "Idle"} + {autosave.label ?? "Idle"} + {autosaveState === "stalled" ? : null} ); @@ -394,7 +478,7 @@ export function InlineEditor({ value={draft} rows={1} onChange={(e) => { - setDraft(e.target.value); + changeDraft(e.target.value); autoSize(e.target); }} onBlur={() => { @@ -415,6 +499,7 @@ export function InlineEditor({ const DisplayTag = value && multiline ? "div" : Tag; return ( + <> {value || placeholder} + {autosaveState === "stalled" ? : null} + ); } diff --git a/ui/src/components/IssueChatThread.test.tsx b/ui/src/components/IssueChatThread.test.tsx index f232996f5ff..f84f183b765 100644 --- a/ui/src/components/IssueChatThread.test.tsx +++ b/ui/src/components/IssueChatThread.test.tsx @@ -12,10 +12,13 @@ import { flushSync } from "react-dom"; import type { ComponentProps, ReactNode } from "react"; import { createRoot } from "react-dom/client"; import { MemoryRouter } from "react-router-dom"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import type { Agent } from "@paperclipai/shared"; import { CommentSubmissionUnknownError } from "../lib/comment-submit-result"; -import { loadDraft, preserveDraftInTab, settleDraftSubmission } from "../lib/composer-draft"; +import { loadDraft, saveDraft, saveDraftSubmission, loadDraftSubmission, preserveDraftInTab, settleDraftSubmission } from "../lib/composer-draft"; +import { queryKeys } from "../lib/queryKeys"; +import { useAccountBoundMutation } from "../hooks/useAccountBoundMutation"; import { IssueAssigneePausedNotice, IssueChatThread, @@ -80,6 +83,12 @@ const { shouldPreserveComposerViewportMock: vi.fn(), })); +// The session lookup never answers on its own; tests set the account directly. +vi.mock("../api/auth", async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, authApi: { ...actual.authApi, getSession: () => new Promise(() => {}) } }; +}); + vi.mock("@assistant-ui/react", () => ({ AssistantRuntimeProvider: ({ children }: { children: ReactNode }) => (
{children}
@@ -2900,13 +2909,156 @@ describe("IssueChatThread", () => { )?.value).toBe(nextDraft); expect(localStorage.getItem(key)).toBe(nextDraft); expect(localStorage.getItem(`${key}:submission:v1`)).toBeNull(); - expect(container.textContent).not.toContain("We couldn’t confirm"); + expect(container.textContent).not.toContain("Sending when reconnected…"); } finally { act(() => root.unmount()); } expect(localStorage.getItem(key)).toBe(nextDraft); }); + it("keeps the original key after account cancellation of an accepted initial send", async () => { + const key = "accepted-thread-account-roundtrip"; + const client = new QueryClient({ defaultOptions: { queries: { retry: false }, mutations: { retry: false } } }); + client.setQueryData(queryKeys.auth.session, { user: { id: "user-1" } }); + let accepted!: () => void; + const post = vi.fn<(body: string, requestId?: string) => Promise>() + .mockImplementationOnce(() => new Promise(resolve => { accepted = resolve; })).mockResolvedValue(undefined); + function OwnedThread() { + const mutation = useAccountBoundMutation({ mutationFn: ({ body, requestId }: { body: string; requestId?: string }) => post(body, requestId) }); + return mutation.mutateAsync({ body, requestId })} />; + } + const root = createRoot(container); + try { + await act(async () => root.render()); + const editor = () => container.querySelector('textarea[aria-label="Issue chat editor"]')!; + act(() => { + Object.getOwnPropertyDescriptor(window.HTMLTextAreaElement.prototype, "value")!.set!.call(editor(), "Accepted before the account changed"); + editor().dispatchEvent(new Event("input", { bubbles: true })); + }); + const button = (label: string) => Array.from(container.querySelectorAll("button")).find(item => item.textContent === label)!; + await act(async () => button("Send").click()); + const requestId = post.mock.calls[0][1]!; + act(() => { + client.setQueryData(queryKeys.auth.session, { user: { id: "user-2" } }); + client.setQueryData(queryKeys.auth.session, { user: { id: "user-1" } }); + }); + await act(async () => accepted()); + expect(loadDraftSubmission(key)?.attemptId).toBe(requestId); + expect(editor().value).toBe("Accepted before the account changed"); + await act(async () => button("Resend now").click()); + expect(post).toHaveBeenCalledTimes(2); + expect(post.mock.calls[1][1]).toBe(requestId); + expect(loadDraftSubmission(key)).toBeNull(); + } finally { act(() => root.unmount()); client.clear(); } + }); + + it.each([false, true])("does not retry a stale account render (returns to original=%s)", async (returns) => { + const key = "stale-thread-account"; + const attemptId = "9af8228f-0be7-45ae-a104-6fbe0af6f1d3"; + saveDraft(key, "Account A text"); + saveDraftSubmission(key, { attemptId, reviewed: false, request: { body: "Account A text" }, requestOwner: "user-1" }); + const client = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + client.setQueryData(queryKeys.auth.session, { user: { id: "user-1" } }); + const onAdd = vi.fn().mockResolvedValue(undefined); + const root = createRoot(container); + try { + await act(async () => root.render( + + )); + const resend = Array.from(container.querySelectorAll("button")).find(button => button.textContent === "Resend now")!; + await act(async () => { + client.setQueryData(queryKeys.auth.session, { user: { id: "user-2" } }); + if (returns) client.setQueryData(queryKeys.auth.session, { user: { id: "user-1" } }); + resend.click(); + }); + expect(onAdd).not.toHaveBeenCalled(); + expect(loadDraftSubmission(key)?.attemptId).toBe(attemptId); + } finally { act(() => root.unmount()); client.clear(); } + }); + + it("resends an unconfirmed comment with the same request ID and keeps the next draft", async () => { + const key = "resend-unconfirmed-comment"; + let rejectSend!: (error: Error) => void; + const onAdd = vi.fn() + .mockReturnValueOnce(new Promise((_, reject) => { rejectSend = reject; })) + .mockResolvedValue(undefined); + const root = createRoot(container); + const editor = () => container.querySelector('textarea[aria-label="Issue chat editor"]')!; + const type = (value: string) => act(() => { + Object.getOwnPropertyDescriptor(window.HTMLTextAreaElement.prototype, "value")!.set!.call(editor(), value); + editor().dispatchEvent(new Event("input", { bubbles: true })); + }); + const button = (label: string) => Array.from(container.querySelectorAll("button")).find(item => item.textContent === label) as HTMLButtonElement; + try { + await act(async () => root.render( + + + , + )); + type("Typed during the outage"); + await act(async () => button("Send").click()); + type("Next thought"); + await act(async () => rejectSend(new CommentSubmissionUnknownError())); + expect(container.textContent).toContain("Sending when reconnected…"); + expect(JSON.parse(localStorage.getItem(`${key}:submission:v1`)!).request.body).toBe("Typed during the outage"); + await act(async () => button("Resend now").click()); + expect(onAdd).toHaveBeenCalledTimes(2); + expect(onAdd.mock.calls[1]![0]).toBe("Typed during the outage"); + expect(onAdd.mock.calls[1]![4]).toBe(onAdd.mock.calls[0]![4]); + expect(container.textContent).not.toContain("Sending when reconnected…"); + expect(editor().value).toBe("Next thought"); + expect(localStorage.getItem(`${key}:submission:v1`)).toBeNull(); + } finally { + await act(async () => root.unmount()); + } + }); + + it("resends a comment sent before the session loaded once the account is known", async () => { + const key = "resend-before-session"; + const client = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + const onAdd = vi.fn() + .mockRejectedValueOnce(new CommentSubmissionUnknownError()) + .mockResolvedValue(undefined); + const root = createRoot(container); + const editor = () => container.querySelector('textarea[aria-label="Issue chat editor"]')!; + const button = (label: string) => Array.from(container.querySelectorAll("button")).find(item => item.textContent === label) as HTMLButtonElement | undefined; + try { + await act(async () => root.render( + + + + + , + )); + act(() => { + Object.getOwnPropertyDescriptor(window.HTMLTextAreaElement.prototype, "value")!.set!.call(editor(), "Sent before the session"); + editor().dispatchEvent(new Event("input", { bubbles: true })); + }); + await act(async () => button("Send")!.click()); + // Until the account is known the send cannot be replayed. + expect(button("Resend now")).toBeUndefined(); + + await act(async () => { + client.setQueryData(queryKeys.auth.session, { user: { id: "user-1" } }); + await new Promise((resolve) => setTimeout(resolve, 0)); + }); + expect(JSON.parse(localStorage.getItem(`${key}:submission:v1`)!).requestOwner).toBe("user-1"); + expect(container.textContent).toContain("Sending when reconnected…"); + await act(async () => button("Resend now")!.click()); + expect(onAdd).toHaveBeenCalledTimes(2); + expect(onAdd.mock.calls[1]![4]).toBe(onAdd.mock.calls[0]![4]); + expect(container.textContent).not.toContain("Sending when reconnected…"); + } finally { + await act(async () => root.unmount()); + client.clear(); + } + }); + it.each(["submission write", "all storage"])( "unlocks a confirmed in-memory submission after failed %s", async (failure) => { @@ -2944,10 +3096,10 @@ describe("IssueChatThread", () => { const attemptId = onAdd.mock.calls[0]![4] as string; type("The full next draft"); await act(async () => rejectSend(new CommentSubmissionUnknownError())); - expect(container.textContent).toContain("We couldn’t confirm"); + expect(container.textContent).toContain("Sending when reconnected…"); await act(async () => root.render(element(attemptId))); expect(editor().value).toBe("The full next draft"); - expect(container.textContent).not.toContain("We couldn’t confirm"); + expect(container.textContent).not.toContain("Sending when reconnected…"); expect(send().disabled).toBe(false); await act(async () => send().click()); expect(onAdd.mock.calls[1]![0]).toBe("The full next draft"); @@ -3000,7 +3152,7 @@ describe("IssueChatThread", () => { await act(async () => root.render(element(true))); const editor = () => container.querySelector('textarea[aria-label="Issue chat editor"]')!; expect(editor().value).toBe(localText); - expect(container.textContent).not.toContain("We couldn’t confirm"); + expect(container.textContent).not.toContain("Sending when reconnected…"); if (scenario === "unavailable recovery storage") expect(container.textContent).toContain("copy your text before leaving"); if (scenario.includes("with new attachments")) expect(container.textContent).toContain("another-tab.txt"); if (scenario === "foreign pending receipt") { @@ -3455,7 +3607,7 @@ describe("IssueChatThread", () => { expect(localStorage.getItem(`${key}:submission:v1`)).toBeNull(); expect(localStorage.getItem(`${key}:attachments:v1`)).toContain(attachmentId); expect(container.textContent).toContain("next-draft.txt"); - expect(container.textContent).not.toContain("We couldn’t confirm"); + expect(container.textContent).not.toContain("Sending when reconnected…"); await act(async () => root.unmount()); await act(async () => resolveSend()); expect(localStorage.getItem(key)).toBe("Newer unsent draft"); diff --git a/ui/src/components/IssueChatThread.tsx b/ui/src/components/IssueChatThread.tsx index a1099bc37c9..6d2a79cd591 100644 --- a/ui/src/components/IssueChatThread.tsx +++ b/ui/src/components/IssueChatThread.tsx @@ -1,3 +1,4 @@ +import { useCurrentAccountGuard } from "../hooks/useAccountBoundMutation"; import { resolveIssueChatHumanAuthor } from "@/lib/issue-chat-human-author"; import { useCurrentWorkspaceRetry, useWorkspaceBaseRefRecovery, useWorkspaceRetry } from "./WorkspaceBaseRefRecovery"; import { WorkspaceBaseRefRecoveryNotice } from "./WorkspaceBaseRefRecoveryNotice"; @@ -76,12 +77,20 @@ import { loadDraftAttachments, saveDraftAttachments, loadDraftSubmission, + replayableRequest, saveDraftSubmission, clearDraftSubmission, settleDraftSubmission, type ComposerDraftSubmission, + type ComposerSendRequest, } from "../lib/composer-draft"; import { CommentSubmissionUnknownError } from "../lib/comment-submit-result"; +import { classifySendFailure, type SendOutcome } from "../lib/pending-send"; +import { useAutoResend } from "../hooks/useDurableSubmit"; +import { useDraftOwner } from "../hooks/useDraftOwner"; +import { useRequestOwnerBinding } from "../hooks/useRequestOwnerBinding"; +import { PendingSendNotice } from "./PendingSendNotice"; +import { describeError } from "../api/errors"; import { buildIssueChatMessages, formatDurationWords, @@ -236,6 +245,7 @@ import { } from "@/components/ui/popover"; import { Textarea } from "@/components/ui/textarea"; import { + RotateCw, AlertTriangle, ArrowRight, Brain, @@ -515,7 +525,6 @@ export interface IssueChatComposerHandle { interface IssueChatComposerProps { onSend: IssueChatThreadProps["onAdd"]; confirmedSubmissionIds: ReadonlySet; - onReviewConversation?: () => Promise; onStop?: () => Promise; stopPending?: boolean; stopScope?: "leaf" | "subtree"; @@ -627,7 +636,6 @@ interface IssueChatThreadProps { clientRequestId?: string, runSettings?: ComposerRunSettings, ) => Promise; - onReviewConversation?: () => Promise; onCancelRun?: () => Promise; stopPending?: boolean; stopScope?: "leaf" | "subtree"; @@ -990,6 +998,11 @@ type ComposerAttachmentItem = { inline: boolean; contentPath?: string; error?: string; + /** + * The picked file, kept after a failed upload so the chip can offer Retry. + * Uploads are not deduped by the server, so they never resend on their own. + */ + file?: File; }; function hasFilePayload(evt: ReactDragEvent) { @@ -4651,7 +4664,6 @@ const IssueChatComposer = forwardRef< { onSend, confirmedSubmissionIds, - onReviewConversation, onStop, stopPending, stopScope = "leaf", @@ -4694,11 +4706,13 @@ const IssueChatComposer = forwardRef< // the stored draft. The effect below handles subsequent task-key changes. const [body, setBody] = useState(() => (draftKey ? loadDraft(draftKey) : "")); const [submitting, setSubmitting] = useState(false); - const [reviewError, setReviewError] = useState(false); const [uncertainSubmission, setUncertainSubmission] = useState(() => draftKey ? loadDraftSubmission(draftKey) : null, ); + // Attempts this composer resent and saw confirmed; they settle exactly like + // a receipt that arrives through the thread. + const [resentSubmissionIds, setResentSubmissionIds] = useState>(() => new Set()); const mountedTaskKey = useRef(draftKey); useEffect(() => { mountedTaskKey.current = draftKey; @@ -4710,12 +4724,20 @@ const IssueChatComposer = forwardRef< const bodyRef = useRef(body); bodyRef.current = body; const reconciledSubmissionRef = useRef<{ draftKey: string | undefined; attemptId: string } | null>(null); + // Only the account that made a request may replay it. + const draftOwner = useDraftOwner(); + const isCurrentAccount = useCurrentAccountGuard(); const pendingDraftRef = useRef<{ draftKey: string; attemptId: string; submittedBody: string; submittedAttachmentIds: string[]; + request: ComposerSendRequest; + requestOwner?: string; } | null>(null); + const markRequestUnbound = useRequestOwnerBinding({ + draftKey, owner: draftOwner, uncertainSubmission, setUncertainSubmission, pendingDraftRef, + }); function changeBody(update: string | ((current: string) => string)) { const value = typeof update === "function" ? update(bodyRef.current) : update; bodyRef.current = value; @@ -4732,6 +4754,8 @@ const IssueChatComposer = forwardRef< attemptId: pending.attemptId, reviewed: false, nextDraftOffset: pending.submittedBody.length + (value ? 2 : 0), submittedAttachmentIds: pending.submittedAttachmentIds, + request: pending.request, + requestOwner: pending.requestOwner, }); } const submittingRef = useRef(submitting); @@ -4847,7 +4871,10 @@ const IssueChatComposer = forwardRef< // A server receipt for this exact request settles a restored submission. // Text equality is not delivery proof: users may intentionally repeat text. useEffect(() => { - if (!uncertainSubmission || !confirmedSubmissionIds.has(uncertainSubmission.attemptId)) return; + if ( + !uncertainSubmission || + !(confirmedSubmissionIds.has(uncertainSubmission.attemptId) || resentSubmissionIds.has(uncertainSubmission.attemptId)) + ) return; const { attemptId } = uncertainSubmission; const reconciled = reconciledSubmissionRef.current; if (reconciled && reconciled.draftKey === draftKey && reconciled.attemptId === attemptId) return; @@ -4891,7 +4918,48 @@ const IssueChatComposer = forwardRef< setBody(nextDraft); bodyRef.current = nextDraft; setComposerAttachments(nextAttachments); - }, [confirmedSubmissionIds, draftKey, sharedDraftKey, uncertainSubmission]); + }, [confirmedSubmissionIds, draftKey, resentSubmissionIds, sharedDraftKey, uncertainSubmission]); + + async function resendUncertainSubmission(): Promise { + if (!isCurrentAccount()) return "pending"; + const submission = uncertainSubmission; + const request = replayableRequest(submission, draftOwner); + if (!submission || !request) return "rejected"; + const taskKey = draftKey; + const args = [ + request.body, + request.reopen, + request.reassignment, + request.attachmentIds, + submission.attemptId, + ] as const; + try { + if (request.runSettings) await onSend(...args, request.runSettings as unknown as ComposerRunSettings); + else await onSend(...args); + } catch (error) { + if (!isCurrentAccount()) return "pending"; + const failure = classifySendFailure(error); + if (failure === "rejected") { + // A definitive answer: the text is already back in the editor. + if (taskKey) clearDraftSubmission(taskKey, submission.attemptId); + if (mountedTaskKey.current === taskKey) setUncertainSubmission(null); + } + return failure; + } + if (mountedTaskKey.current === taskKey) { + setResentSubmissionIds((current) => new Set([...current, submission.attemptId])); + } else if (taskKey) { + settleDraftSubmission(taskKey, submission.attemptId); + } + return "sent"; + } + + const resendSourceId = useId(); + const autoResend = useAutoResend({ + sourceId: `issue-chat-composer:${resendSourceId}`, + active: Boolean(replayableRequest(uncertainSubmission, draftOwner)) && !submitting, + resend: resendUncertainSubmission, + }); useEffect(() => { if ( @@ -5050,6 +5118,8 @@ const IssueChatComposer = forwardRef< bodyRef.current = ""; setBody(""); let attemptId: string | null = null; + let request: ComposerSendRequest | null = null; + const requestOwner = draftOwner; try { if (workModeChanged && onWorkModeChange) { await onWorkModeChange(pendingWorkMode); @@ -5061,10 +5131,20 @@ const IssueChatComposer = forwardRef< return; } attemptId = crypto.randomUUID(); + if (requestOwner === undefined) markRequestUnbound(attemptId); + // The exact request is stored before it is sent, so a lost response or + // an outage can resend it with the same ID instead of losing it. + request = { + body: submittedBody, + ...(reopen === undefined ? {} : { reopen }), + ...(reassignment ? { reassignment } : {}), + ...(attachmentIds.length ? { attachmentIds } : {}), + ...(runSettings ? { runSettings: runSettings as unknown as Record } : {}), + }; if (draftKey) { saveDraft(draftKey, trimmed); - saveDraftSubmission(draftKey, { attemptId, reviewed: false }); - pendingDraftRef.current = { draftKey, attemptId, submittedBody: trimmed, submittedAttachmentIds: attachmentIds }; + saveDraftSubmission(draftKey, { attemptId, reviewed: false, request, requestOwner }); + pendingDraftRef.current = { draftKey, attemptId, submittedBody: trimmed, submittedAttachmentIds: attachmentIds, request, requestOwner }; changeBody(bodyRef.current); } // assistant-ui thread.append is fire-and-forget. Await the actual Board @@ -5090,11 +5170,15 @@ const IssueChatComposer = forwardRef< } catch (error) { if (mountedTaskKey.current !== draftKey) return; const nextDraft = bodyRef.current; - if (attemptId && error instanceof CommentSubmissionUnknownError) { + // Not delivered, or not known to be: keep it and resend with the same + // ID when the connection allows. The server dedupes by that ID. + if (attemptId && request && classifySendFailure(error) === "pending") { const uncertain = { attemptId, reviewed: false, nextDraftOffset: trimmed.length + (nextDraft ? 2 : 0), submittedAttachmentIds: attachmentIds, + request, + requestOwner, }; setUncertainSubmission(uncertain); if (draftKey && loadDraftSubmission(draftKey)?.attemptId === attemptId) @@ -5197,7 +5281,8 @@ const IssueChatComposer = forwardRef< ? { ...item, status: "error", - error: err instanceof Error ? err.message : "Upload failed", + error: describeError(err, { action: "upload the file" }).body, + file, } : item, ), @@ -5205,6 +5290,14 @@ const IssueChatComposer = forwardRef< } } + /** Retry a failed upload from its kept file; only ever on an explicit click. */ + function retryUpload(attachment: ComposerAttachmentItem) { + const file = attachment.file; + if (!file) return; + setComposerAttachments((current) => current.filter((item) => item.id !== attachment.id)); + void attachFile(file); + } + async function handleAttachFile(evt: ChangeEvent) { const file = evt.target.files?.[0]; if (!file) return; @@ -5422,67 +5515,19 @@ const IssueChatComposer = forwardRef<

) : null} {uncertainSubmission ? ( -
-

- We couldn’t confirm whether this comment was saved. It may already - be in the conversation. Review it before starting another draft. -

- - {reviewError ? ( -

Couldn’t refresh the conversation. Try reviewing it again.

- ) : null} - {uncertainSubmission.reviewed ? ( - <> -

- Discarding this draft does not remove any saved comment or - uploaded file. -

- - - ) : null} -
+ { + // Stop resending; the text stays in the editor to edit or send again. + if (draftKey) clearDraftSubmission(draftKey, uncertainSubmission.attemptId); + setUncertainSubmission(null); + }} + /> ) : null} {statusLabel} + {attachment.status === "error" && attachment.file ? ( + + ) : null} {!attachment.inline || attachment.status !== "attached" ? ( : null} {canVoteOnDocument && doc.latestRevisionId ? ( ); } + +export function IssueDocumentsSection(props: Parameters[0]) { + const owner = useDraftOwner(); + const previousOwner = useRef(owner); + const generation = useRef(0); + if (previousOwner.current !== owner) { + if (previousOwner.current !== undefined) generation.current += 1; + previousOwner.current = owner; + } + return ; +} diff --git a/ui/src/components/NewIssueDialog.test.tsx b/ui/src/components/NewIssueDialog.test.tsx index e8e7cb53dcc..edd4adf5d1d 100644 --- a/ui/src/components/NewIssueDialog.test.tsx +++ b/ui/src/components/NewIssueDialog.test.tsx @@ -1,6 +1,7 @@ // @vitest-environment jsdom import type { ComponentProps, ReactNode } from "react"; +import { webcrypto } from "node:crypto"; import { flushSync } from "react-dom"; import { createRoot } from "react-dom/client"; import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; @@ -9,7 +10,7 @@ import { queryKeys } from "../lib/queryKeys"; import { trackRecentAssignee, trackRecentAssigneeUser } from "../lib/recent-assignees"; import { getLastProjectId, trackRecentProject } from "../lib/recent-projects"; import { rememberComposerEffort } from "../lib/recent-composer-effort"; -import { NewIssueDialog } from "./NewIssueDialog"; +import { addStagedFilesToIssue, NewIssueDialog } from "./NewIssueDialog"; const primaryState = vi.hoisted(() => ({ primaryAgentId: null as string | null, loading: false })); vi.mock("./primary-agent/PrimaryAgentPresentation", () => ({ usePrimaryAgentPresentation: () => primaryState })); @@ -550,7 +551,7 @@ describe("NewIssueDialog", () => { primaryState.primaryAgentId = primary; if (recent) trackRecentAssignee(recent, "company-1"); if (explicit) dialogState.newIssueDefaults = { assigneeAgentId: explicit }; - if (draft) localStorage.setItem("paperclip:issue-draft", JSON.stringify({ title: "Saved request", description: "", assigneeValue: `agent:${draft}`, projectId: "" })); + if (draft) localStorage.setItem("paperclip:issue-draft:user-1", JSON.stringify({ title: "Saved request", description: "", assigneeValue: `agent:${draft}`, projectId: "" })); const { root } = renderDialog(container); await waitForAssertion(() => expect(container.querySelector('[data-testid="task-chat-composer-assignee-label"]')?.textContent).toBe(expected)); act(() => root.unmount()); @@ -747,7 +748,7 @@ describe("NewIssueDialog", () => { }); it("restores private visibility from a saved draft without a project", async () => { - localStorage.setItem("paperclip:issue-draft", JSON.stringify({ title: "Private draft", description: "", status: "todo", priority: "", projectId: "", isPrivate: true })); + localStorage.setItem("paperclip:issue-draft:user-1", JSON.stringify({ title: "Private draft", description: "", status: "todo", priority: "", projectId: "", isPrivate: true })); const { root } = renderDialog(container); await flush(); const toggle = container.querySelector('button[data-testid="composer-private-chip"]')!; @@ -807,7 +808,7 @@ describe("NewIssueDialog", () => { // Saving this form must retain only the explicit toggle choice. A later // standalone draft must not acquire a sticky private flag from its parent. await act(async () => { await new Promise(resolve => setTimeout(resolve, 850)); }); - expect(JSON.parse(localStorage.getItem("paperclip:issue-draft")!).isPrivate).toBe(false); + expect(JSON.parse(localStorage.getItem("paperclip:issue-draft:user-1")!).isPrivate).toBe(false); const submit = Array.from(container.querySelectorAll("button")).find(button => button.getAttribute("aria-label") === "Create sub-task")!; await act(async () => submit.click()); await flush(); @@ -1056,7 +1057,7 @@ describe("NewIssueDialog", () => { id: "project-1", name: "Alpha", workspaces: [{ id: "source-default", isPrimary: true }], executionWorkspacePolicy: { enabled: true, defaultMode: "isolated_workspace" }, }]); - localStorage.setItem("paperclip:issue-draft", JSON.stringify({ + localStorage.setItem("paperclip:issue-draft:user-1", JSON.stringify({ title: "Draft task", description: "", status: "todo", priority: "medium", assigneeValue: "", reviewerValue: "", approverValue: "", projectId: "project-1", projectWorkspaceId: "stale-source", selectedExecutionWorkspaceId: "stale-workspace", executionWorkspaceMode: "reuse_existing", @@ -1327,7 +1328,7 @@ describe("NewIssueDialog", () => { it("restores a description-only draft", async () => { - localStorage.setItem("paperclip:issue-draft", JSON.stringify({ + localStorage.setItem("paperclip:issue-draft:user-1", JSON.stringify({ title: "", description: "Keep the request without a title", status: "todo", priority: "medium", assigneeValue: "", reviewerValue: "", approverValue: "", projectId: "", assigneeModelOverride: "", assigneeThinkingEffort: "", assigneeChrome: false, @@ -1565,6 +1566,300 @@ describe("NewIssueDialog", () => { act(() => root.unmount()); }); + it("keeps a create that hit an outage and resends it with the same key", async () => { + const { ApiError } = await import("../api/client"); + dialogState.newIssueDefaults = { title: "Created during a deploy" }; + mockIssuesApi.create + .mockRejectedValueOnce(new ApiError("Unavailable", 503, { error: "tenant_app_unavailable" })) + .mockResolvedValue({ id: "issue-created", identifier: "PAP-9", companyId: "company-1", projectId: null }); + const { root } = renderDialog(container); + await flush(); + const button = (label: string) => Array.from(container.querySelectorAll("button")) + .find((item) => item.getAttribute("aria-label") === label || item.textContent === label)!; + + act(() => button("Create task").click()); + await waitForAssertion(() => expect(container.textContent).toContain("Sending when reconnected…")); + expect(container.textContent).not.toContain("tenant_app_unavailable"); + expect(localStorage.getItem("paperclip:issue-draft:pending-create:user-1")).toContain( + mockIssuesApi.create.mock.calls[0][1].idempotencyKey, + ); + + // The waiting request is what gets created, so later edits are locked out. + expect(container.querySelector('input[aria-label="Task title"]')?.disabled).toBe(true); + + act(() => button("Resend now").click()); + await waitForAssertion(() => expect(mockIssuesApi.create).toHaveBeenCalledTimes(2)); + expect(mockIssuesApi.create.mock.calls[1][1].idempotencyKey).toBe(mockIssuesApi.create.mock.calls[0][1].idempotencyKey); + await waitForAssertion(() => expect(dialogState.closeNewIssue).toHaveBeenCalled()); + expect(localStorage.getItem("paperclip:issue-draft:pending-create:user-1")).toBeNull(); + act(() => root.unmount()); + }); + + it("shows why a resent create was rejected", async () => { + const { ApiError } = await import("../api/client"); + dialogState.newIssueDefaults = { title: "Rejected on resend" }; + mockIssuesApi.create + .mockRejectedValueOnce(new ApiError("Unavailable", 503, { error: "tenant_app_unavailable" })) + .mockRejectedValueOnce(new ApiError("Project is archived", 422, { error: "Project is archived" })); + const { root } = renderDialog(container); + await flush(); + const button = (label: string) => Array.from(container.querySelectorAll("button")) + .find((item) => item.getAttribute("aria-label") === label || item.textContent === label)!; + + act(() => button("Create task").click()); + await waitForAssertion(() => expect(container.textContent).toContain("Sending when reconnected…")); + act(() => button("Resend now").click()); + await waitForAssertion(() => expect(container.querySelector('[role="alert"]')?.textContent).toContain("Project is archived")); + expect(container.textContent).not.toContain("Sending when reconnected…"); + expect(container.querySelector('input[aria-label="Task title"]')?.disabled).toBe(false); + act(() => root.unmount()); + }); + + it("never creates a restored task without the files it was submitted with", async () => { + dialogState.newIssueDefaults = { title: "Has files" }; + localStorage.setItem("paperclip:issue-draft:pending-create:user-1", JSON.stringify({ + version: 1, + key: "paperclip:issue-draft:pending-create:user-1", + idempotencyKey: "create-key-123", + createdAt: Date.now(), + payload: { + fingerprint: "{}", + data: { companyId: "company-1", title: "Has files" }, + stagedFileNames: ["spec.pdf"], + }, + })); + const { root } = renderDialog(container); + await flush(); + + await waitForAssertion(() => expect(container.querySelector('[role="alert"]')?.textContent).toContain("spec.pdf")); + expect(container.querySelector('[role="alert"]')?.textContent).toContain("Attach it again"); + await act(async () => { container.querySelector('[aria-label="Create task"]')!.click(); }); + await flush(); + expect(mockIssuesApi.create).not.toHaveBeenCalled(); + expect(localStorage.getItem("paperclip:issue-draft:pending-create:user-1")).toBeNull(); + // The original key outlives the rejection, so it survives another reload. + expect(localStorage.getItem("paperclip:issue-draft:create-awaiting-files:user-1")).toContain("create-key-123"); + act(() => root.unmount()); + + const reloaded = renderDialog(container); + await flush(); + expect(container.querySelector('[role="alert"]')?.textContent).toContain("spec.pdf"); + expect(localStorage.getItem("paperclip:issue-draft:create-awaiting-files:user-1")).toContain("create-key-123"); + expect(mockIssuesApi.create).not.toHaveBeenCalled(); + act(() => reloaded.root.unmount()); + }); + + it.each([false, true])("keeps restored create payload immutable when files are reattached (changed title: %s)", async changed => { + mockIssuesApi.create.mockResolvedValue({ id: "issue-1", identifier: "PAP-1", companyId: "company-1" }); + mockIssuesApi.uploadAttachment.mockResolvedValue({}); + dialogState.newIssueDefaults = { title: "Has files" }; + const spec = new File(["a"], "spec.pdf", { lastModified: 1 }); + const notes = new File(["bb"], "notes.pdf", { lastModified: 2 }); + localStorage.setItem("paperclip:issue-draft:pending-create:user-1", JSON.stringify({ + version: 1, + key: "paperclip:issue-draft:pending-create:user-1", + idempotencyKey: "create-key-123", + createdAt: Date.now(), + payload: { + fingerprint: JSON.stringify({ stagedFiles: ["attachment::spec.pdf:1:1", "attachment::notes.pdf:2:2"] }), + data: { companyId: "company-1", title: "Has files" }, + stagedFileNames: ["spec.pdf", "notes.pdf"], + }, + })); + const { root } = renderDialog(container); + await flush(); + await waitForAssertion(() => expect(container.querySelector('[role="alert"]')?.textContent).toContain("spec.pdf")); + + const input = container.querySelector('input[type="file"]')!; + Object.defineProperty(input, "files", { configurable: true, value: [notes, spec] }); + await act(async () => { + input.dispatchEvent(new Event("change", { bubbles: true })); + }); + await flush(); + if (changed) await typeTextareaValue(container.querySelector('input[aria-label="Task title"]')!, "Changed after restore"); + const send = container.querySelector('[aria-label="Create task"]')!; + await act(async () => { send.click(); }); + await flush(); + if (changed) { + expect(mockIssuesApi.create).not.toHaveBeenCalled(); + expect(container.textContent).toContain("original fields"); + } else { + await waitForAssertion(() => expect(mockIssuesApi.create).toHaveBeenCalledTimes(1)); + expect(mockIssuesApi.create.mock.calls[0][1].idempotencyKey).toBe("create-key-123"); + expect(mockIssuesApi.create.mock.calls[0][1].title).toBe("Has files"); + } + act(() => root.unmount()); + }); + + it("drops an account's restored create and staged files when the account changes", async () => { + dialogState.newIssueDefaults = { title: "Account A saved task" }; + const file = new File(["a"], "spec.pdf", { lastModified: 1 }); + const original = JSON.stringify({ version: 1, key: "paperclip:issue-draft:create-awaiting-files:user-1", idempotencyKey: "account-a-create-key", createdAt: Date.now(), payload: { fingerprint: JSON.stringify({ stagedFiles: ["attachment::spec.pdf:1:1"] }), data: { companyId: "company-1", title: "Account A saved task" }, stagedFileNames: ["spec.pdf"] } }); + localStorage.setItem("paperclip:issue-draft:create-awaiting-files:user-1", original); + const { root, queryClient } = renderDialog(container); + try { + await waitForAssertion(() => expect(container.textContent).toContain("spec.pdf")); + const input = container.querySelector('input[type="file"]')!; + Object.defineProperty(input, "files", { configurable: true, value: [file] }); + await act(async () => { input.dispatchEvent(new Event("change", { bubbles: true })); }); await flush(); + mockAuthApi.getSession.mockResolvedValue({ user: { id: "user-2" } }); + queryClient.setQueryData(queryKeys.auth.session, { user: { id: "user-2" } }); + await waitForAssertion(() => expect(container.querySelector('input[aria-label="Task title"]')?.value ?? "").toBe("")); + expect(container.textContent).not.toContain("spec.pdf"); + expect(localStorage.getItem("paperclip:issue-draft:create-awaiting-files:user-1")).toBe(original); + await typeTextareaValue(container.querySelector('textarea[aria-label="Describe a task…"]')!, "Account B fresh task"); + mockIssuesApi.create.mockResolvedValue({ id: "task-b", identifier: "PAP-2", companyId: "company-1" }); + await act(async () => container.querySelector('[aria-label="Create task"]')!.click()); + await waitForAssertion(() => expect(mockIssuesApi.create).toHaveBeenCalledTimes(1)); + expect(mockIssuesApi.create.mock.calls[0][1].description).toBe("Account B fresh task"); + expect(mockIssuesApi.create.mock.calls[0][1].idempotencyKey).not.toBe("account-a-create-key"); + expect(mockIssuesApi.uploadAttachment).not.toHaveBeenCalled(); + } finally { act(() => root.unmount()); queryClient.clear(); } + }); + + it.each([false, true])("retains a restored task backup after the debounce (session known=%s)", async (sessionKnown) => { + if (!sessionKnown) mockAuthApi.getSession.mockReturnValue(new Promise(() => {})); + localStorage.setItem("paperclip:issue-draft:user-1", JSON.stringify({ + title: "", description: "Keep this restored backup", status: "todo", priority: "", + assigneeValue: "", reviewerValue: "", approverValue: "", watchdogAgentId: "", watchdogInstructions: "", + projectId: "", projectWorkspaceId: "", assigneeModelLane: "primary", assigneeModelOverride: "", + assigneeThinkingEffort: "", assigneeChrome: false, executionWorkspaceMode: "shared_workspace", + selectedExecutionWorkspaceId: "", workMode: "standard", isPrivate: false, composerSettings: null, + })); + const { root, queryClient } = renderDialog(container); + try { + await flush(); + if (!sessionKnown) queryClient.setQueryData(queryKeys.auth.session, { user: { id: "user-1" } }); + await waitForAssertion(() => expect(container.querySelector('textarea[aria-label="Describe a task…"]')?.value).toBe("Keep this restored backup")); + await act(async () => { await new Promise((resolve) => setTimeout(resolve, 850)); }); + expect(JSON.parse(localStorage.getItem("paperclip:issue-draft:user-1") ?? "null")?.description).toBe("Keep this restored backup"); + dialogState.newIssueOpen = false; + act(() => root.render()); + await flush(); + dialogState.newIssueOpen = true; + act(() => root.render()); + await flush(); + expect(container.querySelector('textarea[aria-label="Describe a task…"]')?.value).toBe("Keep this restored backup"); + } finally { act(() => root.unmount()); queryClient.clear(); } + }); + + it.each([false, true])("keeps erased task text erased after reopening (session known=%s)", async (sessionKnown) => { + if (!sessionKnown) mockAuthApi.getSession.mockReturnValue(new Promise(() => {})); + localStorage.setItem("paperclip:issue-draft:user-1", JSON.stringify({ title: "", description: "Older saved text" })); + const { root, queryClient } = renderDialog(container); + try { + await flush(); + const editor = container.querySelector('textarea[aria-label="Describe a task…"]')!; + await typeTextareaValue(editor, "Erase this task text"); + await typeTextareaValue(editor, ""); + if (!sessionKnown) queryClient.setQueryData(queryKeys.auth.session, { user: { id: "user-1" } }); + await act(async () => { await new Promise((resolve) => setTimeout(resolve, 850)); }); + await waitForAssertion(() => expect(localStorage.getItem("paperclip:issue-draft:user-1")).toBeNull()); + dialogState.newIssueOpen = false; + act(() => root.render()); + await flush(); + dialogState.newIssueOpen = true; + act(() => root.render()); + await flush(); + expect(container.querySelector('textarea[aria-label="Describe a task…"]')?.value).toBe(""); + expect(container.textContent).not.toContain("Erase this task text"); + } finally { act(() => root.unmount()); queryClient.clear(); } + }); + + it.each([false, true])("keeps task typing through the first session lookup (storage unavailable=%s)", async (unavailable) => { + mockAuthApi.getSession.mockReturnValue(new Promise(() => {})); + const { root, queryClient } = renderDialog(container); + let storage: ReturnType | undefined; + try { + await flush(); + await typeTextareaValue(container.querySelector('textarea[aria-label="Describe a task…"]')!, "Typed before account lookup"); + if (unavailable) storage = vi.spyOn(Storage.prototype, "setItem").mockImplementation(() => { throw new Error("Storage unavailable"); }); + queryClient.setQueryData(queryKeys.auth.session, { user: { id: "user-1" } }); + await flush(); await flush(); + expect(container.querySelector('textarea[aria-label="Describe a task…"]')?.value).toBe("Typed before account lookup"); + if (!unavailable) expect(localStorage.getItem("paperclip:issue-draft:user-1")).toContain("Typed before account lookup"); + expect(mockIssuesApi.create).not.toHaveBeenCalled(); + } finally { storage?.mockRestore(); act(() => root.unmount()); queryClient.clear(); } + }); + + it.each(["create", "upload"] as const)("stops old staged work after an account change during %s", async (during) => { + dialogState.newIssueDefaults = { title: "Account A in-flight task" }; + let finish!: (value: unknown) => void; + const waiting = new Promise(resolve => { finish = resolve; }); + const created = { id: "task-a", identifier: "PAP-1", companyId: "company-1" }; + mockIssuesApi.create.mockImplementation(() => during === "create" ? waiting : Promise.resolve(created)); + mockIssuesApi.uploadAttachment.mockImplementation(() => during === "upload" ? waiting : Promise.resolve({})); + const { root, queryClient } = renderDialog(container); + try { + await flush(); await flush(); + const input = container.querySelector('input[type="file"]')!; + Object.defineProperty(input, "files", { configurable: true, value: [new File(["a"], "first.pdf"), new File(["b"], "second.pdf")] }); + await act(async () => { input.dispatchEvent(new Event("change", { bubbles: true })); }); await flush(); + await act(async () => { container.querySelector('[aria-label="Create task"]')!.click(); }); + await waitForAssertion(() => expect(mockIssuesApi.create).toHaveBeenCalledTimes(1)); + if (during === "upload") await waitForAssertion(() => expect(mockIssuesApi.uploadAttachment).toHaveBeenCalledTimes(1)); + const key = mockIssuesApi.create.mock.calls[0][1].idempotencyKey; + mockAuthApi.getSession.mockResolvedValue({ user: { id: "user-2" } }); + queryClient.setQueryData(queryKeys.auth.session, { user: { id: "user-2" } }); + await flush(); await flush(); + finish(during === "create" ? created : {}); + await waitForAssertion(() => { + const mutation = queryClient.getMutationCache().getAll().find(item => (item.state.variables as { idempotencyKey?: string } | undefined)?.idempotencyKey === key); + expect(mutation?.state.status).toMatch(/^(success|error)$/); + }); + expect(mockIssuesApi.uploadAttachment).toHaveBeenCalledTimes(during === "create" ? 0 : 1); + expect(dialogState.closeNewIssue).not.toHaveBeenCalled(); + expect(localStorage.getItem("paperclip:issue-draft:pending-create:user-1")).toContain(key); + expect(container.textContent).not.toContain("Account A in-flight task"); + } finally { act(() => root.unmount()); queryClient.clear(); } + }); + + it("restores original document keys when colliding document names are reattached in reverse", async () => { + dialogState.newIssueDefaults = { title: "Has documents" }; + mockIssuesApi.create.mockResolvedValue({ id: "issue-1", identifier: "PAP-1", companyId: "company-1" }); + mockIssuesApi.upsertDocument.mockResolvedValue({}); + const markdown = new File(["a"], "notes.md", { type: "text/markdown", lastModified: 1 }); + const text = new File(["bb"], "notes.txt", { type: "text/plain", lastModified: 2 }); + localStorage.setItem("paperclip:issue-draft:create-awaiting-files:user-1", JSON.stringify({ version: 1, key: "paperclip:issue-draft:create-awaiting-files:user-1", idempotencyKey: "original-doc-create-key", createdAt: Date.now(), payload: { fingerprint: JSON.stringify({ stagedFiles: ["document:notes:notes.md:1:1", "document:notes-2:notes.txt:2:2"] }), data: { companyId: "company-1", title: "Has documents" }, stagedFileNames: ["notes.md", "notes.txt"] } })); + const { root } = renderDialog(container); + try { + await waitForAssertion(() => expect(container.textContent).toContain("notes.md")); + const input = container.querySelector('input[type="file"]')!; + Object.defineProperty(input, "files", { configurable: true, value: [text, markdown] }); + await act(async () => { input.dispatchEvent(new Event("change", { bubbles: true })); }); await flush(); + await act(async () => container.querySelector('[aria-label="Create task"]')!.click()); + await waitForAssertion(() => expect(mockIssuesApi.create).toHaveBeenCalledTimes(1)); + expect(mockIssuesApi.create.mock.calls[0][1].idempotencyKey).toBe("original-doc-create-key"); + await waitForAssertion(() => expect(mockIssuesApi.upsertDocument).toHaveBeenCalledTimes(2)); + expect(mockIssuesApi.upsertDocument).toHaveBeenCalledWith("issue-1", "notes", expect.objectContaining({ body: "a" })); + expect(mockIssuesApi.upsertDocument).toHaveBeenCalledWith("issue-1", "notes-2", expect.objectContaining({ body: "bb" })); + } finally { act(() => root.unmount()); } + }); + + it("restores a saved subtask's parent and goal when opened from the normal New Task action", async () => { + dialogState.newIssueDefaults = {}; + mockIssuesApi.create.mockResolvedValue({ id: "issue-1", identifier: "PAP-1", companyId: "company-1" }); + mockIssuesApi.uploadAttachment.mockResolvedValue({}); + mockIssuesApi.get.mockResolvedValue({ id: "parent-1", identifier: "PAP-2", title: "Parent", isPrivate: false }); + localStorage.setItem("paperclip:issue-draft:user-1", JSON.stringify({ title: "Saved subtask", description: "", status: "todo", priority: "", projectId: "", assigneeValue: "" })); + localStorage.setItem("paperclip:issue-draft:create-awaiting-files:user-1", JSON.stringify({ + version: 1, key: "paperclip:issue-draft:create-awaiting-files:user-1", idempotencyKey: "subtask-key-123", createdAt: Date.now(), + payload: { fingerprint: JSON.stringify({ stagedFiles: ["attachment::spec.pdf:1:1"] }), + data: { companyId: "company-1", title: "Saved subtask", parentId: "parent-1", goalId: "goal-1" }, stagedFileNames: ["spec.pdf"] }, + })); + const { root } = renderDialog(container); + await flush(); + const input = container.querySelector('input[type="file"]')!; + Object.defineProperty(input, "files", { configurable: true, value: [new File(["a"], "spec.pdf", { lastModified: 1 })] }); + await act(async () => { input.dispatchEvent(new Event("change", { bubbles: true })); }); + await flush(); + await act(async () => { container.querySelector('[aria-label="Create sub-task"]')!.click(); }); + await flush(); + expect(mockIssuesApi.create).toHaveBeenCalledTimes(1); + expect(mockIssuesApi.create.mock.calls[0]?.[1]).toMatchObject({ idempotencyKey: "subtask-key-123", title: "Saved subtask", parentId: "parent-1", goalId: "goal-1" }); + act(() => root.unmount()); + }); + it("submits Chinese, Japanese, and Hindi issue text without normalization", async () => { const title = "验证中文任务"; const description = [ @@ -2035,7 +2330,7 @@ describe("NewIssueDialog", () => { enableIsolatedWorkspaces: false, }); localStorage.setItem( - "paperclip:issue-draft", + "paperclip:issue-draft:user-1", JSON.stringify({ title: "Watched task", description: "", @@ -2099,7 +2394,7 @@ describe("NewIssueDialog", () => { it("restores shared model, effort, and fast settings into the created task", async () => { mockAgentsApi.list.mockResolvedValue([{ id: "agent-1", name: "Coder", status: "active", adapterType: "codex_local", adapterConfig: {}, runtimeConfig: {}, permissions: {} }]); - localStorage.setItem("paperclip:issue-draft", JSON.stringify({ + localStorage.setItem("paperclip:issue-draft:user-1", JSON.stringify({ title: "", description: "Use the saved run settings", status: "todo", priority: "medium", assigneeValue: "agent:agent-1", projectId: "", workMode: "planning", composerSettings: { model: "gpt-6-astra", effort: "ultra", fast: true }, @@ -2135,3 +2430,70 @@ describe("NewIssueDialog", () => { }); }); + +describe("addStagedFilesToIssue", () => { + beforeEach(() => { + vi.stubGlobal("crypto", webcrypto); + localStorage.clear(); + mockIssuesApi.uploadAttachment.mockReset(); + }); + + afterEach(() => vi.unstubAllGlobals()); + + it("keeps a confirmed upload receipt when ownership changes before the next file", async () => { + const files = ["one", "two"].map((name, index) => ({ id: name, kind: "attachment" as const, file: new File([name], `${name}.pdf`, { lastModified: index + 1 }) })); + let owned = true; + const assertCurrentOwner = () => { if (!owned) throw new Error("Account changed"); }; + mockIssuesApi.uploadAttachment.mockResolvedValue({}).mockImplementationOnce(async () => { owned = false; return {}; }); + const input = { companyId: "company-1", issueId: "task-1", stagedFiles: files, idempotencyKey: "original-upload-request", assertCurrentOwner }; + await expect(addStagedFilesToIssue(input)).rejects.toThrow("Account changed"); + expect(mockIssuesApi.uploadAttachment).toHaveBeenCalledTimes(1); + owned = true; + await expect(addStagedFilesToIssue(input)).resolves.toEqual([]); + expect(mockIssuesApi.uploadAttachment.mock.calls.map(call => call[2].name)).toEqual(["one.pdf", "two.pdf"]); + }); + + it("uploads distinct files with the same metadata instead of treating one as a receipt", async () => { + const stagedFiles = ["a", "b"].map((body, index) => ({ + id: `staged-${index}`, + kind: "attachment" as const, + file: new File([body], "report.txt", { lastModified: 123 }), + })); + mockIssuesApi.uploadAttachment.mockResolvedValue({}); + expect(await addStagedFilesToIssue({ companyId: "company-1", issueId: "issue-1", idempotencyKey: "metadata-collision", stagedFiles })).toEqual([]); + expect(mockIssuesApi.uploadAttachment).toHaveBeenCalledTimes(2); + expect(mockIssuesApi.uploadAttachment.mock.calls[0]?.[2]).toBe(stagedFiles[0]!.file); + expect(mockIssuesApi.uploadAttachment.mock.calls[1]?.[2]).toBe(stagedFiles[1]!.file); + }); + + it("still uploads files when browser hashing is unavailable", async () => { + vi.stubGlobal("crypto", {}); + const stagedFiles = ["a", "b"].map((body, index) => ({ + id: `staged-${index}`, kind: "attachment" as const, + file: new File([body], "report.txt", { lastModified: 123 }), + })); + mockIssuesApi.uploadAttachment.mockResolvedValue({}); + expect(await addStagedFilesToIssue({ companyId: "company-1", issueId: "issue-1", idempotencyKey: "hashing-unavailable", stagedFiles })).toEqual([]); + expect(mockIssuesApi.uploadAttachment).toHaveBeenCalledTimes(2); + }); + + it("never adds a file twice when a create replays after a reload mid-upload", async () => { + const spec = { id: "staged-1", kind: "attachment" as const, file: new File(["a"], "spec.pdf", { lastModified: 1 }) }; + const notes = { id: "staged-2", kind: "attachment" as const, file: new File(["bb"], "notes.txt", { lastModified: 2 }) }; + mockIssuesApi.uploadAttachment.mockResolvedValueOnce({}).mockRejectedValueOnce(new Error("Upload interrupted")); + const target = { companyId: "company-1", issueId: "issue-1", idempotencyKey: "create-key-123" }; + expect(await addStagedFilesToIssue({ ...target, stagedFiles: [spec, notes] })).toEqual(["notes.txt"]); + + // Reattached after a reload: new staging ids, the same files, the same key. + mockIssuesApi.uploadAttachment.mockResolvedValue({}); + expect(await addStagedFilesToIssue({ + ...target, + stagedFiles: [{ ...spec, id: "staged-3" }, { ...notes, id: "staged-4" }], + })).toEqual([]); + expect(mockIssuesApi.uploadAttachment.mock.calls.map((call) => (call[2] as File).name)).toEqual([ + "spec.pdf", + "notes.txt", + "notes.txt", + ]); + }); +}); diff --git a/ui/src/components/NewIssueDialog.tsx b/ui/src/components/NewIssueDialog.tsx index 42989bae966..0eae135e726 100644 --- a/ui/src/components/NewIssueDialog.tsx +++ b/ui/src/components/NewIssueDialog.tsx @@ -43,10 +43,74 @@ import { TaskChatPresentationProvider } from "./task-chat/presentation-mode"; import { mergeComposerRunSettings, type ComposerRunSettings } from "./task-chat/composer-run-settings"; import { useSidebar } from "../context/SidebarContext"; import { InlineBanner } from "./InlineBanner"; +import { PendingSendNotice } from "./PendingSendNotice"; +import { useDurableSubmit } from "../hooks/useDurableSubmit"; +import { describeError, type ErrorDescription } from "../api/errors"; +import { clearPendingSend, pendingSendKeyFor, readPendingSend, writePendingSend, PendingSendAccountChangedError } from "../lib/pending-send"; +import { useDraftOwner, useDraftOwnerReader } from "../hooks/useDraftOwner"; import { InlineEntitySelector, type InlineEntityOption } from "./InlineEntitySelector"; import { getTrustPreset } from "../lib/trust-policy-ui"; const DRAFT_KEY = "paperclip:issue-draft"; +/** A create that is waiting to resend: its request body and idempotency key. */ +const PENDING_CREATE_KEY = "paperclip:issue-draft:pending-create"; + +/** + * The create request. Staged files cannot be stored, so they ride in memory and + * only their names are saved: a create restored after reload knows it needs them. + */ +type PendingCreate = { fingerprint: string; data: Record; stagedFileNames?: string[] }; + +function isPendingCreate(value: unknown): value is PendingCreate { + if (!value || typeof value !== "object" || Array.isArray(value)) return false; + const row = value as Record; + return ( + typeof row.fingerprint === "string" && + !!row.data && typeof row.data === "object" && !Array.isArray(row.data) && + typeof (row.data as Record).companyId === "string" && + (row.stagedFileNames === undefined || + (Array.isArray(row.stagedFileNames) && row.stagedFileNames.every((name) => typeof name === "string"))) + ); +} + +/** Thrown before sending a restored create whose staged files are gone from memory. */ +class StagedFilesMissingError extends Error { + constructor(names: string[]) { + super( + `This page no longer has ${names.length === 1 ? "the file" : `the ${names.length} files`} staged for this task (${names.join(", ")}). Attach ${names.length === 1 ? "it" : "them"} again, then create the task.`, + ); + this.name = "StagedFilesMissingError"; + } +} + +/** + * A restored create that could not be sent for lack of its files. Its key is + * kept until a create succeeds or the user cancels, so reattaching after any + * number of reloads replays a create that already reached the server. + */ +const CREATE_AWAITING_FILES_KEY = "paperclip:issue-draft:create-awaiting-files"; + +function stagedFilesMissingDescription(names: string[]): ErrorDescription { + return describeError(new StagedFilesMissingError(names), { action: "create the task" }); +} + +/** Identifies a staged file across reselection from disk, unlike its per-staging `id`. */ +function stagedFileIdentity(file: StagedIssueFile): string { + return [file.kind, file.documentKey ?? "", file.file.name, file.file.size, file.file.lastModified].join(":"); +} +function restoreStagedDocumentKeys(files: StagedIssueFile[], fingerprint: string | undefined): StagedIssueFile[] { + let required: unknown; + try { required = JSON.parse(fingerprint ?? "{}").stagedFiles; } catch { return files; } + if (!Array.isArray(required)) return files; + return files.map(file => { + if (file.kind !== "document") return file; + const suffix = `:${file.file.name}:${file.file.size}:${file.file.lastModified}`; + const matches = required.filter((identity): identity is string => typeof identity === "string" && identity.startsWith("document:") && identity.endsWith(suffix)); + if (matches.length !== 1) return file; + return { ...file, documentKey: matches[0].slice("document:".length, -suffix.length) }; + }); +} + const DEBOUNCE_MS = 800; type VisualViewportLayout = { @@ -146,12 +210,103 @@ type StagedIssueFile = { title?: string | null; }; +/** + * Files a keyed create has already added to its task. A replayed create (same + * key, after a reload mid-upload) gets the existing task back from the + * server, and must not add those files a second time. + */ +const CREATE_UPLOAD_RECEIPTS_PREFIX = "paperclip:issue-draft:create-uploads:"; + +function loadCreateUploadReceipts(idempotencyKey: string): Set { + try { + const stored = JSON.parse(localStorage.getItem(CREATE_UPLOAD_RECEIPTS_PREFIX + idempotencyKey) ?? "[]"); + return new Set(Array.isArray(stored) ? stored.filter((item): item is string => typeof item === "string") : []); + } catch { + return new Set(); + } +} + +function saveCreateUploadReceipts(idempotencyKey: string, receipts: Set) { + try { + localStorage.setItem(CREATE_UPLOAD_RECEIPTS_PREFIX + idempotencyKey, JSON.stringify([...receipts])); + } catch { + // Without storage a replay after reload may add a file twice; nothing is lost. + } +} + +export function clearCreateUploadReceipts(idempotencyKey: string) { + try { + localStorage.removeItem(CREATE_UPLOAD_RECEIPTS_PREFIX + idempotencyKey); + } catch { + // Unavailable storage holds nothing to clear. + } +} + +/** Add staged files to a created task, skipping any this create already added. Returns the names that failed. */ +export async function addStagedFilesToIssue(input: { + companyId: string; + issueId: string; + stagedFiles: StagedIssueFile[]; + idempotencyKey?: string; + assertCurrentOwner?: () => void; +}): Promise { + const { companyId, issueId, stagedFiles, idempotencyKey } = input; + const receipts = idempotencyKey ? loadCreateUploadReceipts(idempotencyKey) : new Set(); + const failures: string[] = []; + for (const stagedFile of stagedFiles) { + input.assertCurrentOwner?.(); + let identity: string | undefined; + if (idempotencyKey && globalThis.crypto?.subtle) { + try { + const bytes = await new Promise((resolve, reject) => { + const reader = new FileReader(); + reader.onload = () => resolve(reader.result as ArrayBuffer); + reader.onerror = () => reject(reader.error); + reader.onabort = () => reject(new Error("File read cancelled")); + reader.readAsArrayBuffer(stagedFile.file); + }); + const digest = await crypto.subtle.digest("SHA-256", bytes); + const hash = Array.from(new Uint8Array(digest), byte => byte.toString(16).padStart(2, "0")).join(""); + identity = JSON.stringify([stagedFileIdentity(stagedFile), hash]); + } catch { + identity = undefined; + } + } + input.assertCurrentOwner?.(); + if (identity && receipts.has(identity)) continue; + try { + if (stagedFile.kind === "document") { + const body = await stagedFile.file.text(); + input.assertCurrentOwner?.(); + await issuesApi.upsertDocument(issueId, stagedFile.documentKey ?? "document", { + title: stagedFile.documentKey === "plan" ? null : (stagedFile.title ?? null), + format: "markdown", + body, + baseRevisionId: null, + }); + } else { + await issuesApi.uploadAttachment(companyId, issueId, stagedFile.file); + } + if (identity && idempotencyKey) { + receipts.add(identity); + saveCreateUploadReceipts(idempotencyKey, receipts); + } + input.assertCurrentOwner?.(); + } catch { + input.assertCurrentOwner?.(); + failures.push(stagedFile.file.name); + } + } + return failures; +} + import { Badge } from "@/components/ui/badge"; import { buildAssigneeAdapterOverrides, type IssueModelLane } from "../lib/issue-assignee-overrides"; -function loadDraft(): IssueDraft | null { +function loadDraft(owner: string | undefined): IssueDraft | null { + if (owner === undefined) return null; try { - const raw = localStorage.getItem(DRAFT_KEY); + const raw = localStorage.getItem(pendingSendKeyFor(DRAFT_KEY, owner)); if (!raw) return null; return JSON.parse(raw) as IssueDraft; } catch { @@ -159,12 +314,12 @@ function loadDraft(): IssueDraft | null { } } -function saveDraft(draft: IssueDraft) { - localStorage.setItem(DRAFT_KEY, JSON.stringify(draft)); +function saveDraft(owner: string, draft: IssueDraft) { + try { localStorage.setItem(pendingSendKeyFor(DRAFT_KEY, owner), JSON.stringify(draft)); } catch {} } -function clearDraft() { - localStorage.removeItem(DRAFT_KEY); +function clearDraft(owner: string | undefined) { + try { if (owner !== undefined) localStorage.removeItem(pendingSendKeyFor(DRAFT_KEY, owner)); } catch {} } function isTextDocumentFile(file: File) { @@ -277,8 +432,24 @@ function isWorkModeEscapeShortcut(e: Pick) { } export function NewIssueDialog() { + const owner = useDraftOwner(); + const { newIssueDefaults } = useDialog(); + const [scope, setScope] = useState({ owner, generation: 0, ignoredDefaults: null as typeof newIssueDefaults | null }); + const changed = scope.owner !== owner; + const knownChange = changed && scope.owner !== undefined; + const next = changed ? { owner, generation: scope.generation + (knownChange ? 1 : 0), ignoredDefaults: knownChange ? newIssueDefaults : scope.ignoredDefaults } : scope; + if (changed) setScope(next); + return ; +} + +function AccountNewIssueDialog({ ignoreDefaults }: { ignoreDefaults: boolean }) { const { visible: workspaceIsolationControlsVisible } = useWorkspaceIsolationControls(); - const { newIssueOpen, newIssueDefaults, closeNewIssue } = useDialog(); + const { newIssueOpen, newIssueDefaults: sharedDefaults, closeNewIssue } = useDialog(); + const newIssueDefaults = useMemo(() => ignoreDefaults ? {} as typeof sharedDefaults : sharedDefaults, [ignoreDefaults, sharedDefaults]); + const readOwner = useDraftOwnerReader(); + const active = useRef(true); + useEffect(() => { active.current = true; return () => { active.current = false; }; }, []); + const unboundFormDraft = useRef(null); const visualViewportLayout = useVisualViewportLayout(newIssueOpen); const dialogBodyRef = useRef(null); const { companies, selectedCompanyId: effectiveCompanyId } = useCompany(); @@ -317,12 +488,24 @@ export function NewIssueDialog() { const initializationKeyRef = useRef(null); const defaultAssigneePendingRef = useRef(false); const defaultProjectPendingRef = useRef(false); - const createRequestRef = useRef<{ fingerprint: string; idempotencyKey: string } | null>(null); + // `recovering`: a create restored without its files keeps its key until it is + // sent or cancelled, whatever the files and fields look like when resubmitted. + const createRequestRef = useRef<{ fingerprint: string; idempotencyKey: string; recovering?: boolean; payload?: PendingCreate } | null>(null); + const draftOwner = useDraftOwner(); + const [restoredLaunch, setRestoredLaunch] = useState<{ owner: string | undefined; parentId?: string; goalId?: string } | null>(null); + const savedLaunch = restoredLaunch?.owner === draftOwner ? restoredLaunch : null; + const parentId = savedLaunch ? savedLaunch.parentId : newIssueDefaults.parentId; + const goalId = savedLaunch ? savedLaunch.goalId : newIssueDefaults.goalId; + const restoreLaunch = (payload: PendingCreate) => setRestoredLaunch({ owner: draftOwner, + ...(typeof payload.data.parentId === "string" ? { parentId: payload.data.parentId } : {}), + ...(typeof payload.data.goalId === "string" ? { goalId: payload.data.goalId } : {}), + }); + const awaitingFilesKey = draftOwner !== undefined ? pendingSendKeyFor(CREATE_AWAITING_FILES_KEY, draftOwner) : null; const primaryAgent = usePrimaryAgentPresentation(effectiveCompanyId); - const isSubIssueMode = Boolean(newIssueDefaults.parentId); + const isSubIssueMode = Boolean(parentId); const parentIssueLabel = - newIssueDefaults.parentIdentifier ?? (newIssueDefaults.parentId ? newIssueDefaults.parentId.slice(0, 8) : ""); + (newIssueDefaults.parentId === parentId ? newIssueDefaults.parentIdentifier : undefined) ?? (parentId ? parentId.slice(0, 8) : ""); const parentExecutionWorkspaceId = newIssueDefaults.executionWorkspaceId ?? ""; const parentExecutionWorkspaceLabel = newIssueDefaults.parentExecutionWorkspaceLabel ?? parentExecutionWorkspaceId; @@ -391,9 +574,9 @@ export function NewIssueDialog() { }); const { data: privacyParent, isError: parentPrivacyError, refetch: refetchParentPrivacy } = useQuery({ - queryKey: queryKeys.issues.detail(newIssueDefaults.parentId ?? ""), - queryFn: () => issuesApi.get(newIssueDefaults.parentId!), - enabled: newIssueOpen && Boolean(newIssueDefaults.parentId), + queryKey: queryKeys.issues.detail(parentId ?? ""), + queryFn: () => issuesApi.get(parentId!), + enabled: newIssueOpen && Boolean(parentId), }); const parentPrivacyUnresolved = isSubIssueMode && !privacyParent; const inheritsPrivateAccess = privacyParent?.visibility === "private" || privacyParent?.project?.visibility === "private"; @@ -423,6 +606,9 @@ export function NewIssueDialog() { }); }, [agents, companyMembers?.users, orderedProjects, mentionIssues]); + const assertCurrentOwner = () => { + if (!active.current || draftOwner === undefined || readOwner() !== draftOwner) throw new PendingSendAccountChangedError(); + }; const createIssue = useMutation({ mutationFn: async ({ companyId, @@ -430,30 +616,22 @@ export function NewIssueDialog() { navigateOnCreate, ...data }: { companyId: string; stagedFiles: StagedIssueFile[]; navigateOnCreate?: boolean } & Record) => { + assertCurrentOwner(); const issue = await issuesApi.create(companyId, data); - const failures: string[] = []; - - for (const stagedFile of pendingStagedFiles) { - try { - if (stagedFile.kind === "document") { - const body = await stagedFile.file.text(); - await issuesApi.upsertDocument(issue.id, stagedFile.documentKey ?? "document", { - title: stagedFile.documentKey === "plan" ? null : (stagedFile.title ?? null), - format: "markdown", - body, - baseRevisionId: null, - }); - } else { - await issuesApi.uploadAttachment(companyId, issue.id, stagedFile.file); - } - } catch { - failures.push(stagedFile.file.name); - } - } + assertCurrentOwner(); + const failures = await addStagedFilesToIssue({ + companyId, + issueId: issue.id, + stagedFiles: pendingStagedFiles, + assertCurrentOwner, + idempotencyKey: typeof data.idempotencyKey === "string" ? data.idempotencyKey : undefined, + }); + assertCurrentOwner(); return { issue, companyId, failures, navigateOnCreate }; }, onSuccess: ({ issue, companyId, failures, navigateOnCreate }) => { + assertCurrentOwner(); trackRecentProject(issue.projectId ?? "", companyId); if (issue.assigneeAgentId) trackRecentAssignee(issue.assigneeAgentId, companyId); if (issue.assigneeUserId) trackRecentAssigneeUser(issue.assigneeUserId, companyId); @@ -483,20 +661,94 @@ export function NewIssueDialog() { action: openIssueAction, }); } - clearDraft(); + clearDraft(draftOwner); reset(); closeNewIssue(); if (navigateOnCreate) navigate(openIssueAction?.href ?? `/issues/${issueRef}`); }, }); + // Issue create is keyed: the server replays the original issue for a known + // idempotency key, so an outage or a lost response resends safely. + const stagedFilesByKey = useRef(new Map()); + // The composer shows a rejection of the first attempt; later (automatic or + // Resend now) rejections show here. + const firstAttemptKey = useRef(null); + const [resendError, setResendError] = useState(null); + // Once the account is known, a create still waiting for its files keeps its + // key and asks for the files again. + useEffect(() => { + if (!awaitingFilesKey) return; + const awaiting = readPendingSend(awaitingFilesKey, isPendingCreate); + if (!awaiting) return; + createRequestRef.current ??= { + fingerprint: awaiting.payload.fingerprint, + idempotencyKey: awaiting.idempotencyKey, + recovering: true, + payload: awaiting.payload, + }; + restoreLaunch(awaiting.payload); + const names = awaiting.payload.stagedFileNames; + if (names?.length) setResendError((current) => current ?? stagedFilesMissingDescription(names)); + }, [awaitingFilesKey]); + const durableCreate = useDurableSubmit({ + sourceId: "new-issue-dialog", + storageKey: PENDING_CREATE_KEY, + isPayload: isPendingCreate, + action: "create the task", + send: async (payload, idempotencyKey) => { + const stagedFiles = stagedFilesByKey.current.get(idempotencyKey); + // Never create a restored task without the files it was submitted with. + if (!stagedFiles && payload.stagedFileNames?.length) { + throw new StagedFilesMissingError(payload.stagedFileNames); + } + const result = await createIssue.mutateAsync({ + ...(payload.data as { companyId: string }), + stagedFiles: stagedFiles ?? [], + idempotencyKey, + }); + stagedFilesByKey.current.delete(idempotencyKey); + clearCreateUploadReceipts(idempotencyKey); + return result; + }, + onRejected: (error, description, pending) => { + if (error instanceof StagedFilesMissingError) { + // Reattaching the same files resubmits under the original key, so a + // create that already reached the server is replayed, not duplicated. + createRequestRef.current = { + fingerprint: pending.payload.fingerprint, + idempotencyKey: pending.idempotencyKey, + recovering: true, + payload: pending.payload, + }; + restoreLaunch(pending.payload); + if (awaitingFilesKey) writePendingSend(awaitingFilesKey, pending); + } + if (pending.idempotencyKey !== firstAttemptKey.current) setResendError(description); + }, + }); + // A create restored after reload without its files cannot be sent; say so + // now rather than after the first resend delay. Nothing reaches the network. + const pendingCreate = durableCreate.pending; + const { resendNow: resendCreateNow } = durableCreate; + useEffect(() => { + if (pendingCreate?.payload.stagedFileNames?.length && !stagedFilesByKey.current.has(pendingCreate.idempotencyKey)) { + resendCreateNow(); + } + }, [pendingCreate, resendCreateNow]); + // Debounced draft saving const scheduleSave = useCallback((draft: IssueDraft) => { + if (draftOwner === undefined && (unboundFormDraft.current || draft.title.trim() || draft.description.trim())) unboundFormDraft.current = draft; if (draftTimer.current) clearTimeout(draftTimer.current); draftTimer.current = setTimeout(() => { - if (draft.title.trim() || draft.description.trim()) saveDraft(draft); + const owner = readOwner(); + if (owner !== undefined && (draftOwner === undefined || owner === draftOwner)) { + if (draft.title.trim() || draft.description.trim()) saveDraft(owner, draft); + else clearDraft(owner); + } }, DEBOUNCE_MS); - }, []); + }, [draftOwner, readOwner]); const setIssueText = useCallback((nextTitle: string, nextDescription: string) => { setHasTitle(Boolean(nextTitle)); @@ -600,13 +852,23 @@ export function NewIssueDialog() { defaultProjectPendingRef.current = false; return; } - const initializationKey = `${effectiveCompanyId ?? ""}:${JSON.stringify(newIssueDefaults)}`; + const initializationKey = `${draftOwner ?? "unknown"}:${effectiveCompanyId ?? ""}:${JSON.stringify(newIssueDefaults)}`; if (initializationKeyRef.current === initializationKey) return; initializationKeyRef.current = initializationKey; + if (draftTimer.current) { + clearTimeout(draftTimer.current); + draftTimer.current = null; + } executionWorkspaceDefaultProjectId.current = null; - const draft = loadDraft(); + if (draftOwner !== undefined && unboundFormDraft.current) { + if (unboundFormDraft.current.title.trim() || unboundFormDraft.current.description.trim()) saveDraft(draftOwner, unboundFormDraft.current); + else clearDraft(draftOwner); + unboundFormDraft.current = null; + return; + } + const draft = loadDraft(draftOwner); defaultProjectPendingRef.current = newIssueDefaults.projectId === undefined && !newIssueDefaults.parentId; defaultAssigneePendingRef.current = !newIssueDefaults.assigneeAgentId && !newIssueDefaults.assigneeUserId; setComposerSettings(null); @@ -745,7 +1007,7 @@ export function NewIssueDialog() { ? defaultProjectId || null : null; } - }, [newIssueOpen, newIssueDefaults, orderedProjects, effectiveCompanyId, setIssueText]); + }, [newIssueOpen, newIssueDefaults, orderedProjects, effectiveCompanyId, setIssueText, draftOwner]); // Cleanup timer on unmount useEffect(() => { @@ -755,7 +1017,9 @@ export function NewIssueDialog() { }, []); function reset() { - createRequestRef.current = null; + unboundFormDraft.current = null; + createRequestRef.current = null; setRestoredLaunch(null); + if (awaitingFilesKey) clearPendingSend(awaitingFilesKey); setIssueText("", ""); setStatus("todo"); setPriority(""); @@ -789,7 +1053,7 @@ export function NewIssueDialog() { async function handleSubmit(body: string, mode: IssueWorkMode, settings: ComposerRunSettings | null) { const currentTitle = titleRef.current.trim(); const currentDescription = body.trim(); - if (!effectiveCompanyId || (!currentTitle && !currentDescription) || createIssue.isPending || worktreeSelectionIncomplete || parentPrivacyUnresolved) return; + if (!effectiveCompanyId || (!currentTitle && !currentDescription) || createIssue.isPending || durableCreate.pending || worktreeSelectionIncomplete || parentPrivacyUnresolved) return; const inheritedOverrides = buildAssigneeAdapterOverrides({ adapterType: assigneeAdapterType, lane: assigneeChrome ? "custom" : assigneeModelLane, @@ -833,7 +1097,6 @@ export function NewIssueDialog() { : inheritedOverrides; const createData = { companyId: effectiveCompanyId, - stagedFiles, ...(currentTitle ? { title: currentTitle } : {}), description: currentDescription || undefined, status, @@ -842,8 +1105,8 @@ export function NewIssueDialog() { ...(effectivePrivate ? { visibility: "private" } : {}), ...(selectedAssigneeAgentId ? { assigneeAgentId: selectedAssigneeAgentId } : {}), ...(selectedAssigneeUserId ? { assigneeUserId: selectedAssigneeUserId } : {}), - ...(newIssueDefaults.parentId ? { parentId: newIssueDefaults.parentId } : {}), - ...(newIssueDefaults.goalId ? { goalId: newIssueDefaults.goalId } : {}), + ...(parentId ? { parentId } : {}), + ...(goalId ? { goalId } : {}), ...(projectId ? { projectId } : {}), ...(requestedProjectWorkspaceId ? { projectWorkspaceId: requestedProjectWorkspaceId } : {}), ...(assigneeAdapterOverrides ? { assigneeAdapterOverrides } : {}), @@ -863,17 +1126,59 @@ export function NewIssueDialog() { : {}), }; // An explicit board create is a new task even when its title already exists. - // Reuse the request key only for retries of the same submitted draft. - const fingerprint = JSON.stringify(createData); - if (createRequestRef.current?.fingerprint !== fingerprint) { - createRequestRef.current = { fingerprint, idempotencyKey: createUuid() }; + // Reuse the request key only for retries of the same submitted draft, and + // always for a create waiting for its files: it may already have reached + // the server, and its key then replays that task instead of a second one. + // The file set is compared without regard to selection order. + let request = createRequestRef.current; + const createFiles = request?.recovering ? restoreStagedDocumentKeys(stagedFiles, request.payload?.fingerprint) : stagedFiles; + const fingerprint = JSON.stringify({ ...createData, stagedFiles: createFiles.map(stagedFileIdentity).sort() }); + if (!request || (!request.recovering && request.fingerprint !== fingerprint)) { + request = { fingerprint, idempotencyKey: createUuid() }; + createRequestRef.current = request; } - await createIssue.mutateAsync({ - ...createData, - allowDuplicate: true, - idempotencyKey: createRequestRef.current.idempotencyKey, - navigateOnCreate: newIssueDefaults.navigateOnCreate === true, - }); + let createPayload: PendingCreate = { + fingerprint, data: { ...createData, allowDuplicate: true, navigateOnCreate: newIssueDefaults.navigateOnCreate === true }, + ...(stagedFiles.length ? { stagedFileNames: stagedFiles.map(file => file.file.name) } : {}), + }; + if (request.recovering) { + const original = request.payload; + const rejectRecovery = (message: string): never => { + setResendError({ title: "Saved request needs attention", body: message, retryable: false }); + throw new Error(message); + }; + if (!original) rejectRecovery("The saved request is unavailable. Cancel it before creating another task."); + let originalFingerprint: Record = {}; + try { originalFingerprint = JSON.parse(original!.fingerprint); } catch {} + const requiredFiles = originalFingerprint.stagedFiles; + if (original!.stagedFileNames?.length && (!Array.isArray(requiredFiles) || + JSON.stringify([...requiredFiles].sort()) !== JSON.stringify(createFiles.map(stagedFileIdentity).sort()))) { + rejectRecovery(`Attach the original files again: ${original!.stagedFileNames.join(", ")}.`); + } + const normalize = (_key: string, value: unknown) => value && typeof value === "object" && !Array.isArray(value) + ? Object.fromEntries(Object.entries(value).sort(([a], [b]) => a.localeCompare(b))) : value; + const fieldsChanged = Object.entries(original!.data).some(([key, value]) => + !["allowDuplicate", "navigateOnCreate"].includes(key) && JSON.stringify(value, normalize) !== JSON.stringify((createData as Record)[key], normalize)); + if (fieldsChanged || (typeof originalFingerprint.companyId === "string" && + JSON.stringify(originalFingerprint, normalize) !== JSON.stringify(JSON.parse(fingerprint), normalize))) { + rejectRecovery("Restore the saved task's original fields before retrying, or cancel the saved request to start another task."); + } + createPayload = original!; + } + request.payload = createPayload; + const { idempotencyKey } = request; + stagedFilesByKey.current.set(idempotencyKey, createFiles); + firstAttemptKey.current = idempotencyKey; + setResendError(null); + let result; + try { + result = await durableCreate.submit(createPayload, { idempotencyKey }); + } finally { + firstAttemptKey.current = null; + } + // A transient failure keeps the request and shows the pending notice; a + // rejection goes back to the composer with readable copy. + if (result.kind === "rejected") throw result.error; } function handleKeyDown(e: React.KeyboardEvent) { @@ -885,7 +1190,7 @@ export function NewIssueDialog() { } function stageFiles(files: File[]) { - if (files.length === 0 || createIssue.isPending) return; + if (files.length === 0 || createIssue.isPending || durableCreate.pending) return; setStagedFiles((current) => { const next = [...current]; for (const file of files) { @@ -1200,13 +1505,44 @@ export function NewIssueDialog() { runSettings: composerSettings, onRunSettingsChange: setComposerSettings, header: - hasTitle || isSubIssueMode ? ( + hasTitle || isSubIssueMode || durableCreate.pending || resendError ? (
+ {durableCreate.pending && durableCreate.status !== "sending" ? ( + { + // The draft (title and description) is still saved; only the send stops. + const cancelledKey = durableCreate.pending?.idempotencyKey; + const cancelled = durableCreate.cancel(); + if (cancelled) { + createRequestRef.current = null; setRestoredLaunch(null); + if (awaitingFilesKey) clearPendingSend(awaitingFilesKey, cancelledKey); + if (cancelledKey) clearCreateUploadReceipts(cancelledKey); + } + }} + /> + ) : null} + {resendError && !durableCreate.pending ? ( +
+

{resendError.title}. {resendError.body}

+ {createRequestRef.current?.recovering ? : null} +
+ ) : null} {hasTitle ? ( { titleRef.current = event.target.value; @@ -1378,7 +1714,8 @@ export function NewIssueDialog() { ) : null} ), - submitDisabled: worktreeSelectionIncomplete || parentPrivacyUnresolved, + submitDisabled: worktreeSelectionIncomplete || parentPrivacyUnresolved || Boolean(durableCreate.pending), + readOnly: Boolean(durableCreate.pending), contextBar: ( <> void; + onCancel: () => void; + className?: string; +}) { + const replayable = Boolean(onResendNow); + const title = !replayable + ? `We couldn’t confirm whether this ${noun} was sent.` + : resending + ? "Sending…" + : stalled + ? `Couldn’t send this ${noun} yet.` + : "Sending when reconnected…"; + const detail = replayable + ? persisted ? `Your ${noun} is saved on this device.` + : "Browser storage is unavailable. Keep this tab open or copy your text before leaving." + : "It may already be in the conversation. Check it before sending again; Cancel keeps your text."; + return ( +
+

+ {resending ? : null} + {title} +

+

{detail}

+
+ {onResendNow ? ( + + ) : null} + +
+
+ ); +} diff --git a/ui/src/components/TaskChatThread.tsx b/ui/src/components/TaskChatThread.tsx index 7101fbdea7c..f7705ab746e 100644 --- a/ui/src/components/TaskChatThread.tsx +++ b/ui/src/components/TaskChatThread.tsx @@ -513,7 +513,6 @@ export function TaskChatThread(props: TaskChatThreadProps) { userProfileMap, currentUserId, onAdd, - onReviewConversation, onCancelRun, stopPending, stopScope, @@ -3309,7 +3308,6 @@ export function TaskChatThread(props: TaskChatThreadProps) { comment.authorUserId === currentUserId && comment.clientRequestId && !("clientStatus" in comment && comment.clientStatus) ).map((comment) => comment.clientRequestId!))} - onReviewConversation={onReviewConversation} onStop={liveRun ? onCancelRun : undefined} stopPending={stopPending} stopScope={stopScope} diff --git a/ui/src/components/chat/ExternallyConnectedTaskBanner.test.tsx b/ui/src/components/chat/ExternallyConnectedTaskBanner.test.tsx index 299535b3a26..e08a0b0aeee 100644 --- a/ui/src/components/chat/ExternallyConnectedTaskBanner.test.tsx +++ b/ui/src/components/chat/ExternallyConnectedTaskBanner.test.tsx @@ -9,6 +9,11 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { ExternallyConnectedTaskBanner } from "./ExternallyConnectedTaskBanner"; import { boardSendDraftKey, readBoardSendDraft } from "./board-send-draft"; import { ApiError } from "@/api/client"; +import { queryKeys } from "@/lib/queryKeys"; +const mockSession = vi.hoisted(() => ({ userId: "user-1" })); +vi.mock("@/api/auth", () => ({ + authApi: { getSession: vi.fn(async () => ({ user: { id: mockSession.userId } })) }, +})); const mockChatEndpointsApi = vi.hoisted(() => ({ getIssueBinding: vi.fn(), @@ -20,6 +25,19 @@ const uploadAttachmentMock = vi.hoisted(() => vi.fn()); vi.mock("@/api/issues", () => ({ issuesApi: { uploadAttachment: uploadAttachmentMock }, })); +// Lets a test report that automatic resends stopped, without running eight +// backoff rounds; otherwise the real hook runs. +const autoResendOverride = vi.hoisted(() => ({ stalled: false })); +vi.mock("@/hooks/useDurableSubmit", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + useAutoResend: (options: Parameters[0]) => { + const result = actual.useAutoResend(options); + return autoResendOverride.stalled ? { ...result, stalled: true } : result; + }, + }; +}); vi.mock("@/hooks/useChatConnectorsEnabled", async (importOriginal) => ({ ...await importOriginal(), useChatConnectorsEnabled: () => ({ enabled: true, githubEnabled: true, loaded: true }), @@ -99,6 +117,7 @@ describe("ExternallyConnectedTaskBanner publication truth", () => { new QueryClient({ defaultOptions: { queries: { retry: false } }, }); + if (!existingQueryClient) queryClient.setQueryData(queryKeys.auth.session, { user: { id: mockSession.userId } }); flushSync(() => { root.render( @@ -131,6 +150,7 @@ describe("ExternallyConnectedTaskBanner publication truth", () => { beforeEach(() => { vi.clearAllMocks(); + mockSession.userId = "user-1"; sessionStorage.clear(); mockChatEndpointsApi.getPublicationBatchStatus.mockImplementation( () => new Promise(() => {}), @@ -154,6 +174,131 @@ describe("ExternallyConnectedTaskBanner publication truth", () => { flushSync(() => root.unmount()); container.remove(); vi.restoreAllMocks(); + autoResendOverride.stalled = false; + }); + + it("cancels an automatic resend and hides its draft after an account change", async () => { + mockChatEndpointsApi.publishBoardMessage.mockRejectedValue(new ApiError("Bad gateway", 502, {})); + const client = await renderBanner(); + await composeAndSubmit("Private account A message"); + expect(mockChatEndpointsApi.publishBoardMessage).toHaveBeenCalledTimes(1); + const retainedA = [...Array(sessionStorage.length)].map((_, index) => sessionStorage.getItem(sessionStorage.key(index)!)); + mockSession.userId = "user-2"; + await act(() => { client.setQueryData(queryKeys.auth.session, { user: { id: "user-2" } }); }); + await flushReact(); + await new Promise(resolve => setTimeout(resolve, 1_100)); + await flushReact(); + expect(mockChatEndpointsApi.publishBoardMessage).toHaveBeenCalledTimes(1); + expect(container.querySelector("textarea")?.value ?? "").not.toContain("Private account A message"); + expect(container.textContent).not.toContain("Sending when reconnected"); + expect([...Array(sessionStorage.length)].map((_, index) => sessionStorage.getItem(sessionStorage.key(index)!))).toEqual(retainedA); + client.clear(); + }); + + it("ignores an old account's late receipt after the session changes", async () => { + let finish!: (result: unknown) => void; + mockChatEndpointsApi.publishBoardMessage.mockImplementationOnce(() => new Promise(resolve => { finish = resolve; })); + const client = await renderBanner(); + await composeAndSubmit("Private pending account A message"); + mockSession.userId = "user-2"; + await act(() => { client.setQueryData(queryKeys.auth.session, { user: { id: "user-2" } }); }); + await flushReact(); + await act(() => { finish({ id: "publication-old", state: "published", attempts: 1 }); }); + await flushReact(); + expect(pushToastMock).not.toHaveBeenCalled(); + expect(container.querySelector("textarea")?.value ?? "").not.toContain("Private pending account A message"); + expect(sessionStorage.length).toBe(1); + client.clear(); + }); + + it("restores only the original account's retained send after returning to it", async () => { + mockChatEndpointsApi.publishBoardMessage.mockRejectedValue(new ApiError("Bad gateway", 502, {})); + const client = await renderBanner(); + await composeAndSubmit("Account A retained request"); + const originalKey = mockChatEndpointsApi.publishBoardMessage.mock.calls[0][3]; + mockSession.userId = "user-2"; + await act(() => { client.setQueryData(queryKeys.auth.session, { user: { id: "user-2" } }); }); + await flushReact(); + await act(() => findButton(container, "Send to channel").click()); + expect(container.querySelector("textarea")?.value).toBe(""); + mockSession.userId = "user-1"; + await act(() => { client.setQueryData(queryKeys.auth.session, { user: { id: "user-1" } }); }); + await flushReact(); + expect(container.querySelector("textarea")?.value).toBe("Account A retained request"); + expect(mockChatEndpointsApi.publishBoardMessage).toHaveBeenCalledTimes(1); + mockChatEndpointsApi.publishBoardMessage.mockResolvedValueOnce({ id: "publication-recovered", state: "published", attempts: 1 }); + await act(() => findButton(container, "Retry safely").click()); + await flushReact(); + expect(mockChatEndpointsApi.publishBoardMessage).toHaveBeenLastCalledWith("endpoint-1", "conversation-1", "Account A retained request", originalKey, []); + }); + + it("does not reveal or replay an older retained send without a known owner", async () => { + const key = boardSendDraftKey("company-1", "issue-1", "endpoint-1", "conversation-1"); + const saved = JSON.stringify({ body: "Unowned private message", attachmentIds: [], idempotencyKey: "legacy-request-key-1", publication: null }); + sessionStorage.setItem(key, saved); + await renderBanner(); + expect(container.querySelector("textarea")?.value).toBe(""); + expect(container.querySelector("textarea")?.disabled).toBe(true); + expect(container.textContent).toContain("Open Activity"); + expect(container.textContent).not.toContain("Unowned private message"); + expect(mockChatEndpointsApi.publishBoardMessage).not.toHaveBeenCalled(); + expect(sessionStorage.getItem(key)).toBe(saved); + }); + + it.each([false, true])("can explicitly discard an unowned receipt without replaying its message (storage failure=%s)", async (storageFailure) => { + const key = boardSendDraftKey("company-1", "issue-1", "endpoint-1", "conversation-1"); + const saved = JSON.stringify({ body: "Unowned private message", attachmentIds: [], idempotencyKey: "legacy-request-key-1", publication: null }); + sessionStorage.setItem(key, saved); + await renderBanner(); + await act(() => findButton(container, "Discard saved delivery record").click()); + await flushReact(); + expect(document.body.textContent).toContain("does not cancel an update"); + const dialog = document.body.querySelector('[role="alertdialog"]')!; + expect(dialog).toBeTruthy(); + await act(() => findButton(dialog, "Keep record").click()); + await flushReact(); + expect(sessionStorage.getItem(key)).toBe(saved); + expect(container.querySelector("textarea")?.disabled).toBe(true); + if (storageFailure) { + const remove = vi.spyOn(Object.getPrototypeOf(sessionStorage), "removeItem").mockImplementation(() => { throw new Error("Storage unavailable"); }); + await act(() => findButton(container, "Discard saved delivery record").click()); + await flushReact(); + await act(() => findButton(document.body, "Discard record").click()); + await flushReact(); + expect(sessionStorage.getItem(key)).toBe(saved); + expect(container.querySelector("textarea")?.disabled).toBe(true); + expect(container.textContent).toContain("could not be discarded"); + expect(mockChatEndpointsApi.publishBoardMessage).not.toHaveBeenCalled(); + remove.mockRestore(); + } + await act(() => findButton(container, "Discard saved delivery record").click()); + await flushReact(); + await act(() => findButton(document.body, "Discard record").click()); + await flushReact(); + expect(sessionStorage.getItem(key)).toBeNull(); + await act(() => findButton(container, "Send to channel").click()); + await flushReact(); + expect(container.querySelector("textarea")?.disabled).toBe(false); + expect(container.querySelector("textarea")?.value).toBe(""); + expect(mockChatEndpointsApi.publishBoardMessage).not.toHaveBeenCalled(); + await act(() => setTextareaValue(container.querySelector("textarea")!, "Fresh deliberate message")); + mockChatEndpointsApi.publishBoardMessage.mockResolvedValueOnce({ id: "fresh-publication", state: "published", attempts: 1 }); + await act(() => [...container.querySelectorAll("button")].filter(button => button.textContent?.trim() === "Send to channel").at(-1)!.click()); + await flushReact(); + expect(mockChatEndpointsApi.publishBoardMessage).toHaveBeenCalledWith("endpoint-1", "conversation-1", "Fresh deliberate message", expect.not.stringMatching("legacy-request-key-1"), []); + }); + + it("asks for Resend now once automatic resends stop", async () => { + autoResendOverride.stalled = true; + mockChatEndpointsApi.publishBoardMessage.mockRejectedValue( + new ApiError("Bad gateway", 502, { error: "Bad gateway" }), + ); + await renderBanner(); + await composeAndSubmit(); + expect(container.textContent).toContain("Couldn’t send this update yet."); + expect(container.textContent).toContain("Automatic resends stopped; choose Resend now"); + expect(container.textContent).not.toContain("It resends on its own"); + expect(findButton(container, "Resend now")).toBeTruthy(); }); it("only reports success and clears the draft after confirmed publication", async () => { @@ -322,6 +467,7 @@ describe("ExternallyConnectedTaskBanner publication truth", () => { "issue-1", "endpoint-1", "conversation-1", + "user-1", ), )?.attachmentNames, ).toEqual([{ id: "uploaded-image", name: "cat.png" }]); @@ -879,7 +1025,12 @@ describe("ExternallyConnectedTaskBanner publication truth", () => { ?.click(), ); await flushReact(); - expect(container.textContent).toContain("Delivery result not confirmed"); + // A 5xx may have committed: it waits to resend with the same identity. + expect(container.textContent).toContain( + kind === "server-error" + ? "Sending when reconnected…" + : "Delivery result not confirmed", + ); expect(container.textContent).not.toContain("Edit rejected send"); expect(container.querySelector("textarea")?.disabled).toBe(true); expect(mockChatEndpointsApi.publishBoardMessage).toHaveBeenCalledTimes(1); @@ -1068,6 +1219,7 @@ describe("ExternallyConnectedTaskBanner publication truth", () => { "issue-1", "endpoint-1", "conversation-1", + "user-1", ), ), ).toBeNull(); @@ -1217,6 +1369,7 @@ describe("ExternallyConnectedTaskBanner publication truth", () => { "issue-1", "endpoint-1", "conversation-1", + "user-1", ), ), ).toMatchObject({ @@ -1231,6 +1384,7 @@ describe("ExternallyConnectedTaskBanner publication truth", () => { "issue-1", "endpoint-2", "conversation-2", + "user-1", ), ), ).toBeNull(); diff --git a/ui/src/components/chat/ExternallyConnectedTaskBanner.tsx b/ui/src/components/chat/ExternallyConnectedTaskBanner.tsx index 1c067ec71a2..cc283572d4a 100644 --- a/ui/src/components/chat/ExternallyConnectedTaskBanner.tsx +++ b/ui/src/components/chat/ExternallyConnectedTaskBanner.tsx @@ -1,6 +1,6 @@ import { TaskVoiceLauncher } from "@/components/voice/TaskVoiceLauncher"; import { useCallback, useEffect, useRef, useState } from "react"; -import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; +import { useQuery, useQueryClient } from "@tanstack/react-query"; import { ExternalLink, Paperclip, Radio } from "lucide-react"; import type { ChatPublicationState, @@ -22,6 +22,11 @@ import { cn } from "@/lib/utils"; import { queryKeys } from "@/lib/queryKeys"; import { useChatConnectorsEnabled, chatProviderVisible } from "@/hooks/useChatConnectorsEnabled"; import { issuesApi } from "@/api/issues"; +import { describeError } from "@/api/errors"; +import { classifySendFailure, type SendOutcome } from "@/lib/pending-send"; +import { useAutoResend } from "@/hooks/useDurableSubmit"; +import { useDraftOwner, useDraftOwnerReader } from "@/hooks/useDraftOwner"; +import { useAccountBoundMutation } from "@/hooks/useAccountBoundMutation"; import { boardSendDraftKey, clearBoardSendDraft, @@ -33,6 +38,8 @@ import { type RetainedBoardSend, } from "./board-send-draft"; +import { LegacyBoardSendNotice } from "./LegacyBoardSendNotice"; + const providerNames: Record = { speko: "Speko", slack: "Slack", @@ -136,11 +143,13 @@ type ConnectedTaskProps = { export function ExternallyConnectedTaskBanner(props: ConnectedTaskProps) { const { binding } = useIssueChatBinding(props.companyId, props.issueId); + const draftOwner = useDraftOwner(); return <> - {binding && binding.provider !== "speko" && binding.provider !== "agentmail" && } ; @@ -153,7 +162,9 @@ function ConnectedTaskComposer({ issueId, issueCacheRefs, binding, -}: ConnectedTaskProps & { binding: ExternalChannelBindingSummary }) { + draftOwner, +}: ConnectedTaskProps & { binding: ExternalChannelBindingSummary; draftOwner: string }) { + const readOwner = useDraftOwnerReader(); const { pushToast } = useToast(); const queryClient = useQueryClient(); const [composing, setComposing] = useState(false); @@ -174,6 +185,7 @@ function ConnectedTaskComposer({ ); const [selectionNotice, setSelectionNotice] = useState(false); const [storageError, setStorageError] = useState(null); + const [unownedRequest, setUnownedRequest] = useState(false); const fileInput = useRef(null); const uploadInFlight = useRef(false); const mounted = useRef(true); @@ -194,13 +206,21 @@ function ConnectedTaskComposer({ issueId, binding.endpointId, binding.conversationId, + draftOwner, ) : null; + const legacyStorageKey = boardSendDraftKey(companyId, issueId, binding.endpointId, binding.conversationId); const loadedStorageKey = useRef(null); useEffect(() => { if (!storageKey || loadedStorageKey.current === storageKey) return; loadedStorageKey.current = storageKey; try { + if (sessionStorage.getItem(legacyStorageKey) !== null) { + setUnownedRequest(true); + setStorageError("An earlier update has an unknown sender. Open Activity to check its delivery before starting a new update."); + setComposing(true); + return; + } const saved = readBoardSendDraft(storageKey); retainedScopeKey.current = storageKey; retainedSend.current = saved; @@ -220,7 +240,7 @@ function ConnectedTaskComposer({ ); setComposing(true); } - }, [storageKey]); + }, [legacyStorageKey, storageKey, unownedRequest]); const deliveryScopeReady = Boolean( storageKey && loadedStorageKey.current === storageKey && @@ -265,6 +285,7 @@ function ConnectedTaskComposer({ const publicationStatus = useQuery({ queryKey: [ "chat-publication-batch", + draftOwner, companyId, binding?.endpointId, binding?.conversationId, @@ -294,21 +315,23 @@ function ConnectedTaskComposer({ finishPublication(); } }, [publication, publicationStatus.data, finishPublication]); - const publish = useMutation({ + const publish = useAccountBoundMutation({ mutationFn: (input: { attachmentIds: string[]; body: string; idempotencyKey: string; endpointId: string; conversationId: string; - }) => - chatEndpointsApi.publishBoardMessage( + }) => { + if (readOwner() !== draftOwner) throw new DOMException("Your account changed. Reopen this task to send from your current account.", "AbortError"); + return chatEndpointsApi.publishBoardMessage( input.endpointId, input.conversationId, input.body, input.idempotencyKey, input.attachmentIds, - ), + ); + }, onSuccess: (result) => { invalidateTask(); const feedback = publicationFeedback[result.state]; @@ -366,16 +389,50 @@ function ConnectedTaskComposer({ }); return; } + // Outages and lost receipts resend on their own with the same request + // identity; the inline notice covers them. + if (classifySendFailure(error) === "pending") return; pushToast({ title: "Couldn't confirm channel delivery", - body: - error instanceof Error - ? `${error.message} Your draft is kept; retrying here reuses the same request identity.` - : "Your draft is kept; retrying here reuses the same request identity.", + body: `${describeError(error).body} Your draft is kept; retrying here reuses the same request identity.`, tone: "error", }); }, }); + // A send that failed with an outage or a lost receipt in this session + // resends with its saved key; the server returns the original publication + // instead of queueing another message. A send restored after reload has an + // unknown outcome and waits for an explicit retry. + const resendWaiting = Boolean( + deliveryScopeReady && + unconfirmedRequest && + retainedSend.current && + !rejection && + !publication && + !storageError && + publish.status === "error" && + classifySendFailure(publish.error) === "pending", + ); + const autoResend = useAutoResend({ + sourceId: `board-send:${storageKey ?? issueId}`, + active: resendWaiting, + resend: async (): Promise => { + const input = retainedSend.current; + if (!input || readOwner() !== draftOwner) return "rejected"; + try { + await publish.mutateAsync({ + ...input, + endpointId: binding.endpointId, + conversationId: binding.conversationId, + }); + return "sent"; + } catch (error) { + return classifySendFailure(error); + } + }, + }); + // Automatic resends gave up while online; only Resend now tries again. + const resendStalled = resendWaiting && autoResend.stalled && !autoResend.resending; const uploadDisabled = Boolean( retainedSend.current || publication || @@ -653,9 +710,21 @@ function ConnectedTaskComposer({ )} {storageError && (

- {storageError} + {storageError}{" "} + Open Activity

)} + {unownedRequest && { + if (readOwner() !== draftOwner) return; + try { clearBoardSendDraft(legacyStorageKey); } + catch { + setStorageError("Saved delivery record could not be discarded. Restore browser storage, then try again."); + return; + } + loadedStorageKey.current = null; + setUnownedRequest(false); + setStorageError(null); + }} />} {rejection && (
-

Delivery result not confirmed

+

+ {resendStalled + ? "Couldn’t send this update yet." + : resendWaiting || autoResend.resending + ? "Sending when reconnected…" + : "Delivery result not confirmed"} +

- Your exact draft and request identity are kept. Retry safely - to learn the authoritative publication state without creating - a duplicate. + {resendStalled + ? "Your exact draft and request identity are kept. Automatic resends stopped; choose Resend now to try again without creating a duplicate." + : resendWaiting || autoResend.resending + ? "Your exact draft and request identity are kept. It resends on its own without creating a duplicate." + : "Your exact draft and request identity are kept. Retry safely to learn the authoritative publication state without creating a duplicate."}

{ if ( uploadInFlight.current || + readOwner() !== draftOwner || !storageKey || loadedStorageKey.current !== storageKey || retainedScopeKey.current !== storageKey ) return; + if (resendWaiting) { + // Same saved request; also restarts the automatic resends. + autoResend.resendNow(); + return; + } idempotencyKey.current ??= crypto.randomUUID(); const input = retainedSend.current ?? { attachmentIds: selectedAttachmentIds, @@ -883,7 +967,9 @@ function ConnectedTaskComposer({ > {publish.isPending ? "Sending…" - : !rejection && (publish.isError || unconfirmedRequest) + : resendWaiting + ? "Resend now" + : !rejection && (publish.isError || unconfirmedRequest) ? "Retry safely" : "Send to channel"} diff --git a/ui/src/components/chat/LegacyBoardSendNotice.tsx b/ui/src/components/chat/LegacyBoardSendNotice.tsx new file mode 100644 index 00000000000..929e6e94ab4 --- /dev/null +++ b/ui/src/components/chat/LegacyBoardSendNotice.tsx @@ -0,0 +1,27 @@ +import { + AlertDialog, AlertDialogAction, AlertDialogCancel, AlertDialogContent, + AlertDialogDescription, AlertDialogFooter, AlertDialogHeader, AlertDialogTitle, + AlertDialogTrigger, +} from "@/components/ui/alert-dialog"; +import { Button } from "@/components/ui/button"; + +export function LegacyBoardSendNotice({ onDiscard }: { onDiscard: () => void }) { + return + + + + + + Discard saved delivery record? + + First check Activity. Discarding this browser record does not cancel an update + that was already accepted. It lets you start a new update. + + + + Keep record + Discard record + + + ; +} diff --git a/ui/src/components/chat/board-send-draft.test.ts b/ui/src/components/chat/board-send-draft.test.ts index 4f729efe8cd..a44dc2785ac 100644 --- a/ui/src/components/chat/board-send-draft.test.ts +++ b/ui/src/components/chat/board-send-draft.test.ts @@ -75,6 +75,18 @@ describe("session-scoped Board send identity", () => { clearBoardSendDraft(key); expect(readBoardSendDraft(key)).toBeNull(); }); + it("keeps account-scoped sends separate from each other and unowned legacy records", () => { + const scope = ["company", "task", "endpoint", "conversation"] as const; + const a = boardSendDraftKey(...scope, "user-a"); + const b = boardSendDraftKey(...scope, "user-b"); + const legacy = boardSendDraftKey(...scope); + const draft = { body: "Private A", attachmentIds: [], idempotencyKey: "stable-request-key-a", publication: null }; + writeBoardSendDraft(a, draft); + expect(readBoardSendDraft(a)).toEqual(draft); + expect(readBoardSendDraft(b)).toBeNull(); + expect(readBoardSendDraft(legacy)).toBeNull(); + }); + it("rejects corrupt retained identity instead of silently enabling a new send", () => { sessionStorage.setItem("corrupt", '{"body":"draft"}'); expect(() => readBoardSendDraft("corrupt")).toThrow( diff --git a/ui/src/components/chat/board-send-draft.ts b/ui/src/components/chat/board-send-draft.ts index b7bc9ad369e..78051bf9211 100644 --- a/ui/src/components/chat/board-send-draft.ts +++ b/ui/src/components/chat/board-send-draft.ts @@ -71,8 +71,12 @@ export function boardSendDraftKey( issueId: string, endpointId: string, conversationId: string, + owner?: string, ) { - return `paperclip:board-send:v1:${JSON.stringify([companyId, issueId, endpointId, conversationId])}`; + const scope = [companyId, issueId, endpointId, conversationId]; + return owner === undefined + ? `paperclip:board-send:v1:${JSON.stringify(scope)}` + : `paperclip:board-send:v2:${JSON.stringify([owner, ...scope])}`; } export function readBoardSendDraft(key: string): RetainedBoardSend | null { diff --git a/ui/src/components/composer-recovery.test.tsx b/ui/src/components/composer-recovery.test.tsx new file mode 100644 index 00000000000..bdb243114ad --- /dev/null +++ b/ui/src/components/composer-recovery.test.tsx @@ -0,0 +1,106 @@ +// @vitest-environment jsdom + +import { act, type ReactElement } from "react"; +import { createRoot, type Root } from "react-dom/client"; +import { MemoryRouter } from "react-router-dom"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { IssueChatThread } from "./IssueChatThread"; +import { TaskChatComposer } from "./task-chat/TaskChatComposer"; +import { queryKeys } from "../lib/queryKeys"; +import { + loadDraftSubmission, + saveDraft, + saveDraftAttachments, + saveDraftSubmission, +} from "../lib/composer-draft"; + +vi.mock("./MarkdownEditor", async () => { + const { forwardRef, useImperativeHandle, useRef } = await import("react"); + return { + MarkdownEditor: forwardRef(function Editor( + { value, onChange, readOnly }: { value: string; onChange: (value: string) => void; readOnly?: boolean }, + ref, + ) { + const input = useRef(null); + useImperativeHandle(ref, () => ({ focus: () => input.current?.focus(), clear: () => onChange("") })); + return