Record a computer reset as soon as the profile is gone (#404)

Co-authored-by: David McKay <david@copilotkit.ai>

Author: kevin9327

CHANGELOG.md

@@ -8,6 +8,14 @@ Newest first. `Unreleased` is what is on `main` and not yet tagged.
 
 ## Unreleased
 
+### Wiping a computer is recorded even if clearing its stored state fails
+
+Resetting a computer destroys the profile first and wrote the audit row last, after two Postgres
+deletes. A connection reset, a failover or a statement timeout in either delete threw before the row
+was written, so the most destructive button in the product could leave a wiped computer -- every
+login gone, no undo -- with nothing on the trail to say who wiped it or when. The row is now written
+as soon as the profile is gone, which is the point after which nothing can be put back. A failure in
+either delete is still reported to the caller.
 ### Pressing Stop is recorded as a stop, not as a computer that is not running
 
 The computer transport answered a Stop correctly only when it arrived before the request left. The

server/src/computer/gateway.ts

@@ -713,6 +713,22 @@ export function createComputerGateway(
      */
     async resetComputer(botId: string, actor: ActionActor) {
       const result = await provider.reset(botId);
+      /*
+       * The row goes in HERE, before the two deletes below, because this line is the point of no
+       * return: the profile is already gone and nothing after it can put the logins back.
+       *
+       * Both clears are Postgres deletes, and a connection reset, a failover or a statement timeout
+       * in either used to throw before the row was written -- leaving a computer wiped with nothing
+       * on the trail to say who wiped it, which is the one outcome the note above rules out. The
+       * failure still propagates, so the caller is told the clears did not finish.
+       */
+      await writeControlEvent(auditStore, "computer.reset", {
+        botId,
+        actor,
+        reason: result.cleared
+          ? "the computer and its saved state were deleted"
+          : "no saved state was present to delete",
+      });
       // The refs the last snapshot handed out describe a page that no longer exists, and a fresh
       // computer counts generations from one again, so the row has to go with the profile.
       await snapshots.clear(botId);
@@ -725,13 +741,6 @@ export function createComputerGateway(
        * anything a person would recognise as private.
        */
       await pageFrames?.clear(botId);
-      await writeControlEvent(auditStore, "computer.reset", {
-        botId,
-        actor,
-        reason: result.cleared
-          ? "the computer and its saved state were deleted"
-          : "no saved state was present to delete",
-      });
       return result;
     },
 

server/tests/computer-gateway.test.ts

@@ -687,6 +687,67 @@ describe("the computer gateway", () => {
     expect(rows[0]?.targetId).toBe("bot-2");
   });
 
+  /*
+   * "The most destructive button we have. Every login the Bot had is gone and no undo exists, so the
+   * row is written whatever happens next."
+   *
+   * The profile is destroyed by `provider.reset` before either of the two Postgres deletes runs, so
+   * a delete that fails cannot put it back -- it can only take the row with it, which is the one
+   * thing that note rules out.
+   */
+  test("resetComputer records the reset even when clearing the stored page fails", async () => {
+    const { provider, fetchImpl } = fakeComputer({
+      resetResult: { cleared: true },
+    });
+    const { store, rows } = fakeAudit();
+    const snapshots: SnapshotStore = {
+      ...createInMemorySnapshotStore(),
+      clear: async () => {
+        throw new Error("connection reset by peer");
+      },
+    };
+    const gateway = createComputerGateway({
+      provider,
+      fetchImpl,
+      auditStore: store,
+      policy: () => PERMISSIVE,
+      snapshots,
+    });
+
+    await expect(gateway.resetComputer("bot-1", ACTOR)).rejects.toThrow(
+      "connection reset by peer",
+    );
+
+    expect(rows.map((row) => row.eventType)).toContain("computer.reset");
+    expect(rows[0]?.targetId).toBe("bot-1");
+  });
+
+  test("resetComputer records the reset even when clearing the screenshots fails", async () => {
+    const { provider, fetchImpl } = fakeComputer({
+      resetResult: { cleared: true },
+    });
+    const { store, rows } = fakeAudit();
+    const gateway = createComputerGateway({
+      provider,
+      fetchImpl,
+      auditStore: store,
+      policy: () => PERMISSIVE,
+      pageFrames: {
+        clear: async () => {
+          throw new Error("statement timeout");
+        },
+      } as unknown as NonNullable<
+        Parameters<typeof createComputerGateway>[0]["pageFrames"]
+      >,
+    });
+
+    await expect(gateway.resetComputer("bot-1", ACTOR)).rejects.toThrow(
+      "statement timeout",
+    );
+
+    expect(rows.map((row) => row.eventType)).toContain("computer.reset");
+  });
+
   test("computers maps provider status 'running' and 'stopped' directly and preserves egress distinctions", async () => {
     const locations: ComputerLocation[] = [
       {