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/.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/claude-client.ts b/server/src/addie/claude-client.ts index ebc672e8ae..1edd68e397 100644 --- a/server/src/addie/claude-client.ts +++ b/server/src/addie/claude-client.ts @@ -228,6 +228,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; } @@ -980,7 +982,7 @@ export class AddieClaudeClient { logger.warn({ toolName, toolInput, error: errorMessage, durationMs }, 'Addie: Tool returned expected error'); } else { logger.error({ toolName, toolInput, error: errorMessage, durationMs }, 'Addie: Tool threw unexpected 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, @@ -1454,7 +1456,7 @@ export class AddieClaudeClient { logger.warn({ toolName, toolInput, error: errorMessage, durationMs }, 'Addie Stream: Tool returned expected error'); } else { logger.error({ toolName, toolInput, error: errorMessage, durationMs }, 'Addie Stream: Tool threw unexpected 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, diff --git a/server/src/addie/error-notifier.ts b/server/src/addie/error-notifier.ts index 0a8bca37e5..3068b5ef32 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,32 @@ 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 sensitivePattern = /password|token|secret|key|auth|credential/i; + const inputLine = ctx.toolInput + ? `*Input:* \`${(() => { + const raw = JSON.stringify(ctx.toolInput, (key, val) => + key && sensitivePattern.test(key) ? '[redacted]' : val + ); + return raw.length > 300 ? raw.substring(0, 300) + '...' : raw; + })()}\`` + : ''; + 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 7418f5676d..ef7305c239 100644 --- a/server/src/addie/mcp/certification-tools.ts +++ b/server/src/addie/mcp/certification-tools.ts @@ -1728,7 +1728,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; @@ -1841,7 +1841,24 @@ 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 + logger.warn({ attemptId, userId }, 'complete_certification_exam received module ID instead of UUID, resolving'); + const trackPrefix = attemptId.replace(/[0-9]+$/, '').toUpperCase(); + if (!/^[A-Z]{1,2}$/.test(trackPrefix)) { + return `Invalid attempt_id "${attemptId}". Provide the UUID returned by start_certification_exam.`; + } + attempt = await certDb.getActiveAttempt(userId, trackPrefix); + if (attempt && attempt.module_id?.toUpperCase() !== attemptId.toUpperCase()) { + logger.warn({ attemptId, attemptModuleId: attempt.module_id }, 'Active attempt module_id does not match, discarding'); + 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.'; @@ -1909,7 +1926,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[] = []; @@ -1919,8 +1939,10 @@ 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) { + try { 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..97078fd13d 100644 --- a/server/tests/unit/error-notifier.test.ts +++ b/server/tests/unit/error-notifier.test.ts @@ -58,6 +58,59 @@ describe('error-notifier', () => { expect(text).toContain('<@U999>'); 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: '