fix(provenance): name the importer that condemned a run - #6493
Conversation
A bundle that arrives already incomplete latches the whole registry one-way, so every later model projection in the run refuses. #6483 made the refusal say what went wrong; it could not say who. In production the answer was reason=source-provenance-incomplete with 23 candidate importers and no way to tell them apart, which is where the last investigation stopped. Carry a stable origin across the import boundary and retain it beside the reason. It inherits through forks and merges — the step that erased attribution before, since a tool crossing forks, imports, then merges back — so the refusal names the importer even though the latch happened frames earlier. Tool crossings take the tool id, so a tool-sourced bundle identifies itself rather than being inferred from timestamps. Origins are caller-supplied strings rather than a closed union, so unlike reasons they carry an explicit bound. No behaviour change: the field is optional, additive, and read only when building a log record.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryLow Risk Overview
All provenance import call sites are tagged with stable dotted ids (API routes, workflow/Mothership handlers, copilot tools, durable/table/logging paths, etc.). Tool responses use Tests assert the new options objects and cover origin attribution through fork-and-merge. Reviewed by Cursor Bugbot for commit 3ef0402. Configure here. |
Greptile SummaryThe PR adds bounded importer attribution to provenance-incompleteness diagnostics and preserves it through registry forks and merges.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| apps/sim/executor/utils/resolved-secret-trace-registry.ts | Adds bounded origin retention and propagates attribution through imports, path-scoped failures, forks, and merges. |
| apps/sim/executor/utils/resolved-secret-projection-refusal.ts | Adds retained importer origins to projection-refusal records when available. |
| apps/sim/lib/copilot/tools/handlers/function-execute.ts | Completes attribution on the previously identified import short-circuit and catch paths. |
| apps/sim/lib/copilot/request/tools/client.ts | Preserves the sealed-context importer identity on the previously identified incompleteness paths. |
| apps/sim/tools/index.ts | Threads the dynamic tool identifier into tool-crossing provenance imports. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
A[Provenance importer] -->|origin| B[Import guard]
B -->|incomplete bundle| C[Registry latches incomplete]
C --> D[Forked registry]
D --> E[Merge into parent]
E --> F[Projection refusal]
F --> G[Diagnostic includes reasons and origins]
Reviews (3): Last reviewed commit: "fix(provenance): stop a broad catch clai..." | Re-trigger Greptile
importProvenanceForValueAtInputPath took only { trusted }, so the five callers
that bind a crossing to an input path — the block, loop, parallel and workflow
resolvers, and the guardrails route — could not name themselves.
Separately, six latches sit beside a tagged import on the path where the import
did not run or returned false: a bundle already marked incomplete short-circuits
the || before the import, and each catch latches directly. Those reported no
origin while their neighbour reported one.
|
@cursor review |
The catch around table-row provenance loading also covers a database failure in loadTableRowSecretProvenance, which is not an incomplete bundle. Naming a reason the catch cannot know is the misattribution this work exists to remove, so it reports 'unspecified' with its origin, matching every sibling catch. The decrypt catch keeps its specific reason because its try wraps only the decrypt call.
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 3ef0402. Configure here.
Why
A provenance bundle that arrives already incomplete latches the registry one-way, so every later model projection in the run refuses. #6483 made the refusal say what went wrong. It could not say who.
That gap is not theoretical — it is exactly where the last investigation stopped. Production produced:
Correct, actionable up to a point, and then a dead end: 23 call sites import provenance, any of which could have accepted that bundle, and nothing distinguishes them. Narrowing by log timing produced a confident wrong answer.
What this changes
An optional
originon the import boundary, carried into the guards that latch:All 23 import sites are tagged; none are untagged. Tool crossings take the tool id (
tool.${toolId}), threaded through the metadata consumers, so a tool-sourced bundle identifies itself instead of being inferred from timestamps.The origin is retained on the registry beside the reason and inherits through forks and merges. That step matters: a tool crossing forks, imports, then merges back, and that is precisely where attribution was being erased. The refusal therefore names the importer even though the latch happened many frames earlier:
Unlike reasons — a closed 16-member union that bounds itself — origins are caller-supplied strings, so they carry an explicit
MAX_RETAINED_ORIGINSbound.Behaviour
None. The field is optional and additive; it is read only when building a log record.
markIncomplete's second parameter moves from a positionalsourceto a{ source, origin }context object — five internal call sites, no external ones (it is only ever called with a reason outside this file).Testing
410 passing in
executor/utils, including new coverage for: attribution recorded at the import guard, attribution surviving the fork-and-merge that hid it in production, and the origin bound. Each verified to fail when its specific guard is reverted.Wider run across
executor,lib,app,tools,providers: 16,784 passing, 0 test failures. Typecheck, biome, andcheck:api-validationclean. (141 suites fail to collect in my worktree from a local tailwind v4/v3 artifact — identical list on a clean tree with this change stashed.)Eight tests genuinely broke and were fixed properly. They pinned the exact options object passed to
importProvenance, so the added field failed them. Loosening the assertions would have discarded coverage, and a blanket replace would have been wrong — one file expects two different origins depending on which mock is asserted. Each assertion now asserts the attribution it should see.What this does not do
It does not fix the failing workflow. It converts the remaining unknown — which importer accepted the untrustworthy bundle — from an investigation into a field on the next log line.
Type of Change
Checklist