fix: Correctly set job.approval correctly on Deny

app/components/chat/function-call.tsx

@@ -168,7 +168,7 @@ function FunctionCall({
                   functionName={targetFn}
                 />
               </div>
-              {approvalRequested && !approved && (
+              {approvalRequested && approved === null && (
                 <div className="flex gap-2">
                   <Button
                     size="sm"

control-plane/src/modules/jobs/jobs.test.ts

@@ -2,7 +2,7 @@ import { ulid } from "ulid";
 import { packer } from "../packer";
 import { upsertServiceDefinition } from "../service-definitions";
 import { createOwner } from "../test/util";
-import { createJob, pollJobs, getJob, requestApproval } from "./jobs";
+import { createJob, pollJobs, getJob, requestApproval, submitApproval } from "./jobs";
 import {
   acknowledgeJob,
   persistJobResult,
@@ -424,3 +424,137 @@ describe("pollJobs", () => {
     expect(result.length).toBe(0);
   });
 });
+
+describe("submitApproval", () => {
+  let owner: Awaited<ReturnType<typeof createOwner>>;
+  beforeAll(async () => {
+    owner = await createOwner();
+
+    await upsertServiceDefinition({
+      service: "testService",
+      definition: {
+        name: "testService",
+        functions: [
+          {
+            name: mockTargetFn,
+            schema: mockTargetSchema,
+          },
+        ],
+      },
+      owner,
+    });
+
+  })
+  it("should mark job as approved", async () => {
+
+    const result = await createJob({
+      targetFn: mockTargetFn,
+      targetArgs: mockTargetArgs,
+      owner,
+      service: "testService",
+    });
+
+    expect(result.id).toBeDefined();
+    expect(result.created).toBe(true);
+
+    await requestApproval({
+      clusterId: owner.clusterId,
+      jobId: result.id,
+    })
+
+    const retreivedJob1 = await getJob({
+      jobId: result.id,
+      clusterId: owner.clusterId,
+    });
+
+    expect(retreivedJob1!.approvalRequested).toBe(true);
+
+    await submitApproval({
+      clusterId: owner.clusterId,
+      call: retreivedJob1!,
+      approved: true,
+    })
+
+    const retreivedJob2 = await getJob({
+      jobId: result.id,
+      clusterId: owner.clusterId,
+    });
+
+    expect(retreivedJob2!.approved).toBe(true);
+    expect(retreivedJob2!.status).toBe("pending");
+    expect(retreivedJob2!.resultType).toBe(null);
+
+    // Re-submitting approval should be a no-op
+    await submitApproval({
+      clusterId: owner.clusterId,
+      call: retreivedJob1!,
+      approved: false,
+    })
+
+    const retreivedJob3 = await getJob({
+      jobId: result.id,
+      clusterId: owner.clusterId,
+    });
+
+    expect(retreivedJob3!.approved).toBe(true);
+    expect(retreivedJob3!.status).toBe("pending");
+    expect(retreivedJob3!.resultType).toBe(null);
+  });
+
+  it("should mark job as denied", async () => {
+
+    const result = await createJob({
+      targetFn: mockTargetFn,
+      targetArgs: mockTargetArgs,
+      owner,
+      service: "testService",
+    });
+
+    expect(result.id).toBeDefined();
+    expect(result.created).toBe(true);
+
+    await requestApproval({
+      clusterId: owner.clusterId,
+      jobId: result.id,
+    })
+
+    const retreivedJob1 = await getJob({
+      jobId: result.id,
+      clusterId: owner.clusterId,
+    });
+
+    expect(retreivedJob1!.approvalRequested).toBe(true);
+
+    await submitApproval({
+      clusterId: owner.clusterId,
+      call: retreivedJob1!,
+      approved: false,
+    })
+
+    const retreivedJob2 = await getJob({
+      jobId: result.id,
+      clusterId: owner.clusterId,
+    });
+
+    expect(retreivedJob2!.approved).toBe(false);
+    expect(retreivedJob2!.status).toBe("success");
+    expect(retreivedJob2!.resultType).toBe("rejection");
+
+    // Re-submitting approval should be a no-op
+    await submitApproval({
+      clusterId: owner.clusterId,
+      call: retreivedJob1!,
+      approved: true,
+    })
+
+    const retreivedJob3 = await getJob({
+      jobId: result.id,
+      clusterId: owner.clusterId,
+    });
+
+    expect(retreivedJob3!.approved).toBe(false);
+    expect(retreivedJob3!.status).toBe("success");
+    expect(retreivedJob3!.resultType).toBe("rejection");
+
+  });
+});

control-plane/src/modules/jobs/jobs.ts

@@ -1,4 +1,4 @@
-import { and, eq, gt, lte, sql } from "drizzle-orm";
+import { and, eq, gt, isNotNull, isNull, lte, sql } from "drizzle-orm";
 import { env } from "../../utilities/env";
 import { JobPollTimeoutError, NotFoundError } from "../../utilities/errors";
 import { getBlobsForJobs } from "../blobs";
@@ -381,21 +381,33 @@ export async function submitApproval({
         remaining_attempts: sql`remaining_attempts + 1`,
       })
       .where(
-        and(eq(data.jobs.id, call.id), eq(data.jobs.cluster_id, clusterId)),
+        and(
+          eq(data.jobs.id, call.id),
+          eq(data.jobs.cluster_id, clusterId),
+          // Do not allow denying a job that has already been approved
+          isNull(data.jobs.approved),
+          eq(data.jobs.approval_requested, true),
+        ),
       );
   } else {
     await data.db
       .update(data.jobs)
       .set({
-        approved: true,
+        approved: false,
         status: "success",
-        result_type: "resolution",
+        result_type: "rejection",
         result: packer.pack({
           message: "This call was denied by the user.",
         }),
       })
       .where(
-        and(eq(data.jobs.id, call.id), eq(data.jobs.cluster_id, clusterId)),
+        and(
+        eq(data.jobs.id, call.id),
+        eq(data.jobs.cluster_id, clusterId),
+        // Do not allow denying a job that has already been approved
+        isNull(data.jobs.approved),
+        eq(data.jobs.approval_requested, true),
+      ),
       );
 
     if (call.runId) {

control-plane/src/modules/workflows/workflows.ts

@@ -5,6 +5,7 @@ import {
   desc,
   eq,
   inArray,
+  isNull,
   ne,
   or,
   sql,
@@ -622,7 +623,10 @@ export const getWaitingJobIds = async ({
         eq(jobs.cluster_id, clusterId),
         or(
           inArray(jobs.status, ["pending", "running"]),
-          and(eq(jobs.approval_requested, true), ne(jobs.approved, true)),
+          and(
+            eq(jobs.approval_requested, true),
+            isNull(jobs.approved)
+          ),
         ),
       ),
     );