From 1e1435e656307be423c7bba7bd85bb949677f180 Mon Sep 17 00:00:00 2001 From: Gabriel Brown Date: Fri, 10 Jul 2026 17:24:15 -0400 Subject: [PATCH] fix(agent-jobs): recover failed claim decryption --- packages/backend/convex/agentJobs.ts | 107 +++++++---- packages/backend/convex/agentJobsNode.ts | 27 ++- .../tests/unit/claim-validation.test.ts | 170 +++++++++++++++--- 3 files changed, 238 insertions(+), 66 deletions(-) diff --git a/packages/backend/convex/agentJobs.ts b/packages/backend/convex/agentJobs.ts index 0572de4..46537b1 100644 --- a/packages/backend/convex/agentJobs.ts +++ b/packages/backend/convex/agentJobs.ts @@ -294,6 +294,40 @@ const deleteWorkspaceRows = async (ctx: MutationCtx, job: Doc<'agentJobs'>) => { await ctx.db.delete(job._id); }; +const failJobClaim = async ( + ctx: MutationCtx, + job: Doc<'agentJobs'>, + reason: string, +) => { + const failedAt = Date.now(); + await ctx.db.patch(job._id, { + status: 'failed', + error: reason, + completedAt: failedAt, + updatedAt: failedAt, + }); + await ctx.db.patch(job.agentRequestId, { + status: 'failed', + updatedAt: failedAt, + }); + if (job.threadId) { + await ctx.db.patch(job.threadId, { + status: 'failed', + updatedAt: failedAt, + resolvedAt: failedAt, + }); + } + await ctx.db.insert('agentJobEvents', { + jobId: job._id, + spoonId: job.spoonId, + ownerId: job.ownerId, + level: 'error', + phase: 'queued', + message: `Job could not be claimed: ${reason}`, + createdAt: failedAt, + }); +}; + const getAgentSettings = async (ctx: MutationCtx, spoon: Doc<'spoons'>) => { const settings = await ctx.db .query('spoonAgentSettings') @@ -1078,59 +1112,48 @@ export const claimNextInternal = internalMutation({ aiProviderProfile?.ownerId === job.ownerId && aiProviderProfile.enabled ? aiProviderProfile : null; - const failClaim = async (reason: string) => { - const failedAt = Date.now(); - await ctx.db.patch(job._id, { - status: 'failed', - error: reason, - completedAt: failedAt, - updatedAt: failedAt, - }); - await ctx.db.patch(job.agentRequestId, { - status: 'failed', - updatedAt: failedAt, - }); - if (job.threadId) { - await ctx.db.patch(job.threadId, { - status: 'failed', - updatedAt: failedAt, - resolvedAt: failedAt, - }); - } - await ctx.db.insert('agentJobEvents', { - jobId: job._id, - spoonId: job.spoonId, - ownerId: job.ownerId, - level: 'error', - phase: 'queued', - message: `Job could not be claimed: ${reason}`, - createdAt: failedAt, - }); - }; if (!spoon) { - await failClaim('The Spoon for this job no longer exists.'); + await failJobClaim( + ctx, + job, + 'The Spoon for this job no longer exists.', + ); return null; } if (!ownedProfile) { - await failClaim( + await failJobClaim( + ctx, + job, 'AI is not configured for this user. Add an AI provider in settings.', ); return null; } if (ownedProfile.authType !== 'none' && !ownedProfile.encryptedSecret) { - await failClaim('Selected AI provider is missing credentials.'); + await failJobClaim( + ctx, + job, + 'Selected AI provider is missing credentials.', + ); return null; } if (agentSettings && !agentSettings.enabled) { - await failClaim('Agent jobs are disabled for this Spoon.'); + await failJobClaim(ctx, job, 'Agent jobs are disabled for this Spoon.'); return null; } if ((job.runtime ?? 'opencode') !== 'opencode') { - await failClaim('Legacy OpenAI direct jobs are no longer supported.'); + await failJobClaim( + ctx, + job, + 'Legacy OpenAI direct jobs are no longer supported.', + ); return null; } if (secrets.length !== job.selectedSecretIds.length) { - await failClaim('Selected secrets must belong to this Spoon.'); + await failJobClaim( + ctx, + job, + 'Selected secrets must belong to this Spoon.', + ); return null; } const now = Date.now(); @@ -1164,6 +1187,20 @@ export const claimNextInternal = internalMutation({ }, }); +export const failClaimInternal = internalMutation({ + args: { + jobId: v.id('agentJobs'), + workerId: v.string(), + reason: v.string(), + }, + handler: async (ctx, { jobId, workerId, reason }) => { + const job = await ctx.db.get(jobId); + if (job?.status !== 'claimed' || job.claimedBy !== workerId) return null; + await failJobClaim(ctx, job, reason); + return null; + }, +}); + export const updateStatus = mutation({ args: { workerToken: v.string(), diff --git a/packages/backend/convex/agentJobsNode.ts b/packages/backend/convex/agentJobsNode.ts index 4c36a0a..6927c48 100644 --- a/packages/backend/convex/agentJobsNode.ts +++ b/packages/backend/convex/agentJobsNode.ts @@ -58,6 +58,24 @@ export const claimNextForWorker = action({ if (!claimed) return null; if (!claimed.spoon || !claimed.aiProviderProfile) return null; const profile = claimed.aiProviderProfile; + let profileSecret: string | undefined; + let secrets: { name: string; value: string }[]; + try { + profileSecret = profile.encryptedSecret + ? decryptSecret(profile.encryptedSecret) + : undefined; + secrets = claimed.secrets.map((secret: Doc<'spoonSecrets'>) => ({ + name: secret.name, + value: decryptSecret(secret.encryptedValue), + })); + } catch { + await ctx.runMutation(internal.agentJobs.failClaimInternal, { + jobId: claimed.job._id, + workerId: args.workerId, + reason: 'Stored AI credentials or Spoon secrets could not be decrypted.', + }); + return null; + } return { job: claimed.job, spoon: claimed.spoon, @@ -70,9 +88,7 @@ export const claimNextForWorker = action({ name: profile.name, provider: profile.provider, authType: profile.authType, - secret: profile.encryptedSecret - ? decryptSecret(profile.encryptedSecret) - : undefined, + secret: profileSecret, baseUrl: profile.baseUrl, model: profile.defaultModel, reasoningEffort: profile.reasoningEffort, @@ -84,10 +100,7 @@ export const claimNextForWorker = action({ claimed.spoon.githubInstallationId ?? process.env.GITHUB_APP_INSTALLATION_ID, }, - secrets: claimed.secrets.map((secret: Doc<'spoonSecrets'>) => ({ - name: secret.name, - value: decryptSecret(secret.encryptedValue), - })), + secrets, }; }, }); diff --git a/packages/backend/tests/unit/claim-validation.test.ts b/packages/backend/tests/unit/claim-validation.test.ts index 8d2f461..78470e0 100644 --- a/packages/backend/tests/unit/claim-validation.test.ts +++ b/packages/backend/tests/unit/claim-validation.test.ts @@ -1,17 +1,24 @@ import { convexTest } from 'convex-test'; -import { describe, expect, test } from 'vitest'; +import { beforeAll, describe, expect, test } from 'vitest'; import type { Id } from '../../convex/_generated/dataModel.js'; -import { internal } from '../../convex/_generated/api.js'; +import { api, internal } from '../../convex/_generated/api.js'; import schema from '../../convex/schema'; +import { encryptSecret } from '../../convex/secretCrypto'; const modules = import.meta.glob('../../convex/**/*.*s'); type SeedOptions = { - profile?: 'valid' | 'disabled' | 'missing-credentials' | 'none'; + profile?: + | 'valid' + | 'disabled' + | 'missing-credentials' + | 'encrypted-valid' + | 'encrypted-malformed' + | 'none'; settingsEnabled?: boolean; runtime?: 'opencode' | 'openai_direct'; - secret?: 'valid' | 'foreign'; + secret?: 'valid' | 'foreign' | 'malformed'; }; const seedQueuedJob = async ( @@ -66,7 +73,16 @@ const seedQueuedJob = async ( name: 'Test provider', provider: 'openai', authType: - profileKind === 'missing-credentials' ? 'api_key' : 'none', + profileKind === 'missing-credentials' || + profileKind.startsWith('encrypted-') + ? 'api_key' + : 'none', + encryptedSecret: + profileKind === 'encrypted-valid' + ? encryptSecret('provider-secret') + : profileKind === 'encrypted-malformed' + ? 'malformed' + : undefined, defaultModel: 'gpt-5.1-codex', modelOptions: ['gpt-5.1-codex'], reasoningEffort: 'medium', @@ -88,7 +104,10 @@ const seedQueuedJob = async ( spoonId, ownerId: secretOwnerId, name: 'TOKEN', - encryptedValue: 'encrypted', + encryptedValue: + options.secret === 'malformed' + ? 'malformed' + : encryptSecret('secret-value'), createdAt: now, updatedAt: now, }), @@ -102,10 +121,22 @@ const seedQueuedJob = async ( createdAt: now, updatedAt: now, }); + const threadId = await ctx.db.insert('threads', { + ownerId, + spoonId, + title: 'Fix the editor', + source: 'user_request', + status: 'queued', + priority: 'normal', + relatedAgentRequestId: requestId, + createdAt: now, + updatedAt: now, + }); const jobId = await ctx.db.insert('agentJobs', { spoonId, ownerId, agentRequestId: requestId, + threadId, status: 'queued', prompt: 'Fix the editor', runtime: options.runtime ?? 'opencode', @@ -125,28 +156,64 @@ const seedQueuedJob = async ( updatedAt: now, }); await ctx.db.patch(requestId, { agentJobId: jobId }); - return { jobId, requestId, spoonId }; + await ctx.db.patch(threadId, { latestAgentJobId: jobId }); + return { jobId, requestId, spoonId, threadId }; }); +const expectFailedClaimState = async ( + t: ReturnType, + jobId: Id<'agentJobs'>, + requestId: Id<'agentRequests'>, + threadId: Id<'threads'>, +) => { + const state = await t.run(async (ctx) => ({ + job: await ctx.db.get(jobId), + request: await ctx.db.get(requestId), + thread: await ctx.db.get(threadId), + events: (await ctx.db.query('agentJobEvents').collect()).filter( + (event) => event.jobId === jobId, + ), + })); + expect(state.job?.status).toBe('failed'); + expect(state.job?.error).toBeTruthy(); + expect(state.request?.status).toBe('failed'); + expect(state.thread?.status).toBe('failed'); + expect(state.thread?.resolvedAt).toBeTypeOf('number'); + expect(state.events).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + level: 'error', + phase: 'queued', + message: expect.stringContaining('Job could not be claimed:'), + }), + ]), + ); +}; + const expectFailedClaim = async ( t: ReturnType, jobId: Id<'agentJobs'>, + requestId: Id<'agentRequests'>, + threadId: Id<'threads'>, ) => { const result = await t.mutation(internal.agentJobs.claimNextInternal, { workerId: 'w1', }); expect(result).toBeNull(); - const job = await t.run(async (ctx) => await ctx.db.get(jobId)); - expect(job?.status).toBe('failed'); - expect(job?.error).toBeTruthy(); + await expectFailedClaimState(t, jobId, requestId, threadId); }; +beforeAll(() => { + process.env.SPOON_WORKER_TOKEN = 'test-worker-token'; + process.env.SPOON_ENCRYPTION_KEY = 'test-encryption-key'; +}); + describe('claimNextInternal validation', () => { test('fails a queued job when its owner has no configured profile', async () => { const t = convexTest(schema, modules); - const { jobId } = await seedQueuedJob(t); + const { jobId, requestId, threadId } = await seedQueuedJob(t); - await expectFailedClaim(t, jobId); + await expectFailedClaim(t, jobId, requestId, threadId); }); test('claims a fully valid queued job', async () => { @@ -165,55 +232,110 @@ describe('claimNextInternal validation', () => { test('fails a queued job when its profile is disabled', async () => { const t = convexTest(schema, modules); - const { jobId } = await seedQueuedJob(t, { profile: 'disabled' }); + const { jobId, requestId, threadId } = await seedQueuedJob(t, { + profile: 'disabled', + }); - await expectFailedClaim(t, jobId); + await expectFailedClaim(t, jobId, requestId, threadId); }); test('fails a queued job when its profile is missing credentials', async () => { const t = convexTest(schema, modules); - const { jobId } = await seedQueuedJob(t, { + const { jobId, requestId, threadId } = await seedQueuedJob(t, { profile: 'missing-credentials', }); - await expectFailedClaim(t, jobId); + await expectFailedClaim(t, jobId, requestId, threadId); }); test('fails a queued job when Spoon agent jobs are disabled', async () => { const t = convexTest(schema, modules); - const { jobId } = await seedQueuedJob(t, { + const { jobId, requestId, threadId } = await seedQueuedJob(t, { profile: 'valid', settingsEnabled: false, }); - await expectFailedClaim(t, jobId); + await expectFailedClaim(t, jobId, requestId, threadId); }); test('fails a queued job with an unsupported legacy runtime', async () => { const t = convexTest(schema, modules); - const { jobId } = await seedQueuedJob(t, { + const { jobId, requestId, threadId } = await seedQueuedJob(t, { profile: 'valid', runtime: 'openai_direct', }); - await expectFailedClaim(t, jobId); + await expectFailedClaim(t, jobId, requestId, threadId); }); test('fails a queued job with a secret owned by another user', async () => { const t = convexTest(schema, modules); - const { jobId } = await seedQueuedJob(t, { + const { jobId, requestId, threadId } = await seedQueuedJob(t, { profile: 'valid', secret: 'foreign', }); - await expectFailedClaim(t, jobId); + await expectFailedClaim(t, jobId, requestId, threadId); }); test('fails a queued job whose Spoon was deleted', async () => { const t = convexTest(schema, modules); - const { jobId, spoonId } = await seedQueuedJob(t, { profile: 'valid' }); + const { jobId, requestId, spoonId, threadId } = await seedQueuedJob(t, { + profile: 'valid', + }); await t.mutation(async (ctx) => await ctx.db.delete(spoonId)); - await expectFailedClaim(t, jobId); + await expectFailedClaim(t, jobId, requestId, threadId); + }); + + test('fails a claim when provider credential decryption fails', async () => { + const t = convexTest(schema, modules); + const { jobId, requestId, threadId } = await seedQueuedJob(t, { + profile: 'encrypted-malformed', + }); + + const result = await t.action(api.agentJobsNode.claimNextForWorker, { + workerId: 'w1', + workerToken: 'test-worker-token', + }); + + expect(result).toBeNull(); + await expectFailedClaimState(t, jobId, requestId, threadId); + }); + + test('fails a claim when selected secret decryption fails', async () => { + const t = convexTest(schema, modules); + const { jobId, requestId, threadId } = await seedQueuedJob(t, { + profile: 'valid', + secret: 'malformed', + }); + + const result = await t.action(api.agentJobsNode.claimNextForWorker, { + workerId: 'w1', + workerToken: 'test-worker-token', + }); + + expect(result).toBeNull(); + await expectFailedClaimState(t, jobId, requestId, threadId); + }); + + test('returns decrypted provider credentials and selected secrets', async () => { + const t = convexTest(schema, modules); + const { jobId } = await seedQueuedJob(t, { + profile: 'encrypted-valid', + secret: 'valid', + }); + + const result = await t.action(api.agentJobsNode.claimNextForWorker, { + workerId: 'w1', + workerToken: 'test-worker-token', + }); + const job = await t.run(async (ctx) => await ctx.db.get(jobId)); + + expect(result?.aiProviderProfile?.secret).toBe('provider-secret'); + expect(result?.secrets).toEqual([ + { name: 'TOKEN', value: 'secret-value' }, + ]); + expect(job?.status).toBe('claimed'); }); });