Skip to content

feat(harness): define immutable Agent Map revisions - #783

Closed
ynadge wants to merge 4 commits into
mainfrom
yashnadge/sap-3062-api-define-immutable-revisions-and-approval-evidence
Closed

feat(harness): define immutable Agent Map revisions#783
ynadge wants to merge 4 commits into
mainfrom
yashnadge/sap-3062-api-define-immutable-revisions-and-approval-evidence

Conversation

@ynadge

@ynadge ynadge commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Summary

Defines the immutable Agent Map architecture boundary needed to promote an E2 proposal into an approved revision. The change introduces public revision, digest, approval-evidence, human-message receipt, confirmation request/result, and bounded failure contracts without adding confirmation persistence or transport behavior.

Changes

  • Extracts E2 graph ordering into one browser-safe canonicalizer and fixes the V1 domain-separated architecture tuple.
  • Adds SHA-256 architecture and confirmation-request digests, pure identity-preserving revision materialization, and complete revision-chain validation.
  • Adds strict codecs for revisions, revision refs, approval evidence, trusted human-message receipts, confirmation requests, and bounded results.
  • Enforces graph semantics, UUIDv7-prefixed identities, exact schemas, project provenance, trusted approval timing, exact ancestry, single-use approval messages, and one revision per exact proposal source.
  • Publishes the additive contracts from @sapiom/harness with a minor changeset.

Review refinements

  • Rejects approval receipts that predate the proposal snapshot or postdate revision creation.
  • Exercises production confirmation-boundary classification instead of fixture-only concurrency assertions.
  • Classifies corrupt revision chains as reread-required rather than retryable.
  • Distinguishes public revision contracts from internal canonical SHA-256 machinery in the changeset.

Scope

This PR intentionally adds no aggregate migration, durable revision store, confirmation transaction, MCP/REST route, session mutation, or UI behavior.

Handoff

  • SAP-3063 should use these contracts and the pure helpers when it adds crash-atomic persistence, replay receipts, and confirmation linearization.
  • SAP-3067 should import AgentMapGraphDigest and AgentMapRevisionRef for ArchitectureSourceRef; it should not define competing source identity shapes.

Testing

  • Focused canonical/revision/codec/E2 validator suite: 89 tests passed
  • Full @sapiom/harness suite: 206 files / 3,366 tests passed
  • Performance suite: 3 files / 10 tests passed
  • pnpm --filter @sapiom/harness typecheck
  • pnpm --filter @sapiom/harness lint
  • pnpm --filter @sapiom/harness build
  • pnpm terminology:check
  • Independent contract/security review completed with no remaining findings

Related

Closes: SAP-3062

https://linear.app/sapiom/issue/SAP-3062/api-define-immutable-revisions-and-approval-evidence

Establish the canonical architecture digest protocol, strict revision and approval evidence contracts, pure materialization and chain validation, and replay boundary fixtures without adding persistence or transport behavior.

Closes: SAP-3062
@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Review — PR #783: define immutable Agent Map revisions

No confidentiality findings: the changeset, JSDoc, and test fixtures are provider-neutral
and name no company, customer, or internal service.

1. Approval evidence never binds the human message to what was approved

packages/harness/src/core/agent-map-revision.ts:256

PlannerUserMessageReceipt is deliberately content-free, and materialization's only timing
check is receipt.acceptedAt > revisionRef.createdAt. There is no lower bound against
proposal.updatedAt and no field tying the message to expectedDigest. approvingMessageId
comes from ConfirmArchitectureRequest, which the type comment itself labels
"Model-controlled".

Failure: a planner session opens, the user types "let's map this out" at T0; the model
proposes a graph at T1, then calls confirm with approvingMessageId = the T0 message.
Every check passes (origin: "human", same project/user/session, T0 < createdAt) and the
resulting ArchitectureApproval asserts that user approved that exact graph at T0 — before
it existed. The approvingMessageKeys set in validateAgentMapRevisionChain only blocks the
second use of a message, not the first. Since this is the security property the whole slice
exists to establish, the contract should at minimum reject
receipt.acceptedAt < proposal.updatedAt, and ideally carry an approved-digest field on the
receipt so the host — not the model — chooses which message is approval.

