diff --git a/apps/sim/app/api/mcp/tools/execute/route.ts b/apps/sim/app/api/mcp/tools/execute/route.ts index eabe79c70ba..d045b407a54 100644 --- a/apps/sim/app/api/mcp/tools/execute/route.ts +++ b/apps/sim/app/api/mcp/tools/execute/route.ts @@ -327,7 +327,7 @@ export const POST = withRouteHandler( return successResponse(transformedResult) } catch (error) { if (getErrorMessage(error) === 'Tool execution timeout') { - resolvedSecretTraceProvenance?.markIncomplete() + resolvedSecretTraceProvenance?.markIncomplete('mcp-tool-execution-timeout') } const bodyErrorResponse = mcpBodyReadErrorResponse(error, request) if (bodyErrorResponse) return bodyErrorResponse diff --git a/apps/sim/app/api/tools/file/manage/route.ts b/apps/sim/app/api/tools/file/manage/route.ts index d22d90f8389..3df9ac4ffd3 100644 --- a/apps/sim/app/api/tools/file/manage/route.ts +++ b/apps/sim/app/api/tools/file/manage/route.ts @@ -295,12 +295,12 @@ async function getFileContentProvenance( for (const source of sources) { if (!source.identity || !source.ownerUserId) { - accumulator.markIncomplete() + accumulator.markIncomplete('file-source-unidentified') continue } const provenance = await getBoundWorkspaceFileSecretProvenance(workspaceId, source.identity) if (provenance.status === 'unknown') { - accumulator.markIncomplete() + accumulator.markIncomplete('workspace-file-provenance-unknown') continue } accumulator.record({ diff --git a/apps/sim/executor/utils/resolved-secret-trace-registry.test.ts b/apps/sim/executor/utils/resolved-secret-trace-registry.test.ts index 3d41d848a93..9ca2188c414 100644 --- a/apps/sim/executor/utils/resolved-secret-trace-registry.test.ts +++ b/apps/sim/executor/utils/resolved-secret-trace-registry.test.ts @@ -144,6 +144,40 @@ describe('ResolvedSecretTraceProvenanceAccumulator', () => { accumulator.markIncomplete('unspecified') expect(accumulator.exportProvenance().entries).toEqual([]) }) + + /** + * The exported bundle carries only `complete`, so an importer can never say more than + * `source-provenance-incomplete`. If this line does not name the guard, nothing does. + */ + it('names the first guard that latched, and stays quiet for the rest of the invocation', () => { + vi.clearAllMocks() + const accumulator = new ResolvedSecretTraceProvenanceAccumulator(scope) + + accumulator.markIncomplete('file-source-unidentified') + accumulator.markIncomplete('workspace-file-provenance-unknown') + + expect(mockLogger.warn).toHaveBeenCalledTimes(1) + expect(mockLogger.warn).toHaveBeenCalledWith( + 'Resolved secret provenance accumulator marked incomplete', + expect.objectContaining({ + reason: 'file-source-unidentified', + scopeWorkspaceId: 'workspace-1', + }) + ) + expect(mockLogger.error).not.toHaveBeenCalled() + }) + + /** A merge of already-reported bundles adds nothing; subflow aggregation runs it per iteration. */ + it('stays silent when a recorded report is what latched it', () => { + vi.clearAllMocks() + const accumulator = new ResolvedSecretTraceProvenanceAccumulator(scope) + + accumulator.record({ version: 1, complete: false, entries: [], scope }) + + expect(accumulator.exportProvenance().complete).toBe(false) + expect(mockLogger.warn).not.toHaveBeenCalled() + expect(mockLogger.error).not.toHaveBeenCalled() + }) }) describe('ResolvedSecretTraceRegistry', () => { @@ -1474,9 +1508,11 @@ describe('incompleteness diagnostics', () => { 'knowledge-result-provenance-unavailable', 'knowledge-response-capacity-exceeded', 'memory-crossing-capacity-exceeded', - 'workspace-scope-missing', 'table-result-provenance-unavailable', 'mounted-file-provenance-unavailable', + 'workspace-file-provenance-unknown', + 'file-source-unidentified', + 'mcp-tool-execution-timeout', 'table-snapshot-unsafe-for-mount', 'restored-provenance-untrusted', 'backfill-checkpoint-absent', diff --git a/apps/sim/executor/utils/resolved-secret-trace-registry.ts b/apps/sim/executor/utils/resolved-secret-trace-registry.ts index 24d91499987..8556e8e38cf 100644 --- a/apps/sim/executor/utils/resolved-secret-trace-registry.ts +++ b/apps/sim/executor/utils/resolved-secret-trace-registry.ts @@ -43,6 +43,7 @@ export type ResolvedSecretIncompletenessReason = | 'durable-provenance-malformed' | 'tool-input-not-enumerable' | 'tool-params-transform-failed' + | 'mcp-tool-execution-timeout' | 'structural-input-projection-incomplete' | 'structural-input-root-unprojected' | 'mothership-provenance-invalid' @@ -60,17 +61,20 @@ export type ResolvedSecretIncompletenessReason = | 'knowledge-row-missing' | 'knowledge-row-content-mismatch' | 'memory-crossing-capacity-exceeded' - | 'workspace-scope-missing' | 'table-result-provenance-unavailable' | 'mounted-file-provenance-unavailable' + | 'workspace-file-provenance-unknown' + | 'file-source-unidentified' | 'table-snapshot-unsafe-for-mount' | 'restored-provenance-untrusted' | 'backfill-checkpoint-absent' | 'backfill-checkpoint-unusable' | 'log-creation-skipped' /** - * Only for a caller that has not been given a reason yet. A refusal reporting this names no - * guard, which is the state that made a production latch untraceable — prefer adding a literal. + * No production caller uses this, and none should: a refusal reporting it names no guard, which + * is the state that made a production latch untraceable. It survives for tests that need a + * latched registry and have no guard to name, where a borrowed real reason would read as a claim + * about which one tripped. A new caller wanting it wants a new literal instead. */ | 'unspecified' @@ -122,6 +126,23 @@ const BY_DESIGN_INCOMPLETENESS_REASONS = new Set +): void { + if (BY_DESIGN_INCOMPLETENESS_REASONS.has(reason)) return + if (ORIGINATING_FAULT_REASONS.has(reason)) logger.error(message, { reason, ...details }) + else logger.warn(message, { reason, ...details }) +} + /** * Origins are caller-supplied strings rather than a closed union, so they carry an explicit bound; * one run reaching this many distinct importers already tells the whole story. @@ -624,6 +645,7 @@ export function isResolvedSecretTraceProvenanceV1( export class ResolvedSecretTraceProvenanceAccumulator { private readonly scope?: ResolvedSecretTraceScopeV1 private provenance: ResolvedSecretTraceProvenanceV1 + private reportedGuard = false constructor(scope?: ResolvedSecretTraceScopeV1) { this.scope = scope ? cloneProvenanceScope(scope) : undefined @@ -677,9 +699,26 @@ export class ResolvedSecretTraceProvenanceAccumulator { return true } - /** Marks the invocation incomplete and discards entries that can no longer be trusted. */ - markIncomplete(): void { + /** + * Marks the invocation incomplete and discards entries that can no longer be trusted. + * + * `reason` is required for the same purpose it is on {@link ResolvedSecretTraceRegistry}, and + * matters more here: the wire format carries only `complete`, so the consumer that imports this + * bundle can only latch with `source-provenance-incomplete` and can never name the guard. This + * line is the sole record of which one tripped. + * + * Only the first guard reports. Later ones restate an invocation that already cannot vouch, and + * a caller walking a list of sources would otherwise emit a line per remaining source. A latch + * from {@link record} does not report at all: it reflects a bundle whose own registry already + * reported, so this would only restate it with less context. + */ + markIncomplete(reason: ResolvedSecretIncompletenessReason): void { this.provenance = this.emptyProvenance(false) + if (this.reportedGuard) return + this.reportedGuard = true + reportIncompleteness('Resolved secret provenance accumulator marked incomplete', reason, { + scopeWorkspaceId: this.scope?.workspaceId, + }) } exportProvenance(): ResolvedSecretTraceProvenanceV1 { @@ -1583,17 +1622,13 @@ export class ResolvedSecretTraceRegistry { if (!this.complete) return this.complete = false this.modelEgressRevision += 1 - if (this.staged || BY_DESIGN_INCOMPLETENESS_REASONS.has(reason)) return - const details = { - reason, + if (this.staged) return + reportIncompleteness('Resolved secret registry marked incomplete', reason, { ...(context.origin ? { origin: context.origin } : {}), scopeWorkspaceId: this.scope?.workspaceId, activeEntryCount: this.activeEntries.size, incompleteInputPathCount: this.incompleteInputPaths.size, - } - const message = 'Resolved secret registry marked incomplete' - if (ORIGINATING_FAULT_REASONS.has(reason)) logger.error(message, details) - else logger.warn(message, details) + }) } /** @@ -2038,17 +2073,13 @@ export class ResolvedSecretTraceRegistry { if (this.incompleteInputPaths.has(key)) return this.incompleteInputPaths.set(key, [...path]) this.modelEgressRevision += 1 - if (this.staged || BY_DESIGN_INCOMPLETENESS_REASONS.has(reason)) return - const details = { - reason, + if (this.staged) return + reportIncompleteness('Resolved secret input path marked incomplete', reason, { ...(origin ? { origin } : {}), inputPath: path.join('.'), scopeWorkspaceId: this.scope?.workspaceId, activeEntryCount: this.activeEntries.size, - } - const message = 'Resolved secret input path marked incomplete' - if (ORIGINATING_FAULT_REASONS.has(reason)) logger.error(message, details) - else logger.warn(message, details) + }) } private copyIncompleteInputPathsTo( diff --git a/apps/sim/lib/copilot/request/tools/client.ts b/apps/sim/lib/copilot/request/tools/client.ts index 88cf40cd35e..de792abd5fc 100644 --- a/apps/sim/lib/copilot/request/tools/client.ts +++ b/apps/sim/lib/copilot/request/tools/client.ts @@ -308,7 +308,9 @@ export async function waitForWorkflowToolCompletion({ !trustedExecution.provenance.complete ) { if (!trustedExecution.provenance.complete) - toolRegistry?.markIncomplete('source-provenance-incomplete') + toolRegistry?.markIncomplete('source-provenance-incomplete', { + origin: 'copilotToolClient.workflowExecution', + }) return structuralWorkflowCompletion( getWorkflowToolConfirmationStatus(trustedExecution.status), workflowId, @@ -328,9 +330,14 @@ export async function waitForWorkflowToolCompletion({ }, { trusted: true } ) - if (!imported) toolRegistry.markIncomplete('value-provenance-import-failed') + if (!imported) + toolRegistry.markIncomplete('value-provenance-import-failed', { + origin: 'copilotToolClient.workflowExecution', + }) } catch (error) { - toolRegistry.markIncomplete('value-provenance-import-failed') + toolRegistry.markIncomplete('value-provenance-import-failed', { + origin: 'copilotToolClient.workflowExecution', + }) logger.warn('Failed to import bound workflow provenance', { toolCallId, workflowId, diff --git a/apps/sim/lib/execution/durable-secret-provenance-enforcement.ts b/apps/sim/lib/execution/durable-secret-provenance-enforcement.ts index 8489ab66424..5058cfa3b2a 100644 --- a/apps/sim/lib/execution/durable-secret-provenance-enforcement.ts +++ b/apps/sim/lib/execution/durable-secret-provenance-enforcement.ts @@ -72,10 +72,21 @@ export function isDurableSecretProvenanceEnforced( return enforcedSurfaces.has(surface) } +/** + * What a surface could not vouch for. + * + * A closed union rather than a free-form string, for the reason the resolved-secret registry's + * reason set is one: a surface stays open on the strength of these lines trending to zero, and a + * cause that a call site can spell freely cannot be aggregated or alerted on. + */ +export type UnrecordedDurableProvenanceCause = + | 'durable-provenance-unknown' + | 'row-sidecar-not-exact' + | 'stored-memory-provenance-unknown' + export interface UnrecordedDurableProvenanceReport { surface: DurableSecretProvenanceSurface - /** What the surface could not vouch for, e.g. `sidecar-status-unknown`. Always a static literal. */ - cause: string + cause: UnrecordedDurableProvenanceCause /** How many records in this one read were unrecorded, when the caller reads a page at a time. */ affectedCount?: number workspaceId?: string