Make project plan revisions idempotent

.env.example

@@ -22,7 +22,7 @@ JWT_MAX_AGE_SECONDS=60
 
 # ---- Rate limit (per wallet, per minute) ----
 RATE_LIMIT_READ_PER_MIN=60
-RATE_LIMIT_ACTION_PER_MIN=10
+RATE_LIMIT_ACTION_PER_MIN=30
 
 # ---- Audit ----
 AUDIT_ENABLED=true

docker-compose.example.yml

@@ -32,7 +32,7 @@ services:
 
       # Rate limits.
       RATE_LIMIT_READ_PER_MIN: ${RATE_LIMIT_READ_PER_MIN:-60}
-      RATE_LIMIT_ACTION_PER_MIN: ${RATE_LIMIT_ACTION_PER_MIN:-10}
+      RATE_LIMIT_ACTION_PER_MIN: ${RATE_LIMIT_ACTION_PER_MIN:-30}
 
       # Audit toggle (production should always be true).
       AUDIT_ENABLED: ${AUDIT_ENABLED:-true}

src/config.ts

@@ -44,7 +44,7 @@ const schema = z.object({
   // Per-wallet token bucket. Reads are cheap; actions are throttled
   // tighter.
   RATE_LIMIT_READ_PER_MIN: z.coerce.number().int().min(1).default(60),
-  RATE_LIMIT_ACTION_PER_MIN: z.coerce.number().int().min(1).default(10),
+  RATE_LIMIT_ACTION_PER_MIN: z.coerce.number().int().min(1).default(30),
 
   // --- Audit ---
   // If false, audit log writes are dropped (used in tests). Production

src/tools/docShared.ts

@@ -174,7 +174,11 @@ export async function touchDocForEdit(
     revision: FieldValue.increment(1),
     updatedAt: now,
   };
-  if (currentStatus === "draft" || currentStatus == null) {
+  if (
+    currentStatus === "draft" ||
+    currentStatus === "plan_proposed" ||
+    currentStatus == null
+  ) {
     patch.status = "under_discussion";
   }
   const batch = docRef.firestore.batch();
@@ -188,3 +192,9 @@ export async function touchDocForEdit(
   });
   await batch.commit();
 }
+
+export function normalizedPlanLabel(value: unknown): string {
+  return typeof value === "string"
+    ? value.trim().replace(/\s+/g, " ").toLocaleLowerCase()
+    : "";
+}

src/tools/postProjectMessage.ts

@@ -52,7 +52,7 @@ export const postProjectMessage: Tool<typeof InputSchema> = {
       | { phase?: string; convId?: string }
       | null;
     const workflowConvId =
-      workflow?.convId && ["running", "pm_review"].includes(String(workflow.phase))
+      workflow?.convId && ["running", "pm_review", "complete"].includes(String(workflow.phase))
         ? workflow.convId
         : undefined;
     const message = await postProjectChat({

src/tools/proposePlan.ts

@@ -24,6 +24,24 @@ const InputSchema = z
   })
   .strict();
 
+export function planProposalDecision(
+  workflow: Record<string, unknown>,
+  docStatus: unknown,
+  docId: string,
+): "propose" | "idempotent" | "conflict" {
+  if (
+    workflow.phase === "awaiting_approval" &&
+    workflow.planId === docId &&
+    docStatus === "plan_proposed"
+  ) {
+    return "idempotent";
+  }
+  if (["running", "pm_review", "complete"].includes(String(workflow.phase ?? ""))) {
+    return "conflict";
+  }
+  return "propose";
+}
+
 export const proposePlan: Tool<typeof InputSchema> = {
   name: "proposePlan",
   kind: "action",
@@ -70,34 +88,72 @@ export const proposePlan: Tool<typeof InputSchema> = {
       };
     }
 
-    const now = FieldValue.serverTimestamp();
     const projectRef = docRef.parent.parent!;
-    const project = await projectRef.get();
-    const workflowConvId =
-      typeof project.data()?.chatConvId === "string"
-        ? (project.data()?.chatConvId as string)
-        : undefined;
-    const batch = projectRef.firestore.batch();
-    batch.set(
-      docRef,
-      { status: "plan_proposed", revision: FieldValue.increment(1), updatedAt: now },
-      { merge: true },
-    );
-    batch.set(
-      projectRef,
-      {
-        workflow: {
-          phase: "awaiting_approval",
-          planId: refs.docId,
-          taskIds: [],
-          ...(workflowConvId ? { convId: workflowConvId } : {}),
+    let workflowConvId: string | undefined;
+    let proposed = false;
+    await projectRef.firestore.runTransaction(async (tx) => {
+      const [freshProject, freshDoc] = await Promise.all([
+        tx.get(projectRef),
+        tx.get(docRef),
+      ]);
+      if (!freshProject.exists || !freshDoc.exists) throw new Error("plan not found");
+      const projectData = freshProject.data() as Record<string, unknown>;
+      const workflow = (projectData.workflow ?? {}) as Record<string, unknown>;
+      workflowConvId =
+        typeof workflow.convId === "string"
+          ? workflow.convId
+          : typeof projectData.chatConvId === "string"
+            ? projectData.chatConvId
+            : undefined;
+      const decision = planProposalDecision(workflow, freshDoc.data()?.status, refs.docId);
+      if (decision === "idempotent") return;
+      if (decision === "conflict") {
+        throw new Error("This workflow has already started; its approved plan is immutable.");
+      }
+      proposed = true;
+      const now = FieldValue.serverTimestamp();
+      const currentRevision = Number(freshDoc.data()?.revision ?? 0);
+      tx.set(
+        docRef,
+        {
+          status: "plan_proposed",
+          revision: currentRevision + 1,
+          proposedRevision: currentRevision + 1,
           updatedAt: now,
         },
-        updatedAt: now,
-      },
-      { merge: true },
-    );
-    await batch.commit();
+        { merge: true },
+      );
+      tx.set(
+        projectRef,
+        {
+          workflow: {
+            ...workflow,
+            phase: "awaiting_approval",
+            planId: refs.docId,
+            taskIds: [],
+            proposalRevision: currentRevision + 1,
+            ...(workflowConvId ? { convId: workflowConvId } : {}),
+            updatedAt: now,
+          },
+          updatedAt: now,
+        },
+        { merge: true },
+      );
+    });
+
+    if (!proposed) {
+      return {
+        ok: true,
+        data: {
+          docId: refs.docId,
+          status: "plan_proposed",
+          groups,
+          tasks,
+          chatDelivered: true,
+          unchanged: true,
+        },
+      };
+    }
 
     await docRef.collection("revisions").doc().set({
       actor: ctx.convId ?? "agent",

src/tools/upsertPlanGroup.ts

@@ -16,6 +16,7 @@ import {
   docIdSchema,
   ensureDoc,
   nextBlockOrder,
+  normalizedPlanLabel,
   projectIdSchema,
   touchDocForEdit,
 } from "./docShared.js";
@@ -61,12 +62,33 @@ export const upsertPlanGroup: Tool<typeof InputSchema> = {
       };
     }
 
+    const project = await docRef.parent.parent!.get();
+    const phase = String((project.data()?.workflow as { phase?: unknown } | undefined)?.phase ?? "");
+    if (phase === "awaiting_approval") {
+      return {
+        ok: false,
+        errorClass: "BAD_INPUT",
+        message: "This plan is awaiting the user's decision. Do not rewrite it until changes are requested.",
+      };
+    }
+
     const blocksCol = docRef.collection("blocks");
-    const groupRef = args.groupId
+    let groupRef = args.groupId
       ? blocksCol.doc(args.groupId)
       : blocksCol.doc();
-
-    const existing = args.groupId ? await groupRef.get() : null;
+    let existing = args.groupId ? await groupRef.get() : null;
+    if (!args.groupId) {
+      const blocks = await blocksCol.get();
+      const match = blocks.docs.find((block) => {
+        const data = block.data() as Record<string, unknown>;
+        return data.type === "planGroup" &&
+          normalizedPlanLabel(data.title) === normalizedPlanLabel(args.title);
+      });
+      if (match) {
+        groupRef = match.ref;
+        existing = match;
+      }
+    }
     if (existing && existing.exists) {
       const data = existing.data() as Record<string, unknown>;
       if (data.type !== "planGroup") {
@@ -84,6 +106,16 @@ export const upsertPlanGroup: Tool<typeof InputSchema> = {
         ? ((existing.data() as Record<string, unknown>).order as number) ?? 0
         : await nextBlockOrder(docRef));
 
+    if (existing?.exists) {
+      const current = existing.data() as Record<string, unknown>;
+      if (
+        normalizedPlanLabel(current.title) === normalizedPlanLabel(args.title) &&
+        Number(current.order ?? 0) === order
+      ) {
+        return { ok: true, data: { docId: refs.docId, groupId: groupRef.id, unchanged: true } };
+      }
+    }
+
     await groupRef.set(
       {
         type: "planGroup",

src/tools/upsertPlanTask.ts

@@ -17,6 +17,7 @@ import {
   docIdSchema,
   ensureDoc,
   nextBlockOrder,
+  normalizedPlanLabel,
   projectIdSchema,
   touchDocForEdit,
 } from "./docShared.js";
@@ -69,12 +70,23 @@ export const upsertPlanTask: Tool<typeof InputSchema> = {
       };
     }
 
+    const projectSnapshot = await docRef.parent.parent!.get();
+    const phase = String(
+      (projectSnapshot.data()?.workflow as { phase?: unknown } | undefined)?.phase ?? "",
+    );
+    if (phase === "awaiting_approval") {
+      return {
+        ok: false,
+        errorClass: "BAD_INPUT",
+        message: "This plan is awaiting the user's decision. Do not rewrite it until changes are requested.",
+      };
+    }
+
     const blocksCol = docRef.collection("blocks");
 
     if (args.suggestedAgent) {
-      const project = await docRef.parent.parent!.get();
-      const roster = Array.isArray(project.data()?.agentIds)
-        ? (project.data()?.agentIds as unknown[]).filter(
+      const roster = Array.isArray(projectSnapshot.data()?.agentIds)
+        ? (projectSnapshot.data()?.agentIds as unknown[]).filter(
             (name): name is string => typeof name === "string" && name.length > 0,
           )
         : [];
@@ -97,8 +109,21 @@ export const upsertPlanTask: Tool<typeof InputSchema> = {
       };
     }
 
-    const taskRef = args.taskId ? blocksCol.doc(args.taskId) : blocksCol.doc();
-    const existing = args.taskId ? await taskRef.get() : null;
+    let taskRef = args.taskId ? blocksCol.doc(args.taskId) : blocksCol.doc();
+    let existing = args.taskId ? await taskRef.get() : null;
+    if (!args.taskId) {
+      const blocks = await blocksCol.get();
+      const match = blocks.docs.find((block) => {
+        const data = block.data() as Record<string, unknown>;
+        return data.type === "planTask" &&
+          data.groupId === args.groupId &&
+          normalizedPlanLabel(data.title) === normalizedPlanLabel(args.title);
+      });
+      if (match) {
+        taskRef = match.ref;
+        existing = match;
+      }
+    }
     if (existing && existing.exists) {
       const data = existing.data() as Record<string, unknown>;
       if (data.type !== "planTask") {
@@ -115,6 +140,22 @@ export const upsertPlanTask: Tool<typeof InputSchema> = {
         ? ((existing.data() as Record<string, unknown>).order as number) ?? 0
         : await nextBlockOrder(docRef);
 
+    if (existing?.exists) {
+      const current = existing.data() as Record<string, unknown>;
+      const currentDeps = Array.isArray(current.deps) ? current.deps : [];
+      const nextDeps = args.deps ?? [];
+      const unchanged =
+        current.groupId === args.groupId &&
+        normalizedPlanLabel(current.title) === normalizedPlanLabel(args.title) &&
+        (current.desc ?? null) === (args.desc ?? null) &&
+        (current.suggestedAgent ?? null) === (args.suggestedAgent ?? null) &&
+        (current.acceptance ?? null) === (args.acceptance ?? null) &&
+        JSON.stringify(currentDeps) === JSON.stringify(nextDeps);
+      if (unchanged) {
+        return { ok: true, data: { docId: refs.docId, taskId: taskRef.id, unchanged: true } };
+      }
+    }
+
     await taskRef.set(
       {
         type: "planTask",

tests/proposal-policy.test.ts

@@ -0,0 +1,39 @@
+import { describe, expect, it } from "vitest";
+
+process.env.FIREBASE_PROJECT_ID = "test-project";
+process.env.FIREBASE_CLIENT_EMAIL = "test@example.com";
+process.env.FIREBASE_PRIVATE_KEY = "x";
+process.env.JWT_SHARED_SECRET = "a".repeat(40);
+
+const { planProposalDecision } = await import("../src/tools/proposePlan.js");
+const { normalizedPlanLabel } = await import("../src/tools/docShared.js");
+
+describe("plan proposal policy", () => {
+  it("makes a repeated proposal for the same pending revision idempotent", () => {
+    expect(
+      planProposalDecision(
+        { phase: "awaiting_approval", planId: "plan-1" },
+        "plan_proposed",
+        "plan-1",
+      ),
+    ).toBe("idempotent");
+  });
+
+  it("rejects proposals after approved work starts", () => {
+    for (const phase of ["running", "pm_review", "complete"]) {
+      expect(planProposalDecision({ phase }, "under_discussion", "plan-1")).toBe("conflict");
+    }
+  });
+
+  it("allows a revised draft to be proposed once", () => {
+    expect(
+      planProposalDecision({ phase: "planning", planId: "plan-1" }, "under_discussion", "plan-1"),
+    ).toBe("propose");
+  });
+});
+
+describe("normalizedPlanLabel", () => {
+  it("collapses cosmetic differences used by retrying PM calls", () => {
+    expect(normalizedPlanLabel("  Final   Synthesis ")).toBe("final synthesis");
+  });
+});