fix(proposals): PR #3581 fresh-review findings — authority invariants, TOCTOU summaries, strict decision routes (cave-v4ep)

fixtures/phase-5/awaiting-human-approval.daemon-summary.json

@@ -10,7 +10,7 @@
   ],
   "approvalPath": {
     "variant": "human_approval_with_rationale",
-    "label": "human_review",
+    "label": "human_required",
     "veto_deadline": null,
     "affected_surfaces": [
       "MEMORY.md"

fixtures/phase-5/blocked.daemon-summary.json

@@ -10,7 +10,7 @@
   ],
   "approvalPath": {
     "variant": "human_approval",
-    "label": "policy_blocked",
+    "label": "human_review",
     "veto_deadline": null,
     "affected_surfaces": [
       "MEMORY.md"

fixtures/phase-5/mismatched.daemon-summary.json

@@ -11,7 +11,7 @@
   ],
   "approvalPath": {
     "variant": "familiar_coherence",
-    "label": "mismatch_review",
+    "label": "familiar_review",
     "veto_deadline": "2026-07-15T10:22:00Z",
     "affected_surfaces": [
       "MEMORY.md",

fixtures/phase-5/ready-for-replay.daemon-summary.json

@@ -10,7 +10,7 @@
   ],
   "approvalPath": {
     "variant": "familiar_coherence",
-    "label": "replay_ready",
+    "label": "familiar_review",
     "veto_deadline": "2026-07-15T09:52:00Z",
     "affected_surfaces": [
       "MEMORY.md"

fixtures/phase-5/unknown.daemon-summary.json

@@ -10,7 +10,7 @@
   ],
   "approvalPath": {
     "variant": "human_approval_with_rationale",
-    "label": "mystery_review",
+    "label": "human_required",
     "veto_deadline": null,
     "affected_surfaces": [
       "MEMORY.md"

fixtures/phase-5/veto-window-open.daemon-summary.json

@@ -10,7 +10,7 @@
   ],
   "approvalPath": {
     "variant": "familiar_coherence",
-    "label": "coherence_review",
+    "label": "familiar_review",
     "veto_deadline": "2026-07-15T09:42:00Z",
     "affected_surfaces": [
       "MEMORY.md"

scripts/run-tests.mjs

@@ -1221,6 +1221,7 @@ const ALIAS_LOADER = new Set([
   "src/lib/cave-inbox-bulk.test.ts",
   "src/lib/cave-inbox-create.test.ts",
   "src/app/api/chat/stream/route.test.ts",
+  "src/app/api/proposals-flow-e2e.test.ts",
   "src/app/api/prompts/route.test.ts",
   "src/app/api/marketplace/pack-prompts-route.test.ts",
   "src/lib/cave-backdrop.test.ts",

src/app/api/api-contracts.test.ts

@@ -270,10 +270,16 @@ function usesJsonResponse(source: string): boolean {
 }
 
 function effectiveRouteSource(file: string, source: string): string {
+  const parts = [source];
   const reexport = source.match(/from\s+"(\.[^"]+\/route)";/);
-  if (!reexport) return source;
-  const target = path.resolve(path.dirname(file), `${reexport[1]}.ts`);
-  return `${source}\n${readFileSync(target, "utf8")}`;
+  if (reexport) {
+    const target = path.resolve(path.dirname(file), `${reexport[1]}.ts`);
+    parts.push(readFileSync(target, "utf8"));
+  }
+  if (source.includes('from "@/lib/proposal-decision-body"')) {
+    parts.push(readFileSync(path.join(apiRoot, "..", "..", "lib", "proposal-decision-body.ts"), "utf8"));
+  }
+  return parts.join("\n");
 }
 
 const routeFiles = walkRoutes(apiRoot);
@@ -306,7 +312,7 @@ for (const contract of contracts) {
     assert.doesNotMatch(source, /invalid json|invalid JSON/, `${contract.route} legacy invalid-JSON behavior changed`);
   }
   if (contract.optionalJsonBody) {
-    assert.match(source, /rawBody\.trim\(\)/, `${contract.route} must accept a missing request body`);
+    assert.match(effectiveSource, /rawBody\.trim\(\)/, `${contract.route} must accept a missing request body`);
   }
   if (contract.pathGuard) {
     assert.match(source, /path not allowed|collection path not allowed/, `${contract.route} must preserve path-deny errors`);

src/app/api/proposals-flow-e2e.test.ts

@@ -1,21 +1,20 @@
 // E2E for the proposal decision flow with REAL staged-write fixtures
 // (threads-986.17.6, spec §3.7).
 //
-// `next/server` cannot be imported under the bare-node test runner, so this
-// test drives the exact composition the route handlers execute —
-// activeThreadsAdapter() (env-selected, no mocks) + httpStatusForEnvelope —
-// over the checked-in fixtures/phase-4/pending/ staged writes and a real
-// temp COVEN_HOME, then pins the route sources to that composition so the
-// handlers cannot drift from what is tested here. Guard behavior
-// (rejectNonLocalRequest, invalid-JSON 400) is enforced per-route by
-// src/app/api/api-contracts.test.ts.
+// This test drives both the route handlers and the exact composition they
+// forward into — activeThreadsAdapter() (env-selected, no mocks) +
+// httpStatusForEnvelope — over checked-in and isolated staged-write fixtures.
 import assert from "node:assert/strict";
 import { mkdtempSync, mkdirSync, readdirSync, readFileSync, rmSync, writeFileSync } from "node:fs";
 import { tmpdir } from "node:os";
 import path from "node:path";
 import { after, afterEach, describe, it } from "node:test";
 
 import { activeThreadsAdapter, httpStatusForEnvelope } from "../../lib/threads-adapters.ts";
+import { parseProposalDecisionBody } from "../../lib/proposal-decision-body.ts";
+
+const { POST: approveProposal } = await import("./proposals/[id]/approve/route.ts");
+const { POST: rejectProposal } = await import("./proposals/[id]/reject/route.ts");
 
 const PROPOSAL_OK = "cccccccc-0001-4001-8001-000000000001";
 const CORRUPT_ID = "dddddddd-0001-4001-8001-000000000001";
@@ -56,28 +55,80 @@ describe("route-source pins: handlers are exactly the composition under test", (
     }
   });
 
-  it("decision routes accept an optional body, validate revisions, and forward revision + note", () => {
+  it("decision routes preserve their local guard and forward the shared parser result exactly", () => {
     for (const file of ["proposals/[id]/approve/route.ts", "proposals/[id]/reject/route.ts"]) {
       const source = readFileSync(new URL(`./${file}`, import.meta.url), "utf8");
       assert.match(source, /rejectNonLocalRequest\(req\)/, `${file} keeps the local-origin guard`);
-      assert.match(source, /await req\.text\(\)/, `${file} reads an optional raw body`);
-      assert.match(source, /rawBody\.trim\(\)/, `${file} preserves bodyless legacy calls`);
-      assert.match(source, /invalid json body/, `${file} answers 400 on malformed JSON`);
-      assert.match(source, /\^\[0-9a-f\]\{64\}\$/, `${file} requires exactly 64 lowercase hex characters`);
-      assert.match(source, /invalid expectedRevision/, `${file} answers 400 on malformed revisions`);
       assert.match(
         source,
-        /\.(approve|reject)\(id, expectedRevision, note\)/,
-        `${file} forwards id + expectedRevision + note to the adapter only`,
+        /parseProposalDecisionBody\(await req\.text\(\)\)/,
+        `${file} parses the optional body through the shared pure helper`,
       );
-      assert.ok(
-        source.indexOf("invalid expectedRevision") < source.search(/\.(approve|reject)\(id, expectedRevision, note\)/),
-        `${file} validates the revision before forwarding`,
+      assert.match(
+        source,
+        /\.(approve|reject)\(id, decision\.expectedRevision, decision\.note\)/,
+        `${file} forwards the parser's exact expectedRevision + note result`,
       );
     }
   });
 });
 
+describe("proposal decision body parser", () => {
+  it("preserves omitted and structurally empty legacy bodies", () => {
+    for (const rawBody of ["", " \n ", "{}", "null", "[]", '"legacy primitive"']) {
+      assert.deepEqual(parseProposalDecisionBody(rawBody), {
+        ok: true,
+        expectedRevision: undefined,
+        note: undefined,
+      });
+    }
+  });
+
+  it("normalizes null notes to omitted and preserves string notes exactly", () => {
+    assert.deepEqual(parseProposalDecisionBody('{"note":null}'), {
+      ok: true,
+      expectedRevision: undefined,
+      note: undefined,
+    });
+    assert.deepEqual(parseProposalDecisionBody('{"note":" principal note "}'), {
+      ok: true,
+      expectedRevision: undefined,
+      note: " principal note ",
+    });
+  });
+
+  it("rejects every non-null, non-string note", () => {
+    for (const note of [42, false, { text: "no" }, ["no"]]) {
+      assert.deepEqual(parseProposalDecisionBody(JSON.stringify({ note })), {
+        ok: false,
+        error: "invalid note",
+      });
+    }
+  });
+
+  it("rejects malformed JSON", () => {
+    assert.deepEqual(parseProposalDecisionBody("{"), {
+      ok: false,
+      error: "invalid json body",
+    });
+  });
+
+  it("accepts only omitted or exact lowercase SHA-256 revisions", () => {
+    const revision = "a".repeat(64);
+    assert.deepEqual(parseProposalDecisionBody(JSON.stringify({ expectedRevision: revision, note: "ship it" })), {
+      ok: true,
+      expectedRevision: revision,
+      note: "ship it",
+    });
+    for (const expectedRevision of [null, "", "a".repeat(63), "A".repeat(64), 42]) {
+      assert.deepEqual(parseProposalDecisionBody(JSON.stringify({ expectedRevision })), {
+        ok: false,
+        error: "invalid expectedRevision",
+      });
+    }
+  });
+});
+
 describe("GET flow — real checked-in staged writes through the env-selected adapter", () => {
   it("serves the legacy and Phase 5 pending fixtures with the freshness envelope", async () => {
     process.env.COVEN_THREADS_ADAPTER = "fixtures";
@@ -110,6 +161,28 @@ describe("GET flow — real checked-in staged writes through the env-selected ad
 });
 
 describe("decision flow — forward-only, fail-closed, staged files untouched", () => {
+  for (const [decision, handler] of [
+    ["approve", approveProposal],
+    ["reject", rejectProposal],
+  ] as const) {
+    it(`${decision} route rejects non-null, non-string notes with 400`, async () => {
+      process.env.COVEN_THREADS_ADAPTER = "fixtures";
+      for (const note of [42, false, { text: "no" }, ["no"]]) {
+        const response = await handler(
+          new Request(`http://127.0.0.1/api/proposals/${PROPOSAL_OK}/${decision}`, {
+            method: "POST",
+            headers: { "content-type": "application/json", host: "127.0.0.1" },
+            body: JSON.stringify({ note }),
+          }),
+          { params: Promise.resolve({ id: PROPOSAL_OK }) },
+        );
+        assert.equal(response.status, 400);
+        assert.deepEqual(await response.json(), { ok: false, error: "invalid note" });
+      }
+    });
+
+  }
+
   it("R5: fixtures mode (no daemon) answers 503 and the checked-in files never change", async () => {
     process.env.COVEN_THREADS_ADAPTER = "fixtures";
     delete process.env.COVEN_THREADS_FIXTURE_SCENARIO;

src/app/api/proposals/[id]/approve/route.ts

@@ -1,12 +1,11 @@
-import { NextResponse } from "next/server";
+import { NextResponse } from "next/server.js";
+import { parseProposalDecisionBody } from "@/lib/proposal-decision-body";
 import { rejectNonLocalRequest } from "@/lib/server/api-security";
 import { activeThreadsAdapter, httpStatusForEnvelope } from "@/lib/threads-adapters";
 
 export const dynamic = "force-dynamic";
 export const runtime = "nodejs";
 
-type DecisionBody = { expectedRevision?: unknown; note?: unknown };
-
 // POST /api/proposals/[id]/approve — thin daemon-forwarder (spec §3.7). The
 // daemon re-validates, applies or refuses, audits, and removes the pending
 // file; this route never mutates anything itself. Fails closed (503) when
@@ -15,27 +14,8 @@ export async function POST(req: Request, { params }: { params: Promise<{ id: str
   const rejected = rejectNonLocalRequest(req);
   if (rejected) return rejected;
   const { id } = await params;
-  let body: DecisionBody = {};
-  const rawBody = await req.text();
-  if (rawBody.trim() !== "") {
-    let parsed: unknown;
-    try {
-      parsed = JSON.parse(rawBody);
-    } catch {
-      return NextResponse.json({ ok: false, error: "invalid json body" }, { status: 400 });
-    }
-    if (typeof parsed === "object" && parsed !== null && !Array.isArray(parsed)) {
-      body = parsed as DecisionBody;
-    }
-  }
-  const expectedRevision = body.expectedRevision;
-  if (
-    expectedRevision !== undefined &&
-    (typeof expectedRevision !== "string" || !/^[0-9a-f]{64}$/.test(expectedRevision))
-  ) {
-    return NextResponse.json({ ok: false, error: "invalid expectedRevision" }, { status: 400 });
-  }
-  const note = typeof body.note === "string" ? body.note : undefined;
-  const envelope = await activeThreadsAdapter().approve(id, expectedRevision, note);
+  const decision = parseProposalDecisionBody(await req.text());
+  if (!decision.ok) return NextResponse.json({ ok: false, error: decision.error }, { status: 400 });
+  const envelope = await activeThreadsAdapter().approve(id, decision.expectedRevision, decision.note);
   return NextResponse.json(envelope, { status: httpStatusForEnvelope(envelope, "POST") });
 }

src/app/api/proposals/[id]/reject/route.ts

@@ -1,40 +1,20 @@
-import { NextResponse } from "next/server";
+import { NextResponse } from "next/server.js";
+import { parseProposalDecisionBody } from "@/lib/proposal-decision-body";
 import { rejectNonLocalRequest } from "@/lib/server/api-security";
 import { activeThreadsAdapter, httpStatusForEnvelope } from "@/lib/threads-adapters";
 
 export const dynamic = "force-dynamic";
 export const runtime = "nodejs";
 
-type DecisionBody = { expectedRevision?: unknown; note?: unknown };
-
 // POST /api/proposals/[id]/reject — thin daemon-forwarder (spec §3.7).
 // Rejection is a daemon-side decision too: the daemon audits it and removes
 // the pending file. Fails closed (503) without a daemon.
 export async function POST(req: Request, { params }: { params: Promise<{ id: string }> }) {
   const rejected = rejectNonLocalRequest(req);
   if (rejected) return rejected;
   const { id } = await params;
-  let body: DecisionBody = {};
-  const rawBody = await req.text();
-  if (rawBody.trim() !== "") {
-    let parsed: unknown;
-    try {
-      parsed = JSON.parse(rawBody);
-    } catch {
-      return NextResponse.json({ ok: false, error: "invalid json body" }, { status: 400 });
-    }
-    if (typeof parsed === "object" && parsed !== null && !Array.isArray(parsed)) {
-      body = parsed as DecisionBody;
-    }
-  }
-  const expectedRevision = body.expectedRevision;
-  if (
-    expectedRevision !== undefined &&
-    (typeof expectedRevision !== "string" || !/^[0-9a-f]{64}$/.test(expectedRevision))
-  ) {
-    return NextResponse.json({ ok: false, error: "invalid expectedRevision" }, { status: 400 });
-  }
-  const note = typeof body.note === "string" ? body.note : undefined;
-  const envelope = await activeThreadsAdapter().reject(id, expectedRevision, note);
+  const decision = parseProposalDecisionBody(await req.text());
+  if (!decision.ok) return NextResponse.json({ ok: false, error: decision.error }, { status: 400 });
+  const envelope = await activeThreadsAdapter().reject(id, decision.expectedRevision, decision.note);
   return NextResponse.json(envelope, { status: httpStatusForEnvelope(envelope, "POST") });
 }