fix(status): reset review state on a new version, and derive shot status everywhere

Requesting changes on a shot and then re-rendering it through the render queue
left every layer of the app still showing the old verdict. Three independent
causes, all on the pipeline path.

finalizePreview inherited the superseded version's approvalStatus
unconditionally. The carry-over exists so a re-export of a signed-off shot does
not reappear as awaiting review, but it applied equally to NEEDS_CHANGES and
REJECTED, so the render queued to address a note arrived already wearing that
note and nothing registered that the work had been redone. carryApproval now
carries only APPROVED.

The pipeline never moved the task. One left on CHANGES by a review or a QC fail
stayed there through every re-render, with no way out but a manual kanban drag.
A genuinely new version now moves it to INTERNAL_REVIEW, as actions/versions.ts
and batch-upload already did. A replaced version is excluded: regeneratePreview
re-delivers the same version after a slate fix and is not a new submission.

The pipeline never called recalcShotStatus at all, so shot boards went stale the
moment a render changed a task. finalizePreview and the QC fail path both
recalculate now; the latter already documented the REVISIONS derivation it was
relying on and never triggering.

Shot.status is derived from tasks and approval flags, but six call sites wrote
it directly with the status they expected: internallyApproveShot,
shareWithClient, unshareFromClient, clientApproveShot, unapproveShot and the
client approve route. Rule 1 of the derivation is that any CHANGES task outranks
an approval, so a shot could be pushed past open rework and then snap back to
REVISIONS on the next unrelated recalc, which is close to unreportable as a bug.
They now set the approval inputs and let recalcShotStatus derive.
unshareFromClient additionally hardcoded READY_FOR_CLIENT even for a
client-approved shot. updateShotStatus keeps its direct write as the deliberate
manual override, now documented as transient for the same reason.

markShotForRevisions replaces the duplicated change-request logic. Both copies
marked all non-DONE tasks CHANGES, which matches nothing on the shot this is
most often called for: one out for client review precisely because all its tasks
are finished. The request then derived to INTERNAL_REVIEW and had no visible
effect anywhere. When nothing is picked up it now reopens the task behind the
shot's latest version, which is the deliverable the reviewer was looking at.

A client's shot-level change request also marks the versions and writes Approval
rows now. The dashboard counters, the version badges and the portal's own
filters all read Version.approvalStatus, so a shot-level verdict was invisible
everywhere those look.

clearStaleShotApproval covers the reverse case: uploading a new version onto an
approved shot moved the task to INTERNAL_REVIEW and changed nothing visible,
because the derivation short-circuits on shotApprovalStatus. The shot kept
reading COMPLETE while carrying work nobody had looked at. This is the one
user-visible behaviour change here, so watch the boards on a live show: an
approved shot now drops back to INTERNAL_REVIEW on a new upload. sharedWithClient
is deliberately left alone, since revoking client access stays a separate act.

submitApproval and unapproveVersion are transactional now. The verdict, the
version, the task and the derived shot status are one decision, and a failure
partway through left the three layers disagreeing with no way to tell which one
was right.

KanbanCard compared approvalStatus against "CHANGES_REQUESTED", which is not a
member of the enum, so the changes-requested icon never rendered at all.

Thirteen tests cover carryApproval, markShotForRevisions including the all-DONE
regression, and clearStaleShotApproval. What is not covered is the pipeline
transaction itself, which needs a database: queue a render against a shot in
REVISIONS and confirm the task and shot both move before trusting this on a
show.

Version.reviewStatus is still written nowhere while being published through
ext/shots/lookup and EXT_API_REFERENCE.md as a permanent "PENDING". Left alone
deliberately: removing a field from that contract could break the connector.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

actions/approvals.ts

@@ -5,7 +5,7 @@ import { db } from "@/lib/db";
 import { revalidatePath } from "next/cache";
 import { z } from "zod";
 import { ApprovalStatus } from "@prisma/client";
-import { recalcShotStatus } from "@/lib/shot-status";
+import { recalcShotStatus, clearStaleShotApproval } from "@/lib/shot-status";
 import { notifyApprovalChange } from "@/lib/notifications";
 import { slackNotifyApproval } from "@/lib/slack";
 import { versionLabel } from "@/lib/utils";
@@ -29,57 +29,62 @@ export async function submitApproval(data: z.infer<typeof approvalSchema>) {
     throw new Error("Insufficient permissions to review versions");
   }
 
