Skip to content

feat(agent): add graph evidence protocol - #752

Open
ynadge wants to merge 3 commits into
mainfrom
yashnadge/sap-2985-api-define-and-validate-the-graph-evidence-protocol
Open

feat(agent): add graph evidence protocol#752
ynadge wants to merge 3 commits into
mainfrom
yashnadge/sap-2985-api-define-and-validate-the-graph-evidence-protocol

Conversation

@ynadge

@ynadge ynadge commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Summary

Define protocol 1 for deterministic package-scoped graph evidence in @sapiom/agent. The contract keeps identity inventory separate while giving static and runtime producers one strict language for explicit endpoints, typed relation/basis pairs, scope, freshness, provenance, diagnostics, and lifecycle behavior.

This is the bounded SAP-2985 foundation only. It does not add source analysis, runtime emission, persistence, production projection/query behavior, or Studio UI.

Changes

  • Add strict Zod contracts for the four protocol-1 relation/basis variants:
    • invokes/static-invocation
    • invokes/runtime-dispatch
    • feeds/static-dataflow
    • feeds/runtime-handoff
  • Add exact inventory-scope reuse, independent analysis fingerprints, producer versions, canonical SHA-256 identities, deterministic normalization, diagnostics, and endpoint quarantine.
  • Add distinct static-result and bundle-only runtime-event envelopes, with reference replacement/retraction and event-id idempotency semantics.
  • Add a scoped conformance projector that preserves every inventory node and supporting basis, diagnoses settled identity or scope mismatches, and drops only affected connectors.
  • Enforce canonical set-like references on parsed wire envelopes so equivalent facts cannot validate with different IDs.
  • Prevent runtime-event state from mixing bundle scopes and document its one-state-per-bundle lifetime.
  • Keep hashing and intermediate composition schemas internal, publish only the producer/consumer value surface plus the types reachable from it, document producer-owned privacy obligations, and add an @sapiom/agent minor changeset.
  • Cover canonical Research → formatter → Growth, sibling-call negative, degraded/provisional local identity, canonical bundles, lifecycle, replay, privacy, and all four evidence variants.

