diff --git a/.env.example b/.env.example index 77d17d8..53cb366 100644 --- a/.env.example +++ b/.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 diff --git a/docker-compose.example.yml b/docker-compose.example.yml index 73b55cc..d3d1988 100644 --- a/docker-compose.example.yml +++ b/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} diff --git a/src/config.ts b/src/config.ts index a145301..d454f47 100644 --- a/src/config.ts +++ b/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 diff --git a/src/tools/docShared.ts b/src/tools/docShared.ts index c97409d..c6f678a 100644 --- a/src/tools/docShared.ts +++ b/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() + : ""; +} diff --git a/src/tools/postProjectMessage.ts b/src/tools/postProjectMessage.ts index 5702795..6a061dd 100644 --- a/src/tools/postProjectMessage.ts +++ b/src/tools/postProjectMessage.ts @@ -52,7 +52,7 @@ export const postProjectMessage: Tool = { | { 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({ diff --git a/src/tools/proposePlan.ts b/src/tools/proposePlan.ts index b384523..4d8beda 100644 --- a/src/tools/proposePlan.ts +++ b/src/tools/proposePlan.ts @@ -24,6 +24,24 @@ const InputSchema = z }) .strict(); +export function planProposalDecision( + workflow: Record, + 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 = { name: "proposePlan", kind: "action", @@ -70,34 +88,72 @@ export const proposePlan: Tool = { }; } - 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; + const workflow = (projectData.workflow ?? {}) as Record; + 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", diff --git a/src/tools/upsertPlanGroup.ts b/src/tools/upsertPlanGroup.ts index 3dd53ad..d4aac4c 100644 --- a/src/tools/upsertPlanGroup.ts +++ b/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 = { }; } + 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; + 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; if (data.type !== "planGroup") { @@ -84,6 +106,16 @@ export const upsertPlanGroup: Tool = { ? ((existing.data() as Record).order as number) ?? 0 : await nextBlockOrder(docRef)); + if (existing?.exists) { + const current = existing.data() as Record; + 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", diff --git a/src/tools/upsertPlanTask.ts b/src/tools/upsertPlanTask.ts index 0adf632..c850bef 100644 --- a/src/tools/upsertPlanTask.ts +++ b/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 = { }; } + 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 = { }; } - 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; + 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; if (data.type !== "planTask") { @@ -115,6 +140,22 @@ export const upsertPlanTask: Tool = { ? ((existing.data() as Record).order as number) ?? 0 : await nextBlockOrder(docRef); + if (existing?.exists) { + const current = existing.data() as Record; + 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", diff --git a/tests/proposal-policy.test.ts b/tests/proposal-policy.test.ts new file mode 100644 index 0000000..d6ce0f7 --- /dev/null +++ b/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"); + }); +});