Skip to content

fix(provenance): report why a projection was refused, at the point of refusal - #6483

Merged
waleedlatif1 merged 3 commits into
stagingfrom
feat/provenance-refusal-diagnostics
Aug 10, 2026
Merged

fix(provenance): report why a projection was refused, at the point of refusal#6483
waleedlatif1 merged 3 commits into
stagingfrom
feat/provenance-refusal-diagnostics

Conversation

@waleedlatif1

@waleedlatif1 waleedlatif1 commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator

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 markIncomplete early-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 was source-provenance-incomplete or inherited-incomplete-source, and projection-mismatch fired 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 returns undefined while the registry can still vouch, so a caller cannot report a cause for a projection that succeeded.

Two placement decisions carry the weight:

  • Recorded before the already-incomplete early return, so a causal chain accumulates rather than only the first reason.
  • Recorded before the silence checks, so 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, and copyIncompleteInputPathsTo carries 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.

  • Returns never, so if (!projection.complete) refuse(...) still narrows — each site is a one-line swap, no restructuring.
  • The message is passed by the caller, so user-facing wording is unchanged; per-file constants mean 50 hand-typed copies can no longer drift apart.
  • Custom error types are preserved (AgentToolInputSafetyError ×29, CopilotModelContentProjectionError ×2), so existing catch clauses still match.
  • Deduplicated per registry per (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 cause discriminator

Two 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 on reason would silently cover half the population. Each record therefore carries cause: 'registry-latched' | 'projection-cross-check'.

Why the instrumentation lives at the throw site

{safe: false} is not synonymous with refusal — pi/local/sim-tools.ts and copilot/tools/server/media/ffmpeg.ts both 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 report source-provenance-incomplete but never say why the producer failed. That is a wire-format change with two hard constraints: isResolvedSecretTraceProvenanceV1 validates 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 against MAX_SERIALIZED_PROVENANCE_BYTES, so added bytes can push a borderline envelope over and cause new provenance-capacity-exceeded refusals. 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 five responseFormat sites were fixed here.

Type of Change

  • Bug fix (diagnosability; no behaviour 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, and check:api-validation clean. (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

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

… 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.
@vercel

vercel Bot commented Aug 10, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Aug 10, 2026 3:14am

Request Review

@cursor

cursor Bot commented Aug 10, 2026

Copy link
Copy Markdown

PR Summary

Low Risk
Diagnosability-only change on secret projection paths; user-facing errors and projection behavior are unchanged, with tests covering refusal logging and reason retention.

Overview
Adds diagnosability when resolved-secret projections fail closed without changing user-facing errors or projection logic.

ResolvedSecretTraceRegistry now keeps incompleteness reasons (insertion-ordered) and exposes getIncompletenessDiagnostics() so a refusal far from the original guard can still report why the registry latched—including inherited reasons when forking/merging or copying incomplete input paths.

refuseResolvedSecretProjection() becomes the single choke point for all projection refusals (~68 sites across executor handlers, MCP serve, copilot, knowledge, tools transport, guardrails). It logs site, optional inputPath, cause (registry-latched vs projection-cross-check), and registry diagnostics, then throws the same caller-supplied message (and optional custom error types like AgentToolInputSafetyError). Logging is deduplicated per registry per (site, inputPath) to avoid loop floods.

Agent/memory paths gain stable refusal constants and pass the registry into projectResponseFormatObject / readFunctionCallArguments so refusals correlate to the failing run.

Reviewed by Cursor Bugbot for commit 335f2c1. Configure here.

@greptile-apps

greptile-apps Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR preserves resolved-secret incompleteness reasons and reports them at projection-refusal boundaries without changing fail-closed behavior.

  • Retains ordered incompleteness diagnostics across registry forks, merges, and copied input paths.
  • Routes projection refusals through a shared reporter while preserving existing messages and custom error types.
  • Deduplicates repeated reports per registry and boundary while reporting every independent registry-less refusal.

Confidence Score: 5/5

The 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.

Important Files Changed

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]
Loading

Reviews (3): Last reviewed commit: "improvement(provenance): inherit copied-..." | Re-trigger Greptile

Comment thread apps/sim/executor/utils/resolved-secret-projection-refusal.ts Outdated
…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.
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ 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.
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ 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.

@waleedlatif1
waleedlatif1 merged commit 3fe2f4f into staging Aug 10, 2026
30 checks passed
@waleedlatif1
waleedlatif1 deleted the feat/provenance-refusal-diagnostics branch August 10, 2026 05:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant