Skip to content

Commit ae1f62d

Browse files
improvement(provenance): make the incompleteness reason set closed and complete (#6559)
The reason a resolved-secret registry latched is the only thing that names which guard tripped, and a refusal surfaces many frames later as one fixed sentence. Two gaps had opened in that set. `workspace-scope-missing` no longer has a producer: the `!context.workspaceId` guard in the copilot table tool went away when `importRowsForModel` was rewritten, and every operation now returns early on a missing workspace before provenance import is reachable. The literal and its warn-classification test case go with it — the test constructed the reason itself, so it asserted on something nothing emits. ResolvedSecretTraceProvenanceAccumulator had no reason concept at all, so its three guards latched anonymously. That matters more there than on the registry: the wire format carries only `complete`, so the consumer can only ever say `source-provenance-incomplete`, and the guard is unrecoverable. Give it the same required `reason` and name all three — a file source with no workspace identity, a workspace file whose sidecar reads unknown, and an MCP tool that timed out. A latch from `record()` stays silent, since it reflects a bundle whose own registry already reported and subflow aggregation runs it per iteration. Fold the error/warn/by-design split into one `reportIncompleteness`. It was copied across both registry latches and would have been copied a third time here, and a copy that can be updated alone lets one reason be a fault in one place and routine in another. Also give the async workflow tool path its own import origin instead of latching with none, and close `UnrecordedDurableProvenanceCause`, which was a free-form string carrying a TSDoc claim that it was always a static literal.
1 parent 81e04a8 commit ae1f62d

6 files changed

Lines changed: 113 additions & 28 deletions

File tree

apps/sim/app/api/mcp/tools/execute/route.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -327,7 +327,7 @@ export const POST = withRouteHandler(
327327
return successResponse(transformedResult)
328328
} catch (error) {
329329
if (getErrorMessage(error) === 'Tool execution timeout') {
330-
resolvedSecretTraceProvenance?.markIncomplete()
330+
resolvedSecretTraceProvenance?.markIncomplete('mcp-tool-execution-timeout')
331331
}
332332
const bodyErrorResponse = mcpBodyReadErrorResponse(error, request)
333333
if (bodyErrorResponse) return bodyErrorResponse

apps/sim/app/api/tools/file/manage/route.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -295,12 +295,12 @@ async function getFileContentProvenance(
295295

296296
for (const source of sources) {
297297
if (!source.identity || !source.ownerUserId) {
298-
accumulator.markIncomplete()
298+
accumulator.markIncomplete('file-source-unidentified')
299299
continue
300300
}
301301
const provenance = await getBoundWorkspaceFileSecretProvenance(workspaceId, source.identity)
302302
if (provenance.status === 'unknown') {
303-
accumulator.markIncomplete()
303+
accumulator.markIncomplete('workspace-file-provenance-unknown')
304304
continue
305305
}
306306
accumulator.record({

apps/sim/executor/utils/resolved-secret-trace-registry.test.ts

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,40 @@ describe('ResolvedSecretTraceProvenanceAccumulator', () => {
144144
accumulator.markIncomplete('unspecified')
145145
expect(accumulator.exportProvenance().entries).toEqual([])
146146
})
147+
148+
/**
149+
* The exported bundle carries only `complete`, so an importer can never say more than
150+
* `source-provenance-incomplete`. If this line does not name the guard, nothing does.
151+
*/
152+
it('names the first guard that latched, and stays quiet for the rest of the invocation', () => {
153+
vi.clearAllMocks()
154+
const accumulator = new ResolvedSecretTraceProvenanceAccumulator(scope)
155+
156+
accumulator.markIncomplete('file-source-unidentified')
157+
accumulator.markIncomplete('workspace-file-provenance-unknown')
158+
159+
expect(mockLogger.warn).toHaveBeenCalledTimes(1)
160+
expect(mockLogger.warn).toHaveBeenCalledWith(
161+
'Resolved secret provenance accumulator marked incomplete',
162+
expect.objectContaining({
163+
reason: 'file-source-unidentified',
164+
scopeWorkspaceId: 'workspace-1',
165+
})
166+
)
167+
expect(mockLogger.error).not.toHaveBeenCalled()
168+
})
169+
170+
/** A merge of already-reported bundles adds nothing; subflow aggregation runs it per iteration. */
171+
it('stays silent when a recorded report is what latched it', () => {
172+
vi.clearAllMocks()
173+
const accumulator = new ResolvedSecretTraceProvenanceAccumulator(scope)
174+
175+
accumulator.record({ version: 1, complete: false, entries: [], scope })
176+
177+
expect(accumulator.exportProvenance().complete).toBe(false)
178+
expect(mockLogger.warn).not.toHaveBeenCalled()
179+
expect(mockLogger.error).not.toHaveBeenCalled()
180+
})
147181
})
148182

149183
describe('ResolvedSecretTraceRegistry', () => {
@@ -1474,9 +1508,11 @@ describe('incompleteness diagnostics', () => {
14741508
'knowledge-result-provenance-unavailable',
14751509
'knowledge-response-capacity-exceeded',
14761510
'memory-crossing-capacity-exceeded',
1477-
'workspace-scope-missing',
14781511
'table-result-provenance-unavailable',
14791512
'mounted-file-provenance-unavailable',
1513+
'workspace-file-provenance-unknown',
1514+
'file-source-unidentified',
1515+
'mcp-tool-execution-timeout',
14801516
'table-snapshot-unsafe-for-mount',
14811517
'restored-provenance-untrusted',
14821518
'backfill-checkpoint-absent',

apps/sim/executor/utils/resolved-secret-trace-registry.ts

Lines changed: 50 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ export type ResolvedSecretIncompletenessReason =
4343
| 'durable-provenance-malformed'
4444
| 'tool-input-not-enumerable'
4545
| 'tool-params-transform-failed'
46+
| 'mcp-tool-execution-timeout'
4647
| 'structural-input-projection-incomplete'
4748
| 'structural-input-root-unprojected'
4849
| 'mothership-provenance-invalid'
@@ -60,17 +61,20 @@ export type ResolvedSecretIncompletenessReason =
6061
| 'knowledge-row-missing'
6162
| 'knowledge-row-content-mismatch'
6263
| 'memory-crossing-capacity-exceeded'
63-
| 'workspace-scope-missing'
6464
| 'table-result-provenance-unavailable'
6565
| 'mounted-file-provenance-unavailable'
66+
| 'workspace-file-provenance-unknown'
67+
| 'file-source-unidentified'
6668
| 'table-snapshot-unsafe-for-mount'
6769
| 'restored-provenance-untrusted'
6870
| 'backfill-checkpoint-absent'
6971
| 'backfill-checkpoint-unusable'
7072
| 'log-creation-skipped'
7173
/**
72-
* Only for a caller that has not been given a reason yet. A refusal reporting this names no
73-
* guard, which is the state that made a production latch untraceable — prefer adding a literal.
74+
* No production caller uses this, and none should: a refusal reporting it names no guard, which
75+
* is the state that made a production latch untraceable. It survives for tests that need a
76+
* latched registry and have no guard to name, where a borrowed real reason would read as a claim
77+
* about which one tripped. A new caller wanting it wants a new literal instead.
7478
*/
7579
| 'unspecified'
7680

@@ -122,6 +126,23 @@ const BY_DESIGN_INCOMPLETENESS_REASONS = new Set<ResolvedSecretIncompletenessRea
122126
'log-creation-skipped',
123127
])
124128

129+
/**
130+
* Sole owner of the report level, shared by every latch that reports one.
131+
*
132+
* The registry, its input paths, and the accumulator each latch for their own reasons but classify
133+
* them identically, and a copy of the split per latch is a copy that can be updated alone — which
134+
* would let the same reason be a fault in one place and routine in another.
135+
*/
136+
function reportIncompleteness(
137+
message: string,
138+
reason: ResolvedSecretIncompletenessReason,
139+
details: Record<string, unknown>
140+
): void {
141+
if (BY_DESIGN_INCOMPLETENESS_REASONS.has(reason)) return
142+
if (ORIGINATING_FAULT_REASONS.has(reason)) logger.error(message, { reason, ...details })
143+
else logger.warn(message, { reason, ...details })
144+
}
145+
125146
/**
126147
* Origins are caller-supplied strings rather than a closed union, so they carry an explicit bound;
127148
* one run reaching this many distinct importers already tells the whole story.
@@ -624,6 +645,7 @@ export function isResolvedSecretTraceProvenanceV1(
624645
export class ResolvedSecretTraceProvenanceAccumulator {
625646
private readonly scope?: ResolvedSecretTraceScopeV1
626647
private provenance: ResolvedSecretTraceProvenanceV1
648+
private reportedGuard = false
627649

628650
constructor(scope?: ResolvedSecretTraceScopeV1) {
629651
this.scope = scope ? cloneProvenanceScope(scope) : undefined
@@ -677,9 +699,26 @@ export class ResolvedSecretTraceProvenanceAccumulator {
677699
return true
678700
}
679701

680-
/** Marks the invocation incomplete and discards entries that can no longer be trusted. */
681-
markIncomplete(): void {
702+
/**
703+
* Marks the invocation incomplete and discards entries that can no longer be trusted.
704+
*
705+
* `reason` is required for the same purpose it is on {@link ResolvedSecretTraceRegistry}, and
706+
* matters more here: the wire format carries only `complete`, so the consumer that imports this
707+
* bundle can only latch with `source-provenance-incomplete` and can never name the guard. This
708+
* line is the sole record of which one tripped.
709+
*
710+
* Only the first guard reports. Later ones restate an invocation that already cannot vouch, and
711+
* a caller walking a list of sources would otherwise emit a line per remaining source. A latch
712+
* from {@link record} does not report at all: it reflects a bundle whose own registry already
713+
* reported, so this would only restate it with less context.
714+
*/
715+
markIncomplete(reason: ResolvedSecretIncompletenessReason): void {
682716
this.provenance = this.emptyProvenance(false)
717+
if (this.reportedGuard) return
718+
this.reportedGuard = true
719+
reportIncompleteness('Resolved secret provenance accumulator marked incomplete', reason, {
720+
scopeWorkspaceId: this.scope?.workspaceId,
721+
})
683722
}
684723

685724
exportProvenance(): ResolvedSecretTraceProvenanceV1 {
@@ -1583,17 +1622,13 @@ export class ResolvedSecretTraceRegistry {
15831622
if (!this.complete) return
15841623
this.complete = false
15851624
this.modelEgressRevision += 1
1586-
if (this.staged || BY_DESIGN_INCOMPLETENESS_REASONS.has(reason)) return
1587-
const details = {
1588-
reason,
1625+
if (this.staged) return
1626+
reportIncompleteness('Resolved secret registry marked incomplete', reason, {
15891627
...(context.origin ? { origin: context.origin } : {}),
15901628
scopeWorkspaceId: this.scope?.workspaceId,
15911629
activeEntryCount: this.activeEntries.size,
15921630
incompleteInputPathCount: this.incompleteInputPaths.size,
1593-
}
1594-
const message = 'Resolved secret registry marked incomplete'
1595-
if (ORIGINATING_FAULT_REASONS.has(reason)) logger.error(message, details)
1596-
else logger.warn(message, details)
1631+
})
15971632
}
15981633

15991634
/**
@@ -2038,17 +2073,13 @@ export class ResolvedSecretTraceRegistry {
20382073
if (this.incompleteInputPaths.has(key)) return
20392074
this.incompleteInputPaths.set(key, [...path])
20402075
this.modelEgressRevision += 1
2041-
if (this.staged || BY_DESIGN_INCOMPLETENESS_REASONS.has(reason)) return
2042-
const details = {
2043-
reason,
2076+
if (this.staged) return
2077+
reportIncompleteness('Resolved secret input path marked incomplete', reason, {
20442078
...(origin ? { origin } : {}),
20452079
inputPath: path.join('.'),
20462080
scopeWorkspaceId: this.scope?.workspaceId,
20472081
activeEntryCount: this.activeEntries.size,
2048-
}
2049-
const message = 'Resolved secret input path marked incomplete'
2050-
if (ORIGINATING_FAULT_REASONS.has(reason)) logger.error(message, details)
2051-
else logger.warn(message, details)
2082+
})
20522083
}
20532084

20542085
private copyIncompleteInputPathsTo(

apps/sim/lib/copilot/request/tools/client.ts

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -308,7 +308,9 @@ export async function waitForWorkflowToolCompletion({
308308
!trustedExecution.provenance.complete
309309
) {
310310
if (!trustedExecution.provenance.complete)
311-
toolRegistry?.markIncomplete('source-provenance-incomplete')
311+
toolRegistry?.markIncomplete('source-provenance-incomplete', {
312+
origin: 'copilotToolClient.workflowExecution',
313+
})
312314
return structuralWorkflowCompletion(
313315
getWorkflowToolConfirmationStatus(trustedExecution.status),
314316
workflowId,
@@ -328,9 +330,14 @@ export async function waitForWorkflowToolCompletion({
328330
},
329331
{ trusted: true }
330332
)
331-
if (!imported) toolRegistry.markIncomplete('value-provenance-import-failed')
333+
if (!imported)
334+
toolRegistry.markIncomplete('value-provenance-import-failed', {
335+
origin: 'copilotToolClient.workflowExecution',
336+
})
332337
} catch (error) {
333-
toolRegistry.markIncomplete('value-provenance-import-failed')
338+
toolRegistry.markIncomplete('value-provenance-import-failed', {
339+
origin: 'copilotToolClient.workflowExecution',
340+
})
334341
logger.warn('Failed to import bound workflow provenance', {
335342
toolCallId,
336343
workflowId,

apps/sim/lib/execution/durable-secret-provenance-enforcement.ts

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,10 +72,21 @@ export function isDurableSecretProvenanceEnforced(
7272
return enforcedSurfaces.has(surface)
7373
}
7474

75+
/**
76+
* What a surface could not vouch for.
77+
*
78+
* A closed union rather than a free-form string, for the reason the resolved-secret registry's
79+
* reason set is one: a surface stays open on the strength of these lines trending to zero, and a
80+
* cause that a call site can spell freely cannot be aggregated or alerted on.
81+
*/
82+
export type UnrecordedDurableProvenanceCause =
83+
| 'durable-provenance-unknown'
84+
| 'row-sidecar-not-exact'
85+
| 'stored-memory-provenance-unknown'
86+
7587
export interface UnrecordedDurableProvenanceReport {
7688
surface: DurableSecretProvenanceSurface
77-
/** What the surface could not vouch for, e.g. `sidecar-status-unknown`. Always a static literal. */
78-
cause: string
89+
cause: UnrecordedDurableProvenanceCause
7990
/** How many records in this one read were unrecorded, when the caller reads a page at a time. */
8091
affectedCount?: number
8192
workspaceId?: string

0 commit comments

Comments
 (0)