diff --git a/coderd/database/constants.go b/coderd/database/constants.go index 931e0d7e09..34ad1005ee 100644 --- a/coderd/database/constants.go +++ b/coderd/database/constants.go @@ -1,5 +1,12 @@ package database -import "github.com/google/uuid" +import ( + "github.com/google/uuid" -var PrebuildsSystemUserID = uuid.MustParse("c42fdf75-3097-471c-8c33-fb52454d81c0") + "github.com/coder/coder/v2/codersdk" +) + +// PrebuildsSystemUserID mirrors codersdk.PrebuildsSystemUserID, parsed +// for use as a uuid.UUID. Both must agree; tests pin the value to the +// codersdk constant so the two cannot drift. +var PrebuildsSystemUserID = uuid.MustParse(codersdk.PrebuildsSystemUserID) diff --git a/codersdk/prebuilds.go b/codersdk/prebuilds.go index 1f428d2f75..979c61bfb7 100644 --- a/codersdk/prebuilds.go +++ b/codersdk/prebuilds.go @@ -6,6 +6,13 @@ import ( "net/http" ) +// PrebuildsSystemUserID is the UUID of the Coder prebuilds system +// user. Prebuilt workspaces are owned by this user until they are +// claimed; build #1 of a claimed workspace remains attributed to +// this user as the initiator forever, which is how callers can +// recognize a prebuild claim after the fact. +const PrebuildsSystemUserID = "c42fdf75-3097-471c-8c33-fb52454d81c0" + type PrebuildsSettings struct { ReconciliationPaused bool `json:"reconciliation_paused"` } diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 6591ad23bf..7cf20be705 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -5922,6 +5922,16 @@ export interface PrebuildsSettings { readonly reconciliation_paused: boolean; } +// From codersdk/prebuilds.go +/** + * PrebuildsSystemUserID is the UUID of the Coder prebuilds system + * user. Prebuilt workspaces are owned by this user until they are + * claimed; build #1 of a claimed workspace remains attributed to + * this user as the initiator forever, which is how callers can + * recognize a prebuild claim after the fact. + */ +export const PrebuildsSystemUserID = "c42fdf75-3097-471c-8c33-fb52454d81c0"; + // From codersdk/presets.go export interface Preset { readonly ID: string; diff --git a/site/src/pages/AgentsPage/AgentsPage.tsx b/site/src/pages/AgentsPage/AgentsPage.tsx index 0799a089c1..aa3ce27ab7 100644 --- a/site/src/pages/AgentsPage/AgentsPage.tsx +++ b/site/src/pages/AgentsPage/AgentsPage.tsx @@ -340,6 +340,19 @@ const AgentsPage: FC = () => { try { const action = await resolveArchiveAndDeleteAction( () => queryClient.fetchQuery(workspaceById(workspaceId)), + // We only need build_number 1 and 2 to recognise a + // prebuild claim. The default page is newest-first; the + // resolver degrades safely ("confirm") if those builds + // aren't in the returned slice. + () => + queryClient.fetchQuery({ + queryKey: [ + "workspaceBuilds", + workspaceId, + "archive-and-delete-resolver", + ], + queryFn: () => API.getWorkspaceBuilds(workspaceId), + }), () => readInfiniteChatsCache(queryClient)?.find((c) => c.id === chatId) ?.created_at, diff --git a/site/src/pages/AgentsPage/utils/agentWorkspaceUtils.test.ts b/site/src/pages/AgentsPage/utils/agentWorkspaceUtils.test.ts index e45ac3bb95..f8820a3294 100644 --- a/site/src/pages/AgentsPage/utils/agentWorkspaceUtils.test.ts +++ b/site/src/pages/AgentsPage/utils/agentWorkspaceUtils.test.ts @@ -1,46 +1,204 @@ import { describe, expect, it, vi } from "vitest"; +import { PrebuildsSystemUserID } from "#/api/typesGenerated"; import { archiveChatAndDeleteWorkspace, isWorkspaceAutoCreated, isWorkspaceNotFound, resolveArchiveAndDeleteAction, shouldNavigateAfterArchive, + workspaceAcquiredAt, } from "./agentWorkspaceUtils"; +const REAL_USER = "11111111-2222-3333-4444-555555555555"; + +describe("workspaceAcquiredAt", () => { + it("returns workspace.created_at when no builds exist", () => { + const ws = { created_at: "2026-01-01T00:00:00Z" }; + expect(workspaceAcquiredAt(ws, [])).toBe("2026-01-01T00:00:00Z"); + }); + + it("returns workspace.created_at when build #1 was initiated by a real user", () => { + const ws = { created_at: "2026-01-01T00:00:00Z" }; + const builds = [ + { + build_number: 1, + initiator_id: REAL_USER, + created_at: "2026-01-01T00:00:01Z", + }, + ]; + expect(workspaceAcquiredAt(ws, builds)).toBe("2026-01-01T00:00:00Z"); + }); + + it("returns build #2 created_at when build #1 was initiated by the prebuilds user", () => { + // Workspace.created_at predates the chat (the prebuild was + // provisioned long before the chat existed), but build #2 is + // the claim and that's the moment the chat acquired the + // workspace. + const ws = { created_at: "2026-01-01T08:00:00Z" }; + const builds = [ + { + build_number: 2, + initiator_id: REAL_USER, + created_at: "2026-01-01T12:00:05Z", + }, + { + build_number: 1, + initiator_id: PrebuildsSystemUserID, + created_at: "2026-01-01T08:00:01Z", + }, + ]; + expect(workspaceAcquiredAt(ws, builds)).toBe("2026-01-01T12:00:05Z"); + }); + + it("returns null when prebuild has no claim build yet", () => { + const ws = { created_at: "2026-01-01T08:00:00Z" }; + const builds = [ + { + build_number: 1, + initiator_id: PrebuildsSystemUserID, + created_at: "2026-01-01T08:00:01Z", + }, + ]; + expect(workspaceAcquiredAt(ws, builds)).toBeNull(); + }); + + it("ignores extra builds beyond #1 and #2", () => { + const ws = { created_at: "2026-01-01T08:00:00Z" }; + const builds = [ + { + build_number: 5, + initiator_id: REAL_USER, + created_at: "2026-02-01T00:00:00Z", + }, + { + build_number: 4, + initiator_id: REAL_USER, + created_at: "2026-01-15T00:00:00Z", + }, + { + build_number: 3, + initiator_id: REAL_USER, + created_at: "2026-01-10T00:00:00Z", + }, + { + build_number: 2, + initiator_id: REAL_USER, + created_at: "2026-01-01T12:00:05Z", + }, + { + build_number: 1, + initiator_id: PrebuildsSystemUserID, + created_at: "2026-01-01T08:00:01Z", + }, + ]; + expect(workspaceAcquiredAt(ws, builds)).toBe("2026-01-01T12:00:05Z"); + }); +}); + describe("isWorkspaceAutoCreated", () => { it.each([ { - name: "workspace created after chat", - workspace: "2026-01-01T00:00:05Z", + name: "from-scratch workspace created after chat", + workspace: { created_at: "2026-01-01T00:00:05Z" }, + builds: [ + { + build_number: 1, + initiator_id: REAL_USER, + created_at: "2026-01-01T00:00:05Z", + }, + ], chat: "2026-01-01T00:00:00Z", expected: true, }, { - name: "workspace created at same time as chat", - workspace: "2026-01-01T12:00:00Z", + name: "from-scratch workspace created at same time as chat", + workspace: { created_at: "2026-01-01T12:00:00Z" }, + builds: [ + { + build_number: 1, + initiator_id: REAL_USER, + created_at: "2026-01-01T12:00:00Z", + }, + ], chat: "2026-01-01T12:00:00Z", expected: true, }, { - name: "workspace created before chat", - workspace: "2026-01-01T11:59:59Z", + name: "from-scratch workspace created before chat", + workspace: { created_at: "2026-01-01T11:59:59Z" }, + builds: [ + { + build_number: 1, + initiator_id: REAL_USER, + created_at: "2026-01-01T11:59:59Z", + }, + ], + chat: "2026-01-01T12:00:00Z", + expected: false, + }, + // Prebuild claim cases: workspace.created_at predates the + // chat, but build #2 (the claim) happened after the chat. + { + name: "prebuild claimed after chat", + workspace: { created_at: "2026-01-01T08:00:00Z" }, + builds: [ + { + build_number: 2, + initiator_id: REAL_USER, + created_at: "2026-01-01T12:00:05Z", + }, + { + build_number: 1, + initiator_id: PrebuildsSystemUserID, + created_at: "2026-01-01T08:00:01Z", + }, + ], + chat: "2026-01-01T12:00:00Z", + expected: true, + }, + { + name: "prebuild claimed before chat", + workspace: { created_at: "2026-01-01T08:00:00Z" }, + builds: [ + { + build_number: 2, + initiator_id: REAL_USER, + created_at: "2026-01-01T11:00:00Z", + }, + { + build_number: 1, + initiator_id: PrebuildsSystemUserID, + created_at: "2026-01-01T08:00:01Z", + }, + ], chat: "2026-01-01T12:00:00Z", expected: false, }, { - name: "sub-second precision difference", - workspace: "2026-01-01T00:00:00.001Z", - chat: "2026-01-01T00:00:00.000Z", - expected: true, - }, - { - name: "workspace predates chat by days", - workspace: "2026-03-10T10:00:00Z", - chat: "2026-03-15T10:00:00Z", + name: "unclaimed prebuild treated as not auto-created", + workspace: { created_at: "2026-01-01T08:00:00Z" }, + builds: [ + { + build_number: 1, + initiator_id: PrebuildsSystemUserID, + created_at: "2026-01-01T08:00:01Z", + }, + ], + chat: "2026-01-01T12:00:00Z", expected: false, }, - ])("$name → $expected", ({ workspace, chat, expected }) => { - expect(isWorkspaceAutoCreated(workspace, chat)).toBe(expected); + { + // Defensive: build history empty. Fall back to + // workspace.created_at so we still allow the proceed + // path in the common case rather than blocking on data. + name: "no builds, falls back to workspace.created_at", + workspace: { created_at: "2026-01-01T12:00:05Z" }, + builds: [], + chat: "2026-01-01T12:00:00Z", + expected: true, + }, + ])("$name → $expected", ({ workspace, builds, chat, expected }) => { + expect(isWorkspaceAutoCreated(workspace, builds, chat)).toBe(expected); }); }); @@ -221,31 +379,103 @@ describe("archiveChatAndDeleteWorkspace", () => { describe("resolveArchiveAndDeleteAction", () => { it.each([ { - name: "auto-created workspace → proceed", - workspaceCreatedAt: "2026-01-01T00:00:05Z", + name: "from-scratch workspace created after chat → proceed", + workspace: { created_at: "2026-01-01T00:00:05Z" }, + builds: [ + { + build_number: 1, + initiator_id: REAL_USER, + created_at: "2026-01-01T00:00:05Z", + }, + ], chatCreatedAt: "2026-01-01T00:00:00Z", expected: "proceed", }, { name: "workspace predates chat → confirm", - workspaceCreatedAt: "2025-12-01T00:00:00Z", + workspace: { created_at: "2025-12-01T00:00:00Z" }, + builds: [ + { + build_number: 1, + initiator_id: REAL_USER, + created_at: "2025-12-01T00:00:00Z", + }, + ], chatCreatedAt: "2026-01-01T00:00:00Z", expected: "confirm", }, { name: "chat not found in cache → confirm", - workspaceCreatedAt: "2026-01-01T00:00:05Z", + workspace: { created_at: "2026-01-01T00:00:05Z" }, + builds: [ + { + build_number: 1, + initiator_id: REAL_USER, + created_at: "2026-01-01T00:00:05Z", + }, + ], chatCreatedAt: undefined, expected: "confirm", }, - ])("$name", async ({ workspaceCreatedAt, chatCreatedAt, expected }) => { + { + // The bug this PR fixes: workspace.created_at predates + // the chat (the prebuild was provisioned earlier) but + // build #2 is the claim and happened after the chat. + name: "prebuild claimed after chat → proceed", + workspace: { created_at: "2025-12-15T00:00:00Z" }, + builds: [ + { + build_number: 2, + initiator_id: REAL_USER, + created_at: "2026-01-01T00:00:05Z", + }, + { + build_number: 1, + initiator_id: PrebuildsSystemUserID, + created_at: "2025-12-15T00:00:00Z", + }, + ], + chatCreatedAt: "2026-01-01T00:00:00Z", + expected: "proceed", + }, + { + name: "prebuild claimed before chat → confirm", + workspace: { created_at: "2025-12-15T00:00:00Z" }, + builds: [ + { + build_number: 2, + initiator_id: REAL_USER, + created_at: "2025-12-31T00:00:00Z", + }, + { + build_number: 1, + initiator_id: PrebuildsSystemUserID, + created_at: "2025-12-15T00:00:00Z", + }, + ], + chatCreatedAt: "2026-01-01T00:00:00Z", + expected: "confirm", + }, + ])("$name", async ({ workspace, builds, chatCreatedAt, expected }) => { const result = await resolveArchiveAndDeleteAction( - async () => ({ created_at: workspaceCreatedAt }), + async () => workspace, + async () => builds, () => chatCreatedAt, ); expect(result).toBe(expected); }); + it("does not fetch builds when the chat is not in the cache", async () => { + const fetchBuilds = vi.fn(async () => []); + const result = await resolveArchiveAndDeleteAction( + async () => ({ created_at: "2026-01-01T00:00:00Z" }), + fetchBuilds, + () => undefined, + ); + expect(result).toBe("confirm"); + expect(fetchBuilds).not.toHaveBeenCalled(); + }); + it("propagates non-404-or-410 workspace fetch errors", async () => { const error = { isAxiosError: true, @@ -260,6 +490,7 @@ describe("resolveArchiveAndDeleteAction", () => { async () => { throw error; }, + async () => [], () => "2026-01-01T00:00:00Z", ), ).rejects.toBe(error); @@ -279,6 +510,7 @@ describe("resolveArchiveAndDeleteAction", () => { async () => { throw error; }, + async () => [], () => "2026-01-01T00:00:00Z", ), ).resolves.toBe("archive-only"); @@ -298,10 +530,51 @@ describe("resolveArchiveAndDeleteAction", () => { async () => { throw error; }, + async () => [], () => "2026-01-01T00:00:00Z", ), ).resolves.toBe("archive-only"); }); + + it("returns archive-only when the builds fetch returns 404", async () => { + const error = { + isAxiosError: true, + response: { + status: 404, + data: { message: "Workspace not found" }, + }, + }; + + await expect( + resolveArchiveAndDeleteAction( + async () => ({ created_at: "2026-01-01T00:00:00Z" }), + async () => { + throw error; + }, + () => "2026-01-01T00:00:00Z", + ), + ).resolves.toBe("archive-only"); + }); + + it("propagates non-404-or-410 builds fetch errors", async () => { + const error = { + isAxiosError: true, + response: { + status: 500, + data: { message: "Internal server error" }, + }, + }; + + await expect( + resolveArchiveAndDeleteAction( + async () => ({ created_at: "2026-01-01T00:00:00Z" }), + async () => { + throw error; + }, + () => "2026-01-01T00:00:00Z", + ), + ).rejects.toBe(error); + }); }); describe("shouldNavigateAfterArchive", () => { diff --git a/site/src/pages/AgentsPage/utils/agentWorkspaceUtils.ts b/site/src/pages/AgentsPage/utils/agentWorkspaceUtils.ts index c49472885a..c2566e31e5 100644 --- a/site/src/pages/AgentsPage/utils/agentWorkspaceUtils.ts +++ b/site/src/pages/AgentsPage/utils/agentWorkspaceUtils.ts @@ -1,17 +1,70 @@ import { isAxiosError } from "axios"; +import { + PrebuildsSystemUserID, + type WorkspaceBuild, +} from "#/api/typesGenerated"; /** - * Determines whether a workspace was auto-created by a chat. - * Workspaces created at or after the chat's creation time are - * considered auto-created (the chat provisioned them). Pre-existing - * workspaces that were manually associated need a confirmation - * dialog before deletion. + * Returns the moment a workspace's identity transferred to its + * current owner. + * + * For workspaces created from scratch, this is `workspace.created_at`: + * build #1 already belongs to the current owner. + * + * For workspaces claimed from a prebuild, this is the start time of + * build #2 (the claim build). `workspace.created_at` for those + * workspaces reflects when the prebuild was provisioned, often well + * before the chat that claimed it existed, which is why the original + * `created_at` heuristic misfired the deletion confirmation dialog. + * + * Returns `null` when the result cannot be determined, for example + * an unclaimed prebuild (build #1 by prebuilds system user, no build + * #2). Callers should treat `null` as "force the confirmation + * dialog"; the deletion path is destructive and should err on the + * side of asking. + */ +export function workspaceAcquiredAt( + workspace: { created_at: string }, + builds: readonly Pick< + WorkspaceBuild, + "build_number" | "initiator_id" | "created_at" + >[], +): string | null { + const build1 = builds.find((b) => b.build_number === 1); + // No history at all (shouldn't happen for an existing workspace); + // fall back to created_at rather than blocking on missing data. + if (!build1) { + return workspace.created_at; + } + if (build1.initiator_id !== PrebuildsSystemUserID) { + return workspace.created_at; + } + const build2 = builds.find((b) => b.build_number === 2); + return build2 ? build2.created_at : null; +} + +/** + * Determines whether a workspace was auto-created by a chat. A + * workspace is "auto-created" if the chat acquired it (via creation + * from scratch or by claiming a prebuild) at or after the chat's own + * creation time. + * + * Pre-existing workspaces that were manually associated with the + * chat need a confirmation dialog before deletion. */ export function isWorkspaceAutoCreated( - workspaceCreatedAt: string, + workspace: { created_at: string }, + builds: readonly Pick< + WorkspaceBuild, + "build_number" | "initiator_id" | "created_at" + >[], chatCreatedAt: string, ): boolean { - return new Date(workspaceCreatedAt) >= new Date(chatCreatedAt); + const acquiredAt = workspaceAcquiredAt(workspace, builds); + if (acquiredAt === null) { + return false; + } + return new Date(acquiredAt) >= new Date(chatCreatedAt); } /** @@ -78,14 +131,18 @@ export function shouldNavigateAfterArchive( /** * Resolves whether an archive-and-delete action should proceed * immediately or require user confirmation. Fetches the workspace - * to compare its creation time against the chat's. Auto-created - * workspaces (provisioned by the chat) skip the confirmation - * dialog; pre-existing workspaces require the user to type the - * workspace name. + * and its build history to determine when the workspace was + * acquired (claim time for prebuilts, creation time otherwise) and + * compares against the chat's creation time. Auto-created + * workspaces (provisioned or claimed by the chat) skip the + * confirmation dialog; pre-existing workspaces require the user to + * type the workspace name. * * @param fetchWorkspace - Retrieves the workspace (e.g. via - * `queryClient.fetchQuery`). The result must include - * `created_at`. + * `queryClient.fetchQuery`). The result must include `created_at`. + * @param fetchBuilds - Retrieves the workspace's build history. The + * first call only needs build_number 1 and 2, but callers will + * typically pass the full list. * @param getChatCreatedAt - Returns the chat's `created_at` * timestamp, or `undefined` if the chat is not in the cache. * @returns `"proceed"` to skip the dialog, `"archive-only"` to archive @@ -94,6 +151,12 @@ export function shouldNavigateAfterArchive( */ export async function resolveArchiveAndDeleteAction( fetchWorkspace: () => Promise<{ created_at: string }>, + fetchBuilds: () => Promise< + readonly Pick< + WorkspaceBuild, + "build_number" | "initiator_id" | "created_at" + >[] + >, getChatCreatedAt: () => string | undefined, ): Promise<"proceed" | "confirm" | "archive-only"> { let workspace: { created_at: string }; @@ -106,10 +169,22 @@ export async function resolveArchiveAndDeleteAction( throw error; } const chatCreatedAt = getChatCreatedAt(); - if ( - chatCreatedAt && - isWorkspaceAutoCreated(workspace.created_at, chatCreatedAt) - ) { + if (!chatCreatedAt) { + return "confirm"; + } + let builds: readonly Pick< + WorkspaceBuild, + "build_number" | "initiator_id" | "created_at" + >[]; + try { + builds = await fetchBuilds(); + } catch (error) { + if (isWorkspaceNotFound(error)) { + return "archive-only"; + } + throw error; + } + if (isWorkspaceAutoCreated(workspace, builds, chatCreatedAt)) { return "proceed"; } return "confirm";