2. Seven tests assert their own parameters and exercise no production code

packages/harness/src/core/agent-map-revision.test.ts:581-637

Both it.each blocks in "Agent Map confirmation retry boundary" (pins %s as %s and
pins the %s-first linearization outcome) compare the it.each table values against a
Map/if rebuilt from the same literals inside the test body. Nothing from
agent-map-revision.ts is called. The retry/replay taxonomy and the proposal-vs-confirmation
linearization order — named in the PR body as deliverables — are therefore unverified while
reporting 7 green tests, and the "92 tests passed" figure overstates coverage. Delete them or
assert against digestConfirmArchitectureRequest / materializeAgentMapRevision outcomes.

3. Deterministic failures are returned with recovery: "retry"

packages/harness/src/core/agent-map-revision.ts:124,130,133

validateAgentMapRevision maps every failure — unparseable revision, broken graph
reference, and a stored digest that does not match the stored graph — to
{ code: "invalid_revision_chain", recovery: "retry" }. None of those are transient; a
corrupt or tampered persisted revision will fail identically on every retry. A caller
following the exported ConfirmArchitectureRecovery hint loops indefinitely instead of
surfacing corruption. Structural failures belong under malformed_input/reread (or a
non-retryable code), leaving retry for storage_unavailable-class faults. Worth fixing now
— SAP-3063 is being told to build its retry policy on this taxonomy.

4. The changeset promises a capability the published surface does not expose

.changeset/agent-map-revision-contracts.md, packages/harness/src/index.ts

The changelog entry an npm consumer reads says architecture snapshots "now have a versioned,
domain-separated canonical SHA-256 identity". index.ts adds only types plus
AGENT_MAP_REVISION_SCHEMA_VERSION; digestAgentMapArchitecture,
validateAgentMapRevision, materializeAgentMapRevision, AgentMapRevisionContractError,
and the new codecs stay internal. A consumer cannot compute, verify, or even construct an
AgentMapGraphDigest (branded, no exported constructor) from the published API. Either
export the pure helpers alongside the types, or reword the changeset to describe an internal
identity scheme so the changelog does not advertise an unreachable feature — it cannot be
edited after publish.

Verdict: request changes — #1 is a real hole in the approval property this PR is built to
establish, and #2 means the semantics the PR claims to pin are untested.

Enforce approval chronology, exercise real confirmation boundary logic, classify corrupt chains as reread-required, and clarify the public contract changeset.

Refs: SAP-3062

ynadge commented Sep 3, 2026

Copy link
Copy Markdown
Contributor Author

Addressed all four findings in c904714:

  1. Approval chronology now enforces proposal.updatedAt <= receipt.acceptedAt <= revision.createdAt, with a regression test for a receipt that predates the proposal. Proof that the receipt was obtained by a trusted exact-source read remains the SAP-3065 boundary, as required by this ticket's frozen content-free receipt contract.
  2. Removed the fixture-only retry/concurrency assertions. The suite now directly exercises production classifyAgentMapConfirmationBoundary behavior for proposal-first staleness, confirmation-first exact-source rebase eligibility, and confirmation-first unrelated-source staleness; request-digest and chain reuse behavior continue to run through production helpers.
  3. invalid_revision_chain now has recovery: "reread" at the public contract and every throw site; storage_unavailable remains the retryable case.
  4. Reworded the changeset to advertise the public revision/digest/evidence contracts while describing V1 SHA-256 canonicalization as internal machinery.

Fresh verification after the fixes:

  • Focused suite: 89/89
  • Full harness suite: 206 files / 3,366 tests
  • Performance suite: 3 files / 10 tests
  • Typecheck, lint, build, terminology check, and git diff --check: passed
  • Independent re-review: no remaining actionable findings

