From 8af21c2add6f4083fb2357cac6021022b11936ff Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Wed, 1 Apr 2026 07:48:01 +0900 Subject: [PATCH 1/4] fix: resolve cert exam completion when Claude sends module ID instead of UUID Claude sometimes passes the module ID (e.g. "S1") as attempt_id instead of the attempt UUID, causing a Postgres type error. The tool now detects non-UUID input and resolves the active attempt via getActiveAttempt. Also improves error notification debugging: includes tool input parameters and web user display name in Slack alerts. Co-Authored-By: Claude Opus 4.6 (1M context) --- .changeset/common-kiwis-hammer.md | 2 ++ server/src/addie/claude-client.ts | 10 ++++--- server/src/addie/error-notifier.ts | 15 ++++++++++- server/src/addie/mcp/certification-tools.ts | 29 +++++++++++++++++---- server/src/routes/addie-chat.ts | 2 ++ server/tests/unit/error-notifier.test.ts | 18 +++++++++++++ 6 files changed, 66 insertions(+), 10 deletions(-) create mode 100644 .changeset/common-kiwis-hammer.md diff --git a/.changeset/common-kiwis-hammer.md b/.changeset/common-kiwis-hammer.md new file mode 100644 index 0000000000..a845151cc8 --- /dev/null +++ b/.changeset/common-kiwis-hammer.md @@ -0,0 +1,2 @@ +--- +--- diff --git a/server/src/addie/claude-client.ts b/server/src/addie/claude-client.ts index 7fb6fac78e..66bf1870d5 100644 --- a/server/src/addie/claude-client.ts +++ b/server/src/addie/claude-client.ts @@ -227,6 +227,8 @@ export interface ProcessMessageOptions { maxMessages?: number; /** Slack user ID — used for error notifications so admins know who was affected */ slackUserId?: string; + /** Fallback display name for error notifications when slackUserId is unavailable (e.g. web chat) */ + userDisplayName?: string; /** Thread ID — used for error notification links to admin view */ threadId?: string; } @@ -966,7 +968,7 @@ export class AddieClaudeClient { result.includes('need to be logged in'); if (looksLikeError) { logger.warn({ toolName, toolInput, result: result.substring(0, 500), durationMs }, 'Addie: Tool returned error result'); - notifyToolError({ toolName, errorMessage: result.substring(0, 500), slackUserId: options?.slackUserId, threadId: options?.threadId, threw: false }); + notifyToolError({ toolName, errorMessage: result.substring(0, 500), toolInput, slackUserId: options?.slackUserId, userDisplayName: options?.userDisplayName, threadId: options?.threadId, threw: false }); } toolResults.push({ tool_use_id: toolUseId, content: result }); toolExecutions.push({ @@ -983,7 +985,7 @@ export class AddieClaudeClient { const durationMs = Date.now() - startTime; const errorMessage = error instanceof Error ? error.message : 'Unknown error'; logger.error({ toolName, toolInput, error: errorMessage, durationMs }, 'Addie: Tool threw exception'); - notifyToolError({ toolName, errorMessage, slackUserId: options?.slackUserId, threadId: options?.threadId, threw: true }); + notifyToolError({ toolName, errorMessage, toolInput, slackUserId: options?.slackUserId, userDisplayName: options?.userDisplayName, threadId: options?.threadId, threw: true }); toolResults.push({ tool_use_id: toolUseId, content: `Error: ${errorMessage}`, @@ -1441,7 +1443,7 @@ export class AddieClaudeClient { result.includes('need to be logged in'); if (looksLikeError) { logger.warn({ toolName, toolInput, result: result.substring(0, 500), durationMs }, 'Addie Stream: Tool returned error result'); - notifyToolError({ toolName, errorMessage: result.substring(0, 500), slackUserId: options?.slackUserId, threadId: options?.threadId, threw: false }); + notifyToolError({ toolName, errorMessage: result.substring(0, 500), toolInput, slackUserId: options?.slackUserId, userDisplayName: options?.userDisplayName, threadId: options?.threadId, threw: false }); } toolResults.push({ tool_use_id: toolUseId, content: result }); toolExecutions.push({ @@ -1460,7 +1462,7 @@ export class AddieClaudeClient { const errorMessage = error instanceof Error ? error.message : 'Unknown error'; const errorResult = `Error: ${errorMessage}`; logger.error({ toolName, toolInput, error: errorMessage, durationMs }, 'Addie Stream: Tool threw exception'); - notifyToolError({ toolName, errorMessage, slackUserId: options?.slackUserId, threadId: options?.threadId, threw: true }); + notifyToolError({ toolName, errorMessage, toolInput, slackUserId: options?.slackUserId, userDisplayName: options?.userDisplayName, threadId: options?.threadId, threw: true }); toolResults.push({ tool_use_id: toolUseId, content: errorResult, diff --git a/server/src/addie/error-notifier.ts b/server/src/addie/error-notifier.ts index 0a8bca37e5..f80ea24af6 100644 --- a/server/src/addie/error-notifier.ts +++ b/server/src/addie/error-notifier.ts @@ -39,10 +39,14 @@ interface ToolErrorContext { errorMessage: string; /** Slack user ID of the person who triggered the conversation */ slackUserId?: string; + /** Fallback display name when slackUserId is unavailable (e.g. web chat users) */ + userDisplayName?: string; /** Addie thread ID for linking to admin view */ threadId?: string; /** Whether the tool threw (true) vs returned an error string (false) */ threw: boolean; + /** Tool input parameters — included in notification for debugging */ + toolInput?: Record; } /** @@ -91,17 +95,26 @@ async function _postToolError(ctx: ToolErrorContext): Promise { recentErrors.set(throttleKey, now); - const userLine = ctx.slackUserId ? `*User:* <@${ctx.slackUserId}>` : '*User:* unknown'; + const userLine = ctx.slackUserId + ? `*User:* <@${ctx.slackUserId}>` + : ctx.userDisplayName + ? `*User:* ${ctx.userDisplayName.replace(/[<>&*_~`]/g, '')} (web)` + : '*User:* unknown'; const threadLine = ctx.threadId ? `` : ''; const kind = ctx.threw ? 'Tool exception' : 'Tool error'; + const inputLine = ctx.toolInput + ? `*Input:* \`${JSON.stringify(ctx.toolInput).substring(0, 300)}\`` + : ''; + const lines = [ `:warning: *${kind}: ${ctx.toolName}*`, '', `> ${ctx.errorMessage.substring(0, 500)}`, '', + inputLine, userLine, threadLine, ].filter(Boolean); diff --git a/server/src/addie/mcp/certification-tools.ts b/server/src/addie/mcp/certification-tools.ts index 930662cc45..e6e11886b5 100644 --- a/server/src/addie/mcp/certification-tools.ts +++ b/server/src/addie/mcp/certification-tools.ts @@ -1727,7 +1727,7 @@ export function createCertificationToolHandlers( // Start the module and create an attempt await certDb.startModule(userId, moduleId); - const attempt = await certDb.createAttempt(userId, mod.track_id, undefined, moduleId); + const attempt = await certDb.createAttempt(userId, mod.track_id, options?.threadId, moduleId); const criteria = mod.assessment_criteria as certDb.AssessmentCriteria | null; const lessonPlan = mod.lesson_plan as certDb.LessonPlan | null; @@ -1840,7 +1840,19 @@ export function createCertificationToolHandlers( if (!attemptId || !scores || typeof scores !== 'object') return 'attempt_id and scores are required.'; - const attempt = await certDb.getAttempt(attemptId); + // Look up the active attempt: accept the UUID directly, or resolve from module ID + const isUuid = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i.test(attemptId); + let attempt: certDb.CertificationAttempt | null; + if (isUuid) { + attempt = await certDb.getAttempt(attemptId); + } else { + // Claude sometimes sends the module ID instead of the attempt UUID + const trackPrefix = attemptId.replace(/[0-9]+$/, '').toUpperCase(); + attempt = await certDb.getActiveAttempt(userId, trackPrefix); + if (attempt && attempt.module_id?.toUpperCase() !== attemptId.toUpperCase()) { + attempt = null; + } + } if (!attempt) return 'Exam attempt not found.'; if (attempt.workos_user_id !== userId) return 'This exam attempt belongs to a different user.'; if (attempt.status !== 'in_progress') return 'This exam attempt is already completed.'; @@ -1908,7 +1920,10 @@ export function createCertificationToolHandlers( const allAboveThreshold = Object.values(scores).every(s => s >= 70); const passing = allAboveThreshold && overallScore >= 70; - await certDb.completeAttempt(attemptId, scores, overallScore, passing); + await certDb.completeAttempt(attempt.id, scores, overallScore, passing); + + // --- From this point the attempt is recorded. Failures below must not --- + // --- surface as "Failed to record capstone results" to the learner. --- const lines: string[] = []; @@ -1918,8 +1933,12 @@ export function createCertificationToolHandlers( lines.push('Congratulate them warmly — they earned this. Do NOT share any scores or percentages.'); // Mark the capstone module as completed - if (capstoneMod) { - await certDb.completeModule(userId, capstoneMod.id, scores); + try { + if (capstoneMod) { + await certDb.completeModule(userId, capstoneMod.id, scores); + } + } catch (modError) { + logger.error({ error: modError, userId, moduleId: capstoneMod?.id }, 'Failed to record module completion after attempt passed'); } // Auto-award credentials (including specialist) diff --git a/server/src/routes/addie-chat.ts b/server/src/routes/addie-chat.ts index 003a7658ff..3b0140c6ed 100644 --- a/server/src/routes/addie-chat.ts +++ b/server/src/routes/addie-chat.ts @@ -758,6 +758,7 @@ export function createAddieChatRouter(): { pageRouter: Router; apiRouter: Router requestContext, maxMessages: hasCertificationContext ? 50 : undefined, threadId: thread.thread_id, + userDisplayName: displayName || undefined, }); } catch (error) { // Provide user-friendly error message based on error type @@ -1009,6 +1010,7 @@ export function createAddieChatRouter(): { pageRouter: Router; apiRouter: Router requestContext, maxMessages: hasCertCtx ? 50 : undefined, threadId: thread.thread_id, + userDisplayName: displayName || undefined, })) { // Break early if client disconnected (still save partial response below) if (connectionClosed) { diff --git a/server/tests/unit/error-notifier.test.ts b/server/tests/unit/error-notifier.test.ts index f4f267291a..f200b65a8a 100644 --- a/server/tests/unit/error-notifier.test.ts +++ b/server/tests/unit/error-notifier.test.ts @@ -58,6 +58,24 @@ describe('error-notifier', () => { expect(text).toContain('<@U999>'); expect(text).toContain('thread-abc'); }); + + it('includes tool input and web user display name', async () => { + const name = uniqueName('tool_input'); + notifyToolError({ + toolName: name, + errorMessage: 'invalid input syntax for type uuid', + toolInput: { attempt_id: 'S1', scores: { mastery: 80 } }, + userDisplayName: 'Bryan', + threadId: 'thread-xyz', + threw: true, + }); + + await vi.waitFor(() => expect(mockSendChannelMessage).toHaveBeenCalled()); + + const text = mockSendChannelMessage.mock.calls[0][1].text; + expect(text).toContain('"attempt_id":"S1"'); + expect(text).toContain('Bryan (web)'); + }); }); describe('notifySystemError', () => { From c15b987822e2fa3975884565ed3022f0fd697fe7 Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Wed, 1 Apr 2026 09:03:05 +0900 Subject: [PATCH 2/4] fix: resolve cert exam completion when Claude sends module ID instead of UUID When Claude sends a module ID (e.g. "S1") as the attempt_id instead of the actual UUID, the Postgres query fails with "invalid input syntax for type uuid". Now accepts both formats by detecting non-UUID input and resolving to the user's active attempt for that track/module. Also improves error notifications: includes tool input and user display name in Slack alerts, redacts sensitive fields, adds structured logging for the fallback path, and guards against completeModule failures blocking the success response. Co-Authored-By: Claude Opus 4.6 (1M context) --- .changeset/hungry-steaks-reply.md | 2 ++ server/src/addie/error-notifier.ts | 5 ++++- server/src/addie/mcp/certification-tools.ts | 1 + server/tests/unit/error-notifier.test.ts | 18 ++++++++++++++++++ 4 files changed, 25 insertions(+), 1 deletion(-) create mode 100644 .changeset/hungry-steaks-reply.md diff --git a/.changeset/hungry-steaks-reply.md b/.changeset/hungry-steaks-reply.md new file mode 100644 index 0000000000..a845151cc8 --- /dev/null +++ b/.changeset/hungry-steaks-reply.md @@ -0,0 +1,2 @@ +--- +--- diff --git a/server/src/addie/error-notifier.ts b/server/src/addie/error-notifier.ts index f80ea24af6..dc6820c0f5 100644 --- a/server/src/addie/error-notifier.ts +++ b/server/src/addie/error-notifier.ts @@ -105,8 +105,11 @@ async function _postToolError(ctx: ToolErrorContext): Promise { : ''; const kind = ctx.threw ? 'Tool exception' : 'Tool error'; + const sensitiveKeys = new Set(['password', 'token', 'secret', 'api_key']); const inputLine = ctx.toolInput - ? `*Input:* \`${JSON.stringify(ctx.toolInput).substring(0, 300)}\`` + ? `*Input:* \`${JSON.stringify(ctx.toolInput, (key, val) => + sensitiveKeys.has(key) ? '[redacted]' : val + ).substring(0, 300)}\`` : ''; const lines = [ diff --git a/server/src/addie/mcp/certification-tools.ts b/server/src/addie/mcp/certification-tools.ts index e6e11886b5..920fd06da3 100644 --- a/server/src/addie/mcp/certification-tools.ts +++ b/server/src/addie/mcp/certification-tools.ts @@ -1847,6 +1847,7 @@ export function createCertificationToolHandlers( attempt = await certDb.getAttempt(attemptId); } else { // Claude sometimes sends the module ID instead of the attempt UUID + logger.warn({ attemptId, userId }, 'complete_certification_exam received module ID instead of UUID, resolving'); const trackPrefix = attemptId.replace(/[0-9]+$/, '').toUpperCase(); attempt = await certDb.getActiveAttempt(userId, trackPrefix); if (attempt && attempt.module_id?.toUpperCase() !== attemptId.toUpperCase()) { diff --git a/server/tests/unit/error-notifier.test.ts b/server/tests/unit/error-notifier.test.ts index f200b65a8a..a23fea592e 100644 --- a/server/tests/unit/error-notifier.test.ts +++ b/server/tests/unit/error-notifier.test.ts @@ -59,6 +59,24 @@ describe('error-notifier', () => { expect(text).toContain('thread-abc'); }); + it('sanitizes display names containing Slack formatting characters', async () => { + const name = uniqueName('tool_sanitize'); + notifyToolError({ + toolName: name, + errorMessage: 'fail', + userDisplayName: '