Fix goal progress verification

.github/workflows/labeler.yml

@@ -1,6 +1,12 @@
 name: PR Labeler
 on:
   pull_request_target:
+
+permissions:
+  contents: read
+  pull-requests: write
+  issues: write
+
 jobs:
   label:
     runs-on: ubuntu-latest

e2e/dashboard-widgets.spec.js

@@ -174,12 +174,13 @@
     });
   });
 });
+
 test("dashboard widgets render with mocked metrics", async ({ page }) => {
   await page.goto("/dashboard", { waitUntil: "load" });
   await expect(page.getByRole("heading", { name: /dashboard/i })).toBeVisible({ timeout: 30000 });
   await expect(page.getByRole("heading", { name: "Your Commits" })).toBeVisible({ timeout: 10000 });
   await expect(page.getByRole("heading", { name: "PR Analytics" })).toBeVisible({ timeout: 10000 });
   await expect(page.getByRole("heading", { name: "Goals" })).toBeVisible({ timeout: 10000 });
   await expect(page.getByText("Make 10 commits")).toBeVisible({ timeout: 10000 });
 });
 

src/app/api/goals/[id]/route.ts

@@ -6,6 +6,8 @@ import { dispatchToAllWebhooks } from "@/lib/webhooks";
 
 export const dynamic = "force-dynamic";
 
+const AUTO_SYNCED_GOAL_UNITS = new Set(["commits", "prs"]);
+
 export async function PATCH(
   req: Request,
   { params }: { params: { id: string } }
@@ -19,9 +21,13 @@ export async function PATCH(
   if (!user) return Response.json({ error: "User not found" }, { status: 404 });
 
   const body = await req.json().catch(() => ({}));
-  const { current } = body;
+  const { current, unit } = body;
 
-  if (typeof current !== "number" || current < 0) {
+  if (
+    typeof current !== "number" ||
+    !Number.isInteger(current) ||
+    current < 0
+  ) {
     return Response.json(
       { error: "Invalid current value" },
       { status: 400 }
@@ -39,6 +45,24 @@ export async function PATCH(
     return Response.json({ error: "Goal not found" }, { status: 404 });
   }
 
+  const requestedUnit = typeof unit === "string" ? unit : existingGoal.unit;
+  if (
+    AUTO_SYNCED_GOAL_UNITS.has(existingGoal.unit) ||
+    AUTO_SYNCED_GOAL_UNITS.has(requestedUnit)
+  ) {
+    return Response.json(
+      { error: "current for GitHub-synced goals can only be updated by sync" },
+      { status: 400 }
+    );
+  }
+
+  if (current > existingGoal.target) {
+    return Response.json(
+      { error: "current cannot exceed target" },
+      { status: 400 }
+    );
+  }
+
   const wasCompleted = existingGoal.current >= existingGoal.target;
   const { data: updatedGoal, error } = await supabaseAdmin
     .from("goals")

test/goals-patch-progress.test.ts

@@ -0,0 +1,116 @@
+import { beforeEach, describe, expect, it, vi } from "vitest";
+
+const mocks = vi.hoisted(() => ({
+  getServerSession: vi.fn(),
+  resolveAppUser: vi.fn(),
+  from: vi.fn(),
+  update: vi.fn(),
+  existingGoal: {
+    id: "goal-1",
+    user_id: "user-1",
+    title: "Ship work",
+    target: 10,
+    current: 2,
+    unit: "tasks",
+  },
+}));
+
+vi.mock("next-auth", () => ({
+  getServerSession: mocks.getServerSession,
+}));
+
+vi.mock("@/lib/auth", () => ({
+  authOptions: {},
+}));
+
+vi.mock("@/lib/resolve-user", () => ({
+  resolveAppUser: mocks.resolveAppUser,
+}));
+
+vi.mock("@/lib/supabase", () => ({
+  supabaseAdmin: {
+    from: mocks.from,
+  },
+}));
+
+import { PATCH } from "@/app/api/goals/[id]/route";
+
+function createSupabaseBuilder() {
+  const builder = {
+    select: vi.fn(() => builder),
+    eq: vi.fn(() => builder),
+    update: mocks.update.mockImplementation(() => builder),
+    maybeSingle: vi.fn(async () => ({ data: mocks.existingGoal, error: null })),
+    single: vi.fn(async () => ({
+      data: { ...mocks.existingGoal, current: 4 },
+      error: null,
+    })),
+  };
+
+  return builder;
+}
+
+async function patchGoal(body: Record<string, unknown>) {
+  const req = new Request("http://localhost/api/goals/goal-1", {
+    method: "PATCH",
+    body: JSON.stringify(body),
+  });
+
+  return PATCH(req, { params: { id: "goal-1" } });
+}
+
+describe("PATCH /api/goals/[id] progress updates", () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+    mocks.getServerSession.mockResolvedValue({
+      githubId: "github-1",
+      githubLogin: "octocat",
+    });
+    mocks.resolveAppUser.mockResolvedValue({ id: "user-1" });
+    mocks.from.mockImplementation(() => createSupabaseBuilder());
+    mocks.existingGoal = {
+      id: "goal-1",
+      user_id: "user-1",
+      title: "Ship work",
+      target: 10,
+      current: 2,
+      unit: "tasks",
+    };
+  });
+
+  it("allows manual progress updates for non-GitHub goals", async () => {
+    const response = await patchGoal({ current: 4 });
+    const json = await response.json();
+
+    expect(response.status).toBe(200);
+    expect(json.goal.current).toBe(4);
+    expect(mocks.update).toHaveBeenCalledWith({ current: 4 });
+  });
+
+  it.each(["commits", "prs"])(
+    "rejects client-supplied progress for %s goals",
+    async (unit) => {
+      mocks.existingGoal = { ...mocks.existingGoal, unit };
+
+      const response = await patchGoal({ current: 9 });
+      const json = await response.json();
+
+      expect(response.status).toBe(400);
+      expect(json.error).toBe(
+        "current for GitHub-synced goals can only be updated by sync"
+      );
+      expect(mocks.update).not.toHaveBeenCalled();
+    }
+  );
+
+  it("rejects setting progress while converting a goal to a GitHub-synced unit", async () => {
+    const response = await patchGoal({ unit: "commits", current: 9 });
+    const json = await response.json();
+
+    expect(response.status).toBe(400);
+    expect(json.error).toBe(
+      "current for GitHub-synced goals can only be updated by sync"
+    );
+    expect(mocks.update).not.toHaveBeenCalled();
+  });
+});