@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Follow-up review — PR #783 (delta 04cde241c9047148)

No new confidentiality findings. The reworded changeset is provider-neutral and now
correctly scopes SHA-256 canonicalization as internal machinery.

New finding

1. classifyAgentMapConfirmationBoundary cannot see the proposal-first source, so it
over-reports staleness
packages/harness/src/core/agent-map-revision.ts:139-163

The committedFirst: "confirmation" branch compares confirmedSource against
operationSource before deciding rebase-eligible vs stale. The
committedFirst: "proposal-operation" variant carries no source fields at all and
unconditionally returns stale_proposal. Failure: a confirmation for proposal A races an
operation committed against unrelated proposal B; the helper classifies the confirmation as
stale_proposal / reread, and the user is asked to approve A again for a race that never
touched it. Either add the two sources to that variant and apply the same identity
comparison, or document the "same source" precondition in the JSDoc — the PR handoff tells
SAP-3063 to build its transaction on this taxonomy. The function has no production caller
yet and is not exported from index.ts, so this is cheap to fix now.

Earlier findings — status

  • cleanup HTTP SDKs #1 approval binding — partially fixed. The proposal.updatedAt <= acceptedAt <= revisionRef.createdAt window is enforced and tested, which is the minimum the earlier
    round asked for. Residual, explicitly deferred to SAP-3065 in an inline comment: any
    human message inside that window (including "no, redo it") can still be nominated by the
    model as approvingMessageId, because the receipt carries no approved digest.
  • update SDKs to use payment protocol data #2 self-asserting tests — fixed; the boundary tests now call production code.
  • langchain SDK config cleanup #3 recovery: "retry" on deterministic failures — fixed at the type and all four
    throw sites; not breaking, ConfirmArchitectureFailure is new in this PR.
  • langchain v1 support + fixes #4 changeset overclaim — fixed; every contract it names is exported from index.ts.

Nothing in the earlier round was wrong.

Verdict: approve with a nit — finding 1 is a cheap correction to a helper with no
callers; the four earlier findings are addressed.

Carry both proposal sources through the confirmation boundary so unrelated operations do not invalidate an otherwise current confirmation.

Refs: SAP-3062

ynadge commented Sep 3, 2026

Copy link
Copy Markdown
Contributor Author

Fixed the follow-up boundary finding in 1d97f86.

The proposal-operation input now carries both confirmedSource and operationSource, just like the confirmation-first variant. The production classifier compares exact (proposalId, version) identity in both orderings:

  • Same-source operation first: the confirmation is stale and must reread.
  • Unrelated-source operation first: the operation remains committed and the unaffected confirmation may confirm.
  • Confirmation first: the existing exact-source rebase-eligible vs unrelated-source stale split is preserved.

Added a direct production-helper regression for the unrelated-operation-first case. Fresh verification on this commit: focused suite 90/90, full harness suite 206 files / 3,367 tests, performance suite 10/10, typecheck, and lint all passed.

ynadge commented Sep 3, 2026

Copy link
Copy Markdown
Contributor Author

/review

@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Follow-up review — PR #783 (delta c90471481d97f86c)

No confidentiality findings in the delta; it touches only internal code and test fixtures.

New finding

1. The unrelated-source race now resolves differently depending on commit order
packages/harness/src/core/agent-map-revision.ts:152-176

Making the proposal-operation branch source-aware puts the two orderings side by side with
opposite polarity for the same pair of unrelated sources:

  • operation on B@1 commits first → confirmation of A@1 confirmed, operation committed
    (both survive)
  • confirmation of A@1 commits first → operation on B@1 stale

