diff --git a/packages/backend/convex/agentJobs.ts b/packages/backend/convex/agentJobs.ts index 091f13d..364bed9 100644 --- a/packages/backend/convex/agentJobs.ts +++ b/packages/backend/convex/agentJobs.ts @@ -234,6 +234,11 @@ const normalizeWorkspacePaths = (values: string[] | undefined, max: number) => .filter((value, index, all) => all.indexOf(value) === index) .slice(0, max); +// Owner-wide scans for old-workspace cleanup are bounded to this many rows so +// the query cost cannot grow without limit. The count is therefore reported as +// "up to N"; delete is per-call bounded and re-runnable to drain the rest. +const OLD_WORKSPACE_SCAN_LIMIT = 500; + const isDeletableWorkspace = (job: Doc<'agentJobs'>) => ['failed', 'cancelled', 'timed_out'].includes(job.status) || ['stopped', 'expired', 'failed'].includes(job.workspaceStatus ?? ''); @@ -1069,7 +1074,7 @@ export const countOldWorkspaces = query({ const jobs = await ctx.db .query('agentJobs') .withIndex('by_owner', (q) => q.eq('ownerId', ownerId)) - .collect(); + .take(OLD_WORKSPACE_SCAN_LIMIT); return jobs.filter( (job) => isDeletableWorkspace(job) && job.updatedAt <= cutoff, ).length; @@ -1091,7 +1096,7 @@ export const deleteOldWorkspaces = mutation({ const jobs = await ctx.db .query('agentJobs') .withIndex('by_owner', (q) => q.eq('ownerId', ownerId)) - .collect(); + .take(OLD_WORKSPACE_SCAN_LIMIT); const deletable = jobs .filter((job) => isDeletableWorkspace(job) && job.updatedAt <= cutoff) .sort((a, b) => a.updatedAt - b.updatedAt) diff --git a/packages/backend/convex/spoonCommits.ts b/packages/backend/convex/spoonCommits.ts index 3f471ba..af4d656 100644 --- a/packages/backend/convex/spoonCommits.ts +++ b/packages/backend/convex/spoonCommits.ts @@ -23,14 +23,26 @@ export const listForSpoon = query({ .order('desc') .take(limit ?? 100); } - const commits = await ctx.db - .query('spoonCommits') - .withIndex('by_owner', (q) => q.eq('ownerId', ownerId)) - .order('desc') - .collect(); - return commits - .filter((commit) => commit.spoonId === spoonId) - .slice(0, limit ?? 100); + const take = limit ?? 100; + const [upstream, fork] = await Promise.all([ + ctx.db + .query('spoonCommits') + .withIndex('by_spoon_side', (q) => + q.eq('spoonId', spoonId).eq('side', 'upstream'), + ) + .order('desc') + .take(take), + ctx.db + .query('spoonCommits') + .withIndex('by_spoon_side', (q) => + q.eq('spoonId', spoonId).eq('side', 'fork'), + ) + .order('desc') + .take(take), + ]); + return [...upstream, ...fork] + .sort((a, b) => (b.committedAt ?? 0) - (a.committedAt ?? 0)) + .slice(0, take); }, }); diff --git a/packages/backend/tests/unit/hot-paths.test.ts b/packages/backend/tests/unit/hot-paths.test.ts new file mode 100644 index 0000000..01c689d --- /dev/null +++ b/packages/backend/tests/unit/hot-paths.test.ts @@ -0,0 +1,273 @@ +import { convexTest } from 'convex-test'; +import { describe, expect, test } from 'vitest'; + +import type { Id } from '../../convex/_generated/dataModel.js'; +import { api } from '../../convex/_generated/api.js'; +import schema from '../../convex/schema'; + +const modules = import.meta.glob('../../convex/**/*.*s'); + +const createUser = async (t: ReturnType, email: string) => + (await t.mutation(async (ctx) => { + return await ctx.db.insert('users', { email, name: email }); + })) as Id<'users'>; + +const authed = (t: ReturnType, userId: string) => + t.withIdentity({ + subject: `${userId}|session`, + issuer: 'https://convex.test', + }); + +const seedSpoon = async ( + t: ReturnType, + ownerId: Id<'users'>, +) => + (await t.mutation(async (ctx) => { + const now = Date.now(); + return await ctx.db.insert('spoons', { + ownerId, + name: 'Hot Path Spoon', + provider: 'github', + upstreamOwner: 'upstream', + upstreamRepo: 'repo', + upstreamDefaultBranch: 'main', + upstreamUrl: 'https://github.com/upstream/repo', + forkOwner: 'me', + forkRepo: 'repo-fork', + forkUrl: 'https://github.com/me/repo-fork', + visibility: 'public', + maintenanceMode: 'watch', + syncCadence: 'daily', + productionRefStrategy: 'default_branch', + status: 'active', + createdAt: now, + updatedAt: now, + }); + })) as Id<'spoons'>; + +type CommitSeed = { + side: 'upstream' | 'fork'; + committedAt: number; + sha: string; +}; + +const seedCommits = async ( + t: ReturnType, + spoonId: Id<'spoons'>, + ownerId: Id<'users'>, + commits: CommitSeed[], +) => + await t.mutation(async (ctx) => { + const now = Date.now(); + for (const commit of commits) { + await ctx.db.insert('spoonCommits', { + spoonId, + ownerId, + sha: commit.sha, + side: commit.side, + message: `commit ${commit.sha}`, + committedAt: commit.committedAt, + createdAt: now, + updatedAt: now, + }); + } + }); + +describe('spoonCommits.listForSpoon (no side) bounded + merged', () => { + test('returns the most-recent-N across both sides, committedAt desc', async () => { + const t = convexTest(schema, modules); + const ownerId = await createUser(t, 'owner@example.com'); + const spoonId = await seedSpoon(t, ownerId); + + // Insert so that DB creation order != committedAt order, forcing the + // implementation to sort by committedAt (not fall back on index order). + // committedAt values interleave the two sides so a correct top-4 must + // include both sides — a naive single-side take(4) would be wrong. + await seedCommits(t, spoonId, ownerId, [ + { side: 'upstream', committedAt: 500, sha: 'u500' }, + { side: 'upstream', committedAt: 300, sha: 'u300' }, + { side: 'upstream', committedAt: 100, sha: 'u100' }, + { side: 'fork', committedAt: 600, sha: 'f600' }, + { side: 'fork', committedAt: 400, sha: 'f400' }, + { side: 'fork', committedAt: 200, sha: 'f200' }, + ]); + + const result = await authed(t, ownerId).query( + api.spoonCommits.listForSpoon, + { spoonId, limit: 4 }, + ); + + expect(result).toHaveLength(4); + // Top-4 by committedAt desc across both sides. + expect(result.map((c) => c.sha)).toEqual(['f600', 'u500', 'f400', 'u300']); + // Both sides represented in the result. + expect(result.some((c) => c.side === 'upstream')).toBe(true); + expect(result.some((c) => c.side === 'fork')).toBe(true); + // Strictly descending committedAt. + const times = result.map((c) => c.committedAt ?? 0); + expect(times).toEqual([...times].sort((a, b) => b - a)); + }); + + test('returns all rows when under the limit', async () => { + const t = convexTest(schema, modules); + const ownerId = await createUser(t, 'owner@example.com'); + const spoonId = await seedSpoon(t, ownerId); + + await seedCommits(t, spoonId, ownerId, [ + { side: 'upstream', committedAt: 10, sha: 'u10' }, + { side: 'fork', committedAt: 20, sha: 'f20' }, + ]); + + const result = await authed(t, ownerId).query( + api.spoonCommits.listForSpoon, + { spoonId, limit: 100 }, + ); + + expect(result.map((c) => c.sha)).toEqual(['f20', 'u10']); + }); + + test('only returns commits for the requested spoon', async () => { + const t = convexTest(schema, modules); + const ownerId = await createUser(t, 'owner@example.com'); + const spoonA = await seedSpoon(t, ownerId); + const spoonB = await seedSpoon(t, ownerId); + + await seedCommits(t, spoonA, ownerId, [ + { side: 'upstream', committedAt: 50, sha: 'a-up' }, + ]); + await seedCommits(t, spoonB, ownerId, [ + { side: 'fork', committedAt: 999, sha: 'b-fork' }, + ]); + + const result = await authed(t, ownerId).query( + api.spoonCommits.listForSpoon, + { spoonId: spoonA, limit: 100 }, + ); + + expect(result.map((c) => c.sha)).toEqual(['a-up']); + }); +}); + +const seedJobs = async ( + t: ReturnType, + spoonId: Id<'spoons'>, + ownerId: Id<'users'>, + count: number, + status: + | 'failed' + | 'cancelled' + | 'timed_out' + | 'running' + | 'queued' = 'failed', + updatedAt = 1_000, +) => + await t.mutation(async (ctx) => { + const now = Date.now(); + for (let i = 0; i < count; i += 1) { + const requestId = await ctx.db.insert('agentRequests', { + spoonId, + ownerId, + prompt: 'work', + status: 'running', + createdAt: now, + updatedAt: now, + }); + await ctx.db.insert('agentJobs', { + spoonId, + ownerId, + agentRequestId: requestId, + status, + prompt: 'work', + runtime: 'opencode', + baseBranch: 'main', + workBranch: `spoon/work-${i}`, + forkOwner: 'me', + forkRepo: 'repo-fork', + forkUrl: 'https://github.com/me/repo-fork', + upstreamOwner: 'upstream', + upstreamRepo: 'repo', + selectedSecretIds: [], + model: 'openai/gpt-5.1-codex', + reasoningEffort: 'medium', + createdAt: now, + updatedAt, + }); + } + }); + +describe('agentJobs.countOldWorkspaces bounded scan', () => { + test('caps the count at the 500-row scan bound', async () => { + const t = convexTest(schema, modules); + const ownerId = await createUser(t, 'owner@example.com'); + const spoonId = await seedSpoon(t, ownerId); + // Seed more deletable jobs than the bound. + await seedJobs(t, spoonId, ownerId, 505, 'failed'); + + const count = await authed(t, ownerId).query( + api.agentJobs.countOldWorkspaces, + {}, + ); + + // Bounded scan means the reported count is capped at 500 ("up to 500"). + expect(count).toBe(500); + }); + + test('returns the exact count when under the bound', async () => { + const t = convexTest(schema, modules); + const ownerId = await createUser(t, 'owner@example.com'); + const spoonId = await seedSpoon(t, ownerId); + await seedJobs(t, spoonId, ownerId, 3, 'failed'); + // Non-deletable jobs must not be counted. + await seedJobs(t, spoonId, ownerId, 4, 'running'); + + const count = await authed(t, ownerId).query( + api.agentJobs.countOldWorkspaces, + {}, + ); + + expect(count).toBe(3); + }); +}); + +describe('agentJobs.deleteOldWorkspaces bounded', () => { + test('deletes at most the per-call max, leaving the rest', async () => { + const t = convexTest(schema, modules); + const ownerId = await createUser(t, 'owner@example.com'); + const spoonId = await seedSpoon(t, ownerId); + await seedJobs(t, spoonId, ownerId, 120, 'failed'); + + const result = await authed(t, ownerId).mutation( + api.agentJobs.deleteOldWorkspaces, + { limit: 100 }, + ); + + expect(result.deleted).toBe(100); + + const remaining = await authed(t, ownerId).query( + api.agentJobs.countOldWorkspaces, + {}, + ); + expect(remaining).toBe(20); + }); + + test('deletes only deletable jobs and leaves active ones', async () => { + const t = convexTest(schema, modules); + const ownerId = await createUser(t, 'owner@example.com'); + const spoonId = await seedSpoon(t, ownerId); + await seedJobs(t, spoonId, ownerId, 5, 'failed'); + await seedJobs(t, spoonId, ownerId, 3, 'running'); + + const result = await authed(t, ownerId).mutation( + api.agentJobs.deleteOldWorkspaces, + { limit: 50 }, + ); + + expect(result.deleted).toBe(5); + + const stillActive = await authed(t, ownerId).query( + api.agentJobs.countOldWorkspaces, + {}, + ); + expect(stillActive).toBe(0); + }); +});