-  // Create the approval record
-  const approval = await db.approval.create({
-    data: {
-      versionId: parsed.versionId,
-      userId: session.user.id,
-      status: parsed.status,
-      notes: parsed.notes,
-    },
-  });
+  // The verdict, the version, the task and the derived shot status are one
+  // decision and are written as one: a failure partway through used to leave the
+  // three layers disagreeing, with no way to tell which one was right.
+  const { approval, version } = await db.$transaction(async (tx) => {
+    const approval = await tx.approval.create({
+      data: {
+        versionId: parsed.versionId,
+        userId: session.user.id,
+        status: parsed.status,
+        notes: parsed.notes,
+      },
+    });
 
-  // Update version approval status
-  await db.version.update({
-    where: { id: parsed.versionId },
-    data: { approvalStatus: parsed.status },
-  });
+    await tx.version.update({
+      where: { id: parsed.versionId },
+      data: { approvalStatus: parsed.status },
+    });
 
-  // Load version + task + shot + project for downstream effects
-  const version = await db.version.findUnique({
-    where: { id: parsed.versionId },
-    include: {
-      task: {
-        include: {
-          shot: true,
-          project: true,
+    // Load version + task + shot + project for downstream effects
+    const version = await tx.version.findUnique({
+      where: { id: parsed.versionId },
+      include: {
+        task: {
+          include: {
+            shot: true,
+            project: true,
+          },
         },
       },
-    },
+    });
+
+    if (!version) throw new Error("Version not found");
+
+    // Update task status based on approval result
+    if (version.task && parsed.status !== "PENDING_REVIEW") {
+      if (parsed.status === "APPROVED") {
+        await tx.task.update({
+          where: { id: version.task.id },
+          data: { status: "DONE" },
+        });
+      } else if (parsed.status === "REJECTED" || parsed.status === "NEEDS_CHANGES") {
+        await tx.task.update({
+          where: { id: version.task.id },
+          data: { status: "CHANGES" },
+        });
+      }
+
+      // Recalculate shot status from updated task states
+      if (version.task.shot) {
+        await recalcShotStatus(version.task.shot.id, tx);
+      }
+    }
+
+    return { approval, version };
   });
 
-  if (!version) throw new Error("Version not found");
-
-  // Update task status based on approval result
-  if (version.task && parsed.status !== "PENDING_REVIEW") {
-    if (parsed.status === "APPROVED") {
-      await db.task.update({
-        where: { id: version.task.id },
-        data: { status: "DONE" },
-      });
-    } else if (parsed.status === "REJECTED" || parsed.status === "NEEDS_CHANGES") {
-      await db.task.update({
-        where: { id: version.task.id },
-        data: { status: "CHANGES" },
-      });
-    }
-
-    // Recalculate shot status from updated task states
-    if (version.task.shot) {
-      await recalcShotStatus(version.task.shot.id).catch(() => {});
-    }
-  }
-
   // Notifications
   const reviewer = await db.user.findUnique({ where: { id: session.user.id } });
   const reviewerName = reviewer?.name ?? "Reviewer";
@@ -166,34 +171,39 @@ export async function unapproveVersion(versionId: string) {
     throw new Error("Only approved versions can be unapproved");
   }
 
-  // Create an audit record for the unapproval
-  await db.approval.create({
-    data: {
-      versionId,
-      userId: session.user.id,
-      status: "PENDING_REVIEW",
-      notes: "Approval undone",
-    },
-  });
-
-  // Reset version approval status
-  await db.version.update({
-    where: { id: versionId },
-    data: { approvalStatus: "PENDING_REVIEW" },
-  });
-
-  // Reset task back to INTERNAL_REVIEW if it was moved to DONE
-  if (version.task && version.task.status === "DONE") {
-    await db.task.update({
-      where: { id: version.task.id },
-      data: { status: "INTERNAL_REVIEW" },
+  // One decision, one write — same reasoning as submitApproval.
+  await db.$transaction(async (tx) => {
+    // Create an audit record for the unapproval
+    await tx.approval.create({
+      data: {
+        versionId,
+        userId: session.user.id,
+        status: "PENDING_REVIEW",
+        notes: "Approval undone",
+      },
     });
-  }
 
-  // Recalculate shot status
-  if (version.task?.shot) {
-    await recalcShotStatus(version.task.shot.id).catch(() => {});
-  }
+    // Reset version approval status
+    await tx.version.update({
+      where: { id: versionId },
+      data: { approvalStatus: "PENDING_REVIEW" },
+    });
+
+    // Reset task back to INTERNAL_REVIEW if it was moved to DONE
+    if (version.task && version.task.status === "DONE") {
+      await tx.task.update({
+        where: { id: version.task.id },
+        data: { status: "INTERNAL_REVIEW" },
+      });
+    }
+
+    // Undoing a version approval also invalidates any shot-level sign-off that
+    // rested on it, otherwise the shot keeps reading COMPLETE / READY_FOR_CLIENT.
+    if (version.task?.shot) {
+      await clearStaleShotApproval(tx, version.task.shot.id);
+      await recalcShotStatus(version.task.shot.id, tx);
+    }
+  });
 
   revalidatePath(`/review/${versionId}`);
   if (version.task) {

actions/shots.ts

@@ -5,7 +5,7 @@ import { db } from "@/lib/db";
 import { revalidatePath } from "next/cache";
 import { z } from "zod";
 import { ShotStatus, ShotPriority, ShotApprovalStatus, Prisma } from "@prisma/client";
-import { recalcShotStatus } from "@/lib/shot-status";
+import { recalcShotStatus, markShotForRevisions } from "@/lib/shot-status";
 
 const createShotSchema = z.object({
   scene: z.string().min(1).max(50).regex(/^[A-Z0-9_]+$/i, "Alphanumeric and underscore only"),
@@ -341,6 +341,15 @@ export async function importShotsFromCsv(
   return { success: true, created, errors };
 }
 
+/**
+ * Manual override of the shot's status.
+ *
+ * Deliberately the one place that writes Shot.status without deriving it — but
+ * note the override is transient: Shot.status is a derived field, so the next
+ * recalc (any task move, approval, upload or render) recomputes it from the
+ * tasks and approval flags and discards whatever was set here. Change the
+ * underlying task states if the new status needs to stick.
+ */
 export async function updateShotStatus(
   shotId: string,
   status: ShotStatus
@@ -737,13 +746,19 @@ export async function internallyApproveShot(shotId: string) {
   });
   if (!shot) throw new Error("Shot not found");
 
-  await db.shot.update({
-    where: { id: shotId },
-    data: {
-      shotApprovalStatus: "INTERNALLY_APPROVED",
-      sharedWithClient: false,
-      status: "READY_FOR_CLIENT",
-    },
+  // Set the approval inputs and let the derivation pick the status. Writing
+  // READY_FOR_CLIENT directly skipped the rule that an outstanding CHANGES task
+  // outranks any approval, so a shot could be pushed past open rework and then
+  // snap back to REVISIONS on the next unrelated recalc.
+  await db.$transaction(async (tx) => {
+    await tx.shot.update({
+      where: { id: shotId },
+      data: {
+        shotApprovalStatus: "INTERNALLY_APPROVED",
+        sharedWithClient: false,
+      },
+    });
+    await recalcShotStatus(shotId, tx);
   });
 
   revalidatePath(`/projects/${shot.projectId}`);
@@ -786,7 +801,7 @@ export async function shareWithClient(shotId: string) {
     // Mark shot as shared
     await tx.shot.update({
       where: { id: shotId },
-      data: { sharedWithClient: true, status: "CLIENT_REVIEW" },
+      data: { sharedWithClient: true },
     });
 
     // For each task, make the latest version client-visible
@@ -806,6 +821,8 @@ export async function shareWithClient(shotId: string) {
         });
       }
     }
+
+    await recalcShotStatus(shotId, tx);
   });
 
   revalidatePath(`/projects/${shot.projectId}`);
@@ -838,7 +855,7 @@ export async function unshareFromClient(shotId: string) {
   await db.$transaction(async (tx) => {
     await tx.shot.update({
       where: { id: shotId },
-      data: { sharedWithClient: false, status: "READY_FOR_CLIENT" },
+      data: { sharedWithClient: false },
     });
 
     // Hide all client-visible versions on this shot's tasks
@@ -849,6 +866,8 @@ export async function unshareFromClient(shotId: string) {
         data: { isClientVisible: false },
       });
     }
+
+    await recalcShotStatus(shotId, tx);
   });
 
   revalidatePath(`/projects/${shot.projectId}`);
@@ -877,9 +896,12 @@ export async function clientApproveShot(shotId: string) {
   if (!shot) throw new Error("Shot not found");
   if (!shot.sharedWithClient) throw new Error("Shot is not shared with client");
 
-  await db.shot.update({
-    where: { id: shotId },
-    data: { shotApprovalStatus: "CLIENT_APPROVED", status: "COMPLETE" },
+  await db.$transaction(async (tx) => {
+    await tx.shot.update({
+      where: { id: shotId },
+      data: { shotApprovalStatus: "CLIENT_APPROVED" },
+    });
+    await recalcShotStatus(shotId, tx);
   });
 
   revalidatePath(`/projects/${shot.projectId}`);
@@ -909,14 +931,15 @@ export async function unapproveShot(shotId: string) {
     throw new Error("Shot is not client-approved");
   }
 
-  const newStatus = shot.sharedWithClient ? "CLIENT_REVIEW" : "READY_FOR_CLIENT";
-
-  await db.shot.update({
-    where: { id: shotId },
-    data: {
-      shotApprovalStatus: "INTERNALLY_APPROVED",
-      status: newStatus,
-    },
+  // The derivation already encodes "CLIENT_REVIEW if still shared, else
+  // READY_FOR_CLIENT" for an INTERNALLY_APPROVED shot, and additionally honours
+  // any open CHANGES task.
+  await db.$transaction(async (tx) => {
+    await tx.shot.update({
+      where: { id: shotId },
+      data: { shotApprovalStatus: "INTERNALLY_APPROVED" },
+    });
+    await recalcShotStatus(shotId, tx);
   });
 
   revalidatePath(`/projects/${shot.projectId}`);
@@ -1217,9 +1240,12 @@ export async function assignShotGroups(
 }
 
 /**
- * Internal: handle client requesting changes on a shot.
- * Resets shotApprovalStatus = PENDING, sharedWithClient = false.
- * Recalculates shot status (will become REVISIONS if tasks set to CHANGES).
+ * Session-authenticated wrapper for requesting changes on a shot — a supervisor
+ * or producer relaying a client note from inside the app.
+ *
+ * The client portal cannot use this: it runs on a review token, not a session,
+ * so it calls markShotForRevisions directly from its route. Both go through that
+ * one helper so the two entry points cannot drift apart again.
  */
 export async function clientRequestShotChanges(shotId: string, taskId?: string) {
   const session = await auth();
@@ -1234,28 +1260,7 @@ export async function clientRequestShotChanges(shotId: string, taskId?: string)
   });
   if (!shot) throw new Error("Shot not found");
 
-  await db.$transaction(async (tx) => {
-    // Reset shot approval
-    await tx.shot.update({
-      where: { id: shotId },
-      data: { shotApprovalStatus: "PENDING", sharedWithClient: false },
-    });
-
-    // If a specific task is called out, mark it CHANGES; otherwise mark all non-DONE tasks
-    if (taskId) {
-      await tx.task.update({
-        where: { id: taskId },
-        data: { status: "CHANGES" },
-      });
-    } else {
-      await tx.task.updateMany({
-        where: { shotId, status: { not: "DONE" } },
-        data: { status: "CHANGES" },
-      });
-    }
-
-    await recalcShotStatus(shotId, tx);
-  });
+  await db.$transaction((tx) => markShotForRevisions(tx, shotId, taskId));
 
   revalidatePath(`/projects/${shot.projectId}`);
   revalidatePath(`/projects/${shot.projectId}/shots/${shotId}`);

actions/versions.ts

@@ -4,7 +4,7 @@ import { auth } from "@/auth";
 import { db } from "@/lib/db";
 import { revalidatePath } from "next/cache";
 import { z } from "zod";
-import { recalcShotStatus } from "@/lib/shot-status";
+import { recalcShotStatus, clearStaleShotApproval } from "@/lib/shot-status";
 import { notifyNewVersionUploaded } from "@/lib/notifications";
 import { slackNotifyVersionUploaded } from "@/lib/slack";
 
@@ -50,9 +50,16 @@ export async function createVersion(data: z.infer<typeof createVersionSchema>) {
     data: { status: "INTERNAL_REVIEW" },
   });
 
-  // Recalculate shot status
+  // Recalculate shot status — clearing any stale sign-off first, so a new
+  // version does not land under a shot still reading COMPLETE.
   if (task.shot) {
-    await recalcShotStatus(task.shot.id).catch(() => {});
+    const shotId = task.shot.id;
+    await db
+      .$transaction(async (tx) => {
+        await clearStaleShotApproval(tx, shotId);
+        await recalcShotStatus(shotId, tx);
+      })
+      .catch(() => {});
   }
 
   // Get the next version number for this task

app/api/batch-upload/upload/route.ts

@@ -2,7 +2,7 @@ import { NextRequest, NextResponse } from "next/server";
 import { auth } from "@/auth";
 import { db } from "@/lib/db";
 import { uploadToHetzner, deleteFromHetzner } from "@/lib/storage";
-import { recalcShotStatus } from "@/lib/shot-status";
+import { recalcShotStatus, clearStaleShotApproval } from "@/lib/shot-status";
 
 export const maxDuration = 300; // 5 min — large .mov high-res files
 
@@ -194,8 +194,14 @@ export async function POST(
     data: { status: "INTERNAL_REVIEW" },
   });
 
-  // Recalculate the parent shot's status
-  await recalcShotStatus(shotId).catch(() => {});
+  // Recalculate the parent shot's status, clearing any stale sign-off first so
+  // the new version does not land under a shot still reading COMPLETE.
+  await db
+    .$transaction(async (tx) => {
+      await clearStaleShotApproval(tx, shotId);
+      await recalcShotStatus(shotId, tx);
+    })
+    .catch(() => {});
 
   return NextResponse.json({
     success: true,

app/api/client/[token]/approve/route.ts

@@ -1,7 +1,7 @@
 import { NextRequest, NextResponse } from "next/server";
 import { db } from "@/lib/db";
 import { ApprovalStatus } from "@prisma/client";
-import { recalcShotStatus } from "@/lib/shot-status";
+import { recalcShotStatus, markShotForRevisions } from "@/lib/shot-status";
 import { validateReviewToken } from "@/lib/review-auth";
 
 async function getOrCreateClientUser(email: string, label?: string | null) {
@@ -48,9 +48,12 @@ export async function POST(
     }
 
     if (action === "APPROVE") {
-      await db.shot.update({
-        where: { id: shotId },
-        data: { shotApprovalStatus: "CLIENT_APPROVED", status: "COMPLETE" },
+      await db.$transaction(async (tx) => {
+        await tx.shot.update({
+          where: { id: shotId },
+          data: { shotApprovalStatus: "CLIENT_APPROVED" },
+        });
+        await recalcShotStatus(shotId, tx);
       });
     } else if (action === "NEEDS_CHANGES") {
       // Increment shotVersion: v001 → v002, v009 → v010, etc.
@@ -59,17 +62,42 @@ export async function POST(
       const nextNum = verMatch ? parseInt(verMatch[1], 10) + 1 : 2;
       const nextVersion = "v" + String(nextNum).padStart(3, "0");
 
+      const email = session.email ?? `client+${token.slice(0, 8)}@review.external`;
+      const user = await getOrCreateClientUser(email, session.label);
+
       await db.$transaction(async (tx) => {
         await tx.shot.update({
           where: { id: shotId },
-          data: { shotApprovalStatus: "PENDING", sharedWithClient: false, shotVersion: nextVersion },
+          data: { shotVersion: nextVersion },
         });
-        // Mark all non-DONE tasks as CHANGES
-        await tx.task.updateMany({
-          where: { shotId, status: { not: "DONE" } },
-          data: { status: "CHANGES" },
+
+        // Clears the approval, unshares, marks the tasks needing rework and
+        // recalculates — including the case where every task was already DONE,
+        // which used to match nothing and leave the request with no effect.
+        await markShotForRevisions(tx, shotId);
+
+        // The shot-level verdict has to land on the versions too: the dashboard
+        // counters, the version badges and the client portal's own filters all
+        // read Version.approvalStatus, so a shot-level change request was
+        // invisible everywhere those look.
+        const shown = await tx.version.findMany({
+          where: { task: { shotId }, isClientVisible: true, isLatest: true },
+          select: { id: true },
         });
-        await recalcShotStatus(shotId, tx);
+        for (const v of shown) {
+          await tx.approval.create({
+            data: {
+              versionId: v.id,
+              userId: user.id,
+              status: "NEEDS_CHANGES",
+              notes: notes ?? "Changes requested on the shot",
+            },
+          });
+          await tx.version.update({
+            where: { id: v.id },
+            data: { approvalStatus: "NEEDS_CHANGES" },
+          });
+        }
       });
     } else {
       return NextResponse.json({ error: "Invalid action" }, { status: 400 });
@@ -126,9 +154,12 @@ export async function POST(
           select: { sharedWithClient: true, shotApprovalStatus: true },
         });
         if (shot?.sharedWithClient && shot.shotApprovalStatus !== "CLIENT_APPROVED") {
+          // status is left to the recalc below — writing COMPLETE here was
+          // overwritten by it anyway, and disagreed with it whenever another
+          // task on the shot was still on CHANGES.
           await db.shot.update({
             where: { id: version.task.shot.id },
-            data: { shotApprovalStatus: "CLIENT_APPROVED", status: "COMPLETE" },
+            data: { shotApprovalStatus: "CLIENT_APPROVED" },
           });
         }
       }

components/tasks/KanbanCard.tsx

@@ -143,7 +143,8 @@ export function KanbanCard({ task, isDragOverlay = false }: KanbanCardProps) {
           </span>
           {latestVersion.approvalStatus === "APPROVED" ? (
             <CheckCircle2 className="h-3.5 w-3.5 text-emerald-500" />
-          ) : latestVersion.approvalStatus === "CHANGES_REQUESTED" ? (
+          ) : latestVersion.approvalStatus === "NEEDS_CHANGES" ||
+            latestVersion.approvalStatus === "REJECTED" ? (
             <XCircle className="h-3.5 w-3.5 text-red-500" />
           ) : (
             <Clock className="h-3.5 w-3.5 text-zinc-600" />

lib/render-pipeline/preview.test.ts

@@ -1,5 +1,5 @@
 import { describe, it, expect } from "vitest";
-import { nextTaskTitle } from "./preview";
+import { nextTaskTitle, carryApproval } from "./preview";
 
 describe("nextTaskTitle", () => {
   it("swaps an existing version suffix", () => {
@@ -30,3 +30,28 @@ describe("nextTaskTitle", () => {
     expect(nextTaskTitle("SHOT_cmp_TT_V12", "v013")).toBe("SHOT_cmp_TT_v013");
   });
 });
+
+describe("carryApproval", () => {
+  it("carries an approval forward, so a re-export does not reopen a signed-off shot", () => {
+    expect(carryApproval("APPROVED")).toBe("APPROVED");
+  });
+
+  // The regression: a render queued to address a note used to arrive already
+  // wearing that note, so nothing registered that the work had been redone.
+  it("resets NEEDS_CHANGES — this render is the fix for it", () => {
+    expect(carryApproval("NEEDS_CHANGES")).toBe("PENDING_REVIEW");
+  });
+
+  it("resets REJECTED for the same reason", () => {
+    expect(carryApproval("REJECTED")).toBe("PENDING_REVIEW");
+  });
+
+  it("leaves an unreviewed version pending", () => {
+    expect(carryApproval("PENDING_REVIEW")).toBe("PENDING_REVIEW");
+  });
+
+  it("defaults to pending when there is no previous version", () => {
+    expect(carryApproval(null)).toBe("PENDING_REVIEW");
+    expect(carryApproval(undefined)).toBe("PENDING_REVIEW");
+  });
+});

lib/render-pipeline/preview.ts

@@ -1,10 +1,11 @@
-import { ExportStatus, Prisma } from "@prisma/client";
+import { ApprovalStatus, ExportStatus, Prisma } from "@prisma/client";
 import { db } from "@/lib/db";
 import { PipelineError } from "./errors";
 import { applyTransition } from "./transitions";
 import { getConfigNumber, getPreviewConfig } from "./config";
 import { resolveProjectTemplates, autoShotThumbnailEnabled } from "./project-templates";
 import { clientShotCode } from "@/lib/shot-code";
+import { recalcShotStatus, clearStaleShotApproval } from "@/lib/shot-status";
 import { renderOutputBase } from "./exports";
 
 /** Slate date format used by the panel's getDateString (YYYY/MM/DD). */
@@ -261,6 +262,22 @@ export async function createPreviewJob(
   });
 }
 
+/**
+ * What approval a pipeline-created Version inherits from the one it supersedes.
+ *
+ * Only APPROVED carries. A re-export re-renders EXRs for delivery from a shot
+ * that has usually already been signed off, and defaulting to PENDING_REVIEW
+ * would make an approved shot silently reappear as awaiting review.
+ *
+ * The rejecting verdicts are the opposite case: NEEDS_CHANGES and REJECTED
+ * describe the version someone asked to be fixed, and the render coming through
+ * here is the fix. Carrying them forward left every re-render wearing the note
+ * that prompted it, so nothing in the app registered that the work was redone.
+ */
+export function carryApproval(previous: ApprovalStatus | null | undefined): ApprovalStatus {
+  return previous === "APPROVED" ? "APPROVED" : "PENDING_REVIEW";
+}
+
 /**
  * `UNG_102_019_010_cmp_TT_v004` + v005 → `UNG_102_019_010_cmp_TT_v005`.
  * Mirrors `nextExrOutputBase` (exports.ts) so the task title, the EXR base and
@@ -330,6 +347,7 @@ export async function finalizePreview(jobId: string, machineId: string, body: Fi
 
     let versionId: string | null = null;
     let versionReplaced = false;
+    let versionCreated = false;
     let inheritedApproval: string | null = null;
     if (artifacts.previewMovKey) {
       const mediaFields = {
@@ -362,12 +380,6 @@ export async function finalizePreview(jobId: string, machineId: string, body: Fi
         // Version numbering stays monotonic for the task (existing review flows
         // rely on the newest version having the highest number) while matching
         // the export version whenever it is ahead.
-        // A re-export does not change the picture — it re-renders EXRs for
-        // delivery from a shot that has usually already been approved. A fresh
-        // Version defaults to PENDING_REVIEW, which is what the dashboard, the
-        // shot page and the client portal all read, so an approved shot would
-        // silently reappear as awaiting review. The approval carries over from
-        // the version this one supersedes.
         const previousLatest = exportRow.taskId
           ? await tx.version.findFirst({
               where: { taskId: exportRow.taskId },
@@ -375,6 +387,7 @@ export async function finalizePreview(jobId: string, machineId: string, body: Fi
               select: { approvalStatus: true, versionNumber: true },
             })
           : null;
+        const carriedApproval = carryApproval(previousLatest?.approvalStatus);
 
         const taskMax = exportRow.taskId
           ? await tx.version.aggregate({
@@ -395,13 +408,14 @@ export async function finalizePreview(jobId: string, machineId: string, body: Fi
             // client portal, so the pipeline stays out of it. What this version
             // is and where it came from lives on the Export and its timeline,
             // which is where someone would go looking anyway.
-            approvalStatus: previousLatest?.approvalStatus ?? "PENDING_REVIEW",
+            approvalStatus: carriedApproval,
             isLatest: true,
             isClientVisible: false, // never shared to the client portal (§10.0)
           },
         });
         versionId = version.id;
-        inheritedApproval = previousLatest?.approvalStatus ?? null;
+        versionCreated = true;
+        inheritedApproval = carriedApproval;
       }
 
       if (exportRow.taskId) {
@@ -473,6 +487,40 @@ export async function finalizePreview(jobId: string, machineId: string, body: Fi
       }
     }
 
+    // A genuinely new version is new work awaiting review, so the task moves the
+    // same way a manual upload moves it (actions/versions.ts, batch-upload) —
+    // otherwise a task left on CHANGES by a review or a QC fail stays there
+    // through every re-render, and the shot stays on REVISIONS with no way out
+    // but a manual kanban drag.
+    //
+    // A *replaced* version is deliberately excluded: regeneratePreview
+    // re-delivers the same version after a slate fix, and that is not a new
+    // submission. Nor is an already-approved carry-over, which would drag a
+    // signed-off shot back into review.
+    let taskStatusMoved = false;
+    let approvalCleared = false;
+    if (versionCreated && exportRow.taskId && inheritedApproval !== "APPROVED") {
+      const task = await tx.task.findUnique({
+        where: { id: exportRow.taskId },
+        select: { status: true },
+      });
+      if (task && task.status !== "INTERNAL_REVIEW") {
+        await tx.task.update({
+          where: { id: exportRow.taskId },
+          data: { status: "INTERNAL_REVIEW" },
+        });
+        taskStatusMoved = true;
+      }
+      approvalCleared = await clearStaleShotApproval(tx, exportRow.shotId);
+    }
+
+    // Shot.status is derived from task states (lib/shot-status.ts); nothing in
+    // the pipeline recalculated it, so shot boards went stale the moment a
+    // render changed a task.
+    if (taskStatusMoved || approvalCleared) {
+      await recalcShotStatus(exportRow.shotId, tx);
+    }
+
     const updated = await applyTransition(
       tx,
       exportRow,
@@ -488,6 +536,7 @@ export async function finalizePreview(jobId: string, machineId: string, body: Fi
               : ` — review version created (${inheritedApproval ?? "PENDING_REVIEW"})`
             : " — no preview media reported") +
           (taskRenamed ? ` — task renamed to ${taskTitle}` : "") +
+          (taskStatusMoved ? " — task moved to INTERNAL_REVIEW" : "") +
           thumbnailNote,
       },
       {

lib/render-pipeline/qc.ts

@@ -3,6 +3,7 @@ import { z } from "zod";
 import { db } from "@/lib/db";
 import { PipelineError } from "./errors";
 import { applyTransition } from "./transitions";
+import { recalcShotStatus } from "@/lib/shot-status";
 
 /**
  * QC workflow (RenderPipeline2 §10) — technical/final-delivery QC on shots the
@@ -104,6 +105,9 @@ export async function submitQCReview(exportId: string, input: QCSubmitInput, act
       if (task && task.status !== "CHANGES") {
         await tx.task.update({ where: { id: task.id }, data: { status: "CHANGES" } });
         taskUpdated = { id: task.id, status: "CHANGES" };
+        // Shot.status is derived from its tasks and does not recompute on its
+        // own, so without this the shot never actually reached REVISIONS.
+        await recalcShotStatus(exportRow.shotId, tx);
       }
     }
 

lib/shot-status.test.ts

@@ -3,8 +3,13 @@ import { describe, it, expect, vi } from "vitest";
 // Mock Prisma db so the pure deriveShotStatus function can be tested in isolation
 vi.mock("@/lib/db", () => ({ db: {} }));
 
-import { deriveShotStatus } from "./shot-status";
-import type { TaskStatus } from "@prisma/client";
+import {
+  deriveShotStatus,
+  clearStaleShotApproval,
+  markShotForRevisions,
+  recalcShotStatus,
+} from "./shot-status";
+import type { TaskStatus, ShotApprovalStatus } from "@prisma/client";
 
 // Helper to build task arrays quickly
 const tasks = (...statuses: TaskStatus[]) => statuses.map((status) => ({ status }));
@@ -142,3 +147,186 @@ describe("deriveShotStatus", () => {
     });
   });
 });
+
+// ─────────────────────────────────────────────────────────────────────────────
+// Fake Prisma client — enough of the surface for the two write helpers. Each
+// call mutates the in-memory shot so the assertions read final state, not spies.
+// ─────────────────────────────────────────────────────────────────────────────
+
+type FakeTask = { id: string; status: TaskStatus };
+type FakeVersion = { id: string; taskId: string; isLatest: boolean; createdAt: Date };
+
+// The helpers take a Prisma client; the fake implements just the calls they make.
+type TxArg = Parameters<typeof recalcShotStatus>[1];
+const asTx = (c: { shotState: unknown }): NonNullable<TxArg> =>
+  c as unknown as NonNullable<TxArg>;
+
+function fakeClient(shot: {
+  id: string;
+  shotApprovalStatus: ShotApprovalStatus;
+  sharedWithClient: boolean;
+  status?: string;
+  tasks: FakeTask[];
+  versions?: FakeVersion[];
+}) {
+  const versions = shot.versions ?? [];
+  return {
+    shot: {
+      findUnique: async () => shot,
+      update: async ({ data }: { data: Record<string, unknown> }) => {
+        Object.assign(shot, data);
+        return shot;
+      },
+    },
+    task: {
+      findMany: async ({ where }: { where: { status?: { not: TaskStatus } } }) =>
+        where.status?.not
+          ? shot.tasks.filter((t) => t.status !== where.status!.not)
+          : shot.tasks,
+      update: async ({ where, data }: { where: { id: string }; data: { status: TaskStatus } }) => {
+        const t = shot.tasks.find((x) => x.id === where.id);
+        if (t) t.status = data.status;
+        return t;
+      },
+      updateMany: async ({
+        where,
+        data,
+      }: {
+        where: { status?: { not: TaskStatus } };
+        data: { status: TaskStatus };
+      }) => {
+        for (const t of shot.tasks) {
+          if (!where.status?.not || t.status !== where.status.not) t.status = data.status;
+        }
+        return { count: shot.tasks.length };
+      },
+    },
+    version: {
+      findFirst: async () =>
+        [...versions].sort(
+          (a, b) =>
+            Number(b.isLatest) - Number(a.isLatest) ||
+            b.createdAt.getTime() - a.createdAt.getTime()
+        )[0] ?? null,
+    },
+    shotState: shot,
+  };
+}
+
+describe("clearStaleShotApproval", () => {
+  it("clears an internal sign-off so new work is visible", async () => {
+    const c = fakeClient({
+      id: "s1",
+      shotApprovalStatus: "INTERNALLY_APPROVED",
+      sharedWithClient: false,
+      tasks: [{ id: "t1", status: "INTERNAL_REVIEW" }],
+    });
+    expect(await clearStaleShotApproval(asTx(c), "s1")).toBe(true);
+    expect(c.shotState.shotApprovalStatus).toBe("PENDING");
+  });
+
+  it("clears a client sign-off, so an upload cannot hide under COMPLETE", async () => {
+    const c = fakeClient({
+      id: "s1",
+      shotApprovalStatus: "CLIENT_APPROVED",
+      sharedWithClient: true,
+      tasks: [{ id: "t1", status: "INTERNAL_REVIEW" }],
+    });
+    expect(await clearStaleShotApproval(asTx(c), "s1")).toBe(true);
+    expect(c.shotState.shotApprovalStatus).toBe("PENDING");
+  });
+
+  it("is a no-op when there is no sign-off to clear", async () => {
+    const c = fakeClient({
+      id: "s1",
+      shotApprovalStatus: "PENDING",
+      sharedWithClient: false,
+      tasks: [{ id: "t1", status: "IN_PROGRESS" }],
+    });
+    expect(await clearStaleShotApproval(asTx(c), "s1")).toBe(false);
+  });
+
+  it("leaves sharedWithClient alone — revoking access is a separate act", async () => {
+    const c = fakeClient({
+      id: "s1",
+      shotApprovalStatus: "CLIENT_APPROVED",
+      sharedWithClient: true,
+      tasks: [{ id: "t1", status: "DONE" }],
+    });
+    await clearStaleShotApproval(asTx(c), "s1");
+    expect(c.shotState.sharedWithClient).toBe(true);
+  });
+});
+
+describe("markShotForRevisions", () => {
+  it("marks the open tasks and lands the shot on REVISIONS", async () => {
+    const c = fakeClient({
+      id: "s1",
+      shotApprovalStatus: "CLIENT_APPROVED",
+      sharedWithClient: true,
+      tasks: [
+        { id: "t1", status: "DONE" },
+        { id: "t2", status: "IN_PROGRESS" },
+      ],
+    });
+    const changed = await markShotForRevisions(asTx(c), "s1");
+    expect(changed).toEqual(["t2"]);
+    expect(c.shotState.tasks.find((t) => t.id === "t1")!.status).toBe("DONE");
+    expect(c.shotState.tasks.find((t) => t.id === "t2")!.status).toBe("CHANGES");
+    expect(c.shotState.shotApprovalStatus).toBe("PENDING");
+    expect(c.shotState.sharedWithClient).toBe(false);
+    expect(c.shotState.status).toBe("REVISIONS");
+  });
+
+  // The regression this helper exists for: a shot out for client review has all
+  // its tasks DONE, so the old "non-DONE" filter matched nothing and the change
+  // request silently derived to INTERNAL_REVIEW instead of REVISIONS.
+  it("reopens the task behind the latest version when every task is DONE", async () => {
+    const c = fakeClient({
+      id: "s1",
+      shotApprovalStatus: "CLIENT_APPROVED",
+      sharedWithClient: true,
+      tasks: [
+        { id: "t1", status: "DONE" },
+        { id: "t2", status: "DONE" },
+      ],
+      versions: [
+        { id: "v1", taskId: "t1", isLatest: false, createdAt: new Date("2026-01-01") },
+        { id: "v2", taskId: "t2", isLatest: true, createdAt: new Date("2026-02-01") },
+      ],
+    });
+    const changed = await markShotForRevisions(asTx(c), "s1");
+    expect(changed).toEqual(["t2"]);
+    expect(c.shotState.tasks.find((t) => t.id === "t2")!.status).toBe("CHANGES");
+    expect(c.shotState.status).toBe("REVISIONS");
+  });
+
+  it("marks only the called-out task when one is named", async () => {
+    const c = fakeClient({
+      id: "s1",
+      shotApprovalStatus: "INTERNALLY_APPROVED",
+      sharedWithClient: false,
+      tasks: [
+        { id: "t1", status: "TODO" },
+        { id: "t2", status: "DONE" },
+      ],
+    });
+    const changed = await markShotForRevisions(asTx(c), "s1", "t2");
+    expect(changed).toEqual(["t2"]);
+    expect(c.shotState.tasks.find((t) => t.id === "t1")!.status).toBe("TODO");
+    expect(c.shotState.tasks.find((t) => t.id === "t2")!.status).toBe("CHANGES");
+  });
+
+  it("still clears the approval when a shot has no tasks to mark", async () => {
+    const c = fakeClient({
+      id: "s1",
+      shotApprovalStatus: "CLIENT_APPROVED",
+      sharedWithClient: true,
+      tasks: [],
+    });
+    const changed = await markShotForRevisions(asTx(c), "s1");
+    expect(changed).toEqual([]);
+    expect(c.shotState.shotApprovalStatus).toBe("PENDING");
+    expect(c.shotState.status).toBe("WAITING");
+  });
+});

lib/shot-status.ts

@@ -77,3 +77,100 @@ export async function recalcShotStatus(
     data: { status: newStatus },
   });
 }
+
+/**
+ * New review material arrived on a shot, so any standing sign-off is stale.
+ *
+ * deriveShotStatus short-circuits on shotApprovalStatus: a CLIENT_APPROVED shot
+ * reads COMPLETE and an INTERNALLY_APPROVED one reads READY_FOR_CLIENT no matter
+ * what its tasks say. Without this, uploading a new version onto an approved
+ * shot moved the task to INTERNAL_REVIEW and changed nothing visible — the shot
+ * still read as signed off while carrying work nobody had looked at.
+ *
+ * sharedWithClient is left alone; clearing the approval is enough to pull the
+ * shot back to INTERNAL_REVIEW, and revoking client access is a separate,
+ * deliberate act (unshareFromClient).
+ *
+ * @returns true if a stale approval was actually cleared.
+ */
+export async function clearStaleShotApproval(
+  client: TxClient,
+  shotId: string
+): Promise<boolean> {
+  const shot = await client.shot.findUnique({
+    where: { id: shotId },
+    select: { shotApprovalStatus: true },
+  });
+  if (!shot || shot.shotApprovalStatus === "PENDING") return false;
+
+  await client.shot.update({
+    where: { id: shotId },
+    data: { shotApprovalStatus: "PENDING" },
+  });
+  return true;
+}
+
+/**
+ * Put a shot into revisions: clear its approval, unshare it, and mark the tasks
+ * that need rework so the derivation lands on REVISIONS.
+ *
+ * The task selection is the subtle part. Marking every non-DONE task as CHANGES
+ * matches nothing at all on the shot this is most often called for — one that is
+ * out for client review precisely *because* all of its tasks are finished. That
+ * left the shot deriving to INTERNAL_REVIEW and the change request with no
+ * visible effect anywhere. So when nothing was picked up, the task behind the
+ * shot's latest version is marked instead: that is the deliverable the reviewer
+ * was actually looking at.
+ *
+ * @param taskId  Mark only this task (a reviewer calling out one specific task).
+ * @returns ids of the tasks moved to CHANGES.
+ */
+export async function markShotForRevisions(
+  client: TxClient,
+  shotId: string,
+  taskId?: string
+): Promise<string[]> {
+  await client.shot.update({
+    where: { id: shotId },
+    data: { shotApprovalStatus: "PENDING", sharedWithClient: false },
+  });
+
+  let changed: string[];
+
+  if (taskId) {
+    await client.task.update({ where: { id: taskId }, data: { status: "CHANGES" } });
+    changed = [taskId];
+  } else {
+    const open = await client.task.findMany({
+      where: { shotId, status: { not: "DONE" } },
+      select: { id: true },
+    });
+
+    if (open.length > 0) {
+      await client.task.updateMany({
+        where: { shotId, status: { not: "DONE" } },
+        data: { status: "CHANGES" },
+      });
+      changed = open.map((t) => t.id);
+    } else {
+      // Every task is DONE — reopen the one that produced what was reviewed.
+      const latest = await client.version.findFirst({
+        where: { task: { shotId } },
+        orderBy: [{ isLatest: "desc" }, { createdAt: "desc" }],
+        select: { taskId: true },
+      });
+      if (latest?.taskId) {
+        await client.task.update({
+          where: { id: latest.taskId },
+          data: { status: "CHANGES" },
+        });
+        changed = [latest.taskId];
+      } else {
+        changed = [];
+      }
+    }
+  }
+
+  await recalcShotStatus(shotId, client);
+  return changed;
+}