improvement(provenance): name every guard that can latch a registry - #6513
Conversation
A production latch reported `reason: "unspecified"` because 44 call sites took the default. The reason is the only thing that names which guard tripped, and a refusal surfaces many frames later as one fixed sentence, so an unnamed latch is undiagnosable — that is what left an incident's origin unidentified for a day. Give each call site a literal that names its guard, add the 20 new literals to the reason union, and sort them into the existing error/warn split: a guard that should not trip on a healthy run reports at error, everything else stays at warn. `log-creation-skipped` joins the by-design set since it fires on every run that does not persist a log. Make `reason` required on both `markIncomplete` and `markInputPathIncomplete`, so omission is a compile error rather than a silent `unspecified`. A caller with genuinely nothing to say now passes `'unspecified'` where a reviewer can see it. The three remaining bare calls are on ResolvedSecretTraceProvenanceAccumulator, a different class with no reason concept. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…roduction Cover what the reason set is for rather than only that it compiles: the non-enumerable tool-params branch now asserts it names `tool-input-not-enumerable`, which is the guard the production logs showed reporting `unspecified`, and every new literal asserts which stream it reports on — error for a guard that cannot trip on a healthy run, warn for one reachable without a fault, silent for the by-design log-less session. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two fields had grown into competing answers to the same question. `origin` is a
free-form label for which importer accepted an already-incomplete bundle; four
latches had started passing `markIncomplete('unspecified', { origin })`, using it
to stand in for a reason that did not exist yet. That splits one fact across a
closed enum and an open string, leaving neither worth alerting on.
Give those four the literal they were reaching for — none needed a new one — and
split the five reasons that covered genuinely different guards, so the reason
alone locates the site rather than needing an origin beside it. `origin` keeps its
narrow job, now documented: it disambiguates importers that share one guard, and a
latch that wants an origin because no reason fits should add a reason instead.
No production call site passes 'unspecified' any more.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryMedium Risk Overview The change wires specific reason literals through knowledge search, memory/knowledge provenance APIs, mothership execution, generic block input projection, copilot client tools (seal/completion/workflow restore), table backfill, logging sessions, and tool param enumeration—replacing bare
Reviewed by Cursor Bugbot for commit f4ac6a1. Configure here. |
Greptile SummaryThis PR makes provenance-registry latches explicitly name their cause and classifies those causes for clearer warning/error diagnostics.
Confidence Score: 5/5The PR appears safe to merge because it improves provenance diagnostics without weakening or changing the registry’s fail-closed behavior. All current latch callers provide explicit reasons, and reason classification changes only logging severity while preserving completeness, propagation, and refusal semantics.
|
| Filename | Overview |
|---|---|
| apps/sim/executor/utils/resolved-secret-trace-registry.ts | Expands the closed reason taxonomy, requires explicit latch reasons, and classifies reasons for diagnostic severity without changing fail-closed semantics. |
| apps/sim/lib/copilot/request/tools/client.ts | Names client-tool sealing, completion, trust, content, and provenance-import failure paths. |
| apps/sim/executor/handlers/mothership/mothership-handler.ts | Attributes invalid, unreadable, and missing Mothership provenance across buffered and streaming responses. |
| apps/sim/lib/knowledge/secret-provenance.ts | Distinguishes knowledge provenance capacity, missing-row, and content-mismatch failures. |
| apps/sim/tools/index.ts | Names the non-enumerable tool-input guard and adds regression coverage for getter-backed parameters. |
| apps/sim/lib/logs/execution/logging-session.ts | Separates untrusted restored provenance from intentionally skipped log creation. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
Guard[Provenance guard fails] --> Mark[markIncomplete with named reason]
Mark --> Latch[Registry becomes incomplete]
Mark --> Classify{Reason category}
Classify -->|Originating fault| Error[Error diagnostic]
Classify -->|Expected incompleteness| Warn[Warning diagnostic]
Classify -->|By design| Silent[No diagnostic]
Latch --> Refusal[Existing fail-closed projection and import behavior]
Reviews (1): Last reviewed commit: "refactor(provenance): make reason the on..." | Re-trigger Greptile
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Cursor Bugbot caught `backfill-checkpoint-*`: the guard covered four conditions under one reason classified as an originating fault, and one of them — a state persisted before the checkpoint contract existed — is what essentially every legacy row looks like. A backfill over historical rows would have put one error line per row into the stream the error/warn split exists to protect. Auditing the rest of the error-level reasons for the same shape found a second: a client tool invoked without a run id has no binding to unseal against, so it took the `[null, null]` path and reported `client-tool-seal-failed` at error on an ordinary configuration. Split both along the line that matters — absent versus unusable, not attempted versus failed — and classify each half: expected states warn, genuine faults keep error. `backfill-scope-mismatch` is retired; it named one of its four conditions. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
bugbot run |
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 f4ac6a1. Configure here.
1 participant
Summary
Better logs for provenance failures
Type of Change
Testing
Tested manually
Checklist