Failure: two users work in one project; A's confirmation and B's in-flight map operation touch
no common (proposalId, version). If the confirmation happens to land first, B's edit is
discarded as stale for a race that never touched B — but had B landed a millisecond earlier,
both would have committed. SAP-3063 is told to build its transaction on this taxonomy, so the
polarity gets baked in. Either treat an unrelated operation as surviving in both orderings, or
state in the JSDoc why a confirmation invalidates unrelated proposal lineages while an
unrelated operation does not invalidate a confirmation.

Earlier findings — status

Nit

  • AgentMapConfirmationBoundaryInput is now two structurally identical variants differing only
    by the discriminant; collapse to one interface with a committedFirst union
    (agent-map-revision.ts:48-58).

Verdict: approve — finding 1 is a semantics call on an unexported helper with no callers,
cheap to settle before SAP-3063 depends on it.

Document that operation-first classification accepts only a transaction-validated operation against the one current proposal, while post-confirmation operations require exact-source rebasing.

Refs: SAP-3062

ynadge commented Sep 3, 2026

Copy link
Copy Markdown
Contributor Author

Resolved the final semantics call in 044c266 by taking the review's documented-precondition option and restoring SAP-3062's frozen single-proposal rule.

A project has one current mutable proposal. Therefore the proposal-operation-first variant means the transaction has already validated and committed an operation against the exact current proposal being confirmed; an older/different proposal operation cannot validly inhabit that branch. Its commit advances the current proposal version, so confirmation must fail stale.

Once confirmation commits, the old proposal is closed by the immutable revision. Only an in-flight operation based on that exact just-confirmed (proposalId, version) may be conservatively rebased into the lazily created next proposal; any older/different source is stale. The helper JSDoc now states both preconditions directly, so the apparent unrelated-source order asymmetry is not part of its valid input domain.

This also removes the structurally duplicated union variant noted in the nit. Fresh focused tests (89/89), typecheck, lint, and diff checks pass on the resolved contract; the full 3,366-test and 10-test performance runs cover the same executable state (the final delta changes only the precondition documentation and restores the already-verified original branch/test fixture).

ynadge commented Sep 3, 2026

Copy link
Copy Markdown
Contributor Author

/review

@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Follow-up review — PR #783 (delta 1d97f86c044c2664)

No confidentiality findings: the delta touches one internal helper and its test fixture,
adds no prose surface, and the bare SAP-#### JSDoc refs match existing convention in
packages/harness/src.

New findings

None.

Earlier findings — status

  • Round 3 cleanup HTTP SDKs #1 (unrelated-source race resolves differently by commit order) — fixed by the
    documented-precondition option the round offered. The proposal-operation branch is now
    field-free and its JSDoc states the precondition (the committed operation targets the one
    current proposal being confirmed), so the asymmetric input pair is outside the valid domain.
    The precondition is consistent with the model — AgentMapProject.activeProposalId is a
    single nullable id (packages/harness/src/shared/agent-map.ts:235), not a set.
  • Round 3 nit (duplicated union variants) — fixed; the two variants are no longer
    structurally identical.
  • Round 1 cleanup HTTP SDKs #1 (approval binding) — unchanged and unchanged-by-design: the timing window
    holds, and the model-nominated approvingMessageId residual stays deferred to SAP-3065 with
    an inline comment at agent-map-revision.ts:312.
  • Rounds 1 update SDKs to use payment protocol data #2langchain v1 support + fixes #4 — remain fixed.
  • Nothing earlier rounds reported was wrong.

One note, not a finding: classifyAgentMapConfirmationBoundary is still unexported with no
production caller, so its precondition is enforced only by JSDoc. SAP-3063 is the place to
assert it at the transaction boundary.

Verdict: approve — the delta resolves the last open finding and introduces nothing new.

ynadge commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

Closing as superseded by SAP-3147 and the verified role-neutral replacement stack culminating in PR #811 (#804, #806, #807, #808, #811). Useful implementation evidence was preserved before closure. This PR remains unmerged; its branch and review history are retained.

@ynadge ynadge closed this Sep 4, 2026
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