fix: remove a saved draft once its email is sent

apps/mobile/src/details.tsx

@@ -255,6 +255,7 @@ function EmailEditor({ draft }: { draft?: Partial<EmailDraft> & { id?: string }
         const action = await api.request<ActionProposal>("/api/actions", {
           kind: "email.send",
           data: parsed.data,
+          draftId: draft?.id,
         });
         await refresh();
         open({ type: "review", action });
@@ -571,7 +572,10 @@ function ReviewDetail({ initial }: { initial: ActionProposal }) {
     try {
       let next: Detail;
       if (action.kind === "email.send")
-        next = { type: "email", draft: emailDraftSchema.parse(action.data) };
+        next = {
+          type: "email",
+          draft: { ...emailDraftSchema.parse(action.data), id: action.draftId },
+        };
       else {
         const draft = eventDraftSchema.parse(action.data);
         if (action.kind === "calendar.update") {
@@ -581,11 +585,16 @@ function ReviewDetail({ initial }: { initial: ActionProposal }) {
           next = { type: "event", event: { ...draft, id: eventId } };
         } else next = { type: "event", draft };
       }
-      await api.request(`/api/actions/${action.id}/decide`, {
+      const result = await api.request<ActionProposal>(`/api/actions/${action.id}/decide`, {
         decision: "deny",
         hash: action.hash,
       });
       await refresh();
+      // A review already replaced or sent elsewhere shows its outcome instead of an editor.
+      if (result.status !== "denied") {
+        setLocal(result);
+        return;
+      }
       open(next);
     } catch (e) {
       setError(e instanceof Error ? e.message : String(e));

apps/server/src/actions.ts

@@ -7,6 +7,7 @@ import {
 } from "../../../packages/domain/src/index.ts";
 import type { Store } from "./db.ts";
 import { AppError } from "./errors.ts";
+import { backgroundFailure } from "./log.ts";
 
 interface Options {
   execute: (
@@ -51,6 +52,12 @@ export class ActionService {
       if (existing) return existing;
     }
     const parsed = proposalSchema.parse(raw);
+    if (
+      parsed.kind === "email.send" &&
+      parsed.draftId &&
+      !(await this.db.get(owner, "drafts", parsed.draftId))
+    )
+      throw new AppError("Draft not found", 404);
     const connection = await this.options.connection?.(owner);
     if (this.options.connection && !connection)
       throw new AppError("Connect Google before preparing an action", 409);
@@ -66,6 +73,7 @@ export class ActionService {
     const proposal: ActionProposal = {
       id,
       taskId,
+      draftId: input.kind === "email.send" ? input.draftId : undefined,
       title,
       kind: input.kind,
       data: input.data,
@@ -96,9 +104,32 @@ export class ActionService {
       if (!existing) throw new AppError("Prepared action could not be loaded", 409);
       return existing;
     }
+    if (saved.draftId) {
+      await this.db.compareAndSwap(owner, "drafts", saved.draftId, {}, { actionId: saved.id });
+      await this.cancelDraftReviews(owner, saved.draftId, saved.id);
+    }
     await this.record(owner, saved, "Ready for your review");
     return saved;
   }
+  /** Leaves at most one pending send per draft, so one draft cannot be sent twice. */
+  async cancelDraftReviews(owner: string, draftId: string, keepId?: string) {
+    for (const action of await this.db.listByStatus<ActionProposal>(
+      owner,
+      "actions",
+      "awaiting_review",
+    )) {
+      if (action.draftId !== draftId || action.id === keepId) continue;
+      const cancelled = await this.db.compareAndSwap<ActionProposal>(
+        owner,
+        "actions",
+        action.id,
+        { status: "awaiting_review" },
+        { status: "cancelled" },
+      );
+      if (cancelled)
+        await this.record(owner, cancelled, "Replaced by a newer version of the draft");
+    }
+  }
   async decide(
     owner: string,
     id: string,
@@ -203,6 +234,15 @@ export class ActionService {
     }
     await this.db.put(owner, "actions", finished);
     await this.record(owner, finished, finished.result ?? finished.error ?? finished.status);
+    if (finished.status === "succeeded" && finished.draftId) {
+      // The email has already gone out, so a cleanup failure must not read as a failed send.
+      // A draft saved again after this review no longer links to it and is kept.
+      try {
+        await this.db.removeIf(owner, "drafts", finished.draftId, { actionId: finished.id });
+      } catch (error) {
+        backgroundFailure("sent draft cleanup", error);
+      }
+    }
     return finished;
   }
   private async record(owner: string, action: ActionProposal, detail: string) {

apps/server/src/app.ts

@@ -196,14 +196,13 @@ export async function createApp(
       ? await db.get<{ createdAt: string }>(c.get("owner"), "drafts", body.id)
       : null;
     if (body.id && !existing) throw new AppError("Draft not found", 404);
-    return c.json(
-      await db.put(c.get("owner"), "drafts", {
-        ...body,
-        id: body.id ?? randomUUID(),
-        createdAt: existing?.createdAt ?? new Date().toISOString(),
-      }),
-      201,
-    );
+    const saved = await db.put(c.get("owner"), "drafts", {
+      ...body,
+      id: body.id ?? randomUUID(),
+      createdAt: existing?.createdAt ?? new Date().toISOString(),
+    });
+    if (existing) await actions.cancelDraftReviews(c.get("owner"), saved.id);
+    return c.json(saved, 201);
   });
   app.get("/api/main-thread", async (c) => {
     const owner = c.get("owner");

apps/server/src/db.ts

@@ -66,6 +66,17 @@ export class Store {
       id,
     ]);
   }
+  async removeIf(
+    owner: string,
+    kind: string,
+    id: string,
+    expected: Record<string, unknown>,
+  ): Promise<void> {
+    await this.db.query(
+      "DELETE FROM records WHERE owner=$1 AND kind=$2 AND id=$3 AND data @> $4::jsonb",
+      [owner, kind, id, JSON.stringify(expected)],
+    );
+  }
   async compareAndSwap<T>(
     owner: string,
     kind: string,

packages/domain/src/index.ts

@@ -126,7 +126,11 @@ export const eventDraftSchema = z
     }
   });
 export const proposalSchema = z.discriminatedUnion("kind", [
-  z.object({ kind: z.literal("email.send"), data: emailDraftSchema }),
+  z.object({
+    kind: z.literal("email.send"),
+    data: emailDraftSchema,
+    draftId: z.uuid().optional(),
+  }),
   z.object({ kind: z.literal("calendar.create"), data: eventDraftSchema }),
   z.object({
     kind: z.literal("calendar.update"),
@@ -144,6 +148,7 @@ export interface ActionProposal {
   target?: CalendarEvent;
   targetVersion?: string;
   taskId?: string;
+  draftId?: string;
   account?: string;
   connectionId?: string;
   id: string;

tests/actions.test.ts

@@ -1,4 +1,5 @@
 import assert from "node:assert/strict";
+import { randomUUID } from "node:crypto";
 import { after, before, test } from "node:test";
 import { ActionService } from "../apps/server/src/actions.ts";
 import { createStore, type Store } from "../apps/server/src/db.ts";
@@ -119,6 +120,118 @@ test("uncertain writes retain uncertainty and cannot be retried", async () => {
   await service.decide("uncertain-user", proposal.id, proposal.hash, "approve");
   assert.equal(calls, 1);
 });
+async function sendDraft(
+  owner: string,
+  decision: "approve" | "deny",
+  execute: () => Promise<string>,
+) {
+  const service = new ActionService(db, { execute, connected: async () => true });
+  const draft = await db.put(owner, "drafts", { ...email.data, id: randomUUID() });
+  const proposal = await service.propose(owner, { ...email, draftId: draft.id });
+  const result = await service.decide(owner, proposal.id, proposal.hash, decision);
+  return { status: result.status, draft: await db.get(owner, "drafts", draft.id) };
+}
+test("sending a saved draft removes it from drafts", async () => {
+  const sent = await sendDraft("draft-sent-user", "approve", async () => "sent");
+  assert.equal(sent.status, "succeeded");
+  assert.equal(sent.draft, null);
+});
+test("a draft stays when its email is declined, fails or has an unknown outcome", async () => {
+  const denied = await sendDraft("draft-denied-user", "deny", async () => "sent");
+  assert.equal(denied.status, "denied");
+  assert.ok(denied.draft);
+  const failed = await sendDraft("draft-failed-user", "approve", async () => {
+    throw new Error("Provider rejected the message");
+  });
+  assert.equal(failed.status, "failed");
+  assert.ok(failed.draft);
+  const uncertain = await sendDraft("draft-uncertain-user", "approve", async () => {
+    throw Object.assign(new Error("Provider response lost"), { outcomeUnknown: true });
+  });
+  assert.equal(uncertain.status, "outcome_unknown");
+  assert.ok(uncertain.draft);
+});
+test("an expired review keeps its draft", async () => {
+  let now = Date.now();
+  const service = new ActionService(db, {
+    execute: async () => "sent",
+    connected: async () => true,
+    now: () => now,
+  });
+  const draft = await db.put("draft-expired-user", "drafts", { ...email.data, id: randomUUID() });
+  const proposal = await service.propose("draft-expired-user", { ...email, draftId: draft.id });
+  now += 31 * 60 * 1000;
+  await assert.rejects(
+    service.decide("draft-expired-user", proposal.id, proposal.hash, "approve"),
+    /expired/i,
+  );
+  assert.ok(await db.get("draft-expired-user", "drafts", draft.id));
+});
+test("a draft rewritten after its review started is kept when that review is approved", async () => {
+  const service = new ActionService(db, {
+    execute: async () => "sent",
+    connected: async () => true,
+  });
+  const id = randomUUID();
+  await db.put("draft-rewrite-user", "drafts", { ...email.data, id });
+  const proposal = await service.propose("draft-rewrite-user", { ...email, draftId: id });
+  await db.put("draft-rewrite-user", "drafts", { ...email.data, id, body: "Rewritten" });
+  await service.decide("draft-rewrite-user", proposal.id, proposal.hash, "approve");
+  const kept = await db.get<{ body: string }>("draft-rewrite-user", "drafts", id);
+  assert.equal(kept?.body, "Rewritten");
+});
+test("a newer review of a draft replaces the older one, so the email is sent once", async () => {
+  let sends = 0;
+  const service = new ActionService(db, {
+    execute: async () => {
+      sends++;
+      return "sent";
+    },
+    connected: async () => true,
+  });
+  const draft = await db.put("draft-twice-user", "drafts", { ...email.data, id: randomUUID() });
+  const first = await service.propose("draft-twice-user", { ...email, draftId: draft.id });
+  const second = await service.propose("draft-twice-user", { ...email, draftId: draft.id });
+  const stale = await service.decide("draft-twice-user", first.id, first.hash, "approve");
+  assert.equal(stale.status, "cancelled");
+  const sent = await service.decide("draft-twice-user", second.id, second.hash, "approve");
+  assert.equal(sent.status, "succeeded");
+  assert.equal(sends, 1);
+  assert.equal(await db.get("draft-twice-user", "drafts", draft.id), null);
+});
+test("a failed draft cleanup still reports the email as sent", async (t) => {
+  const store = await createStore();
+  t.after(() => store.close());
+  const service = new ActionService(store, {
+    execute: async () => "sent",
+    connected: async () => true,
+  });
+  const draft = await store.put("cleanup-user", "drafts", { ...email.data, id: randomUUID() });
+  const proposal = await service.propose("cleanup-user", { ...email, draftId: draft.id });
+  const unavailable = async () => {
+    throw new Error("Database briefly unavailable");
+  };
+  Object.assign(store, { remove: unavailable, removeIf: unavailable });
+  const logged = t.mock.method(console, "error", () => {});
+  const result = await service.decide("cleanup-user", proposal.id, proposal.hash, "approve");
+  assert.equal(result.status, "succeeded");
+  assert.equal(logged.mock.callCount(), 1);
+});
+test("a review only links a draft that exists for the same owner", async () => {
+  const service = new ActionService(db, {
+    execute: async () => "sent",
+    connected: async () => true,
+  });
+  const draft = await db.put("draft-owner", "drafts", { ...email.data, id: randomUUID() });
+  await assert.rejects(
+    service.propose("draft-other-owner", { ...email, draftId: draft.id }),
+    /Draft not found/,
+  );
+  await assert.rejects(
+    service.propose("draft-owner", { ...email, draftId: randomUUID() }),
+    /Draft not found/,
+  );
+});
 test("another service instance sees persisted proposals", async () => {
   const options = { execute: async () => "created", connected: async () => true };
   const first = new ActionService(db, options);

tests/api.test.ts

@@ -212,6 +212,26 @@ test("sample workspace serves a real PDF and filling creates a new version", asy
   forged.searchParams.set("owner", "another-user");
   assert.equal((await app.request(forged.toString())).status, 403);
 });
+test("saving a draft again cancels its pending review, so the older text is never sent", async () => {
+  const post = (path: string, body: object) =>
+    app.request(path, { method: "POST", headers: headers(), body: JSON.stringify(body) });
+  const message = { to: ["sample@example.com"], subject: "Trip", body: "First version" };
+  const draft: { id: string } = await (await post("/api/drafts", message)).json();
+  const review: ActionProposal = await (
+    await post("/api/actions", { kind: "email.send", data: message, draftId: draft.id })
+  ).json();
+  assert.equal(review.status, "awaiting_review");
+  const resaved = await post("/api/drafts", { ...message, id: draft.id, body: "Second version" });
+  assert.equal(resaved.status, 201);
+  const decided: ActionProposal = await (
+    await post(`/api/actions/${review.id}/decide`, { hash: review.hash, decision: "approve" })
+  ).json();
+  assert.equal(decided.status, "cancelled");
+  const drafts: { id: string; body: string }[] = await (
+    await app.request("/api/drafts", { headers: headers() })
+  ).json();
+  assert.equal(drafts.find((d) => d.id === draft.id)?.body, "Second version");
+});
 test("reviewed sample email persists a receipt, then revocation blocks another proposal", async () => {
   const propose = () =>
     app.request("/api/actions", {