Clean up Artifacts repos on package delete and account deletion

packages/worker/src/app/account-deletion.ts

@@ -3,6 +3,7 @@ import { jobVectorId } from '#mcp/jobs-vectorize.ts'
 import { memoryVectorId } from '#mcp/memory/memory-vectorize.ts'
 import { savedPackageVectorId } from '#worker/package-registry/repo.ts'
 import { getCapabilityVectorIndex } from '#mcp/capabilities/capability-search.ts'
+import { cleanupAllUserArtifactRepos } from '#worker/repo/artifact-repo-cleanup.ts'
 
 // Imported manually instead of via `@cloudflare/workers-oauth-provider` so
 // node-only unit tests can require this module without dragging in
@@ -27,6 +28,7 @@ type AccountDeletionEnv = Env & {
 export type AccountDeletionResult = {
 	deletedRowCounts: Record<string, number>
 	deletedKvKeys: number
+	deletedArtifactRepos: number
 	revokedOAuthGrants: number
 	clearedDurableObjects: Record<string, number>
 	deletedVectors: number
@@ -377,6 +379,7 @@ export async function deleteUserAccount(input: {
 	const result: AccountDeletionResult = {
 		deletedRowCounts: {},
 		deletedKvKeys: 0,
+		deletedArtifactRepos: 0,
 		revokedOAuthGrants: 0,
 		clearedDurableObjects: {},
 		deletedVectors: 0,
@@ -407,6 +410,16 @@ export async function deleteUserAccount(input: {
 		warnings,
 	})
 
+	result.deletedArtifactRepos = await cleanupAllUserArtifactRepos({
+		env: input.env,
+		userId: input.mcpUserId,
+		warnings,
+	}).catch((error) => {
+		const message = error instanceof Error ? error.message : String(error)
+		warnings.push(`Artifact repo cleanup failed unexpectedly: ${message}`)
+		return 0
+	})
+
 	result.clearedDurableObjects.storageRunners = await clearStorageRunners({
 		env: input.env,
 		userId: input.mcpUserId,

packages/worker/src/mcp/capabilities/packages/delete-package.ts

@@ -10,7 +10,7 @@ export const deletePackageCapability = defineDomainCapability(
 	{
 		name: 'package_delete',
 		description:
-			'Delete a saved package projection for the signed-in user. This removes the saved package from discovery. Repo-backed source cleanup is handled separately.',
+			'Delete a saved package projection for the signed-in user. This removes the saved package from discovery and attempts to clean up associated Artifacts repos (best-effort).',
 		keywords: ['package', 'delete', 'remove'],
 		readOnly: false,
 		idempotent: false,

packages/worker/src/package-registry/service.node.test.ts

@@ -34,6 +34,7 @@ const mockModule = vi.hoisted(() => ({
 	syncPackageJobsForPackage: vi.fn(),
 	updateSavedPackage: vi.fn(),
 	upsertSavedPackageVector: vi.fn(),
+	cleanupArtifactReposForPackage: vi.fn(),
 }))
 
 vi.mock('./manifest.ts', () => ({
@@ -111,6 +112,11 @@ vi.mock('#worker/jobs/service.ts', () => ({
 		mockModule.syncPackageJobsForPackage(...args),
 }))
 
+vi.mock('#worker/repo/artifact-repo-cleanup.ts', () => ({
+	cleanupArtifactReposForPackage: (...args: Array<unknown>) =>
+		mockModule.cleanupArtifactReposForPackage(...args),
+}))
+
 const { deleteSavedPackageProjection, refreshSavedPackageProjection } =
 	await import('./service.ts')
 
@@ -152,6 +158,7 @@ beforeEach(() => {
 	mockModule.deleteSavedPackage.mockResolvedValue(undefined)
 	mockModule.deleteSavedPackageVector.mockResolvedValue(undefined)
 	mockModule.deleteJobRow.mockResolvedValue(undefined)
+	mockModule.cleanupArtifactReposForPackage.mockResolvedValue(0)
 	mockModule.listSavedPackageServices.mockResolvedValue({
 		savedPackage: {
 			id: 'package-1',
@@ -352,6 +359,11 @@ test('deleteSavedPackageProjection resyncs the job manager after removing packag
 		packageId: 'package-1',
 	})
 
+	expect(mockModule.cleanupArtifactReposForPackage).toHaveBeenCalledWith({
+		env,
+		userId: 'user-1',
+		sourceId: 'source-1',
+	})
 	expect(mockModule.packageServiceRpc).toHaveBeenCalledWith({
 		env,
 		userId: 'user-1',

packages/worker/src/package-registry/service.ts

@@ -31,6 +31,7 @@ import {
 	refreshPackageRetrieverManifestCache,
 	removePackageRetrieverManifestCacheEntries,
 } from '#worker/package-retrievers/manifest-cache.ts'
+import { cleanupArtifactReposForPackage } from '#worker/repo/artifact-repo-cleanup.ts'
 
 function logPackageRetrieverProjectionError(input: {
 	action: 'refresh' | 'delete'
@@ -270,6 +271,21 @@ export async function deleteSavedPackageProjection(input: {
 		packageId: input.packageId,
 	})
 	if (savedPackage) {
+		await cleanupArtifactReposForPackage({
+			env: input.env,
+			userId: input.userId,
+			sourceId: savedPackage.sourceId,
+		}).catch((error) => {
+			console.warn(
+				JSON.stringify({
+					message: 'package artifact repo cleanup failed',
+					userId: input.userId,
+					packageId: input.packageId,
+					sourceId: savedPackage.sourceId,
+					error: error instanceof Error ? error.message : String(error),
+				}),
+			)
+		})
 		const listedServices = await listSavedPackageServices({
 			env: input.env,
 			userId: input.userId,

packages/worker/src/repo/artifact-repo-cleanup.node.test.ts

@@ -0,0 +1,173 @@
+import { afterEach, expect, test, vi } from 'vitest'
+
+const mockModule = vi.hoisted(() => ({
+	deleteArtifactRepo: vi.fn(),
+	getEntitySourceById: vi.fn(),
+	listEntitySourcesByUser: vi.fn(),
+	listRepoSessionsBySource: vi.fn(),
+	listRepoSessionsByUser: vi.fn(),
+	hasArtifactsAccess: vi.fn(),
+}))
+
+vi.mock('./artifacts.ts', () => ({
+	getArtifactsBinding: () => ({
+		delete: (...args: Array<unknown>) => mockModule.deleteArtifactRepo(...args),
+	}),
+	hasArtifactsAccess: (...args: Array<unknown>) =>
+		mockModule.hasArtifactsAccess(...args),
+}))
+
+vi.mock('./entity-sources.ts', () => ({
+	getEntitySourceById: (...args: Array<unknown>) =>
+		mockModule.getEntitySourceById(...args),
+	listEntitySourcesByUser: (...args: Array<unknown>) =>
+		mockModule.listEntitySourcesByUser(...args),
+}))
+
+vi.mock('./repo-sessions.ts', () => ({
+	listRepoSessionsBySource: (...args: Array<unknown>) =>
+		mockModule.listRepoSessionsBySource(...args),
+	listRepoSessionsByUser: (...args: Array<unknown>) =>
+		mockModule.listRepoSessionsByUser(...args),
+}))
+
+const {
+	cleanupAllUserArtifactRepos,
+	cleanupArtifactReposForPackage,
+	deleteUserScopedArtifactRepo,
+} = await import('./artifact-repo-cleanup.ts')
+
+const env = { APP_DB: {} } as Env
+
+afterEach(() => {
+	vi.clearAllMocks()
+})
+
+test('deleteUserScopedArtifactRepo deletes a repo and treats 404 as success', async () => {
+	mockModule.hasArtifactsAccess.mockReturnValue(true)
+	mockModule.deleteArtifactRepo.mockResolvedValueOnce({
+		id: 'repo_1',
+		alreadyDeleted: false,
+	})
+	mockModule.deleteArtifactRepo.mockResolvedValueOnce({
+		id: null,
+		alreadyDeleted: true,
+	})
+
+	expect(
+		await deleteUserScopedArtifactRepo({
+			env,
+			userId: 'user-1',
+			repoName: 'package-src-1',
+		}),
+	).toBe(true)
+	expect(
+		await deleteUserScopedArtifactRepo({
+			env,
+			userId: 'user-1',
+			repoName: 'package-src-1-session-abc',
+		}),
+	).toBe(true)
+	expect(mockModule.deleteArtifactRepo).toHaveBeenCalledTimes(2)
+})
+
+test('cleanupArtifactReposForPackage deletes session fork repos before entity repo', async () => {
+	mockModule.hasArtifactsAccess.mockReturnValue(true)
+	mockModule.getEntitySourceById.mockResolvedValue({
+		id: 'source-1',
+		user_id: 'user-1',
+		repo_id: 'package-pkg-1',
+	})
+	mockModule.listRepoSessionsBySource.mockResolvedValue([
+		{
+			id: 'session-1',
+			user_id: 'user-1',
+			source_id: 'source-1',
+			session_repo_name: 'package-pkg-1-abc123',
+		},
+	])
+	mockModule.deleteArtifactRepo.mockResolvedValue({
+		id: 'repo_deleted',
+		alreadyDeleted: false,
+	})
+
+	const deleted = await cleanupArtifactReposForPackage({
+		env,
+		userId: 'user-1',
+		sourceId: 'source-1',
+	})
+
+	expect(deleted).toBe(2)
+	expect(mockModule.deleteArtifactRepo).toHaveBeenCalledWith(
+		'package-pkg-1-abc123',
+	)
+	expect(mockModule.deleteArtifactRepo).toHaveBeenCalledWith('package-pkg-1')
+	expect(mockModule.getEntitySourceById).toHaveBeenCalledWith({}, 'source-1')
+})
+
+test('cleanupArtifactReposForPackage skips repos owned by another user', async () => {
+	mockModule.getEntitySourceById.mockResolvedValue({
+		id: 'source-1',
+		user_id: 'user-other',
+		repo_id: 'package-pkg-1',
+	})
+	const warnings: Array<string> = []
+
+	const deleted = await cleanupArtifactReposForPackage({
+		env,
+		userId: 'user-1',
+		sourceId: 'source-1',
+		warnings,
+	})
+
+	expect(deleted).toBe(0)
+	expect(mockModule.deleteArtifactRepo).not.toHaveBeenCalled()
+	expect(warnings[0]).toContain('user scope mismatch')
+})
+
+test('cleanupAllUserArtifactRepos deduplicates repos across sources and sessions', async () => {
+	mockModule.hasArtifactsAccess.mockReturnValue(true)
+	mockModule.listEntitySourcesByUser.mockResolvedValue([
+		{ id: 'source-1', user_id: 'user-1', repo_id: 'package-pkg-1' },
+		{ id: 'source-2', user_id: 'user-1', repo_id: 'job-job-1' },
+	])
+	mockModule.listRepoSessionsByUser.mockResolvedValue([
+		{
+			id: 'session-1',
+			user_id: 'user-1',
+			session_repo_name: 'package-pkg-1-abc123',
+		},
+	])
+	mockModule.deleteArtifactRepo.mockResolvedValue({
+		id: 'repo_deleted',
+		alreadyDeleted: false,
+	})
+
+	const warnings: Array<string> = []
+	const deleted = await cleanupAllUserArtifactRepos({
+		env,
+		userId: 'user-1',
+		warnings,
+	})
+
+	expect(deleted).toBe(3)
+	expect(mockModule.deleteArtifactRepo).toHaveBeenCalledTimes(3)
+})
+
+test('cleanupAllUserArtifactRepos records a warning when Artifacts access is unavailable', async () => {
+	mockModule.hasArtifactsAccess.mockReturnValue(false)
+	mockModule.listEntitySourcesByUser.mockResolvedValue([
+		{ id: 'source-1', user_id: 'user-1', repo_id: 'package-pkg-1' },
+	])
+	mockModule.listRepoSessionsByUser.mockResolvedValue([])
+
+	const warnings: Array<string> = []
+	const deleted = await cleanupAllUserArtifactRepos({
+		env,
+		userId: 'user-1',
+		warnings,
+	})
+
+	expect(deleted).toBe(0)
+	expect(warnings[0]).toContain('Cloudflare Artifacts access was unavailable')
+})