From 4e83a43f55ad7a499a619c8c2ae6ec18b44d787b Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Wed, 23 Sep 2026 01:44:02 +0000 Subject: [PATCH 1/4] fix(addie): enforce persisted outcomes and clarify recovery telemetry --- docs/runbooks/addie-gemini-direct.md | 25 ++- docs/runbooks/addie-reliability-audit.md | 88 ++++++++ server/src/addie/claude-client.ts | 24 ++- server/src/addie/config-version.ts | 2 +- server/src/addie/gemini-direct-experiment.ts | 19 +- .../tool-surface-inventory.generated.json | 10 +- server/src/addie/mcp/certification-tools.ts | 3 +- server/src/addie/mcp/escalation-tools.ts | 59 ++++-- server/src/addie/mcp/member-tools.ts | 47 +++++ .../model-providers/tool-orchestration.ts | 45 ++++ server/src/addie/outcome-claims.ts | 196 ++++++++++++++++++ server/src/addie/rules/behaviors.md | 16 +- server/src/addie/rules/constraints.md | 4 +- .../607_addie_tool_error_containment.sql | 17 ++ .../certification-outcome-claims.test.ts | 98 +++++++++ .../unit/addie/escalation-receipts.test.ts | 103 +++++++++ .../addie/gemini-direct-experiment.test.ts | 50 ++++- .../model-provider-tool-orchestration.test.ts | 7 +- .../unit/addie/support-outcome-claims.test.ts | 94 +++++++++ .../unit/addie/tool-error-containment.test.ts | 101 +++++++++ .../certification-attempt-recovery.test.ts | 33 +++ .../unit/claude-client-replay-policy.test.ts | 72 +++++++ tests/addie/member-tools.test.ts | 76 +++++++ 23 files changed, 1140 insertions(+), 49 deletions(-) create mode 100644 docs/runbooks/addie-reliability-audit.md create mode 100644 server/src/addie/outcome-claims.ts create mode 100644 server/src/db/migrations/607_addie_tool_error_containment.sql create mode 100644 server/tests/unit/addie/certification-outcome-claims.test.ts create mode 100644 server/tests/unit/addie/escalation-receipts.test.ts create mode 100644 server/tests/unit/addie/support-outcome-claims.test.ts create mode 100644 server/tests/unit/addie/tool-error-containment.test.ts diff --git a/docs/runbooks/addie-gemini-direct.md b/docs/runbooks/addie-gemini-direct.md index 15949c0738e..9f6b7acc3e2 100644 --- a/docs/runbooks/addie-gemini-direct.md +++ b/docs/runbooks/addie-gemini-direct.md @@ -237,8 +237,29 @@ checks remain synchronous and fail closed. SSE clients receive a content-free `call_adcp_task` failures now carry structured status, safe code/category, retryability, operation, attempt count, and same-turn recovery state through -tool checkpoints. `recovered_tool_errors` and `unrecovered_tool_errors` should -be compared separately; do not parse Markdown result prefixes. A typed +tool checkpoints. `recovered_tool_errors` requires later success with the exact +operation, agent URL, and idempotency key. `unrecovered_tool_errors` retains its +historical meaning: every error without that exact recovery. + +The admin results also split classified unrecovered errors into +`contained_tool_errors` and `unresolved_tool_errors`. Containment means a +validation rejection was followed by a successful AdCP read from the same +literal agent URL and a complete, unflagged generated answer. For example, +unsupported `request_proposals` followed by successful product discovery can +be contained without claiming proposals were created. A corrected +`get_products` call with a changed key can be contained but is not exact +recovery. Unrelated targets, arbitrary tools, mutation successes, transport or +authentication failures, and text-only answers do not establish containment. +This is observed workflow continuation, not proof of fulfillment or answer +quality; consult delivery outcomes separately. `unresolved_tool_errors` means +neither form of evidence was observed. Historical/incomplete turns retain +NULL containment; their errors without exact recovery are reported separately +as `tool_recovery_unclassified_errors` and excluded from the contained/unresolved +split. The four categories (exact, contained, unresolved, unclassified) do not +overlap: a historical turn with two errors and one exact recovery has one +unclassified error. This uses total errors minus exact recoveries, since rows +predating recovery telemetry have default-zero recovery counters. No historical +backfill is inferred from answer prose. Do not parse Markdown result prefixes. A typed `call_adcp_get_products` read surface makes the adapter's stable idempotency-key and buying-mode contract machine-checkable without acquiring a mutation reservation. One automatic retry is allowed only for typed transient diff --git a/docs/runbooks/addie-reliability-audit.md b/docs/runbooks/addie-reliability-audit.md new file mode 100644 index 00000000000..39442729bad --- /dev/null +++ b/docs/runbooks/addie-reliability-audit.md @@ -0,0 +1,88 @@ +# Addie reliability audit follow-up + +This change starts at `0f4512093` on main, after #7649. No production queries, +writes, or synthetic evaluations were used to implement or validate it. + +## Existing fixes + +- #7641 added exact AdCP recovery keyed by operation, literal endpoint, and + idempotency key. A corrected call with the same identity already recovers; + a different operation or key must not be reported as that operation succeeding. + The audit did not include request tuples, so it does not establish why the + corrected production call failed to match. +- #7623 reconciles durable certification completion outcomes. It does not stop + later model prose from claiming completion after a rejected attempt. +- #7649 provides durable evaluation ownership, fenced publication, and observer + receipts. Owned admissions and coalesced observations previously lacked + distinct lifecycle logs. +- #7650, which landed after #7649, repairs secure organization creation. It does + not address Addie's unsupported escalation promises to anonymous users. + +## Response truthfulness + +The common terminal response pipeline now checks certification outcomes and +support promises before streaming or non-streaming delivery. Teaching history +identifies context only; neither user assertions nor prior assistant statements +are proof of completion. Current successful certification tool receipts or a +current `get_learner_progress` read provide authoritative evidence. The parser +accepts anchored application-owned receipt lines, not arbitrary retrieved text. +Capstone module receipts are emitted only after the module write/reconciliation +succeeds, independently of the passed attempt. + +Recognized completion, mastery, and credential claims are rendered from exact +recorded outcomes or replaced with a brief unconfirmed-state explanation. Other +teaching sentences remain available. Credential award and external issuance are +separate claims. A later exchange alone never makes a rejected completion valid. +A historical completion can be confirmed by reading persisted progress again. + +This is a deterministic delivery backstop for the covered English outcome +phrasing, not a semantic proof for every paraphrase or language. Expanding the +phrase set should include both false-claim and teaching-flow regressions. Tool +receipt format changes must update these tests and the parser together. + +Support claims require an `escalate_to_admin` receipt containing the persisted +request ID. Notification claims additionally require successful notification. +Ancillary thread-flag or notification failures preserve the saved request +receipt. Anonymous sessions cannot create escalation rows, even if they supply +contact details. Guest registration complaints receive the public +`support@agenticadvertising.org` route without soliciting unusable contact data. + +## Telemetry and evaluation observations + +See [Gemini direct](./addie-gemini-direct.md) for exact, contained, unresolved, +and historically unclassified error definitions. Containment records successful +same-agent continuation after a validation rejection; it is not proof of task +fulfillment, original-operation success, or delivery. + +The content-free `agent_quality_evaluation` lifecycle event records admission +(`owned` or `coalesced`, including expired-lease recovery), committed canonical +or audit-only publication, and completion/failure/lease-loss/unconfirmed outcomes. +No URLs, prompts, contact details, credential identities, or lease tokens are +added to these events. Coalescing remains unproven organically until genuine +production overlap occurs; that observation does not justify synthetic traffic. + +## Deferred isolated outliers + +The shadow warning is a best-effort stale-recovery catch in +`jobs/shadow-evaluator.ts`. Cleanup is retried at subsequent scheduled +invocations. Heartbeat expiry defaults to 20 minutes and is clamped to 15–60 +minutes, beyond the ten-minute provider timeout. Terminal writes and subsequent +paid dispatches check ownership. Code and tests do not establish a stale worker +publication defect; changing leases based on one warning would be speculative. + +The persistence duration surrounds `threadService.addMessage`; it does not +measure provider time or browser receipt. Per-thread lock and statement timeouts +are bounded at five and ten seconds, with five-second pool acquisition. The +assistant insert and turn finalization share a transaction, and later route +errors preserve an already-completed turn. Without the outlier's trace, timing, +and contention evidence, there is no scoped defect to repair here. + +## Deployment and rollback + +Migration 607 adds one nullable containment count and consistency constraint. +Apply migrations before the new application version (normal startup ordering). +Historical rows remain unclassified; there is no speculative backfill. Rollback +can restore the prior application while retaining the additive column. Do not +drop it during an application rollback. Old writers leave new containment +values null. Response enforcement and lifecycle logging need no other schema +change, tool-surface expansion, model-provider change, or protocol changeset. diff --git a/server/src/addie/claude-client.ts b/server/src/addie/claude-client.ts index c3d08b77cb4..33a6bd875f5 100644 --- a/server/src/addie/claude-client.ts +++ b/server/src/addie/claude-client.ts @@ -5,6 +5,7 @@ * with tool reference always appended from code. */ +import { enforceOutcomeClaims } from './outcome-claims.js'; import Anthropic from '@anthropic-ai/sdk'; import { createHash, createHmac } from 'node:crypto'; import { createLogger } from '../logger.js'; @@ -334,9 +335,9 @@ export const HALLUCINATION_PATTERNS: ReadonlyArray<{ pattern: RegExp; expectedTo { pattern: /(?:I'?ve\s+|I\s+)?(?:created|generated|sent)\s+(?:a\s+)?payment\s+link/i, expectedTools: ['create_payment_link'] }, { pattern: /(?:I'?ve\s+|I\s+)?(?:sent|delivered)\s+(?:a\s+)?(?:DM|direct message|notification)/i, expectedTools: ['send_member_dm', 'resolve_escalation'] }, { pattern: /(?:I'?ve\s+|I\s+)?added\s+\S+(?:\s+\S+){0,5}\s+to\s+the\s+(?:meeting|call|series)/i, expectedTools: ['add_meeting_attendee'] }, - // Fake-escalation patterns. `escalate_to_admin` is in the always-available - // tool set, so claiming an escalation/notification was made without firing - // it is the same class of fabrication as the rest. Real GitHub-issue tools + // Fake-escalation monitoring complements the receipt-bound delivery guard. + // Escalation availability depends on the authenticated surface. Claiming + // escalation without a successful receipt is fabrication. Real GitHub-issue tools // count too because Addie sometimes describes filing a ticket as creating // an issue. // @@ -498,6 +499,7 @@ function finalizeAssistantText( githubIssueRetryReceipts: readonly GithubIssueRetryReceipt[] = [], clientRequestId: string | undefined, forceTruncation: boolean = false, + conversationContext: string = question, ): FinalizedAssistantText { // This list is supplied only by the same client-request retry path. const retryExecutions = rehydratedGithubIssueRetryExecutions(githubIssueRetryReceipts, clientRequestId); @@ -522,9 +524,13 @@ function finalizeAssistantText( 'Addie: Replaced unsupported provider prose after failed source lookups', ); } + const outcome = enforceOutcomeClaims(evidenceBoundary.text, toolExecutions, conversationContext); + if (outcome.reason) { + logger.warn({ event: 'addie_unconfirmed_outcome_replaced', reason: outcome.reason }, 'Addie: Replaced unsupported outcome claim'); + } const processed = applyResponsePipelineWithEmptyMonitoring( question, - evidenceBoundary.text, + outcome.text, toolExecutions, ); const lengthExceeded = processed.text.length > MAX_OUTPUT_LENGTH; @@ -532,7 +538,7 @@ function finalizeAssistantText( return { text: truncated ? formatTruncatedOutput(processed.text) : processed.text, emptyReason: processed.reason, - localReplacementReason: githubIssueOutcome.reason ?? evidenceBoundary.reason, + localReplacementReason: githubIssueOutcome.reason ?? evidenceBoundary.reason ?? outcome.reason, lengthExceeded, }; } @@ -890,6 +896,7 @@ function grantGeminiDirectBoundaryOpportunity(input: Readonly<{ interface TerminalAddieResponseCommon { userMessage: string; + conversationContext?: string; githubIssueCreationRequested: boolean; clientRequestId?: string; githubIssueRetryReceipts?: readonly GithubIssueRetryReceipt[]; @@ -937,6 +944,7 @@ function buildTerminalAddieResponse(input: TerminalAddieResponseInput): Terminal input.githubIssueRetryReceipts, input.clientRequestId, input.kind === 'provider' && input.disposition === 'truncated', + input.conversationContext, ); const terminalExecutions = [ ...rehydratedGithubIssueRetryExecutions(input.githubIssueRetryReceipts ?? [], input.clientRequestId), @@ -2054,6 +2062,7 @@ export class AddieClaudeClient { kind: 'provider', disposition: 'truncated', userMessage, + conversationContext: [userMessage, options?.requestContext ?? '', ...(threadContext ?? []).map(entry => entry.text)].join('\n'), githubIssueCreationRequested, clientRequestId: options?.clientRequestId, githubIssueRetryReceipts: options?.githubIssueRetryReceipts, @@ -2107,6 +2116,7 @@ export class AddieClaudeClient { kind: 'provider', disposition: 'complete', userMessage, + conversationContext: [userMessage, options?.requestContext ?? '', ...(threadContext ?? []).map(entry => entry.text)].join('\n'), githubIssueCreationRequested, clientRequestId: options?.clientRequestId, githubIssueRetryReceipts: options?.githubIssueRetryReceipts, @@ -2170,6 +2180,7 @@ export class AddieClaudeClient { const terminal = buildTerminalAddieResponse({ kind: 'max_iterations', userMessage, + conversationContext: [userMessage, options?.requestContext ?? '', ...(threadContext ?? []).map(entry => entry.text)].join('\n'), githubIssueCreationRequested, clientRequestId: options?.clientRequestId, githubIssueRetryReceipts: options?.githubIssueRetryReceipts, @@ -2871,6 +2882,7 @@ export class AddieClaudeClient { kind: 'provider', disposition: 'truncated', userMessage, + conversationContext: [userMessage, options?.requestContext ?? '', ...(threadContext ?? []).map(entry => entry.text)].join('\n'), githubIssueCreationRequested, clientRequestId: options?.clientRequestId, githubIssueRetryReceipts: options?.githubIssueRetryReceipts, @@ -2927,6 +2939,7 @@ export class AddieClaudeClient { kind: 'provider', disposition: 'complete', userMessage, + conversationContext: [userMessage, options?.requestContext ?? '', ...(threadContext ?? []).map(entry => entry.text)].join('\n'), githubIssueCreationRequested, clientRequestId: options?.clientRequestId, githubIssueRetryReceipts: options?.githubIssueRetryReceipts, @@ -2987,6 +3000,7 @@ export class AddieClaudeClient { const terminal = buildTerminalAddieResponse({ kind: 'max_iterations', userMessage, + conversationContext: [userMessage, options?.requestContext ?? '', ...(threadContext ?? []).map(entry => entry.text)].join('\n'), githubIssueCreationRequested, clientRequestId: options?.clientRequestId, githubIssueRetryReceipts: options?.githubIssueRetryReceipts, diff --git a/server/src/addie/config-version.ts b/server/src/addie/config-version.ts index 69e66ebc924..5820c9b80a1 100644 --- a/server/src/addie/config-version.ts +++ b/server/src/addie/config-version.ts @@ -30,7 +30,7 @@ import { loadRules, loadResponseStyle } from './rules/index.js'; * Format: YYYY.MM.N where N is incremented for multiple changes in a month * Example: 2025.01.1, 2025.01.2, 2025.02.1 */ -export const CODE_VERSION = '2026.09.66'; +export const CODE_VERSION = '2026.09.67'; // Types export interface ConfigVersion { diff --git a/server/src/addie/gemini-direct-experiment.ts b/server/src/addie/gemini-direct-experiment.ts index d236686d380..2c8c490e180 100644 --- a/server/src/addie/gemini-direct-experiment.ts +++ b/server/src/addie/gemini-direct-experiment.ts @@ -9,6 +9,7 @@ import type { WebChatModelPreference } from './web-chat-model-selection.js'; import { responseClient } from './response-client.js'; import { getResponseProviderPolicy, responseProviderModel } from './response-provider-policy.js'; import { anonymousSessionSubjectHmac } from '../routes/helpers/anonymous-session-capability.js'; +import { countContainedToolErrors } from './model-providers/tool-orchestration.js'; const logger = createLogger('addie-gemini-direct'); export const GEMINI_DIRECT_EXPERIMENT = 'gemini-3.7-direct-v2'; @@ -192,6 +193,9 @@ class ExperimentTurn { const totalToolErrors = this.fallbackToolErrors + (response?.tool_executions.filter(tool => tool.is_error).length ?? 0); const unrecoveredToolErrors = Math.max(0, totalToolErrors - recoveredToolErrors); + const containedToolErrors = countContainedToolErrors(response?.tool_executions ?? [], Boolean( + response?.text.trim() && !failed && !response.flagged && !response.output_truncation, + )); logger.info({ event: 'addie_response_stage', stage: 'provider_and_tools', @@ -216,7 +220,8 @@ class ExperimentTurn { member_context_ms = $24, workos_context_ms = $25, experiment_routing_ms = $26, pre_provider_ms = $27, provider_ms = $28, tool_ms = $29, - recovered_tool_errors = $30, unrecovered_tool_errors = $31 + recovered_tool_errors = $30, unrecovered_tool_errors = $31, + contained_tool_errors = $32 WHERE id = $1`, [ this.id, this.firstVisibleMs, Date.now() - this.startedAt, this.routerMs, this.providerCalls, costMicros, this.usageComplete, this.fallbackReason, @@ -240,6 +245,7 @@ class ExperimentTurn { response?.timing?.total_tool_execution_ms ?? null, recoveredToolErrors, unrecoveredToolErrors, + containedToolErrors, ]); } catch (error) { logger.error({ error, turnId: this.id }, 'Failed to save Gemini Direct outcome'); @@ -457,6 +463,10 @@ async function getResults(experimentId: string) { SUM(tool_errors)::int AS tool_errors, SUM(recovered_tool_errors)::int AS recovered_tool_errors, SUM(unrecovered_tool_errors)::int AS unrecovered_tool_errors, + SUM(contained_tool_errors)::int AS contained_tool_errors, + SUM(unrecovered_tool_errors - contained_tool_errors)::int AS unresolved_tool_errors, + COUNT(*) FILTER (WHERE contained_tool_errors IS NOT NULL)::int AS tool_recovery_classified_turns, + SUM(tool_errors - recovered_tool_errors) FILTER (WHERE contained_tool_errors IS NULL)::int AS tool_recovery_unclassified_errors, COUNT(m.rating)::int AS rated_turns, ROUND(AVG(m.rating), 2) AS mean_rating, COUNT(*) FILTER (WHERE m.outcome = 'resolved')::int AS resolved_turns, CASE WHEN bool_and(usage_complete AND completed_at IS NOT NULL) @@ -478,6 +488,13 @@ async function getResults(experimentId: string) { assignment_version: ANONYMOUS_WEB_ASSIGNMENT_VERSION, }, cohorts: result.rows, + tool_recovery_definitions: { + recovered_tool_errors: 'Later success with the exact operation, agent URL, and idempotency key.', + unrecovered_tool_errors: 'Errors without exact recovery, including contained errors; retained for historical comparison.', + contained_tool_errors: 'Validation rejection followed by a successful read from the same agent and a complete generated answer. Observed continuation, not original-operation success or fulfillment; delivery is reported separately.', + unresolved_tool_errors: 'Classified errors with neither exact recovery nor observed containment. Absence of evidence, not a judgment of answer quality.', + tool_recovery_unclassified_errors: 'Errors without exact recovery on historical or incomplete turns without containment classification; excluded from contained and unresolved counts.', + }, }; } diff --git a/server/src/addie/generated/tool-surface-inventory.generated.json b/server/src/addie/generated/tool-surface-inventory.generated.json index c2dbeab4b86..5800d9dfbfa 100644 --- a/server/src/addie/generated/tool-surface-inventory.generated.json +++ b/server/src/addie/generated/tool-surface-inventory.generated.json @@ -63,7 +63,7 @@ } }, "registration_source_sha256": { - "server/src/addie/claude-client.ts": "27cdda3be371dff2793bc1b0556dd58c835d66577fa62eed4c4ee1a068e999e0", + "server/src/addie/claude-client.ts": "db7fb7f6cff9e967ea0ca2f6c3f80240ec4f36db527184019703cf97cddd7bf1", "server/src/addie/request-tool-assembly.ts": "cf4443d27ec69940f22aa9fcf46fdefd3f444762259c2700ac8fa01554ead4da", "server/src/addie/request-tool-replay-binding.ts": "3aed4c0bd18032e4418b8fff7e8deb263936b6c75b27941f9818a2a9a87287f2", "server/src/addie/prompts.ts": "3ec621f3e97eb9109fb561b94ff625a0639460c84506aa63aeebd57f683790f4", @@ -94,12 +94,12 @@ "not_declared_to_router_scope": "Modern routed Slack Bolt profiles only" }, "prompt": { - "rules_bytes": 154069, + "rules_bytes": 153537, "tool_reference_bytes": 40541, "complete_offline_tool_reference_bytes": 42589, "response_style_bytes": 9513, - "system_prompt_bytes": 204137, - "system_prompt_sha256": "ccb91e8e1d4189cc7e5d99a758c30064e6b2acad4fb45c350c8cf0f33cf57060" + "system_prompt_bytes": 203605, + "system_prompt_sha256": "ad110ad95d7f67bc142025f1ca75439807bb821b13cf43e583c6a5e6ecba1862" }, "catalog": { "missing_runtime_tool_count": 0, @@ -39600,7 +39600,7 @@ "prompt_catalog_tools_missing_runtime": -2, "always_available_tools": -13, "tool_reference_bytes": -3252, - "system_prompt_bytes": 2106, + "system_prompt_bytes": 1574, "maximum_slack_member_tools": -114, "maximum_slack_admin_tools": -181, "maximum_slack_public_tools": -3, diff --git a/server/src/addie/mcp/certification-tools.ts b/server/src/addie/mcp/certification-tools.ts index 952a6dda96c..8c04e16f54a 100644 --- a/server/src/addie/mcp/certification-tools.ts +++ b/server/src/addie/mcp/certification-tools.ts @@ -2763,7 +2763,7 @@ export function createCertificationToolHandlers( } const expectedCredential = await getCredentialForModule(capstoneMod.id); - const lines: string[] = [CAPSTONE_COMPLETED_PREFIX, '']; + const lines: string[] = [CAPSTONE_COMPLETED_PREFIX, `Module ${capstoneMod.id} completed!`, '']; lines.push('The capstone was already recorded, so I rechecked module completion and credential issuance.'); try { @@ -2915,6 +2915,7 @@ export function createCertificationToolHandlers( // Mark the capstone module as completed try { await certDb.completeModule(userId, capstoneMod.id, scores); + lines.push(`Module ${capstoneMod.id} completed!`); } catch (modError) { logger.error({ error: modError, userId, moduleId: capstoneMod.id }, 'Failed to record module completion after attempt passed'); } diff --git a/server/src/addie/mcp/escalation-tools.ts b/server/src/addie/mcp/escalation-tools.ts index 092b8475112..89ad00ac927 100644 --- a/server/src/addie/mcp/escalation-tools.ts +++ b/server/src/addie/mcp/escalation-tools.ts @@ -266,6 +266,12 @@ export function createEscalationToolHandlers( // ESCALATE TO ADMIN // ============================================ handlers.set('escalate_to_admin', async (input) => { + // Contact details supplied by a caller are not an authenticated identity. + // The web catalog excludes this tool for guests; also fail closed here. + if (!memberContext?.workos_user?.workos_user_id && !slackUserId) { + throw new ToolError('Please email support@agenticadvertising.org for help. Creating a support request here requires signing in.'); + } + // Validate required inputs if (typeof input.summary !== 'string' || !input.summary.trim()) { throw new ToolError('summary is required and must be a non-empty string'); @@ -315,12 +321,11 @@ export function createEscalationToolHandlers( : undefined); const orgName = memberContext?.organization?.name; + // Resolve email: explicit input > WorkOS user email > undefined + const resolvedEmail = userEmail || memberContext?.workos_user?.email; + let escalation; try { - // Resolve email: explicit input > WorkOS user email > undefined - const resolvedEmail = userEmail || memberContext?.workos_user?.email; - - // 1. Create escalation record - const escalation = await createEscalation({ + escalation = await createEscalation({ thread_id: threadId, slack_user_id: slackUserId, workos_user_id: memberContext?.workos_user?.workos_user_id, @@ -336,20 +341,30 @@ export function createEscalationToolHandlers( perspective_slug: perspectiveSlug, }); - logger.info( - { escalationId: escalation.id, category, priority, threadId }, - 'Created escalation' - ); + } catch (error) { + logger.error({ error, category, threadId }, 'Failed to create escalation'); + throw new ToolError('I could not confirm that your support request was created. Please email support@agenticadvertising.org for help.'); + } - // 2. Flag the thread - if (threadId) { + logger.info( + { escalationId: escalation.id, category, priority, threadId }, + 'Created escalation' + ); + + // The persisted request is authoritative. Ancillary failures must not erase + // its receipt or invite a retry that would create a duplicate request. + if (threadId) { + try { const threadService = getThreadService(); await threadService.flagThread(threadId, `Escalation: ${category}`); + } catch (error) { + logger.warn({ error, escalationId: escalation.id, threadId }, 'Support request persisted but thread flag failed'); } + } - // 3. Send notification to escalation channel + let notificationSent = false; + try { const escalationChannelId = await getEscalationChannelId(); - let notificationSent = false; if (escalationChannelId) { const result = await sendEscalationNotification( escalation.id, @@ -370,9 +385,9 @@ export function createEscalationToolHandlers( ); if (result.ok && result.ts) { + notificationSent = true; try { await markNotificationSent(escalation.id, escalationChannelId, result.ts); - notificationSent = true; logger.info({ escalationId: escalation.id, channelId: escalationChannelId }, 'Sent escalation notification'); } catch (notifyError) { logger.error( @@ -384,14 +399,18 @@ export function createEscalationToolHandlers( } else { logger.warn({ escalationId: escalation.id }, 'No escalation channel configured - notification not sent'); } - - return notificationSent - ? `Support request created (ID: ${escalation.id}). I've notified the AgenticAdvertising.org team and they'll follow up with you soon.` - : `Support request created (ID: ${escalation.id}). The request is in your dashboard, but the team notification channel is not configured. You can add details or close it from your dashboard.`; } catch (error) { - logger.error({ error, category, threadId }, 'Failed to create escalation'); - return 'I tried to escalate this but encountered an error. Please reach out directly to the AgenticAdvertising.org team for help.'; + logger.warn({ error, escalationId: escalation.id }, 'Support request persisted but team notification failed'); } + + return JSON.stringify({ + success: true, + escalation_id: escalation.id, + notification_sent: notificationSent, + message: notificationSent + ? `Support request created (ID: ${escalation.id}). The AgenticAdvertising.org team has been notified.` + : `Support request created (ID: ${escalation.id}). I could not confirm a team notification. You can also email support@agenticadvertising.org and mention this request ID.`, + }); }); // ============================================ diff --git a/server/src/addie/mcp/member-tools.ts b/server/src/addie/mcp/member-tools.ts index 33d0b129fe5..3a83d47aca7 100644 --- a/server/src/addie/mcp/member-tools.ts +++ b/server/src/addie/mcp/member-tools.ts @@ -4793,6 +4793,16 @@ export function createMemberToolHandlers( ownerId: agentQualityEvaluationOwnerId, leaseMs: AGENT_QUALITY_EVALUATION_LEASE_MS, }); + // Observations do not create rows, so admissions must be visible in logs + // to distinguish coalescing from a lack of duplicate organic requests. + // IDs and categorical outcomes suffice; omit URLs and credential identity. + logger.info({ + event: 'agent_quality_evaluation', + phase: 'admission', + evaluationId: claim.evaluation.id, + outcome: claim.owned ? 'owned' : 'coalesced', + recoveredExpiredLease: claim.recoveredExpiredLease, + }, 'evaluate_agent_quality: admission'); if (!claim.owned) return formatRunningAgentQualityEvaluation(claim.evaluation); evaluationLease = new AgentQualityEvaluationLease( agentQualityEvaluationDb, @@ -4820,8 +4830,21 @@ export function createMemberToolHandlers( let canonicalPublication: { runId: string; authoritative: boolean } | undefined; let observedUnconfirmedEvidence: string | undefined; let evaluationFinalized = false; + const publicationLogFields = () => ({ + event: 'agent_quality_evaluation', + evaluationId: evaluationLease!.evaluation.id, + publication: canonicalPublication + ? canonicalPublication.authoritative ? 'canonical_committed' : 'audit_only_committed' + : canonicalPublicationAttempted ? 'unconfirmed' : 'not_attempted', + canonicalRunId: canonicalPublication?.runId ?? null, + }); const finalizationUncertainMessage = (error: unknown, observedEvidence?: string): string => { const leaseLost = error instanceof AgentQualityEvaluationLeaseLostError || evaluationLease!.signal.aborted; + logger.warn({ + ...publicationLogFields(), + phase: 'finalization', + outcome: leaseLost ? 'lease_lost' : 'unconfirmed', + }, 'evaluate_agent_quality: finalization unconfirmed'); let message = leaseLost ? '**Evaluation ownership changed**\n\nThis worker no longer has a confirmed execution lease.\n\n' : '**Evaluation completion unconfirmed**\n\nThe database completion receipt could not be confirmed. This does not establish that execution ownership changed.\n\n'; @@ -4850,6 +4873,11 @@ export function createMemberToolHandlers( try { await evaluationLease!.complete(receiptMetadata); evaluationFinalized = true; + logger.info({ + ...publicationLogFields(), + phase: 'finalization', + outcome: 'completed', + }, 'evaluate_agent_quality: completed'); return message; } catch (error) { logger.warn({ error, agentUrl: displayAgentUrl }, 'evaluate_agent_quality: completion unconfirmed'); @@ -4859,6 +4887,14 @@ export function createMemberToolHandlers( const finishFailed = async (message: string, failureCode: string): Promise => { try { const recorded = await evaluationLease!.fail(failureCode); + if (recorded) { + logger.info({ + ...publicationLogFields(), + phase: 'finalization', + outcome: 'failed', + failureCode, + }, 'evaluate_agent_quality: failed'); + } return recorded ? message : finalizationUncertainMessage(new AgentQualityEvaluationLeaseLostError(), message); } catch (error) { logger.warn({ error, agentUrl: displayAgentUrl }, 'evaluate_agent_quality: failure transition failed'); @@ -5016,6 +5052,11 @@ export function createMemberToolHandlers( canonicalPublicationAttempted = true; const { run } = await complianceDb.recordComplianceRun(dbInput); canonicalPublication = { runId: run.id, authoritative: isAuthoritativeComplianceRun(dbInput) }; + logger.info({ + ...publicationLogFields(), + phase: 'publication', + outcome: 'committed', + }, 'evaluate_agent_quality: publication committed'); // notifyComplianceChange intentionally omitted: owner test runs are // exploratory; compliance-change notifications fire on heartbeat // transitions only to prevent iteration-loop spam. @@ -5300,6 +5341,12 @@ export function createMemberToolHandlers( return finalizationUncertainMessage(finalizationError); } if (!failureRecorded) return finalizationUncertainMessage(new AgentQualityEvaluationLeaseLostError()); + logger.info({ + ...publicationLogFields(), + phase: 'finalization', + outcome: 'failed', + failureCode: 'evaluation_failed', + }, 'evaluate_agent_quality: failed'); throw new ToolError(`Failed to evaluate agent quality for ${resolved.resolvedUrl}: ${msg}`); } }); diff --git a/server/src/addie/model-providers/tool-orchestration.ts b/server/src/addie/model-providers/tool-orchestration.ts index b1f03aa3805..05b814d86f5 100644 --- a/server/src/addie/model-providers/tool-orchestration.ts +++ b/server/src/addie/model-providers/tool-orchestration.ts @@ -303,6 +303,51 @@ function toolRecoveryIdentity(execution: ToolExecution): string | null { return JSON.stringify([operation, agentUrl, idempotencyKey]); } +/** + * Containment is an observation of continued work, not operation recovery or + * proof that the user's request was fulfilled. Only a validation rejection + * followed by a successful AdCP read against the same literal agent target + * qualifies, and only when the turn produced a complete, usable answer. + * Transport/auth/mutation ambiguity and unrelated successes remain unresolved. + */ +export function countContainedToolErrors( + executions: readonly ToolExecution[], + hasCompleteAnswer: boolean, +): number { + if (!hasCompleteAnswer) return 0; + return executions.filter((failed, index) => { + const presentation = failed.normalized_result; + const agentUrl = failed.parameters.agent_url; + if ( + !failed.is_error + || failed.blocked_by_policy + || presentation?.status !== 'invalid_input' + || presentation.telemetry?.error_category !== 'validation' + || presentation.telemetry.recovered_by_later_success === true + || !isAdcpExecution(failed) + || typeof agentUrl !== 'string' + || !agentUrl.trim() + ) return false; + return executions.slice(index + 1).some(later => ( + !later.is_error + && !later.blocked_by_policy + && later.normalized_result?.status === 'ok' + && later.parameters.agent_url === agentUrl + && isAdcpExecution(later) + && /^(?:get|list)_/.test(later.normalized_result.telemetry?.operation ?? '') + && !isSideEffectToolCall(later.tool_name, later.parameters) + )); + }).length; +} + +function isAdcpExecution(execution: ToolExecution): boolean { + const operation = execution.normalized_result?.telemetry?.operation; + if (!operation) return false; + return (execution.tool_name === 'call_adcp_task' && execution.parameters.task === operation) + || (execution.tool_name === 'call_adcp_get_products' && operation === 'get_products') + || (execution.tool_name === 'get_adcp_capabilities' && operation === 'get_adcp_capabilities'); +} + /** * Apply one accepted provider-neutral turn to Addie's shared tool and message * state. Delivery adapters consume the emitted events for logging/UI only; diff --git a/server/src/addie/outcome-claims.ts b/server/src/addie/outcome-claims.ts new file mode 100644 index 00000000000..29433cb2668 --- /dev/null +++ b/server/src/addie/outcome-claims.ts @@ -0,0 +1,196 @@ +import type { ToolExecution } from './model-providers/tool-orchestration.js'; + +/** These are application-owned receipt formats, not assertions from chat history. */ +const CERTIFICATION_TOOLS = new Set([ + 'complete_certification_module', 'complete_certification_exam', 'test_out_modules', + 'get_learner_progress', 'check_credentials', +]); +const MODULE_ID = /\b[A-Z]{1,2}\d{1,2}\b/gi; +const CERTIFICATION_CONTEXT = /\b(?:certification|capstone|module|credential|certificate|[A-Z]{1,2}\d{1,2})\b/i; +const COMPLETION = /\b(?:completed?|concludes?|concluded|finished|mastered|passed|earned|certified|awarded|issued|done|locked in|in the books|wrapped up|wraps? up|you(?:'re| are) through)\b/gi; +const CREDENTIAL = /\b(?:credential|certificate|badge|certified|certification)\b/i; +const ESCALATION_CLAIM = /\b(?:I(?:'ve| have| just| will|'ll| am going to|'m going to|'m| am)?|we(?:'ve| have| will|'ll| are going to|'re going to|'re| are)?)\s+(?:(?:will|have|already|just)\s+)?(flag(?:ged|ging)?|escalat(?:e|ed|ing)|notif(?:y|ied|ying)|rais(?:e|ed|ing)|fil(?:e|ed|ing)|creat(?:e|ed|ing)|open(?:ed|ing)?|send(?:ing)?|sent|pass(?:ed|ing)?|forward(?:ed|ing)?|hand(?:ed|ing)?|contact(?:ed|ing)?|reach(?:ed|ing)? out)\b/i; +const SUPPORT_OBJECT = /\b(?:ticket|support request|team|admins?|support|github issue)\b/i; +const PASSIVE_ESCALATION = /\b(?:(?:team|admins?) (?:has been|have been|will be|is being|was|were) (?:notified|alerted)|(?:ticket|support request|escalation)(?: #\d+)? (?:has been |was |is |will be )?(?:created|filed|opened|raised|saved)|(?:this|it|issue|request|problem|bug) (?:has been|was|will be|is being) (?:flagged|escalated|forwarded|passed|sent))\b/gi; +const GUARANTEED_NOTIFICATION = /\b(?:I(?:'ll| will)|we(?:'ll| will))\s+(?:make sure|ensure|let)\b/i; + +export const UNCONFIRMED_CERTIFICATION = "You're making progress. I haven't confirmed a saved completion for this module or credential yet. We can keep working; completion needs to be recorded before I can confirm it."; +export const DIRECT_SUPPORT = "I haven't created or escalated a support request. For help, email support@agenticadvertising.org with a brief description of what went wrong. Don't include passwords or sign-in codes."; + +/** Qualifiers apply to the claimed predicate, not every other clause in a sentence. */ +function hasSupportClaim(text: string, context: string): boolean { + const action = ESCALATION_CLAIM.exec(text)?.[1]?.toLowerCase(); + const contextualRequest = /\b(?:registration|sign[ -]?up|verification|support)\b/i.test(context) + && /\b(?:request|issue)\b/i.test(text); + if (action && (action.startsWith('flag') || action.startsWith('escalat') || SUPPORT_OBJECT.test(text) || contextualRequest)) return true; + if (GUARANTEED_NOTIFICATION.test(text) && /\b(?:team|admins?|support)\b/i.test(text) + && /\b(?:hears?|sees?|receives?|notified|alerted|knows?)\b/i.test(text)) return true; + for (const match of text.matchAll(PASSIVE_ESCALATION)) { + const prefix = text.slice(0, match.index); + if (/\b(?:no|not|never)\s+(?:(?:a|the|any)\s+)?$/i.test(prefix)) continue; + if (/\b(?:can't|cannot|couldn't|could not|haven't|have not|unable to)\s+(?:confirm|verify|say)\s+(?:(?:that|whether)\s+)?(?:(?:the|a)\s+)?$/i.test(prefix)) continue; + if (/\b(?:not sure|unclear)\s+(?:(?:that|whether|if)\s+)?(?:(?:the|a)\s+)?$/i.test(prefix)) continue; + if (/(?:^|[,;:]\s*)\s*(?:if|whether|when|once|until)\s+(?:(?:the|a)\s+)?$/i.test(prefix)) continue; + return true; + } + return false; +} + +interface CertificationEvidence { + modules: Set; + credentials: Set; + issuedCredentials: Set; +} + +/** Only anchored, application-generated lines from successful certification tools count. */ +function certificationEvidence(executions: readonly ToolExecution[]): CertificationEvidence { + const evidence: CertificationEvidence = { modules: new Set(), credentials: new Set(), issuedCredentials: new Set() }; + for (const execution of executions) { + if (execution.is_error || !CERTIFICATION_TOOLS.has(execution.tool_name)) continue; + const text = execution.result; + if (text.startsWith('NOT COMPLETED')) continue; + const lines = text.split('\n'); + if (execution.tool_name === 'complete_certification_module') { + const match = /^Module ([A-Z]{1,2}\d{1,2}) completed!/.exec(lines[0] ?? ''); + if (match) evidence.modules.add(match[1]!); + } + if (execution.tool_name === 'complete_certification_exam' && text.startsWith('# Congratulations! The learner passed the capstone!')) { + // The module receipt is emitted only after its separate persistence succeeds. + for (const line of lines) { + const match = /^Module ([A-Z]{1,2}\d{1,2}) completed!$/.exec(line); + if (match) evidence.modules.add(match[1]!); + } + } + if (execution.tool_name === 'get_learner_progress' && text.startsWith('# Your certification progress')) { + let section = ''; + for (const line of lines) { + if (line.startsWith('## ')) section = line; + const module = section === '## Module details' && /^- ([A-Z]{1,2}\d{1,2}): (?:completed|tested out)$/.exec(line); + if (module) evidence.modules.add(module[1]!); + const credential = section === '## Earned credentials' && /^- \*\*(.+)\*\* \(Level \d+\) — earned .+$/.exec(line); + if (credential) evidence.credentials.add(credential[1]!); + } + } + if (execution.tool_name === 'test_out_modules' && /^Marked \d+ module\(s\) as tested out:/.test(text)) { + // Stop before the user-supplied assessment notes. + for (const line of lines.slice(2)) { + if (!line.startsWith('- ')) break; + const match = /^- ([A-Z]{1,2}\d{1,2}): (?:tested out|already completed \(kept existing status\))$/.exec(line); + if (match) evidence.modules.add(match[1]!); + } + } + // Credential award and external badge issuance are separate outcomes. + // These lines originate in checkAndFormatCredentials, after database writes. + if (execution.tool_name !== 'get_learner_progress' && execution.tool_name !== 'test_out_modules') { + let credential: string | undefined; + for (const line of lines) { + const match = /^\*\*Credential earned: (.+)!\*\*$/.exec(line); + if (match) { credential = match[1]!; evidence.credentials.add(credential); } + if (credential && line.startsWith('- [View and share your credential](https://credsverse.com/credentials/')) { + evidence.issuedCredentials.add(credential); + } + } + } + } + return evidence; +} + +function supportReceipt(executions: readonly ToolExecution[]): { id: number; notified: boolean } | null { + for (const execution of [...executions].reverse()) { + if (execution.tool_name !== 'escalate_to_admin' || execution.is_error) continue; + try { + const receipt = JSON.parse(execution.result); + if (receipt?.success === true && Number.isSafeInteger(receipt.escalation_id) && receipt.escalation_id > 0 + && typeof receipt.notification_sent === 'boolean') { + return { id: receipt.escalation_id, notified: receipt.notification_sent }; + } + } catch { /* Legacy prose is not a persisted receipt. */ } + } + return null; +} + +/** Test each outcome predicate; an unrelated negation cannot exempt a claim. */ +function assertsCertificationOutcome(text: string): boolean { + if (/^(?:complete|finish|pass|master|earn)\b/i.test(text.trim())) return false; + if (/^(?:have|has|is|are|did|can|could|would|will)\b/i.test(text.trim()) && text.endsWith('?')) return false; + const predicates = [...text.matchAll(COMPLETION)]; + if (CREDENTIAL.test(text)) predicates.push(...text.matchAll(/\b(?:yours|ready(?=\s*[.!?]?$| to (?:download|share)| for (?:download|sharing)))/gi)); + if (/\bmastery\b/i.test(text)) predicates.push(...text.matchAll(/\b(?:confirmed|demonstrated|recorded)\b/gi)); + for (const match of predicates) { + const prefix = text.slice(0, match.index); + // Negation must apply to this predicate, not another clause. + if (/\b(?:not|never|haven't|hasn't|isn't|aren't|cannot|can't)(?:\s+\w+){0,2}\s+$/i.test(prefix)) continue; + const clause = prefix.split(/[,;:]|\b(?:but|and|so)\b/i).at(-1)!.trim(); + if (/^(?:once|if|when|until|before|to)\b/i.test(clause)) continue; + if (/\bto\s+$/i.test(prefix)) continue; + return true; + } + return false; +} + +/** + * Delivery backstop shared by streaming and non-streaming terminal responses. + * History identifies teaching context only; it can never authorize a claim. + * Replace unsupported outcome sentences, preserving teaching and next steps. + */ +export function enforceOutcomeClaims( + text: string, + executions: readonly ToolExecution[], + conversationContext = '', +): { text: string; reason: string | null } { + const evidence = certificationEvidence(executions); + const support = supportReceipt(executions); + const certificationContext = CERTIFICATION_CONTEXT.test(conversationContext) + || executions.some(execution => CERTIFICATION_TOOLS.has(execution.tool_name)); + const reasons = new Set(); + let certificationReplaced = false; + let certificationRendered = false; + let supportReplaced = false; + const parts = text.split(/((?<=[.!?])\s+|\n+)/); + const output: string[] = []; + for (const [index, part] of parts.entries()) { + if (index % 2 === 1) { output.push(part); continue; } + const plain = part.replace(/[*_`]/g, '').replace(/[’‘]/g, "'"); + if (hasSupportClaim(plain, conversationContext)) { + if (!supportReplaced) output.push(support + ? `Support request #${support.id} is saved.${support.notified ? ' The team notification was sent.' : ' I could not confirm a team notification.'}` + : DIRECT_SUPPORT); + supportReplaced = true; + if (!support) reasons.add('Unconfirmed support escalation'); + continue; + } + const ids = [...plain.matchAll(MODULE_ID)].map(match => match[0].toUpperCase()); + const teachingSubtask = ids.length === 0 && !CREDENTIAL.test(plain) + && !/\b(?:module|capstone|mastery)\b/i.test(plain) + && [...plain.matchAll(COMPLETION)].length === 1 + && /\b(?:completed?|finished|passed|mastered)\s+(?:(?:the|this|that|an?|your)\s+)?(?:example|exercise|question|practice|request|media buy|deployment)\b/i.test(plain); + const isCertificationClaim = !teachingSubtask && assertsCertificationOutcome(plain) + && (ids.length > 0 || CREDENTIAL.test(plain) || /\b(?:module|capstone)\b/i.test(plain) + || (certificationContext && /\b(?:you|your|we|that|this|it)\b/i.test(plain))); + if (isCertificationClaim) { + const credentialClaim = CREDENTIAL.test(plain); + const externalIssuance = /\b(?:issued|sent|delivered|download|share|ready)\b/i.test(plain); + const credentials = externalIssuance ? evidence.issuedCredentials : evidence.credentials; + const modulesSupported = ids.every(id => evidence.modules.has(id)); + const namedCredentials = [...credentials].filter(name => plain.toLowerCase().includes(name.toLowerCase())); + const modules = ids.length ? ids : [...evidence.modules]; + const supported = modulesSupported && (credentialClaim ? namedCredentials.length > 0 : modules.length > 0); + if (!supported) { + if (!certificationReplaced) output.push(UNCONFIRMED_CERTIFICATION); + certificationReplaced = true; + reasons.add('Unconfirmed certification completion'); + continue; + } + // Render only the precise recorded outcomes. A valid receipt must not + // license extra credentials, modules, or delivery claims in model prose. + output.push([ + ...modules.filter(id => evidence.modules.has(id)).map(id => `${id} is recorded as complete.`), + ...(credentialClaim ? namedCredentials.map(name => `Credential ${externalIssuance ? 'issued' : 'earned'}: ${name}.`) : []), + ].join(' ')); + certificationRendered = true; + continue; + } + output.push(part); + } + return { text: reasons.size > 0 || supportReplaced || certificationRendered ? output.join('').trim() : text, reason: [...reasons].join('; ') || null }; +} diff --git a/server/src/addie/rules/behaviors.md b/server/src/addie/rules/behaviors.md index c9afd69c1ca..36b432d7c35 100644 --- a/server/src/addie/rules/behaviors.md +++ b/server/src/addie/rules/behaviors.md @@ -47,14 +47,14 @@ This is distinct from the Conversation Pivot section below — that is about opp When sharing the Slack invite link or telling someone they can join the Slack community, always add a proactive caveat about domain restrictions: -"The invite link is public, but if it doesn't work — Gmail, personal email addresses, and some non-company domains are sometimes restricted — reply here with your email address and I'll flag it for a direct invite from the team." +"The invite link is public, but Gmail, personal email addresses, and some non-company domains are sometimes restricted. If it doesn't work, email support@agenticadvertising.org for help with a direct invite." Do NOT share the invite link silently and walk away. The silent-failure pattern (link shared, user tries it, gets rejected with no explanation, assumes the link is broken) is the #1 source of preventable escalations on this topic. If someone reports that the invite failed for them: -1. Acknowledge it specifically — it's a domain allowlist issue, not a broken link -2. Ask for their email address -3. Call `escalate_to_admin` with category `needs_human_action`, including their email address, so the admin team can issue a direct invite +1. Acknowledge the failure; domain restrictions are a possible cause. +2. Without `escalate_to_admin` in the catalog, immediately give support@agenticadvertising.org. No contact collection or promise to flag it. +3. With the tool, get consent and contact details, then call it with category `needs_human_action`. Confirm creation from the persisted receipt; notification requires `notification_sent: true`. The help page at https://docs.adcontextprotocol.org/docs/community/joining-slack has the full explanation of what happens and what to do. @@ -62,10 +62,10 @@ The help page at https://docs.adcontextprotocol.org/docs/community/joining-slack When a user reports that they never received a verification email, password reset, or any other platform notification: -1. Acknowledge it as a platform-side delivery failure, not user error. Do not suggest "check your spam folder" as the primary response — lead with the fact that this is a known failure mode on our end. -2. Ask for or confirm their email address in the same turn so the escalation is actionable. If you already have it from their member context, confirm it: "I have your email as [address] — is that the one you're expecting the email at?" -3. Call `escalate_to_admin` with category `needs_human_action` and include the email address and the type of email that failed (verification, password reset, notification, etc.) before telling the user the team will investigate. -4. Tell the user the team will follow up — do not commit to a specific timeline. +1. Acknowledge the delivery problem without diagnosing or blaming. Do not lead with "check your spam folder." +2. Without `escalate_to_admin`, immediately give support@agenticadvertising.org, including for registration problems. No contact collection or escalation promise. +3. With the tool, get consent and contact details. Call it with `needs_human_action` and the failed email type. +4. Confirm creation only from the persisted receipt; notification requires `notification_sent: true`. No response timeline promises. Do NOT: - Claim to check email delivery logs, async operation status, or email provider dashboards (you have no such tool) diff --git a/server/src/addie/rules/constraints.md b/server/src/addie/rules/constraints.md index a4d59157afe..ab4bc5fc63a 100644 --- a/server/src/addie/rules/constraints.md +++ b/server/src/addie/rules/constraints.md @@ -59,9 +59,9 @@ Actions that REQUIRE a tool call before claiming success: - Sending DMs or notifications → send_member_dm must succeed - Creating payment links → create_payment_link must succeed - Scheduling meetings → schedule_meeting must succeed -- **Escalating to a human / opening a ticket → escalate_to_admin must succeed.** `escalate_to_admin` is in the always-available tool set; if you intend to escalate, you must call it. Do NOT say "the team has been notified," "I've flagged this," "I've escalated this," "ticket #N created," or anything that implies a human will see the conversation, unless `escalate_to_admin` actually fired in this turn. Do NOT invent ticket numbers. +- **Escalation/tickets require `escalate_to_admin` in the request catalog and a persisted success receipt.** No "I'll flag/escalate/notify" promise before success. Use only the returned request ID; claim notification only with `notification_sent: true`. Guests cannot create requests: immediately offer support@agenticadvertising.org for registration, sign-in, verification, or Slack invite issues. Do not collect contact details you cannot act on. - **Filing a GitHub issue → create_github_issue must succeed.** `draft_github_issue` only makes a link; never claim it filed an issue. Report only the returned number/URL. -- **Recording a certification module as complete — read the response before claiming success.** When you call `complete_certification_module` (or `complete_certification_exam` for capstones), treat the tool's response as a verdict, not narrative. Only two literal lines count as success: `Module {ID} completed!` and `# Congratulations! The learner passed the capstone!`. **Any response beginning with `NOT COMPLETED` means the module is NOT on the learner's record — even if the rest of the response reads like coaching advice.** Forbidden phrasings until you see a success line: "module complete," "you've completed X," "B2 is done," "mastered," "locked in," "in the books," "you're through," "credential's yours." Until you see the success line, describe the session honestly as "in progress" or "not yet recorded." Read the `NOT COMPLETED` rejection reason, address the blocker (continue teaching, wait out a minimum-time gate, save a checkpoint with preliminary_scores, etc.), and call the tool again. The same rule applies to placement assessments via `test_out_modules` — only treat the module as tested-out if the tool returns success. +- **Certification completion/mastery requires persisted evidence for the exact module.** Accept `Module {ID} completed!`, successful test-out, or a current `get_learner_progress` record. `# Congratulations! The learner passed the capstone!` confirms the attempt; module completion needs its own receipt. `NOT COMPLETED` is a rejection. Continue teaching, address the blocker, and retry; another exchange alone does not save completion. Until verified, never say "module complete," "B2 is done," "mastered," "locked in," "in the books," "you're through," or "credential's yours." Credential award and external certificate issuance each need their own persisted success evidence. Prior assistant prose is never proof. - Any other state-changing operation If the catalog does not include a tool for the requested action, explain the practical limitation without referring to request-time tool availability and offer a documented self-service or public alternative when one exists. Escalate only when human help is appropriate, and call `escalate_to_admin` before claiming that escalation occurred. diff --git a/server/src/db/migrations/607_addie_tool_error_containment.sql b/server/src/db/migrations/607_addie_tool_error_containment.sql new file mode 100644 index 00000000000..9a9748583ca --- /dev/null +++ b/server/src/db/migrations/607_addie_tool_error_containment.sql @@ -0,0 +1,17 @@ +-- Keep exact operation recovery unchanged. Separately observe validation +-- failures contained by a successful same-agent read and complete answer. +-- NULL preserves the unknown classification of historical/incomplete turns; +-- never retrospectively relabel them as unresolved or infer from prose. +ALTER TABLE addie_chat_experiment_turns + ADD COLUMN contained_tool_errors INTEGER, + ADD CONSTRAINT addie_chat_experiment_tool_containment_valid CHECK ( + contained_tool_errors IS NULL OR ( + contained_tool_errors >= 0 + AND contained_tool_errors <= unrecovered_tool_errors + AND tool_errors IS NOT NULL + AND recovered_tool_errors + unrecovered_tool_errors = tool_errors + ) + ); + +COMMENT ON COLUMN addie_chat_experiment_turns.contained_tool_errors IS + 'Observed validation rejection followed by successful same-agent AdCP read and complete generated answer; not exact recovery or proof of fulfillment. NULL means unclassified. Remaining unresolved errors = unrecovered_tool_errors - contained_tool_errors. Delivery is tracked separately.'; diff --git a/server/tests/unit/addie/certification-outcome-claims.test.ts b/server/tests/unit/addie/certification-outcome-claims.test.ts new file mode 100644 index 00000000000..63f2b208d1f --- /dev/null +++ b/server/tests/unit/addie/certification-outcome-claims.test.ts @@ -0,0 +1,98 @@ +import { describe, expect, it } from 'vitest'; +import { enforceOutcomeClaims, UNCONFIRMED_CERTIFICATION } from '../../../src/addie/outcome-claims.js'; +import type { ToolExecution } from '../../../src/addie/model-providers/tool-orchestration.js'; + +function execution(tool_name: string, result: string, is_error = false): ToolExecution { + return { tool_name, result, is_error, parameters: {}, duration_ms: 1 }; +} +const completed = execution('complete_certification_module', 'Module B2 completed! The learner has demonstrated mastery of all learning objectives.'); + +describe('receipt-bound certification claims', () => { + it.each([ + 'B2 is complete.', 'B2 is concluded.', 'B2 is done.', 'B2 mastery is confirmed.', + "The credential's yours.", 'Your Foundations certificate is ready to download.', 'Your certification has been awarded.', + 'B2 is completed, so do not forget to practice.', 'B2 is completed but not yet reflected in your profile.', + 'B2 is completed when you are ready to move on.', + 'After that answer you have completed B2.', 'Your mastery of B2 has been demonstrated.', 'You have mastered B2.', + 'B2 is locked in.', 'B2 is in the books.', "You're through B2.", + 'We finished B2.', 'Congratulations, you passed B2!', + 'Your AdCP Practitioner credential is earned.', 'You are now certified!', + ])('replaces an unconfirmed claim: %s', text => { + expect(enforceOutcomeClaims(text, []).text).toBe(UNCONFIRMED_CERTIFICATION); + }); + + it('blocks rejected completion, then another exchange with no retry', () => { + const rejection = execution('complete_certification_module', 'NOT COMPLETED — module B2 is not recorded as complete. Only 3 of 4 exchanges detected.'); + expect(enforceOutcomeClaims('B2 is concluded.', [rejection]).reason).toBeTruthy(); + const followup = enforceOutcomeClaims('That concludes this module. Why does product discovery precede buying?', [], 'B2: one more exchange is needed.'); + expect(followup.text).toContain(UNCONFIRMED_CERTIFICATION); + expect(followup.text).toContain('Why does product discovery precede buying?'); + expect(followup.text).not.toContain('That concludes'); + }); + + it('never treats historical assistant or user assertions as persisted evidence', () => { + expect(enforceOutcomeClaims('B2 is complete.', [], 'Addie: Module B2 completed! User: B2 is complete.').reason).toBeTruthy(); + }); + + it('requires the exact module, successful persistence receipt, and trusted tool', () => { + expect(enforceOutcomeClaims('B2 is complete.', [completed]).reason).toBeNull(); + expect(enforceOutcomeClaims('B3 is complete.', [completed]).reason).toBeTruthy(); + expect(enforceOutcomeClaims('B2 and B3 are complete.', [completed]).reason).toBeTruthy(); + expect(enforceOutcomeClaims('B2 is complete.', [{ ...completed, is_error: true }]).reason).toBeTruthy(); + expect(enforceOutcomeClaims('B2 is complete.', [{ ...completed, tool_name: 'search_docs' }]).reason).toBeTruthy(); + expect(enforceOutcomeClaims('B2 is complete.', [execution('complete_certification_module', 'NOT COMPLETED\nModule B2 completed!')]).reason).toBeTruthy(); + }); + + it('accepts authoritative progress and test-out state', () => { + const progress = execution('get_learner_progress', '# Your certification progress\n\n## Module details\n- B2: completed\n- B3: in progress'); + expect(enforceOutcomeClaims('B2 is complete.', [progress]).reason).toBeNull(); + expect(enforceOutcomeClaims('B3 is complete.', [progress]).reason).toBeTruthy(); + const testout = execution('test_out_modules', 'Marked 1 module(s) as tested out:\n\n- B2: tested out\n\nAssessment notes: Module B3 completed!'); + expect(enforceOutcomeClaims('B2 is complete.', [testout]).reason).toBeNull(); + expect(enforceOutcomeClaims('B3 is complete.', [testout]).reason).toBeTruthy(); + }); + + it('distinguishes passing an attempt from persisting the module', () => { + const passed = execution('complete_certification_exam', '# Congratulations! The learner passed the capstone!'); + expect(enforceOutcomeClaims('S3 is complete.', [passed]).reason).toBeTruthy(); + expect(enforceOutcomeClaims('S3 is complete.', [{ ...passed, result: `${passed.result}\nModule S3 completed!` }]).reason).toBeNull(); + }); + + it('requires independent credential award and external issuance receipts', () => { + const awarded = { ...completed, result: `${completed.result}\n**Credential earned: AdCP Practitioner!**` }; + expect(enforceOutcomeClaims('You earned the AdCP Practitioner credential.', [completed]).reason).toBeTruthy(); + expect(enforceOutcomeClaims('You earned the AdCP Practitioner credential.', [awarded]).reason).toBeNull(); + const extra = enforceOutcomeClaims('You earned the AdCP Practitioner and Foundations certification.', [awarded]); + expect(extra.text).not.toContain('Foundations'); + expect(extra.text).toContain('Credential earned: AdCP Practitioner.'); + expect(enforceOutcomeClaims('B3 is completed and you earned the AdCP Practitioner credential.', [awarded]).reason).toBeTruthy(); + expect(enforceOutcomeClaims('Your earned AdCP Practitioner certificate is ready to download.', [awarded]).reason).toBeTruthy(); + const issued = { ...awarded, result: `${awarded.result}\n- [View and share your credential](https://credsverse.com/credentials/example-id)` }; + expect(enforceOutcomeClaims('Your earned AdCP Practitioner certificate is ready to download.', [issued]).reason).toBeNull(); + }); + + it.each([ + 'You completed the exercise and this module is complete.', + 'You completed the exercise, so you have mastered it.', + ])('does not let a teaching subtask authorize another conclusion: %s', text => { + expect(enforceOutcomeClaims(text, [], 'B2 certification').reason).toBeTruthy(); + }); + + it.each([ + 'The media buy was completed after the seller accepted the proposal.', + 'You are ready to try another exercise.', 'You are ready for B2.', + 'You have completed the exercise.', 'You have mastered this example.', 'Mastery comes from practice.', 'Complete B2 before starting B3.', + 'When your certificate is issued, you can share it.', 'Has your certificate been issued?', + ])('preserves teaching inside certification context: %s', text => { + expect(enforceOutcomeClaims(text, [], 'B2 certification').text).toBe(text); + }); + + it.each([ + 'B2 is not complete yet.', 'Once B2 is complete, we can move on.', + 'Have you completed B2?', 'Before we complete B2, try another example.', + 'Your explanation of targeting is clear. How would you apply it to this product?', + 'The media buy completed successfully.', + ])('preserves teaching, questions, conditional guidance and ordinary actions: %s', text => { + expect(enforceOutcomeClaims(text, []).text).toBe(text); + }); +}); diff --git a/server/tests/unit/addie/escalation-receipts.test.ts b/server/tests/unit/addie/escalation-receipts.test.ts new file mode 100644 index 00000000000..234846d04c3 --- /dev/null +++ b/server/tests/unit/addie/escalation-receipts.test.ts @@ -0,0 +1,103 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import type { MemberContext } from '../../../src/addie/member-context.js'; + +const mocks = vi.hoisted(() => ({ + createEscalation: vi.fn(), + markNotificationSent: vi.fn(), + flagThread: vi.fn(), + sendChannelMessage: vi.fn(), + getEscalationChannel: vi.fn(), +})); + +vi.mock('../../../src/db/escalation-db.js', () => ({ + createEscalation: mocks.createEscalation, + markNotificationSent: mocks.markNotificationSent, + listEscalationsForUser: vi.fn(), +})); +vi.mock('../../../src/addie/thread-service.js', () => ({ + getThreadService: () => ({ flagThread: mocks.flagThread }), +})); +vi.mock('../../../src/slack/client.js', () => ({ sendChannelMessage: mocks.sendChannelMessage })); +vi.mock('../../../src/db/system-settings-db.js', () => ({ getEscalationChannel: mocks.getEscalationChannel })); + +import { createEscalationToolHandlers } from '../../../src/addie/mcp/escalation-tools.js'; +import { ToolError } from '../../../src/addie/tool-error.js'; + +const memberContext = { + workos_user: { workos_user_id: 'user_dayo', email: 'dayo@example.com' }, +} as MemberContext; +const request = { summary: 'Registration verification email never arrived', category: 'needs_human_action' }; + +function handler(context: MemberContext | null = memberContext, slackUserId?: string) { + return createEscalationToolHandlers(context, slackUserId, 'thread_1').get('escalate_to_admin')!; +} + +describe('escalation persisted receipts', () => { + beforeEach(() => { + vi.clearAllMocks(); + mocks.createEscalation.mockResolvedValue({ id: 42 }); + mocks.flagThread.mockResolvedValue(undefined); + mocks.getEscalationChannel.mockResolvedValue({ channel_id: 'private_support' }); + mocks.sendChannelMessage.mockResolvedValue({ ok: true, ts: '123.456' }); + mocks.markNotificationSent.mockResolvedValue(undefined); + }); + + it('rejects anonymous writes even when the caller supplies contact details', async () => { + await expect(handler(null)({ ...request, user_email: 'dayo@example.com' })) + .rejects.toThrow('support@agenticadvertising.org'); + expect(mocks.createEscalation).not.toHaveBeenCalled(); + expect(mocks.sendChannelMessage).not.toHaveBeenCalled(); + }); + + it('returns a persisted receipt and requires private notification delivery', async () => { + const result = JSON.parse(await handler()(request)); + expect(result).toMatchObject({ success: true, escalation_id: 42, notification_sent: true }); + expect(mocks.createEscalation).toHaveBeenCalledWith(expect.objectContaining({ + workos_user_id: 'user_dayo', user_email: 'dayo@example.com', + })); + expect(mocks.sendChannelMessage).toHaveBeenCalledWith( + 'private_support', expect.any(Object), { requirePrivate: true }, + ); + expect(JSON.stringify(result)).not.toContain('dayo@example.com'); + }); + + it('preserves the existing authenticated Slack escalation path', async () => { + const result = JSON.parse(await handler(null, 'U_DAYO')(request)); + expect(result).toMatchObject({ success: true, escalation_id: 42 }); + expect(mocks.createEscalation).toHaveBeenCalledWith(expect.objectContaining({ slack_user_id: 'U_DAYO' })); + }); + + it('reports failed persistence as a tool error rather than a successful narrative', async () => { + mocks.createEscalation.mockRejectedValue(new Error('database unavailable')); + await expect(handler()(request)).rejects.toBeInstanceOf(ToolError); + expect(mocks.flagThread).not.toHaveBeenCalled(); + expect(mocks.sendChannelMessage).not.toHaveBeenCalled(); + }); + + it('retains the persisted receipt if flagging the conversation fails', async () => { + mocks.flagThread.mockRejectedValue(new Error('flag failed')); + expect(JSON.parse(await handler()(request))) + .toMatchObject({ success: true, escalation_id: 42, notification_sent: true }); + expect(mocks.createEscalation).toHaveBeenCalledTimes(1); + }); + + it.each(['unconfigured', 'settings failure', 'delivery failure', 'delivery rejected']) + ('does not invent a notification when %s', async (scenario) => { + if (scenario === 'unconfigured') mocks.getEscalationChannel.mockResolvedValue({ channel_id: null }); + if (scenario === 'settings failure') mocks.getEscalationChannel.mockRejectedValue(new Error('settings failed')); + if (scenario === 'delivery failure') mocks.sendChannelMessage.mockRejectedValue(new Error('delivery failed')); + if (scenario === 'delivery rejected') mocks.sendChannelMessage.mockResolvedValue({ ok: false }); + + const result = JSON.parse(await handler()(request)); + expect(result).toMatchObject({ success: true, escalation_id: 42, notification_sent: false }); + expect(result.message).toContain('support@agenticadvertising.org'); + expect(result.message).not.toContain('has been notified'); + expect(mocks.markNotificationSent).not.toHaveBeenCalled(); + }); + + it('reports delivery truthfully when recording its status fails after Slack accepted it', async () => { + mocks.markNotificationSent.mockRejectedValue(new Error('status update failed')); + expect(JSON.parse(await handler()(request))) + .toMatchObject({ success: true, escalation_id: 42, notification_sent: true }); + }); +}); diff --git a/server/tests/unit/addie/gemini-direct-experiment.test.ts b/server/tests/unit/addie/gemini-direct-experiment.test.ts index 4a40774537c..8870cc10783 100644 --- a/server/tests/unit/addie/gemini-direct-experiment.test.ts +++ b/server/tests/unit/addie/gemini-direct-experiment.test.ts @@ -123,14 +123,60 @@ beforeEach(() => { afterEach(() => vi.unstubAllEnvs()); describe('Gemini Direct production integration', () => { - it('reports preparation and recovered versus unrecovered tool errors to staff', async () => { - await getGeminiDirectResults(); + it('reports exact recovery, observed containment, unresolved errors, and historical coverage separately', async () => { + const results = await getGeminiDirectResults(); const sql = String(mocks.query.mock.calls.at(-1)?.[0]); expect(sql).toContain('median_pre_provider_ms'); expect(sql).toContain('p95_provider_ms'); expect(sql).toContain('relationship_analytics_pending'); expect(sql).toContain('recovered_tool_errors'); expect(sql).toContain('unrecovered_tool_errors'); + expect(sql).toContain('SUM(contained_tool_errors)::int AS contained_tool_errors'); + expect(sql).toContain('SUM(unrecovered_tool_errors - contained_tool_errors)::int AS unresolved_tool_errors'); + expect(sql).toContain('SUM(tool_errors - recovered_tool_errors) FILTER (WHERE contained_tool_errors IS NULL)'); + expect(sql).toContain('tool_recovery_classified_turns'); + expect(results.tool_recovery_definitions.contained_tool_errors).toContain('not original-operation success'); + }); + + it.each([ + ['complete answer', {}, false, 1], + ['flagged answer', { flagged: true }, false, 0], + ['empty answer', { text: ' ' }, false, 0], + ['truncated answer', { output_truncation: { source: 'provider_output_limit' as const } }, false, 0], + ['failed turn', {}, true, 0], + ])('persists conservative containment for a %s', async (_label, overrides, failed, expectedContained) => { + const f = fixture([]); + const turn = await prepareGeminiDirectTurn(f.input); + const toolExecutions: AddieResponse['tool_executions'] = [ + { + tool_name: 'call_adcp_task', parameters: { task: 'request_proposals', agent_url: 'https://agent.example/mcp' }, + result: 'Unsupported.', is_error: true, duration_ms: 1, sequence: 1, + normalized_result: { status: 'invalid_input', user_summary: 'Unsupported.', source: 'structured', + telemetry: { operation: 'request_proposals', error_category: 'validation' } }, + }, + { + tool_name: 'call_adcp_task', parameters: { task: 'get_products', agent_url: 'https://agent.example/mcp' }, + result: 'Products.', is_error: false, duration_ms: 1, sequence: 2, + normalized_result: { status: 'ok', user_summary: 'Products.', source: 'structured', telemetry: { operation: 'get_products' } }, + }, + ]; + await turn.experiment?.finish({ ...answer, tool_executions: toolExecutions, ...overrides }, undefined, failed); + const finish = mocks.query.mock.calls.find(([sql]) => sql.startsWith('UPDATE addie_chat_experiment_turns SET\n completed_at'))!; + expect(finish[1][11]).toBe(1); // total errors + expect(finish[1].slice(29, 32)).toEqual([0, 1, expectedContained]); // exact, unrecovered, contained + }); + + it('keeps exactly recovered errors out of the contained/unresolved partition', async () => { + const f = fixture([]); + const turn = await prepareGeminiDirectTurn(f.input); + await turn.experiment?.finish({ ...answer, tool_executions: [{ + tool_name: 'call_adcp_task', parameters: { task: 'get_products', agent_url: 'https://agent.example/mcp' }, + result: 'Correct the request.', is_error: true, duration_ms: 1, sequence: 1, + normalized_result: { status: 'invalid_input', user_summary: 'Correct the request.', source: 'structured', + telemetry: { operation: 'get_products', error_category: 'validation', recovered_by_later_success: true } }, + }] }); + const finish = mocks.query.mock.calls.find(([sql]) => sql.startsWith('UPDATE addie_chat_experiment_turns SET\n completed_at'))!; + expect(finish[1].slice(29, 32)).toEqual([1, 0, 0]); }); it('persists preparation, provider, tool, and relationship stage timing separately', async () => { diff --git a/server/tests/unit/addie/model-provider-tool-orchestration.test.ts b/server/tests/unit/addie/model-provider-tool-orchestration.test.ts index 979e8abe589..a73c52a452e 100644 --- a/server/tests/unit/addie/model-provider-tool-orchestration.test.ts +++ b/server/tests/unit/addie/model-provider-tool-orchestration.test.ts @@ -694,11 +694,14 @@ describe('AddieToolExecutionLedger', () => { expect(ledger.executions[1]?.normalized_result?.telemetry?.recovered_by_later_success).toBeUndefined(); }); - it('does not let another agent success recover a failed target with the same operation', async () => { + it.each([ + ['another agent', 'https://agent-b.example', 'same-products-request-key'], + ['another logical request', 'https://agent-a.example', 'a-different-products-key'], + ])('does not let %s success recover a failed target with the same operation', async (_label, agentUrl, idempotencyKey) => { const ledger = new AddieToolExecutionLedger(); const calls = [ { ...call({ agent_url: 'https://agent-a.example', task: 'get_products', params: { idempotency_key: 'same-products-request-key' } }), id: 'call_error', name: 'call_adcp_task' }, - { ...call({ agent_url: 'https://agent-b.example', idempotency_key: 'same-products-request-key', buying_mode: 'wholesale' }), id: 'call_success', name: 'call_adcp_get_products' }, + { ...call({ agent_url: agentUrl, idempotency_key: idempotencyKey, buying_mode: 'wholesale' }), id: 'call_success', name: 'call_adcp_get_products' }, ]; let invocation = 0; const execute = vi.fn(async (toolCall: ModelToolCallContent, sequence: number) => { diff --git a/server/tests/unit/addie/support-outcome-claims.test.ts b/server/tests/unit/addie/support-outcome-claims.test.ts new file mode 100644 index 00000000000..2866b062ca4 --- /dev/null +++ b/server/tests/unit/addie/support-outcome-claims.test.ts @@ -0,0 +1,94 @@ +import { describe, expect, it } from 'vitest'; +import { DIRECT_SUPPORT, enforceOutcomeClaims } from '../../../src/addie/outcome-claims.js'; +import type { ToolExecution } from '../../../src/addie/model-providers/tool-orchestration.js'; + +function execution(result: string, overrides: Partial = {}): ToolExecution { + return { + tool_name: 'escalate_to_admin', parameters: {}, result, + is_error: false, duration_ms: 1, sequence: 0, ...overrides, + }; +} + +function receipt(notified = true): ToolExecution { + return execution(JSON.stringify({ success: true, escalation_id: 42, notification_sent: notified })); +} + +describe('receipt-bound support outcome claims', () => { + it.each([ + "I'll flag it.", + "I’ll flag this because I can't fix registration.", + "I've escalated this but cannot promise a reply.", + "I'll flag it, okay?", + 'When registration fails, the team will be notified.', + "I'm escalating your issue now.", + "I'll pass this to the team.", + "I'll forward your registration issue to the team.", + "I'll make sure the team hears about this.", + "I'll let the support team know.", + 'Your issue has been escalated.', + 'A support request was created.', + 'Ticket #99 is saved.', + ])('replaces unsupported claim %s with the direct support route', (text) => { + expect(enforceOutcomeClaims(text, [])).toEqual({ text: DIRECT_SUPPORT, reason: 'Unconfirmed support escalation' }); + }); + + it.each([ + "I haven't escalated this.", + "I will not flag this without your consent.", + 'No support request has been created.', + "I can't confirm the team has been notified.", + "I'm not sure whether the team has been notified.", + 'If the team has been notified, they can check your request.', + 'You can email support@agenticadvertising.org for help registering.', + "I've created this example to explain the lesson.", + "I've sent this media buy request to the seller.", + ])('preserves truthful limitations and self-service guidance: %s', (text) => { + expect(enforceOutcomeClaims(text, [])).toEqual({ text, reason: null }); + }); + + it.each([ + execution('Support request created (ID: 42).'), + execution(JSON.stringify({ success: false, escalation_id: 42, notification_sent: true })), + execution(JSON.stringify({ success: true, escalation_id: 42 })), + execution(JSON.stringify({ success: true, escalation_id: -1, notification_sent: true })), + execution(JSON.stringify({ success: true, escalation_id: 42, notification_sent: true }), { is_error: true }), + execution(JSON.stringify({ success: true, escalation_id: 42, notification_sent: true }), { tool_name: 'search_docs' }), + ])('rejects unavailable, failed, or unrelated execution evidence %#', (toolExecution) => { + expect(enforceOutcomeClaims("I've escalated this.", [toolExecution]).text).toBe(DIRECT_SUPPORT); + }); + + it('does not trust an earlier assistant receipt or a user-supplied receipt in conversation context', () => { + const history = JSON.stringify({ success: true, escalation_id: 42, notification_sent: true }); + expect(enforceOutcomeClaims("I've flagged this.", [], history).text).toBe(DIRECT_SUPPORT); + }); + + it('recognizes a generic request creation promise in a guest registration conversation', () => { + expect(enforceOutcomeClaims("I'll create a request for you.", [], 'Registration is broken; can you help?').text) + .toBe(DIRECT_SUPPORT); + }); + + it('does not use a GitHub receipt to authorize a team notification', () => { + const github = execution('GitHub issue created', { + tool_name: 'create_github_issue', + github_issue_receipt: { + toolName: 'create_github_issue', issueNumber: 42, + issueUrl: 'https://github.com/adcontextprotocol/adcp/issues/42', + }, + }); + expect(enforceOutcomeClaims("I've notified the team about the GitHub issue.", [github]).text).toBe(DIRECT_SUPPORT); + }); + + it('renders only the persisted ID and independently confirmed notification', () => { + expect(enforceOutcomeClaims("I've escalated this as ticket #999 and notified the team.", [receipt(false)]).text) + .toBe('Support request #42 is saved. I could not confirm a team notification.'); + expect(enforceOutcomeClaims("I'll flag this.", [receipt()]).text) + .toBe('Support request #42 is saved. The team notification was sent.'); + }); + + it('preserves helpful instructions around an unsupported promise', () => { + const result = enforceOutcomeClaims("Use the same browser to retry registration. I'll flag it. You can keep learning while support investigates.", []); + expect(result.text).toContain('Use the same browser to retry registration.'); + expect(result.text).toContain(DIRECT_SUPPORT); + expect(result.text).toContain('You can keep learning'); + }); +}); diff --git a/server/tests/unit/addie/tool-error-containment.test.ts b/server/tests/unit/addie/tool-error-containment.test.ts new file mode 100644 index 00000000000..24646b49e56 --- /dev/null +++ b/server/tests/unit/addie/tool-error-containment.test.ts @@ -0,0 +1,101 @@ +import { describe, expect, it } from 'vitest'; +import { countContainedToolErrors, type ToolExecution } from '../../../src/addie/model-providers/tool-orchestration.js'; + +function execution(operation: string, failed: boolean, overrides: Partial = {}): ToolExecution { + return { + tool_name: 'call_adcp_task', + parameters: { task: operation, agent_url: 'https://agent.example/mcp', params: { idempotency_key: 'discovery-request-key' } }, + result: failed ? 'The parameters need correction.' : 'Verified result.', + is_error: failed, + duration_ms: 1, + sequence: failed ? 1 : 2, + normalized_result: { + status: failed ? 'invalid_input' : 'ok', + user_summary: failed ? 'The parameters need correction.' : 'Verified result.', + source: 'structured', + telemetry: { operation, ...(failed && { error_category: 'validation' }) }, + }, + ...overrides, + }; +} + +describe('observed tool error containment', () => { + it('contains unsupported proposals followed by same-target product discovery without marking proposals recovered', () => { + const rejected = execution('request_proposals', true); + const discovery = execution('get_products', false); + expect(countContainedToolErrors([rejected, discovery], true)).toBe(1); + expect(rejected.normalized_result?.telemetry?.recovered_by_later_success).toBeUndefined(); + }); + + it('contains corrected discovery with a different idempotency key without claiming exact recovery', () => { + const rejected = execution('get_products', true); + const corrected = execution('get_products', false, { + tool_name: 'call_adcp_get_products', + parameters: { agent_url: 'https://agent.example/mcp', idempotency_key: 'a-different-request-key' }, + }); + expect(countContainedToolErrors([rejected, corrected], true)).toBe(1); + expect(rejected.normalized_result?.telemetry?.recovered_by_later_success).toBeUndefined(); + }); + + it('does not double-count exactly recovered failures as contained', () => { + const rejected = execution('get_products', true); + rejected.normalized_result!.telemetry!.recovered_by_later_success = true; + expect(countContainedToolErrors([rejected, execution('get_products', false)], true)).toBe(0); + }); + + it.each(['get_adcp_capabilities', 'list_creative_formats', 'get_media_buys'])( + 'observes same-agent continuation with %s without depending on a hardcoded task pair', operation => { + expect(countContainedToolErrors([ + execution('request_proposals', true), execution(operation, false), + ], true)).toBe(1); + }, + ); + + it.each([ + ['another target', { parameters: { task: 'get_products', agent_url: 'https://other.example/mcp' } }], + ['another tenant', { parameters: { task: 'get_products', agent_url: 'https://agent.example/mcp/tenant-b' } }], + ['a failed read', { is_error: true }], + ['a policy-blocked read', { blocked_by_policy: true }], + ['an arbitrary tool', { tool_name: 'search_docs' }], + ['a missing operation', { normalized_result: { status: 'ok', user_summary: 'Done.', source: 'structured' } }], + ['an empty result', { normalized_result: { status: 'empty', user_summary: 'None.', source: 'structured', telemetry: { operation: 'get_products' } } }], + ['mismatched operation metadata', { parameters: { task: 'list_creative_formats', agent_url: 'https://agent.example/mcp' } }], + ] satisfies Array<[string, Partial]>)( + 'leaves errors unresolved after %s', (_label, overrides) => { + expect(countContainedToolErrors([ + execution('request_proposals', true), execution('get_products', false, overrides), + ], true)).toBe(0); + }, + ); + + it('does not use a later successful mutation as containment evidence', () => { + expect(countContainedToolErrors([ + execution('request_proposals', true), execution('create_media_buy', false), + ], true)).toBe(0); + }); + + it.each(['transport', 'authentication', 'authorization', 'application', 'unknown'] as const)( + 'leaves %s errors unresolved despite successful subsequent reads', category => { + const failed = execution('get_products', true); + failed.normalized_result!.telemetry!.error_category = category; + expect(countContainedToolErrors([failed, execution('get_products', false)], true)).toBe(0); + }, + ); + + it('requires a complete answer and subsequent read, rather than prose alone or an earlier success', () => { + const failed = execution('request_proposals', true); + const success = execution('get_products', false); + expect(countContainedToolErrors([failed, success], false)).toBe(0); + expect(countContainedToolErrors([success, failed], true)).toBe(0); + expect(countContainedToolErrors([failed], true)).toBe(0); + }); + + it('does not correlate missing targets or policy rejections', () => { + const failed = execution('request_proposals', true, { parameters: { task: 'request_proposals' } }); + const success = execution('get_products', false, { parameters: { task: 'get_products' } }); + expect(countContainedToolErrors([failed, success], true)).toBe(0); + expect(countContainedToolErrors([ + execution('request_proposals', true, { blocked_by_policy: true }), execution('get_products', false), + ], true)).toBe(0); + }); +}); diff --git a/server/tests/unit/certification-attempt-recovery.test.ts b/server/tests/unit/certification-attempt-recovery.test.ts index 878ee0143f3..245ee689110 100644 --- a/server/tests/unit/certification-attempt-recovery.test.ts +++ b/server/tests/unit/certification-attempt-recovery.test.ts @@ -42,6 +42,7 @@ vi.mock('../../src/db/client.js', () => ({ getPool: vi.fn(), })); +import { enforceOutcomeClaims } from '../../src/addie/outcome-claims.js'; import { createCertificationToolHandlers } from '../../src/addie/mcp/certification-tools.js'; const USER_ID = 'user_attempt_recovery'; @@ -66,6 +67,38 @@ describe('certification attempt recovery', () => { mocks.hasEffectiveMembershipForUser.mockResolvedValue(true); }); + it('rejects B2 at 3/4 exchanges and requires a persisted retry after the fourth', async () => { + const scores = { protocol_mastery: 85 }; + mocks.getModule.mockResolvedValue({ + id: 'B2', is_free: true, + assessment_criteria: { dimensions: [{ name: 'protocol_mastery', weight: 100 }], passing_threshold: 70 }, + exercise_definitions: [], + }); + mocks.getProgress.mockResolvedValue([{ module_id: 'B2', status: 'in_progress', started_at: new Date(Date.now() - 600_000).toISOString() }]); + mocks.getLatestCheckpoint.mockResolvedValue({ preliminary_scores: scores, demonstrations_verified: [] }); + mocks.query.mockResolvedValue({ rows: [{ count: '3' }] }); + const complete = createCertificationToolHandlers(memberContext(), { threadId: 'thread_b2' }).get('complete_certification_module')!; + const rejected = await complete({ module_id: 'B2', scores }); + expect(rejected).toContain('at least 4 conversation exchanges'); + expect(rejected).toContain('Only 3 detected'); + expect(mocks.completeModule).not.toHaveBeenCalled(); + + // A later learner exchange is evidence for retry eligibility, not a write receipt. + mocks.query.mockResolvedValue({ rows: [{ count: '4' }] }); + expect(enforceOutcomeClaims('B2 is concluded.', [], String(rejected)).reason).toBe('Unconfirmed certification completion'); + expect(mocks.completeModule).not.toHaveBeenCalled(); + + mocks.completeModule.mockResolvedValue({ module_id: 'B2', status: 'completed' }); + const saved = await complete({ module_id: 'B2', scores }); + expect(mocks.completeModule).toHaveBeenCalledExactlyOnceWith(USER_ID, 'B2', scores); + const delivered = enforceOutcomeClaims('B2 is concluded.', [{ + tool_name: 'complete_certification_module', parameters: { module_id: 'B2' }, + result: String(saved), is_error: false, duration_ms: 1, + }]); + expect(delivered.text).toBe('B2 is recorded as complete.'); + expect(delivered.reason).toBeNull(); + }); + it('surfaces an in-progress specialist attempt across handler sessions', async () => { mocks.getProgress.mockResolvedValue([ { module_id: 'S3', status: 'in_progress' }, diff --git a/server/tests/unit/claude-client-replay-policy.test.ts b/server/tests/unit/claude-client-replay-policy.test.ts index 64b4b292115..bbc6c9b6e8a 100644 --- a/server/tests/unit/claude-client-replay-policy.test.ts +++ b/server/tests/unit/claude-client-replay-policy.test.ts @@ -1295,3 +1295,75 @@ describe('AddieClaudeClient isolated execution policy', () => { expect(prospects.system_blocks[2].sha256).toBe(organizations.system_blocks[2].sha256); }); }); + +describe('persisted outcome delivery guard', () => { + it.each(['streaming', 'non_streaming'] as const)('contains rejected completion and a later no-retry claim before %s delivery', async delivery => { + const handler = vi.fn().mockResolvedValue('NOT COMPLETED — module B2 is not recorded as complete. Only 3/4 conversation exchanges detected.'); + const scoped = requestTools([tool('complete_certification_module', 'mutation')], [['complete_certification_module', handler]]); + const client = new AddieClaudeClient('unused'); + const queue = delivery === 'streaming' ? sdkState.streamingResponses : sdkState.nonStreamingResponses; + const replies = [ + toolUseResponse([{ id: 'complete-b2', name: 'complete_certification_module', input: { module_id: 'B2' } }]), + textResponse('One more example will help. Why does discovery precede buying?'), + textResponse('B2 is concluded. How would you use those products in a plan?'), + ]; + queue.push(...replies); + const options: ProcessMessageOptions = { uncapped: true, disableServerTools: true, reserveSideEffect: async () => {} }; + if (delivery === 'streaming') { + const first = []; + for await (const event of client.processMessageStream('Finish B2', undefined, scoped, options)) first.push(event); + const next: StreamEvent[] = []; + for await (const event of client.processMessageStream('I would discover products first.', [ + { user: 'Addie', text: 'B2 needs another exchange; completion was not recorded.' }, + ], scoped, options)) next.push(event); + const emitted = next.filter(event => event.type === 'text').map(event => event.text).join(''); + const final = next.find(event => event.type === 'done'); + expect(emitted).not.toContain('B2 is concluded'); + expect(final?.type === 'done' && final.response.text).toContain("haven't confirmed a saved completion"); + expect(final?.type === 'done' && final.response.text).toContain('How would you use those products'); + } else { + await client.processMessage('Finish B2', undefined, scoped, { systemPrompt: 'system' }, options); + const next = await client.processMessage('I would discover products first.', [ + { user: 'Addie', text: 'B2 needs another exchange; completion was not recorded.' }, + ], scoped, { systemPrompt: 'system' }, options); + expect(next.text).not.toContain('B2 is concluded'); + expect(next.text).toContain("haven't confirmed a saved completion"); + expect(next.text).toContain('How would you use those products'); + } + expect(handler).toHaveBeenCalledTimes(1); + }); + + it.each(['streaming', 'non_streaming'] as const)('requires an actual escalation receipt on %s delivery', async delivery => { + const client = new AddieClaudeClient('unused'); + const reply = textResponse("I'll flag this registration problem for the team."); + const options: ProcessMessageOptions = { uncapped: true, disableServerTools: true }; + if (delivery === 'streaming') { + sdkState.streamingResponses.push(reply); + const events: StreamEvent[] = []; + for await (const event of client.processMessageStream('Registration is broken.', undefined, undefined, options)) events.push(event); + const emitted = events.filter(event => event.type === 'text').map(event => event.text).join(''); + const final = events.find(event => event.type === 'done'); + expect(emitted).not.toContain("I'll flag"); + expect(final?.type === 'done' && final.response.text).toContain('support@agenticadvertising.org'); + } else { + sdkState.nonStreamingResponses.push(reply); + const response = await client.processMessage('Registration is broken.', undefined, undefined, { systemPrompt: 'system' }, options); + expect(response.text).not.toContain("I'll flag"); + expect(response.text).toContain('support@agenticadvertising.org'); + } + }); + + it('delivers confirmed completion from the real executor receipt', async () => { + const handler = vi.fn().mockResolvedValue('Module B2 completed! The learner has demonstrated mastery of all learning objectives.'); + sdkState.nonStreamingResponses.push( + toolUseResponse([{ id: 'complete-b2', name: 'complete_certification_module', input: { module_id: 'B2' } }]), + textResponse('You have mastered B2.'), + ); + const result = await new AddieClaudeClient('unused').processMessage('Finish B2', undefined, + requestTools([tool('complete_certification_module', 'mutation')], [['complete_certification_module', handler]]), + { systemPrompt: 'system' }, { uncapped: true, disableServerTools: true, reserveSideEffect: async () => {} }); + expect(handler).toHaveBeenCalledTimes(1); + expect(result.text).toBe('B2 is recorded as complete.'); + expect(result.flagged).toBe(false); + }); +}); diff --git a/tests/addie/member-tools.test.ts b/tests/addie/member-tools.test.ts index f210f8f3f80..f203e86b5e3 100644 --- a/tests/addie/member-tools.test.ts +++ b/tests/addie/member-tools.test.ts @@ -19,6 +19,12 @@ const memberToolMocks = vi.hoisted(() => ({ getComplianceStoryboardById: vi.fn(), runStoryboard: vi.fn(), runStoryboardStep: vi.fn(), + logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() }, +})); + +vi.mock('../../server/src/logger.js', async (importOriginal) => ({ + ...await importOriginal() as Record, + createLogger: () => memberToolMocks.logger, })); vi.mock('../../server/src/services/pipes.js', () => ({ @@ -80,6 +86,7 @@ beforeEach(() => { memberToolMocks.getComplianceStoryboardById.mockReset(); memberToolMocks.runStoryboard.mockReset(); memberToolMocks.runStoryboardStep.mockReset(); + for (const log of Object.values(memberToolMocks.logger)) log.mockClear(); }); describe('MEMBER_TOOLS definitions', () => { @@ -1350,6 +1357,15 @@ describe('createMemberToolHandlers', () => { displayAgentUrl: 'https://seller.example.com/mcp', }), ); + const lifecycleFields = memberToolMocks.logger.info.mock.calls + .map(([fields]) => fields) + .filter(fields => fields.event === 'agent_quality_evaluation'); + expect(lifecycleFields).toEqual([ + expect.objectContaining({ phase: 'admission', outcome: 'owned', recoveredExpiredLease: false }), + expect.objectContaining({ phase: 'publication', publication: 'canonical_committed', canonicalRunId: 'run_123' }), + expect.objectContaining({ phase: 'finalization', outcome: 'completed', publication: 'canonical_committed' }), + ]); + expect(JSON.stringify(lifecycleFields)).not.toMatch(/opaque|seller\.example|lease.token|auth.scope|request.key/i); }); it.each(['database error', 'lease lost'])('preserves a committed canonical receipt after completion %s', async failure => { @@ -1370,6 +1386,13 @@ describe('createMemberToolHandlers', () => { expect(result).not.toContain('**Evidence status:** AUTHORITATIVE'); expect(result).toContain(failure === 'database error' ? 'Evaluation completion unconfirmed' : 'Evaluation ownership changed'); expect(memberToolMocks.recordAgentTestRun).not.toHaveBeenCalled(); + expect(memberToolMocks.logger.warn).toHaveBeenCalledWith(expect.objectContaining({ + event: 'agent_quality_evaluation', + phase: 'finalization', + outcome: failure === 'database error' ? 'unconfirmed' : 'lease_lost', + publication: 'canonical_committed', + canonicalRunId: 'run_123', + }), expect.any(String)); }); it('does not claim public publication for a committed partial audit receipt', async () => { @@ -1388,6 +1411,9 @@ describe('createMemberToolHandlers', () => { expect(result).not.toContain('The canonical compliance result was already committed'); expect(result).toContain('**Completeness:** timed_out'); expect(memberToolMocks.runBadgeFanOut).not.toHaveBeenCalled(); + expect(memberToolMocks.logger.info).toHaveBeenCalledWith(expect.objectContaining({ + phase: 'publication', publication: 'audit_only_committed', canonicalRunId: 'run_123', + }), expect.any(String)); }); it('reports ambiguous canonical write acknowledgement without claiming no publication', async () => { @@ -1403,6 +1429,9 @@ describe('createMemberToolHandlers', () => { expect(result).not.toContain('did not publish a result'); expect(result).not.toContain('**Saved compliance run:**'); expect(result).toContain('All applicable tracks passed'); + expect(memberToolMocks.logger.warn).toHaveBeenCalledWith(expect.objectContaining({ + phase: 'finalization', outcome: 'unconfirmed', publication: 'unconfirmed', canonicalRunId: null, + }), expect.any(String)); }); it('keeps publication uncertainty visible even when execution completion is recorded', async () => { @@ -1488,9 +1517,56 @@ describe('createMemberToolHandlers', () => { expect(result).toContain('database-recorded start time'); expect(result).toContain('No new evaluation was started'); expect(memberToolMocks.comply).not.toHaveBeenCalled(); + expect(memberToolMocks.logger.info).toHaveBeenCalledWith(expect.objectContaining({ + event: 'agent_quality_evaluation', + phase: 'admission', + evaluationId: '10000000-0000-4000-8000-000000000002', + outcome: 'coalesced', + recoveredExpiredLease: false, + }), expect.any(String)); + expect(memberToolMocks.logger.info.mock.calls.filter(([fields]) => + fields.event === 'agent_quality_evaluation')).toHaveLength(1); expect(memberToolMocks.checkToolRateLimit).not.toHaveBeenCalled(); }); + it('distinguishes a recovered expired lease from a coalesced observation before a safety rejection', async () => { + const claim = vi.mocked(AgentQualityEvaluationDatabase.prototype.claimOrObserve); + const owned = await claim({} as never); + claim.mockClear(); + claim.mockResolvedValueOnce({ ...owned, owned: true, recoveredExpiredLease: true } as never); + memberToolMocks.checkToolRateLimit.mockResolvedValueOnce({ ok: false, retryAfterMs: 1_000 }); + + const result = await createMemberToolHandlers(ownerContext).get('evaluate_agent_quality')!({ + agent_url: 'https://seller.example.com/mcp', + compliance_target: '3.0', + }); + + expect(result).toContain('Rate limit exceeded'); + expect(memberToolMocks.comply).not.toHaveBeenCalled(); + expect(memberToolMocks.logger.info).toHaveBeenCalledWith(expect.objectContaining({ + phase: 'admission', outcome: 'owned', recoveredExpiredLease: true, + }), expect.any(String)); + expect(memberToolMocks.logger.info).toHaveBeenCalledWith(expect.objectContaining({ + phase: 'finalization', outcome: 'failed', failureCode: 'rate_limited', publication: 'not_attempted', + }), expect.any(String)); + }); + + it('records a terminal lifecycle event for a persisted runner failure', async () => { + memberToolMocks.comply.mockRejectedValueOnce(new Error('Runner stopped unexpectedly')); + + await expect(createMemberToolHandlers(ownerContext).get('evaluate_agent_quality')!({ + agent_url: 'https://seller.example.com/mcp', + compliance_target: '3.0', + })).rejects.toThrow('Failed to evaluate agent quality'); + + expect(AgentQualityEvaluationDatabase.prototype.markFailed).toHaveBeenCalledWith( + expect.any(String), expect.any(String), 'evaluation_failed', + ); + expect(memberToolMocks.logger.info).toHaveBeenCalledWith(expect.objectContaining({ + phase: 'finalization', outcome: 'failed', failureCode: 'evaluation_failed', publication: 'not_attempted', + }), expect.any(String)); + }); + it('marks timed-out receipts incomplete while retaining partial findings and coverage', async () => { memberToolMocks.comply.mockResolvedValueOnce({ ...makeComplianceResult(['3.0']), From 065718b2f3933f439b2aa6eed96e082f1e5507ab Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Wed, 23 Sep 2026 01:46:35 +0000 Subject: [PATCH 2/4] docs(addie): add audit runbook metadata --- docs/runbooks/addie-reliability-audit.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/docs/runbooks/addie-reliability-audit.md b/docs/runbooks/addie-reliability-audit.md index 39442729bad..f5e7206e879 100644 --- a/docs/runbooks/addie-reliability-audit.md +++ b/docs/runbooks/addie-reliability-audit.md @@ -1,3 +1,9 @@ +--- +title: Addie reliability audit follow-up +description: "Receipt-backed certification and support claims, recovery telemetry, evaluation lifecycle logs, and deployment guidance for Addie." +"og:title": "AdCP — Addie reliability audit follow-up" +--- + # Addie reliability audit follow-up This change starts at `0f4512093` on main, after #7649. No production queries, From 08f3168a409a6518b187050137383520f473580b Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Wed, 23 Sep 2026 02:05:34 +0000 Subject: [PATCH 3/4] fix(addie): scope outcome claims to their conversation context --- docs/runbooks/addie-reliability-audit.md | 7 ++ server/src/addie/claude-client.ts | 14 +-- .../tool-surface-inventory.generated.json | 2 +- server/src/addie/outcome-claims.ts | 33 +++++-- .../607_addie_tool_error_containment.sql | 4 +- .../certification-outcome-claims.test.ts | 89 +++++++++++++------ .../unit/addie/support-outcome-claims.test.ts | 26 +++++- .../unit/claude-client-replay-policy.test.ts | 23 ++++- server/tests/unit/member-context.test.ts | 6 ++ 9 files changed, 155 insertions(+), 49 deletions(-) diff --git a/docs/runbooks/addie-reliability-audit.md b/docs/runbooks/addie-reliability-audit.md index f5e7206e879..ff34090c071 100644 --- a/docs/runbooks/addie-reliability-audit.md +++ b/docs/runbooks/addie-reliability-audit.md @@ -40,6 +40,9 @@ recorded outcomes or replaced with a brief unconfirmed-state explanation. Other teaching sentences remain available. Credential award and external issuance are separate claims. A later exchange alone never makes a rejected completion valid. A historical completion can be confirmed by reading persisted progress again. +Bare identifiers require teaching context; ordinary Q4 or V2 conclusions do not +count as module claims. Generic sign-in/tool boilerplate cannot establish that +context; actual conversation or active certification facts can. This is a deterministic delivery backstop for the covered English outcome phrasing, not a semantic proof for every paraphrase or language. Expanding the @@ -86,6 +89,10 @@ and contention evidence, there is no scoped defect to repair here. ## Deployment and rollback Migration 607 adds one nullable containment count and consistency constraint. +The constraint is `NOT VALID`: future writes are checked immediately, while +startup avoids scanning historical rows under the schema lock. Historical rows +have the new column null and already satisfy the check. Optional validation must +run in a separate transaction, not inside the startup migration transaction. Apply migrations before the new application version (normal startup ordering). Historical rows remain unclassified; there is no speculative backfill. Rollback can restore the prior application while retaining the additive column. Do not diff --git a/server/src/addie/claude-client.ts b/server/src/addie/claude-client.ts index 33a6bd875f5..0b2a7f6a743 100644 --- a/server/src/addie/claude-client.ts +++ b/server/src/addie/claude-client.ts @@ -5,7 +5,7 @@ * with tool reference always appended from code. */ -import { enforceOutcomeClaims } from './outcome-claims.js'; +import { enforceOutcomeClaims, outcomeClaimContext } from './outcome-claims.js'; import Anthropic from '@anthropic-ai/sdk'; import { createHash, createHmac } from 'node:crypto'; import { createLogger } from '../logger.js'; @@ -2062,7 +2062,7 @@ export class AddieClaudeClient { kind: 'provider', disposition: 'truncated', userMessage, - conversationContext: [userMessage, options?.requestContext ?? '', ...(threadContext ?? []).map(entry => entry.text)].join('\n'), + conversationContext: outcomeClaimContext([userMessage, ...(threadContext ?? []).map(entry => entry.text)], options?.requestContext), githubIssueCreationRequested, clientRequestId: options?.clientRequestId, githubIssueRetryReceipts: options?.githubIssueRetryReceipts, @@ -2116,7 +2116,7 @@ export class AddieClaudeClient { kind: 'provider', disposition: 'complete', userMessage, - conversationContext: [userMessage, options?.requestContext ?? '', ...(threadContext ?? []).map(entry => entry.text)].join('\n'), + conversationContext: outcomeClaimContext([userMessage, ...(threadContext ?? []).map(entry => entry.text)], options?.requestContext), githubIssueCreationRequested, clientRequestId: options?.clientRequestId, githubIssueRetryReceipts: options?.githubIssueRetryReceipts, @@ -2180,7 +2180,7 @@ export class AddieClaudeClient { const terminal = buildTerminalAddieResponse({ kind: 'max_iterations', userMessage, - conversationContext: [userMessage, options?.requestContext ?? '', ...(threadContext ?? []).map(entry => entry.text)].join('\n'), + conversationContext: outcomeClaimContext([userMessage, ...(threadContext ?? []).map(entry => entry.text)], options?.requestContext), githubIssueCreationRequested, clientRequestId: options?.clientRequestId, githubIssueRetryReceipts: options?.githubIssueRetryReceipts, @@ -2882,7 +2882,7 @@ export class AddieClaudeClient { kind: 'provider', disposition: 'truncated', userMessage, - conversationContext: [userMessage, options?.requestContext ?? '', ...(threadContext ?? []).map(entry => entry.text)].join('\n'), + conversationContext: outcomeClaimContext([userMessage, ...(threadContext ?? []).map(entry => entry.text)], options?.requestContext), githubIssueCreationRequested, clientRequestId: options?.clientRequestId, githubIssueRetryReceipts: options?.githubIssueRetryReceipts, @@ -2939,7 +2939,7 @@ export class AddieClaudeClient { kind: 'provider', disposition: 'complete', userMessage, - conversationContext: [userMessage, options?.requestContext ?? '', ...(threadContext ?? []).map(entry => entry.text)].join('\n'), + conversationContext: outcomeClaimContext([userMessage, ...(threadContext ?? []).map(entry => entry.text)], options?.requestContext), githubIssueCreationRequested, clientRequestId: options?.clientRequestId, githubIssueRetryReceipts: options?.githubIssueRetryReceipts, @@ -3000,7 +3000,7 @@ export class AddieClaudeClient { const terminal = buildTerminalAddieResponse({ kind: 'max_iterations', userMessage, - conversationContext: [userMessage, options?.requestContext ?? '', ...(threadContext ?? []).map(entry => entry.text)].join('\n'), + conversationContext: outcomeClaimContext([userMessage, ...(threadContext ?? []).map(entry => entry.text)], options?.requestContext), githubIssueCreationRequested, clientRequestId: options?.clientRequestId, githubIssueRetryReceipts: options?.githubIssueRetryReceipts, diff --git a/server/src/addie/generated/tool-surface-inventory.generated.json b/server/src/addie/generated/tool-surface-inventory.generated.json index 5800d9dfbfa..c5a9ce77548 100644 --- a/server/src/addie/generated/tool-surface-inventory.generated.json +++ b/server/src/addie/generated/tool-surface-inventory.generated.json @@ -63,7 +63,7 @@ } }, "registration_source_sha256": { - "server/src/addie/claude-client.ts": "db7fb7f6cff9e967ea0ca2f6c3f80240ec4f36db527184019703cf97cddd7bf1", + "server/src/addie/claude-client.ts": "fe5cf39b29f9598773beab2a08235c52f5c573741dcc3f86866aba36efc79fff", "server/src/addie/request-tool-assembly.ts": "cf4443d27ec69940f22aa9fcf46fdefd3f444762259c2700ac8fa01554ead4da", "server/src/addie/request-tool-replay-binding.ts": "3aed4c0bd18032e4418b8fff7e8deb263936b6c75b27941f9818a2a9a87287f2", "server/src/addie/prompts.ts": "3ec621f3e97eb9109fb561b94ff625a0639460c84506aa63aeebd57f683790f4", diff --git a/server/src/addie/outcome-claims.ts b/server/src/addie/outcome-claims.ts index 29433cb2668..506247050c4 100644 --- a/server/src/addie/outcome-claims.ts +++ b/server/src/addie/outcome-claims.ts @@ -6,23 +6,35 @@ const CERTIFICATION_TOOLS = new Set([ 'get_learner_progress', 'check_credentials', ]); const MODULE_ID = /\b[A-Z]{1,2}\d{1,2}\b/gi; -const CERTIFICATION_CONTEXT = /\b(?:certification|capstone|module|credential|certificate|[A-Z]{1,2}\d{1,2})\b/i; +const CERTIFICATION_CONTEXT = /\b(?:certification|capstone|credential|certificate)\b|\bmodule\s+[A-Z]{1,2}\d{1,2}\b/i; const COMPLETION = /\b(?:completed?|concludes?|concluded|finished|mastered|passed|earned|certified|awarded|issued|done|locked in|in the books|wrapped up|wraps? up|you(?:'re| are) through)\b/gi; const CREDENTIAL = /\b(?:credential|certificate|badge|certified|certification)\b/i; const ESCALATION_CLAIM = /\b(?:I(?:'ve| have| just| will|'ll| am going to|'m going to|'m| am)?|we(?:'ve| have| will|'ll| are going to|'re going to|'re| are)?)\s+(?:(?:will|have|already|just)\s+)?(flag(?:ged|ging)?|escalat(?:e|ed|ing)|notif(?:y|ied|ying)|rais(?:e|ed|ing)|fil(?:e|ed|ing)|creat(?:e|ed|ing)|open(?:ed|ing)?|send(?:ing)?|sent|pass(?:ed|ing)?|forward(?:ed|ing)?|hand(?:ed|ing)?|contact(?:ed|ing)?|reach(?:ed|ing)? out)\b/i; const SUPPORT_OBJECT = /\b(?:ticket|support request|team|admins?|support|github issue)\b/i; +const SUPPORT_DESTINATION = /\b(?:ticket|support request|github issue)\b|\b(?:for|to|with)\s+(?:(?:the|our|your|support)\s+){0,2}(?:team|admins?|support)\b/i; const PASSIVE_ESCALATION = /\b(?:(?:team|admins?) (?:has been|have been|will be|is being|was|were) (?:notified|alerted)|(?:ticket|support request|escalation)(?: #\d+)? (?:has been |was |is |will be )?(?:created|filed|opened|raised|saved)|(?:this|it|issue|request|problem|bug) (?:has been|was|will be|is being) (?:flagged|escalated|forwarded|passed|sent))\b/gi; const GUARANTEED_NOTIFICATION = /\b(?:I(?:'ll| will)|we(?:'ll| will))\s+(?:make sure|ensure|let)\b/i; export const UNCONFIRMED_CERTIFICATION = "You're making progress. I haven't confirmed a saved completion for this module or credential yet. We can keep working; completion needs to be recorded before I can confirm it."; export const DIRECT_SUPPORT = "I haven't created or escalated a support request. For help, email support@agenticadvertising.org with a brief description of what went wrong. Don't include passwords or sign-in codes."; +/** Generic tool/sign-in boilerplate is not evidence of a teaching conversation. */ +export function outcomeClaimContext(messages: readonly string[], requestContext = ''): string { + const activeCertification = /^## Active certification modules\r?$/m.test(requestContext) + || /^### Certification\r?\nCurrently working on:/m.test(requestContext); + return [...messages, ...(activeCertification ? ['Active certification module.'] : [])].join('\n'); +} + /** Qualifiers apply to the claimed predicate, not every other clause in a sentence. */ function hasSupportClaim(text: string, context: string): boolean { - const action = ESCALATION_CLAIM.exec(text)?.[1]?.toLowerCase(); - const contextualRequest = /\b(?:registration|sign[ -]?up|verification|support)\b/i.test(context) - && /\b(?:request|issue)\b/i.test(text); - if (action && (action.startsWith('flag') || action.startsWith('escalat') || SUPPORT_OBJECT.test(text) || contextualRequest)) return true; + const actionMatch = ESCALATION_CLAIM.exec(text); + const action = actionMatch?.[1]?.toLowerCase(); + const supportContext = /\b(?:registration|sign[ -]?up|verification|support)\b/i.test(`${context}\n${text}`); + const directObject = actionMatch ? text.slice(actionMatch.index + actionMatch[0].length) : ''; + const contextualRequest = supportContext && /^\s+(?:(?:this|that|it)|(?:(?:this|the|your|an?)\s+)?(?:(?:registration|sign[ -]?up|verification|support)\s+)?(?:issue|problem|bug|request))(?=\s*(?:[.!?,;:]|$)|\s+(?:because|for you)\b)/i.test(directObject); + if (action && (action.startsWith('escalat') + || (action.startsWith('flag') ? SUPPORT_DESTINATION.test(text) : SUPPORT_OBJECT.test(text)) + || contextualRequest)) return true; if (GUARANTEED_NOTIFICATION.test(text) && /\b(?:team|admins?|support)\b/i.test(text) && /\b(?:hears?|sees?|receives?|notified|alerted|knows?)\b/i.test(text)) return true; for (const match of text.matchAll(PASSIVE_ESCALATION)) { @@ -31,6 +43,9 @@ function hasSupportClaim(text: string, context: string): boolean { if (/\b(?:can't|cannot|couldn't|could not|haven't|have not|unable to)\s+(?:confirm|verify|say)\s+(?:(?:that|whether)\s+)?(?:(?:the|a)\s+)?$/i.test(prefix)) continue; if (/\b(?:not sure|unclear)\s+(?:(?:that|whether|if)\s+)?(?:(?:the|a)\s+)?$/i.test(prefix)) continue; if (/(?:^|[,;:]\s*)\s*(?:if|whether|when|once|until)\s+(?:(?:the|a)\s+)?$/i.test(prefix)) continue; + const genericAction = /\b(?:flagged|forwarded|passed|sent)$/i.test(match[0]); + if (genericAction && !SUPPORT_DESTINATION.test(text) + && !(supportContext && /^\s*(?:[.!?,;:]|$|because\b)/i.test(text.slice(match.index + match[0].length)))) continue; return true; } return false; @@ -151,7 +166,7 @@ export function enforceOutcomeClaims( for (const [index, part] of parts.entries()) { if (index % 2 === 1) { output.push(part); continue; } const plain = part.replace(/[*_`]/g, '').replace(/[’‘]/g, "'"); - if (hasSupportClaim(plain, conversationContext)) { + if (hasSupportClaim(plain, `${conversationContext}\n${text}${support ? '\nSupport request receipt.' : ''}`)) { if (!supportReplaced) output.push(support ? `Support request #${support.id} is saved.${support.notified ? ' The team notification was sent.' : ' I could not confirm a team notification.'}` : DIRECT_SUPPORT); @@ -163,10 +178,10 @@ export function enforceOutcomeClaims( const teachingSubtask = ids.length === 0 && !CREDENTIAL.test(plain) && !/\b(?:module|capstone|mastery)\b/i.test(plain) && [...plain.matchAll(COMPLETION)].length === 1 - && /\b(?:completed?|finished|passed|mastered)\s+(?:(?:the|this|that|an?|your)\s+)?(?:example|exercise|question|practice|request|media buy|deployment)\b/i.test(plain); + && /\b(?:completed?|finished|passed|mastered)\s+(?:(?:the|this|that|an?|your)\s+)?(?:example|exercise|question|practice|tutorial|request|media buy|deployment)\b/i.test(plain); const isCertificationClaim = !teachingSubtask && assertsCertificationOutcome(plain) - && (ids.length > 0 || CREDENTIAL.test(plain) || /\b(?:module|capstone)\b/i.test(plain) - || (certificationContext && /\b(?:you|your|we|that|this|it)\b/i.test(plain))); + && (CREDENTIAL.test(plain) || /\bcapstone\b|\bmodule\s+[A-Z]{1,2}\d{1,2}\b/i.test(plain) + || (certificationContext && (ids.length > 0 || /\b(?:module|you|your|we|that|this|it)\b/i.test(plain)))); if (isCertificationClaim) { const credentialClaim = CREDENTIAL.test(plain); const externalIssuance = /\b(?:issued|sent|delivered|download|share|ready)\b/i.test(plain); diff --git a/server/src/db/migrations/607_addie_tool_error_containment.sql b/server/src/db/migrations/607_addie_tool_error_containment.sql index 9a9748583ca..4e32639abea 100644 --- a/server/src/db/migrations/607_addie_tool_error_containment.sql +++ b/server/src/db/migrations/607_addie_tool_error_containment.sql @@ -2,6 +2,8 @@ -- failures contained by a successful same-agent read and complete answer. -- NULL preserves the unknown classification of historical/incomplete turns; -- never retrospectively relabel them as unresolved or infer from prose. +-- NOT VALID avoids scanning historical rows under the startup schema lock. +-- All old rows have the new column NULL; new writes are checked immediately. ALTER TABLE addie_chat_experiment_turns ADD COLUMN contained_tool_errors INTEGER, ADD CONSTRAINT addie_chat_experiment_tool_containment_valid CHECK ( @@ -11,7 +13,7 @@ ALTER TABLE addie_chat_experiment_turns AND tool_errors IS NOT NULL AND recovered_tool_errors + unrecovered_tool_errors = tool_errors ) - ); + ) NOT VALID; COMMENT ON COLUMN addie_chat_experiment_turns.contained_tool_errors IS 'Observed validation rejection followed by successful same-agent AdCP read and complete generated answer; not exact recovery or proof of fulfillment. NULL means unclassified. Remaining unresolved errors = unrecovered_tool_errors - contained_tool_errors. Delivery is tracked separately.'; diff --git a/server/tests/unit/addie/certification-outcome-claims.test.ts b/server/tests/unit/addie/certification-outcome-claims.test.ts index 63f2b208d1f..7c432ba5e9c 100644 --- a/server/tests/unit/addie/certification-outcome-claims.test.ts +++ b/server/tests/unit/addie/certification-outcome-claims.test.ts @@ -1,13 +1,48 @@ import { describe, expect, it } from 'vitest'; -import { enforceOutcomeClaims, UNCONFIRMED_CERTIFICATION } from '../../../src/addie/outcome-claims.js'; +import { enforceOutcomeClaims, outcomeClaimContext, UNCONFIRMED_CERTIFICATION } from '../../../src/addie/outcome-claims.js'; import type { ToolExecution } from '../../../src/addie/model-providers/tool-orchestration.js'; function execution(tool_name: string, result: string, is_error = false): ToolExecution { return { tool_name, result, is_error, parameters: {}, duration_ms: 1 }; } +// These fixtures take place in a teaching conversation. +function enforceCertificationClaims(text: string, executions: readonly ToolExecution[], context = 'B2 certification') { + return enforceOutcomeClaims(text, executions, context); +} const completed = execution('complete_certification_module', 'Module B2 completed! The learner has demonstrated mastery of all learning objectives.'); describe('receipt-bound certification claims', () => { + it.each(['Q4', 'H1', 'H2', 'V2', 'P1', 'S3', 'b2'])( + 'preserves ordinary identifiers outside a teaching conversation: %s', id => { + const text = `Your ${id} migration is complete.`; + const context = outcomeClaimContext([`How is the ${id} migration?`]); + expect(enforceOutcomeClaims(text, [], context).text).toBe(text); + }, + ); + + it.each([ + '## Active certification modules\n- B2 (in progress)', + '### Certification\nCurrently working on: B2 (in progress, started 1 days ago).', + ])('uses active teaching facts without treating them as completion receipts', requestContext => { + const context = outcomeClaimContext(['How did I do?'], requestContext); + expect(enforceOutcomeClaims('b2 is concluded.', [], context).reason).toBeTruthy(); + }); + + it('requires receipts for explicit module claims even without prior context', () => { + expect(enforceOutcomeClaims('Module B2 is complete.', []).reason).toBeTruthy(); + }); + + it.each(['The get_products module needs a tutorial.', 'Update the reporting module.', 'The S3 migration uses v2.'])( + 'does not infer certification from software context: %s', context => { + const text = "You've completed the tutorial."; + expect(enforceOutcomeClaims(text, [], context).text).toBe(text); + expect(enforceOutcomeClaims('You are done.', [], context).text).toBe('You are done.'); + for (const conclusion of ['The reporting module is complete.', 'The get_products module is complete.']) { + expect(enforceOutcomeClaims(conclusion, [], context).text).toBe(conclusion); + } + }, + ); + it.each([ 'B2 is complete.', 'B2 is concluded.', 'B2 is done.', 'B2 mastery is confirmed.', "The credential's yours.", 'Your Foundations certificate is ready to download.', 'Your certification has been awarded.', @@ -18,73 +53,73 @@ describe('receipt-bound certification claims', () => { 'We finished B2.', 'Congratulations, you passed B2!', 'Your AdCP Practitioner credential is earned.', 'You are now certified!', ])('replaces an unconfirmed claim: %s', text => { - expect(enforceOutcomeClaims(text, []).text).toBe(UNCONFIRMED_CERTIFICATION); + expect(enforceCertificationClaims(text, []).text).toBe(UNCONFIRMED_CERTIFICATION); }); it('blocks rejected completion, then another exchange with no retry', () => { const rejection = execution('complete_certification_module', 'NOT COMPLETED — module B2 is not recorded as complete. Only 3 of 4 exchanges detected.'); - expect(enforceOutcomeClaims('B2 is concluded.', [rejection]).reason).toBeTruthy(); - const followup = enforceOutcomeClaims('That concludes this module. Why does product discovery precede buying?', [], 'B2: one more exchange is needed.'); + expect(enforceCertificationClaims('B2 is concluded.', [rejection]).reason).toBeTruthy(); + const followup = enforceCertificationClaims('That concludes this module. Why does product discovery precede buying?', [], rejection.result); expect(followup.text).toContain(UNCONFIRMED_CERTIFICATION); expect(followup.text).toContain('Why does product discovery precede buying?'); expect(followup.text).not.toContain('That concludes'); }); it('never treats historical assistant or user assertions as persisted evidence', () => { - expect(enforceOutcomeClaims('B2 is complete.', [], 'Addie: Module B2 completed! User: B2 is complete.').reason).toBeTruthy(); + expect(enforceCertificationClaims('B2 is complete.', [], 'Addie: Module B2 completed! User: B2 is complete.').reason).toBeTruthy(); }); it('requires the exact module, successful persistence receipt, and trusted tool', () => { - expect(enforceOutcomeClaims('B2 is complete.', [completed]).reason).toBeNull(); - expect(enforceOutcomeClaims('B3 is complete.', [completed]).reason).toBeTruthy(); - expect(enforceOutcomeClaims('B2 and B3 are complete.', [completed]).reason).toBeTruthy(); - expect(enforceOutcomeClaims('B2 is complete.', [{ ...completed, is_error: true }]).reason).toBeTruthy(); - expect(enforceOutcomeClaims('B2 is complete.', [{ ...completed, tool_name: 'search_docs' }]).reason).toBeTruthy(); - expect(enforceOutcomeClaims('B2 is complete.', [execution('complete_certification_module', 'NOT COMPLETED\nModule B2 completed!')]).reason).toBeTruthy(); + expect(enforceCertificationClaims('B2 is complete.', [completed]).reason).toBeNull(); + expect(enforceCertificationClaims('B3 is complete.', [completed]).reason).toBeTruthy(); + expect(enforceCertificationClaims('B2 and B3 are complete.', [completed]).reason).toBeTruthy(); + expect(enforceCertificationClaims('B2 is complete.', [{ ...completed, is_error: true }]).reason).toBeTruthy(); + expect(enforceCertificationClaims('B2 is complete.', [{ ...completed, tool_name: 'search_docs' }]).reason).toBeTruthy(); + expect(enforceCertificationClaims('B2 is complete.', [execution('complete_certification_module', 'NOT COMPLETED\nModule B2 completed!')]).reason).toBeTruthy(); }); it('accepts authoritative progress and test-out state', () => { const progress = execution('get_learner_progress', '# Your certification progress\n\n## Module details\n- B2: completed\n- B3: in progress'); - expect(enforceOutcomeClaims('B2 is complete.', [progress]).reason).toBeNull(); - expect(enforceOutcomeClaims('B3 is complete.', [progress]).reason).toBeTruthy(); + expect(enforceCertificationClaims('B2 is complete.', [progress]).reason).toBeNull(); + expect(enforceCertificationClaims('B3 is complete.', [progress]).reason).toBeTruthy(); const testout = execution('test_out_modules', 'Marked 1 module(s) as tested out:\n\n- B2: tested out\n\nAssessment notes: Module B3 completed!'); - expect(enforceOutcomeClaims('B2 is complete.', [testout]).reason).toBeNull(); - expect(enforceOutcomeClaims('B3 is complete.', [testout]).reason).toBeTruthy(); + expect(enforceCertificationClaims('B2 is complete.', [testout]).reason).toBeNull(); + expect(enforceCertificationClaims('B3 is complete.', [testout]).reason).toBeTruthy(); }); it('distinguishes passing an attempt from persisting the module', () => { const passed = execution('complete_certification_exam', '# Congratulations! The learner passed the capstone!'); - expect(enforceOutcomeClaims('S3 is complete.', [passed]).reason).toBeTruthy(); - expect(enforceOutcomeClaims('S3 is complete.', [{ ...passed, result: `${passed.result}\nModule S3 completed!` }]).reason).toBeNull(); + expect(enforceCertificationClaims('S3 is complete.', [passed]).reason).toBeTruthy(); + expect(enforceCertificationClaims('S3 is complete.', [{ ...passed, result: `${passed.result}\nModule S3 completed!` }]).reason).toBeNull(); }); it('requires independent credential award and external issuance receipts', () => { const awarded = { ...completed, result: `${completed.result}\n**Credential earned: AdCP Practitioner!**` }; - expect(enforceOutcomeClaims('You earned the AdCP Practitioner credential.', [completed]).reason).toBeTruthy(); - expect(enforceOutcomeClaims('You earned the AdCP Practitioner credential.', [awarded]).reason).toBeNull(); - const extra = enforceOutcomeClaims('You earned the AdCP Practitioner and Foundations certification.', [awarded]); + expect(enforceCertificationClaims('You earned the AdCP Practitioner credential.', [completed]).reason).toBeTruthy(); + expect(enforceCertificationClaims('You earned the AdCP Practitioner credential.', [awarded]).reason).toBeNull(); + const extra = enforceCertificationClaims('You earned the AdCP Practitioner and Foundations certification.', [awarded]); expect(extra.text).not.toContain('Foundations'); expect(extra.text).toContain('Credential earned: AdCP Practitioner.'); - expect(enforceOutcomeClaims('B3 is completed and you earned the AdCP Practitioner credential.', [awarded]).reason).toBeTruthy(); - expect(enforceOutcomeClaims('Your earned AdCP Practitioner certificate is ready to download.', [awarded]).reason).toBeTruthy(); + expect(enforceCertificationClaims('B3 is completed and you earned the AdCP Practitioner credential.', [awarded]).reason).toBeTruthy(); + expect(enforceCertificationClaims('Your earned AdCP Practitioner certificate is ready to download.', [awarded]).reason).toBeTruthy(); const issued = { ...awarded, result: `${awarded.result}\n- [View and share your credential](https://credsverse.com/credentials/example-id)` }; - expect(enforceOutcomeClaims('Your earned AdCP Practitioner certificate is ready to download.', [issued]).reason).toBeNull(); + expect(enforceCertificationClaims('Your earned AdCP Practitioner certificate is ready to download.', [issued]).reason).toBeNull(); }); it.each([ 'You completed the exercise and this module is complete.', 'You completed the exercise, so you have mastered it.', ])('does not let a teaching subtask authorize another conclusion: %s', text => { - expect(enforceOutcomeClaims(text, [], 'B2 certification').reason).toBeTruthy(); + expect(enforceCertificationClaims(text, [], 'B2 certification').reason).toBeTruthy(); }); it.each([ 'The media buy was completed after the seller accepted the proposal.', 'You are ready to try another exercise.', 'You are ready for B2.', - 'You have completed the exercise.', 'You have mastered this example.', 'Mastery comes from practice.', 'Complete B2 before starting B3.', + 'You have completed the exercise.', "You've completed the tutorial.", 'You have mastered this example.', 'Mastery comes from practice.', 'Complete B2 before starting B3.', 'When your certificate is issued, you can share it.', 'Has your certificate been issued?', ])('preserves teaching inside certification context: %s', text => { - expect(enforceOutcomeClaims(text, [], 'B2 certification').text).toBe(text); + expect(enforceCertificationClaims(text, [], 'B2 certification').text).toBe(text); }); it.each([ @@ -93,6 +128,6 @@ describe('receipt-bound certification claims', () => { 'Your explanation of targeting is clear. How would you apply it to this product?', 'The media buy completed successfully.', ])('preserves teaching, questions, conditional guidance and ordinary actions: %s', text => { - expect(enforceOutcomeClaims(text, []).text).toBe(text); + expect(enforceCertificationClaims(text, []).text).toBe(text); }); }); diff --git a/server/tests/unit/addie/support-outcome-claims.test.ts b/server/tests/unit/addie/support-outcome-claims.test.ts index 2866b062ca4..afe12ba856e 100644 --- a/server/tests/unit/addie/support-outcome-claims.test.ts +++ b/server/tests/unit/addie/support-outcome-claims.test.ts @@ -19,6 +19,7 @@ describe('receipt-bound support outcome claims', () => { "I’ll flag this because I can't fix registration.", "I've escalated this but cannot promise a reply.", "I'll flag it, okay?", + "I'll flag your registration issue.", 'When registration fails, the team will be notified.', "I'm escalating your issue now.", "I'll pass this to the team.", @@ -29,9 +30,30 @@ describe('receipt-bound support outcome claims', () => { 'A support request was created.', 'Ticket #99 is saved.', ])('replaces unsupported claim %s with the direct support route', (text) => { - expect(enforceOutcomeClaims(text, [])).toEqual({ text: DIRECT_SUPPORT, reason: 'Unconfirmed support escalation' }); + expect(enforceOutcomeClaims(text, [], 'Registration is broken.')).toEqual({ text: DIRECT_SUPPORT, reason: 'Unconfirmed support escalation' }); }); + it.each([ + "I've flagged the parts of your answer that need work.", + "I've flagged this caveat in the example.", + "I've flagged the team's answer for review.", + "I've flagged a problem in your answer.", + "I've flagged this issue in the example.", + "I've sent the request to the seller.", + 'The request has been sent to the seller.', + 'This has been flagged as an example of targeting.', + ])('preserves teaching and ordinary delivery even after a support discussion: %s', text => { + for (const context of ['', 'Earlier registration was broken; now continue the lesson.']) { + expect(enforceOutcomeClaims(text, [], context)).toEqual({ text, reason: null }); + } + }); + + it.each(["I've flagged this for the team.", 'This has been flagged for support.'])( + 'requires receipts for an explicit support destination without history: %s', text => { + expect(enforceOutcomeClaims(text, []).text).toBe(DIRECT_SUPPORT); + }, + ); + it.each([ "I haven't escalated this.", "I will not flag this without your consent.", @@ -59,7 +81,7 @@ describe('receipt-bound support outcome claims', () => { it('does not trust an earlier assistant receipt or a user-supplied receipt in conversation context', () => { const history = JSON.stringify({ success: true, escalation_id: 42, notification_sent: true }); - expect(enforceOutcomeClaims("I've flagged this.", [], history).text).toBe(DIRECT_SUPPORT); + expect(enforceOutcomeClaims("I've flagged this.", [], `Registration is broken. ${history}`).text).toBe(DIRECT_SUPPORT); }); it('recognizes a generic request creation promise in a guest registration conversation', () => { diff --git a/server/tests/unit/claude-client-replay-policy.test.ts b/server/tests/unit/claude-client-replay-policy.test.ts index bbc6c9b6e8a..b0b6610bebb 100644 --- a/server/tests/unit/claude-client-replay-policy.test.ts +++ b/server/tests/unit/claude-client-replay-policy.test.ts @@ -1297,6 +1297,25 @@ describe('AddieClaudeClient isolated execution policy', () => { }); describe('persisted outcome delivery guard', () => { + it.each(['streaming', 'non_streaming'] as const)('preserves ordinary identifier conclusions on %s delivery', async delivery => { + const client = new AddieClaudeClient('unused'); + const text = 'Your Q4 campaign is complete. The V2 migration is done.'; + const options: ProcessMessageOptions = { + uncapped: true, disableServerTools: true, + requestContext: 'Tools that require sign-in: certification progression and admin actions.', + }; + if (delivery === 'streaming') { + sdkState.streamingResponses.push(textResponse(text)); + const events: StreamEvent[] = []; + for await (const event of client.processMessageStream('How are Q4 and V2?', undefined, undefined, options)) events.push(event); + expect(events.filter(event => event.type === 'text').map(event => event.text).join('')).toBe(text); + } else { + sdkState.nonStreamingResponses.push(textResponse(text)); + const result = await client.processMessage('How are Q4 and V2?', undefined, undefined, { systemPrompt: 'system' }, options); + expect(result.text).toBe(text); + } + }); + it.each(['streaming', 'non_streaming'] as const)('contains rejected completion and a later no-retry claim before %s delivery', async delivery => { const handler = vi.fn().mockResolvedValue('NOT COMPLETED — module B2 is not recorded as complete. Only 3/4 conversation exchanges detected.'); const scoped = requestTools([tool('complete_certification_module', 'mutation')], [['complete_certification_module', handler]]); @@ -1314,7 +1333,7 @@ describe('persisted outcome delivery guard', () => { for await (const event of client.processMessageStream('Finish B2', undefined, scoped, options)) first.push(event); const next: StreamEvent[] = []; for await (const event of client.processMessageStream('I would discover products first.', [ - { user: 'Addie', text: 'B2 needs another exchange; completion was not recorded.' }, + { user: 'Addie', text: 'Module B2 needs another exchange; completion was not recorded.' }, ], scoped, options)) next.push(event); const emitted = next.filter(event => event.type === 'text').map(event => event.text).join(''); const final = next.find(event => event.type === 'done'); @@ -1324,7 +1343,7 @@ describe('persisted outcome delivery guard', () => { } else { await client.processMessage('Finish B2', undefined, scoped, { systemPrompt: 'system' }, options); const next = await client.processMessage('I would discover products first.', [ - { user: 'Addie', text: 'B2 needs another exchange; completion was not recorded.' }, + { user: 'Addie', text: 'Module B2 needs another exchange; completion was not recorded.' }, ], scoped, { systemPrompt: 'system' }, options); expect(next.text).not.toContain('B2 is concluded'); expect(next.text).toContain("haven't confirmed a saved completion"); diff --git a/server/tests/unit/member-context.test.ts b/server/tests/unit/member-context.test.ts index 5cccf485ad4..296e10067f7 100644 --- a/server/tests/unit/member-context.test.ts +++ b/server/tests/unit/member-context.test.ts @@ -55,6 +55,7 @@ vi.mock('../../src/db/org-filters.js', () => ({ })); import { formatMemberContextForPrompt, MemberContext } from '../../src/addie/member-context.js'; +import { enforceOutcomeClaims, outcomeClaimContext } from '../../src/addie/outcome-claims.js'; /** * Member Context Unit Tests @@ -87,6 +88,11 @@ describe('formatMemberContextForPrompt', () => { expect(result).toContain('Use only the tool names shown there'); expect(result).not.toContain('`get_schema`'); expect(result).not.toContain('everything in ALWAYS_AVAILABLE'); + // Guest capability boilerplate mentions certification but is not a lesson. + expect(result).toContain('certification progression'); + const conversation = outcomeClaimContext(['How is my Q4 campaign?'], result ?? ''); + const response = 'Your Q4 campaign is complete.'; + expect(enforceOutcomeClaims(response, [], conversation).text).toBe(response); }); it('should include user name from workos_user', () => { From c84aeb9c2be9972e5a99132ff1de2588adc197b4 Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Wed, 23 Sep 2026 03:05:39 +0000 Subject: [PATCH 4/4] fix(addie): preserve confirmed credential success and share links --- server/src/addie/outcome-claims.ts | 39 +++++++++++--- .../certification-outcome-claims.test.ts | 53 +++++++++++++++++++ .../unit/claude-client-replay-policy.test.ts | 29 ++++++++++ 3 files changed, 113 insertions(+), 8 deletions(-) diff --git a/server/src/addie/outcome-claims.ts b/server/src/addie/outcome-claims.ts index 506247050c4..1ca4b0e12fe 100644 --- a/server/src/addie/outcome-claims.ts +++ b/server/src/addie/outcome-claims.ts @@ -54,12 +54,12 @@ function hasSupportClaim(text: string, context: string): boolean { interface CertificationEvidence { modules: Set; credentials: Set; - issuedCredentials: Set; + issuedCredentials: Map; } /** Only anchored, application-generated lines from successful certification tools count. */ function certificationEvidence(executions: readonly ToolExecution[]): CertificationEvidence { - const evidence: CertificationEvidence = { modules: new Set(), credentials: new Set(), issuedCredentials: new Set() }; + const evidence: CertificationEvidence = { modules: new Set(), credentials: new Set(), issuedCredentials: new Map() }; for (const execution of executions) { if (execution.is_error || !CERTIFICATION_TOOLS.has(execution.tool_name)) continue; const text = execution.result; @@ -101,8 +101,10 @@ function certificationEvidence(executions: readonly ToolExecution[]): Certificat for (const line of lines) { const match = /^\*\*Credential earned: (.+)!\*\*$/.exec(line); if (match) { credential = match[1]!; evidence.credentials.add(credential); } - if (credential && line.startsWith('- [View and share your credential](https://credsverse.com/credentials/')) { - evidence.issuedCredentials.add(credential); + const share = /^- \[View and share your credential\]\((https:\/\/credsverse\.com\/credentials\/[A-Za-z0-9._~!%'*+-]+)\)$/.exec(line); + const sharePath = share ? new URL(share[1]!).pathname : ''; + if (credential && share && sharePath.startsWith('/credentials/') && sharePath.length > '/credentials/'.length) { + evidence.issuedCredentials.set(credential, share[1]!); } } } @@ -110,6 +112,19 @@ function certificationEvidence(executions: readonly ToolExecution[]): Certificat return evidence; } +/** A singular generic reference can bind to one receipt, never a different named award. */ +function hasOnlyGenericCredentialReferences(text: string): boolean { + const mentions = [...text.matchAll(/\b(?:credential|certificate|badge|certification|certified)\b/gi)]; + return mentions.length > 0 && mentions.every(match => { + const prefix = text.slice(0, match.index); + const suffix = text.slice(match.index + match[0].length); + if (/^\s+(?:in|for|of|as)\b/i.test(suffix)) return false; + return match[0].toLowerCase() === 'certified' + ? /\byou(?:'re| are)(?:\s+now)?\s+$/i.test(prefix) + : /\b(?:your|the|an?|this|that)\s+(?:(?:new|earned)\s+)?$/i.test(prefix); + }); +} + function supportReceipt(executions: readonly ToolExecution[]): { id: number; notified: boolean } | null { for (const execution of [...executions].reverse()) { if (execution.tool_name !== 'escalate_to_admin' || execution.is_error) continue; @@ -165,7 +180,8 @@ export function enforceOutcomeClaims( const output: string[] = []; for (const [index, part] of parts.entries()) { if (index % 2 === 1) { output.push(part); continue; } - const plain = part.replace(/[*_`]/g, '').replace(/[’‘]/g, "'"); + const plain = part.replace(/\[([^\[\]\n]*)\]\([^\[\]\s()]+\)/g, '$1') + .replace(/[*_`]/g, '').replace(/[’‘]/g, "'"); if (hasSupportClaim(plain, `${conversationContext}\n${text}${support ? '\nSupport request receipt.' : ''}`)) { if (!supportReplaced) output.push(support ? `Support request #${support.id} is saved.${support.notified ? ' The team notification was sent.' : ' I could not confirm a team notification.'}` @@ -187,9 +203,12 @@ export function enforceOutcomeClaims( const externalIssuance = /\b(?:issued|sent|delivered|download|share|ready)\b/i.test(plain); const credentials = externalIssuance ? evidence.issuedCredentials : evidence.credentials; const modulesSupported = ids.every(id => evidence.modules.has(id)); - const namedCredentials = [...credentials].filter(name => plain.toLowerCase().includes(name.toLowerCase())); + const claimedCredentials = [...credentials.keys()].filter(name => plain.toLowerCase().includes(name.toLowerCase())); + if (claimedCredentials.length === 0 && credentials.size === 1 && hasOnlyGenericCredentialReferences(plain)) { + claimedCredentials.push(...credentials.keys()); + } const modules = ids.length ? ids : [...evidence.modules]; - const supported = modulesSupported && (credentialClaim ? namedCredentials.length > 0 : modules.length > 0); + const supported = modulesSupported && (credentialClaim ? claimedCredentials.length > 0 : modules.length > 0); if (!supported) { if (!certificationReplaced) output.push(UNCONFIRMED_CERTIFICATION); certificationReplaced = true; @@ -200,7 +219,11 @@ export function enforceOutcomeClaims( // license extra credentials, modules, or delivery claims in model prose. output.push([ ...modules.filter(id => evidence.modules.has(id)).map(id => `${id} is recorded as complete.`), - ...(credentialClaim ? namedCredentials.map(name => `Credential ${externalIssuance ? 'issued' : 'earned'}: ${name}.`) : []), + ...(credentialClaim ? claimedCredentials.map(name => { + const shareUrl = evidence.issuedCredentials.get(name); + return `Credential ${externalIssuance ? 'issued' : 'earned'}: ${name}.` + + (shareUrl ? ` [View and share your credential](${shareUrl})` : ''); + }) : []), ].join(' ')); certificationRendered = true; continue; diff --git a/server/tests/unit/addie/certification-outcome-claims.test.ts b/server/tests/unit/addie/certification-outcome-claims.test.ts index 7c432ba5e9c..332fff31f63 100644 --- a/server/tests/unit/addie/certification-outcome-claims.test.ts +++ b/server/tests/unit/addie/certification-outcome-claims.test.ts @@ -10,6 +10,9 @@ function enforceCertificationClaims(text: string, executions: readonly ToolExecu return enforceOutcomeClaims(text, executions, context); } const completed = execution('complete_certification_module', 'Module B2 completed! The learner has demonstrated mastery of all learning objectives.'); +const awarded = { ...completed, result: `${completed.result}\n**Credential earned: AdCP Practitioner!**` }; +const shareUrl = 'https://credsverse.com/credentials/abc-b2'; +const issued = { ...awarded, result: `${awarded.result}\n- [View and share your credential](${shareUrl})` }; describe('receipt-bound certification claims', () => { it.each(['Q4', 'H1', 'H2', 'V2', 'P1', 'S3', 'b2'])( @@ -106,6 +109,56 @@ describe('receipt-bound certification claims', () => { expect(enforceCertificationClaims('Your earned AdCP Practitioner certificate is ready to download.', [issued]).reason).toBeNull(); }); + it.each([ + 'Congratulations, you earned your certificate!', 'You earned a badge.', + "The credential's yours.", 'Your certification has been awarded.', 'You are now certified!', + ])('renders an unambiguous persisted credential for generic wording: %s', text => { + const result = enforceCertificationClaims(text, [awarded]); + expect(result.reason).toBeNull(); + expect(result.text).toContain('Credential earned: AdCP Practitioner.'); + expect(result.text).not.toContain('haven\'t confirmed'); + expect(result.text).not.toContain('credsverse.com'); + expect(enforceCertificationClaims(text, [completed]).reason).toBeTruthy(); + }); + + it.each([ + 'Your certificate has been issued.', 'Your certificate is ready to download.', + `Your AdCP Practitioner certificate has been issued: [View and share](${shareUrl}).`, + `You earned the [AdCP Practitioner credential](${shareUrl})!`, + 'Your AdCP Practitioner certificate has been issued: [View and share](https://example.com/untrusted).', + ])('preserves only the receipt-owned credential link: %s', text => { + const result = enforceCertificationClaims(text, [issued]); + expect(result.reason).toBeNull(); + expect(result.text).toContain(`[View and share your credential](${shareUrl})`); + expect(result.text).not.toContain('https://example.com'); + if (!text.startsWith('You earned')) expect(enforceCertificationClaims(text, [awarded]).reason).toBeTruthy(); + }); + + it.each([ + 'You earned the Advanced Specialist certificate.', + 'You earned your certificate and the Advanced Specialist certificate.', + 'Your certificate for Advanced Specialist has been issued.', + 'B3 is complete and you earned your certificate.', + ])('does not let a generic receipt authorize another named outcome: %s', text => { + expect(enforceCertificationClaims(text, [issued]).reason).toBeTruthy(); + }); + + it('does not guess between multiple saved credentials or accept unsuccessful evidence', () => { + const multiple = { ...awarded, result: `${awarded.result}\n**Credential earned: AdCP Specialist!**` }; + expect(enforceCertificationClaims('You earned your certificate.', [multiple]).reason).toBeTruthy(); + expect(enforceCertificationClaims('You earned your certificate.', [{ ...awarded, is_error: true }]).reason).toBeTruthy(); + expect(enforceCertificationClaims('You earned your certificate.', [{ ...awarded, tool_name: 'search_docs' }]).reason).toBeTruthy(); + }); + + it.each([ + 'https://credsverse.com.example.com/credentials/id', + 'https://credsverse.com/credentials/%2e%2e', + 'https://credsverse.com/credentials/.', + ])('does not treat malformed receipt links as credential issuance: %s', url => { + const invalid = { ...awarded, result: `${awarded.result}\n- [View and share your credential](${url})` }; + expect(enforceCertificationClaims('Your certificate has been issued.', [invalid]).reason).toBeTruthy(); + }); + it.each([ 'You completed the exercise and this module is complete.', 'You completed the exercise, so you have mastered it.', diff --git a/server/tests/unit/claude-client-replay-policy.test.ts b/server/tests/unit/claude-client-replay-policy.test.ts index b0b6610bebb..cac3769c683 100644 --- a/server/tests/unit/claude-client-replay-policy.test.ts +++ b/server/tests/unit/claude-client-replay-policy.test.ts @@ -1385,4 +1385,33 @@ describe('persisted outcome delivery guard', () => { expect(result.text).toBe('B2 is recorded as complete.'); expect(result.flagged).toBe(false); }); + + it.each(['streaming', 'non_streaming'] as const)('delivers a generic credential success and its saved share link on %s delivery', async delivery => { + const shareUrl = 'https://credsverse.com/credentials/abc-b2'; + const handler = vi.fn().mockResolvedValue(`Module B2 completed!\n**Credential earned: AdCP Practitioner!**\n- [View and share your credential](${shareUrl})`); + const scoped = requestTools([tool('complete_certification_module', 'mutation')], [['complete_certification_module', handler]]); + const queue = delivery === 'streaming' ? sdkState.streamingResponses : sdkState.nonStreamingResponses; + queue.push( + toolUseResponse([{ id: 'award-b2', name: 'complete_certification_module', input: { module_id: 'B2' } }]), + textResponse('Congratulations, you earned your certificate!'), + ); + const options: ProcessMessageOptions = { uncapped: true, disableServerTools: true, reserveSideEffect: async () => {} }; + const client = new AddieClaudeClient('unused'); + let text: string; + if (delivery === 'streaming') { + const events: StreamEvent[] = []; + for await (const event of client.processMessageStream('Finish module B2', undefined, scoped, options)) events.push(event); + text = events.filter(event => event.type === 'text').map(event => event.text).join(''); + const done = events.find(event => event.type === 'done'); + expect(done?.type === 'done' && done.response.flagged).toBe(false); + } else { + const result = await client.processMessage('Finish module B2', undefined, scoped, { systemPrompt: 'system' }, options); + text = result.text; + expect(result.flagged).toBe(false); + } + expect(text).toContain('Credential earned: AdCP Practitioner.'); + expect(text).toContain(`[View and share your credential](${shareUrl})`); + expect(text).not.toContain('haven\'t confirmed'); + expect(handler).toHaveBeenCalledTimes(1); + }); });