Testing

  • pnpm build
  • pnpm typecheck (after the repository's required build step)
  • pnpm lint
  • pnpm --filter @sapiom/agent test --runInBand — 11 suites, 195 tests
  • pnpm provider-copy:check
  • CJS/ESM public export smoke checks and package dry-run
  • pnpm changeset status --since=origin/main
  • git diff --check

The full workspace test command was also exercised. All affected @sapiom/agent tests pass; the run stops later on an unrelated existing agent-core unreadable-directory classification test in this sandbox (bundle-error.spec.ts).

Related

Closes: SAP-2985

Checklist

  • Contract consumes the finalized PackageInventoryVersion
  • Public DTOs carry only restricted reference handles; producers remain responsible for keeping raw paths, IDs, and payload content behind their resolver
  • No analyzer, runtime instrumentation, persistence, or UI scope
  • Self-reviewed against the finalized Linear acceptance criteria

@github-actions

Copy link
Copy Markdown
Contributor

Review — PR #752 feat(agent): add graph evidence protocol

Public-copy check: clean. The changeset, README section, JSDoc, and test fixtures use
generic role names (coordinator / research / growth / formatter) and acme
placeholders; no customer, sibling-product, or internal-service names; no URLs or
hostnames. The SAP-2950 JSDoc reference matches existing precedent in published source
(agent-core/src/types.ts, agent/src/agent.ts). Changeset level (minor) is right for
a purely additive export.

Findings

1. projectPackageGraphEvidence throws on inputs the protocol admits as valid

packages/agent/src/package-graph-evidence.ts:1268

Scope identity is the inventory version, which excludes status
(package-inventory.ts:223: working-tree = workspaceKey + revision). Two inventories
at the same version can therefore carry different agentKeys — a degraded inventory
resolves an agent as local:agents/research (provisional) where a complete one resolves
research. Pass a static result built against the canonical inventory plus a degraded
inventory of the same revision and scopeMatches passes, then the endpoint check throws
TypeError("Reference projection accepts validated evidence only"). A consumer
(Studio/server) rendering a graph gets no graph at all instead of one dropped connector —
and this is data the producers here will legitimately hand it, not a programmer error. The
PR's own test at package-graph-evidence.spec.ts:300 exercises provisional identities, but
only within one inventory. Everywhere else this protocol degrades via quarantine +
diagnostics; this path should emit a diagnostic and drop the connector, not throw.

2. The wire schema does not enforce reference normalization — same facts, two identities

packages/agent/src/package-graph-evidence.ts:716

normalizeCandidate sorts and dedupes callsites, but only inside the constructor.
staticResultBaseSchema declares callsites: z.array(...).min(1) with no order or
uniqueness constraint, and expectedStaticEvidenceId hashes candidateFromRecord(record)
verbatim. So a payload with callsites: [b, a] (or with a duplicate callsite) and IDs
computed over that unnormalized content validates cleanly — with an evidenceId and
resultId different from what this repo's constructor produces for identical facts. The
changeset advertises "deterministic static results"; that only holds for producers that
route through this TypeScript constructor. Note the asymmetry: coverage gaps, diagnostics,
and quarantine are each checked against their normalizer
(lines 804, 839, 850) — reference arrays are not, which reads as an oversight. Either
compare each record against normalizeCandidate in superRefine, or document that
callsite order is semantic.

3. Published JSDoc asserts a privacy property the schema does not enforce

packages/agent/src/package-graph-evidence.ts:49-53 and README.md

"Paths, execution IDs, lineage IDs, and payloads do not enter the graph-evidence wire
contract itself" ships in the .d.ts and reads as a guarantee. OPAQUE_REFERENCE
(^[A-Za-z0-9][A-Za-z0-9._:@+-]{0,255}$) only enforces a character class: a raw execution
UUID (550e8400-e29b-41d4-a716-446655440000) matches, as does a slash-free dotted path.
Opacity is a producer obligation, not a contract property. Reword both surfaces to state
the obligation ("producers must keep … behind their resolver; the schema enforces only a
safe character class"), or the first producer that inlines an execution ID will do so
believing the contract prevented it. Same wording issue in the PR checklist item "No raw
paths or payload content enter public evidence DTOs".

4. 44 new names on a published API surface with no consumer yet

packages/agent/src/index.ts:124-172

20 values + 24 types, and grep finds no use of any of them outside
package-graph-evidence.ts and its spec — no analyzer, no runtime, no Studio (by design
per the PR body). Every one is a semver commitment before a single producer has shaped the
contract. Load-bearing today: PACKAGE_GRAPH_EVIDENCE_PROTOCOL, the two constructors, the
two envelope schemas, the projector, and the top-level types. Not load-bearing:
canonicalPackageGraphEvidenceJson and packageGraphEvidenceSha256 (generic JSON/hash
utilities with nothing graph-specific about them, now permanent public API), plus the
intermediate sub-schemas (…ProducerSchema, …CoverageGapSchema,
…DiagnosticCodeSchema, …QuarantineSchema, …ReferenceSchema) that exist only to
compose the envelopes. Keep those internal until a producer needs them; widening later is
a minor, narrowing is a break.

Nits

  • README.md:237 — the closing ``` fence was dedented from 2 spaces to 0, which ends the
    enclosing list item; unrelated churn in a shipped README, and renderer-dependent.
  • appendPackageGraphRuntimeEvidenceEvent doesn't check that the incoming event's bundle
    scope matches events already in state, so a state object can mix bundles until
    projection rejects it.
  • packageInventoryVersionSchema is re-exported from index.ts although only
    package-graph-evidence.ts needs it; a plain module export would keep it internal.

Verdict: Request changes — findings 1 and 2 are contract defects that get harder to
fix after this ships as a published protocol; 3 and 4 are cheap now and expensive later.

ynadge added 2 commits August 31, 2026 17:43
Make projection tolerate settled identity changes, enforce canonical callsite sets on the wire, reject cross-bundle runtime state, and narrow the public API before release.\n\nRefs: SAP-2985
@ynadge
ynadge force-pushed the yashnadge/sap-2985-api-define-and-validate-the-graph-evidence-protocol branch from e0032b3 to ebc7f06 Compare August 31, 2026 17:56
@github-actions

Copy link
Copy Markdown
Contributor

Review — PR #752 (round 2, delta only: ebc7f06d)

Public-copy check on the delta: clean. The reworded README paragraph and JSDoc stay generic
(acme producer id, role names); no company, host, or internal reference added. Changeset
unchanged and still accurate.

Earlier findings — all four fixed

  1. Projector no longer throws on stale endpoints: it emits unknown/ambiguous/invalid-endpoint
    • illegal-self-relationship diagnostics, drops only the affected connector, keeps all
      inventory nodes. Covered by a same-version degraded-inventory test.
  2. staticResultBaseSchema now rejects non-canonical set-like references in superRefine
    (normalizeCandidate round-trip), with an it.each over reordered and duplicated callsites.
    Runtime bases have no set-like fields, so the static-only check is complete.
  3. JSDoc and README now state the obligation ("schema enforces a restricted character set; it
    cannot determine whether a permitted string contains a sensitive identifier"). PR checklist
    reworded too.
  4. Public surface narrowed by 15 names: the two generic JSON/hash utilities and the intermediate
    sub-schemas are internal again. All three nits (README fence, cross-bundle runtime state,
    packageInventoryVersionSchema) fixed; none of the removals touch anything published on main.

New

  • appendPackageGraphRuntimeEvidenceEvent state is now implicitly bundle-scoped, and nothing
    says so.
    package-graph-evidence.ts:1176 — a consumer that keeps one long-lived state across
    a redeploy gets status: "conflict" for every event of the new bundle, forever, with no hint
    that the fix is a fresh state per bundle. The JSDoc still only mentions event-ID idempotency.
    Document the per-bundle lifetime on the function and in the README.

Nits

  • The narrowing overshot for types reachable from exported signatures — PackageGraphEvidenceConnector,
    …ProjectedSupport, …ProjectionSource, …Record, …Quarantine, …DiagnosticCode. With only a
    "." export, consumers can name them solely via indexed access. Type-only re-exports cost nothing.
  • Scope mismatch in projectPackageGraphEvidence still hard-throws — same shape as fixed finding 1
    for a consumer holding one stale result.
  • Projection diagnostics carry quarantineId handles that resolve to nothing, since quarantine
    entries are not projected.

Verdict: Approve — the round-1 contract defects are genuinely fixed; the remainder is docs and
type re-exports.

ynadge commented Aug 31, 2026

Copy link
Copy Markdown
Contributor Author

/review

@github-actions

Copy link
Copy Markdown
Contributor

Review — PR #752 (round 3, delta only: ef35e82b)

Public-copy check on the delta: clean. The new README paragraph and JSDoc stay generic; no
company, host, or internal reference added. Changeset unchanged.

Round-2 items — all fixed

Bundle-scoped state documented on the function and in the README; scope mismatch now emits a
cross-scope diagnostic and drops the source instead of throwing; quarantineId stripped from
projected diagnostics; the six reachable types re-exported.

New

  • A cross-scope static result is dropped with no handle to say which one.
    package-graph-evidence.ts:1285 pushes { code: "cross-scope", severity: "error" } with no
    evidenceId, candidateFingerprint, or producer — and normalizeDiagnostics dedupes by
    canonical JSON, so five stale results from five producers collapse into a single diagnostic.
    Runtime events at least carry eventId. A consumer rendering the projection can tell that
    something was out of scope but not what or how much; that defeats the point of the round-1
    fix this commit generalizes. Attach the source's record evidenceIds (or add a resultId
    field to the diagnostic schema).
  • continue at line 1292 also discards the source's own diagnostics. A stale static result's
    incomplete-analysis / producer-failed / quarantine diagnostics never reach the projection,
    because the diagnostic merge at line 1296 is below the scope check. Silent loss, not a drop of
    a connector.

Nits

  • README.md:76 — "cross-bundle appends conflict and leave the existing state unchanged" is
    wrong in one detail: the returned state gains a cross-scope diagnostic. A consumer that
    discards the returned state on status: "conflict" loses it. Same sentence says "when the
    bundle digest changes", but a working-tree–scoped state rotates on revision.
  • Stripping quarantineId also merges two different producers' diagnostics for the same
    candidate+code into one. Round 2 overstated the dangling handle — the consumer holds the
    source result and could resolve it there — but the merge is a fair trade.

Verdict: Approve — the round-2 items are genuinely fixed; the new items are diagnostic
fidelity in a path that already degrades correctly.

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