fix(provenance): report why a projection was refused, at the point of refusal - #6483
Conversation
… refusal A refusal fails closed and reaches the user as one fixed sentence. The guard that caused it may have tripped many frames — or a whole process — earlier, and the incompleteness latch is one-way, so by then the causing call has long returned. #6478 recorded the reason when the guard tripped, but marking early-returns once a registry is already incomplete, so a run that inherits an incomplete registry refused with nothing recorded anywhere. That is the case production actually hits. Retain reasons on the registry and report them where the refusal happens. The reason is recorded before the already-incomplete return so a causal chain accumulates, and before the silence checks so a by-design origin that logs nothing when marked is still nameable at refusal. Propagation inherits through markIncomplete's source argument, and copying incomplete input paths carries their reasons, so a fork cannot latch without its cause. Route all 68 refusal sites through one choke point that logs boundary, cause, input path and workspace before throwing. It returns never, so callers still narrow; messages and thrown types are unchanged at every site. Records carry a cause discriminator, since a latched registry and a caller-side cross-check of the projection's own output both arrive here and only the former has reasons. No behaviour change.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryLow Risk Overview
Agent/memory paths gain stable refusal constants and pass the registry into Reviewed by Cursor Bugbot for commit 335f2c1. Configure here. |
Greptile SummaryThe PR preserves resolved-secret incompleteness reasons and reports them at projection-refusal boundaries without changing fail-closed behavior.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains; the previously reported cross-request suppression was removed, and current registry-less refusal paths report independently and abort before they can repeat within one request.
|
| Filename | Overview |
|---|---|
| apps/sim/executor/utils/resolved-secret-trace-registry.ts | Retains bounded incompleteness reasons and propagates them through registry lifecycle operations for later diagnostics. |
| apps/sim/executor/utils/resolved-secret-projection-refusal.ts | Adds centralized refusal reporting with per-registry deduplication while leaving registry-less requests independently observable. |
| apps/sim/executor/utils/resolved-secret-projection-refusal.test.ts | Covers diagnostic attribution, error preservation, deduplication boundaries, and exclusion of secret material. |
| apps/sim/lib/copilot/request/lifecycle/run.ts | Routes attachment projection failures through the shared reporter; each registry-less refusal still aborts its request immediately. |
| apps/sim/app/api/mcp/serve/[serverId]/route.ts | Routes MCP projection refusals through the shared helper while preserving existing request failure behavior. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
Guard[Provenance guard trips] --> Registry[Registry retains ordered reasons]
Registry --> Propagation[Fork or merge propagates reasons]
Propagation --> Projection[Projection boundary checks result]
Projection -->|Accepted| Continue[Continue execution]
Projection -->|Refused| Reporter[Log boundary and diagnostics]
Reporter --> Throw[Throw existing message and error type]
Reviews (3): Last reviewed commit: "improvement(provenance): inherit copied-..." | Re-trigger Greptile
…uests A refusal with no registry aborts the request rather than iterating, so it reaches the reporter at most once per request. Remembering it for the process silenced every later request, including the one being investigated.
|
@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 398e2a3. Configure here.
Semantics are unchanged — reasons are still inherited only when at least one incomplete path was actually copied — but the source set is walked once rather than once per copied path.
|
@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 335f2c1. Configure here.
Why
A resolved-secret projection that fails closed reaches the user as one fixed sentence (
… could not be safely projected). The guard that caused it may have tripped many frames — or a whole process — earlier, and the incompleteness latch is one-way, so by the time a run refuses, the causing call has long since returned.#6478 recorded the reason when the guard tripped. But
markIncompleteearly-returns once a registry is already incomplete, so a run that inherits an incomplete registry refuses with nothing recorded anywhere. That is the case production actually hits: in the hours after #6478 deployed, every observed incompleteness wassource-provenance-incompleteorinherited-incomplete-source, andprojection-mismatchfired zero times.What this changes
Nothing behavioural. Two pieces:
1. The registry retains its reasons. Previously the reason was handed to the logger and dropped. It is now kept in an insertion-ordered set with a
getIncompletenessDiagnostics()accessor that returnsundefinedwhile the registry can still vouch, so a caller cannot report a cause for a projection that succeeded.Two placement decisions carry the weight:
constructed-incomplete— which deliberately logs nothing when marked, because it sits on the per-webhook hot path — is still nameable at refusal. That was the blind spot fix(provenance): record why a resolved-secret registry became incomplete #6478 introduced.Propagation is folded into the latch:
markIncomplete(reason, source?)inherits the source's reasons, andcopyIncompleteInputPathsTocarries reasons with the paths (a target becomes permanently incomplete through the copy alone, with no mark call to attach a cause to). A future fork/merge path therefore cannot latch without its cause.2. One choke point for refusal.
refuseResolvedSecretProjection()logs boundary, cause, input path and workspace, then throws. All 68 refusal sites across 13 files now route through it.never, soif (!projection.complete) refuse(...)still narrows — each site is a one-line swap, no restructuring.AgentToolInputSafetyError×29,CopilotModelContentProjectionError×2), so existingcatchclauses still match.(site, inputPath)— an agent projects tool input on every loop iteration, and a latched registry would otherwise emit a line per iteration. A refusal with no registry is never deduplicated: those sites abort the request rather than iterate, so any process-wide memory of them would silence every later request, including the one being investigated.The
causediscriminatorTwo different failure families reach this one event: a registry that latched and genuinely cannot vouch, and a caller-side cross-check finding the projection's own output malformed (
!isPlainRecord(projected), arity mismatches). Only the first carries reasons, so a query filtered onreasonwould silently cover half the population. Each record therefore carriescause: 'registry-latched' | 'projection-cross-check'.Why the instrumentation lives at the throw site
{safe: false}is not synonymous with refusal —pi/local/sim-tools.tsandcopilot/tools/server/media/ffmpeg.tsboth consume the same projection helpers and degrade gracefully. Instrumenting the projection API would log fleet-visible errors for handled outcomes, and would still miss most of these 68 sites, which are not projection-call failures at all but caller-side cross-checks the projection function cannot see. Refusal is a caller-side concept.Deliberately not in this PR
The reason still does not cross a process boundary. The serialized envelope carries a bare
complete: false, so a consumer can reportsource-provenance-incompletebut never say why the producer failed. That is a wire-format change with two hard constraints:isResolvedSecretTraceProvenanceV1validates with an exact-property check, so an added field makes an old consumer reject the envelope outright during a rolling deploy; and envelope size is checked againstMAX_SERIALIZED_PROVENANCE_BYTES, so added bytes can push a borderline envelope over and cause newprovenance-capacity-exceededrefusals. It needs its own version bump and byte accounting.This PR is still a large win without it: the producer process now logs the real cause locally, so cross-process correlation becomes "join two log lines" instead of correlating CloudWatch against S3 blobs by workspace id.
Three sites still pass no registry (
copilot.initialAttachments*×2,mcpServe.executionError) — each would need the registry threaded up several frames. Down from 8; the fiveresponseFormatsites were fixed here.Type of Change
Testing
Registry + refusal suites 406 passing, including coverage for: retention before the early return, retention of a silenced by-design origin, inheritance through a fork naming the originating guard, per-registry dedup under a 25-iteration loop, distinct paths reported separately, the latched/cross-check discriminator, and absence of secret material in the record. Each was verified to fail when its specific guard is reverted.
Wider run across
executor,lib,app,tools,providers: 16,726 passing, 0 test failures. Typecheck, biome, andcheck:api-validationclean. (140 suites fail to collect in my worktree from a local tailwind v4/v3 artifact — byte-identical list on a clean tree with this change stashed.)Messages verified byte-identical against the base commit per file, with counts.
Checklist