Skip to content

feat(engine): establish canonical tool-call identity and private replay - #2404

Draft
jbeda wants to merge 10 commits into
mainfrom
impl/2117-engine-identity
Draft

jbeda wants to merge 10 commits into
mainfrom
impl/2117-engine-identity

Conversation

@jbeda

@jbeda jbeda commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor

Engine foundation: canonical tool-call identity and private replay

Review scope

Engine foundation for #2117. Review engine/** and the necessary .golangci.yml depguard allowance. Provider implementations, external persistence drivers, host/contracts/SDK/client changes, and TUI presentation belong to the dependent drafts, not this PR.

  • Mint globally unique canonical invocation IDs at accepted-turn conversion, separate from provider pairing/item IDs.
  • Validate whole incoming call/result batches before mutation; match results and parked continuations to exact accepted occurrences.
  • Keep provider-neutral LLM DTOs at the port boundary and session-owned, occurrence-bound opaque replay outside executable domain identity.
  • Preserve nonpending legacy snapshot history and provable complete-event controls/evidence; reject unsupported legacy snapshot continuations and abandon unprovable complete-event continuations.
  • Supply schema-2 snapshot/event codecs, inert historical projections, and engine-local conformance/regression coverage.

Current engine checkpoint

Rebased the original commits from 701df1f6a onto 55b980d34b4518221c9385a2ab32e2bb5c3f4f8a. Published engine head: 80ddd21affb4dcf7c6141f64866e17d7456ef88b; this is not a claim of qualification against today's main.

  • Preserve main's permission-mode inheritance, yolo background-review behavior, and provider stop-detail reporting. The loop conflict moves StopDetail handling into the new foldTurnChunk terminal branch; the regenerated port API snapshot retains the field.
  • Keep both sets of changelog entries. git range-diff confirms the cache-review and codec-abandonment commits replay unchanged.
  • Follow-up 1928b1875 adapts new main tests to accepted canonical call IDs and returned child handles. It also makes background-child denial/collection and evidence-failure result assertions non-vacuous. No production change in that follow-up.
  • feat(providers): preserve pairing and replay behind canonical identity #2405, feat(adapters): migrate canonical snapshot and event identity formats #2406, and feat: propagate canonical identity through harness API v2 and clients #2407 source branches remain unchanged. Their descriptions now identify the current policy, but stacked comparisons and assembled qualification still need refreshing before coordinated delivery.
  • 31e9f27eb rejects unsupported legacy snapshot continuations and never-shipped hold formats. 71b1352aa isolates legacy/current decoding and normalizes before Restore. 80ddd21af moves complete-history reconstruction into eventsource and renames the deterministic snapshot helper without changing its derivation.

Migration contract: reject unsupported snapshots, preserve evidence

This supersedes the original draft's persistent identity-hold policy and the earlier blanket load-time abandonment policy:

  • Legacy snapshots: preserve nonpending /1 history, canonical identity/provenance/replay migration, argument key order and stable metadata. Reject legacy awaiting/authorizing states even with absent/null pending payloads; reject any non-null legacy pending approval/private-authorization payload in any state. These snapshots are unloadable, not normalized to idle. Resolve parked work before upgrade or start fresh; no silent fallback.
  • Version isolation: separate /1 DTO decoding migrates before Session construction; /2 uses the current decoder. Both yield a current Snapshot. Restore only reconstructs the current schema, and re-encoding writes /2. Valid stored incarnation survives; absent identity derives deterministically; invalid nonempty identity rejects.
  • Never-shipped formats: reject any identity_hold key, including null and schema 2. Direct schema-zero Snapshot inputs reject; supported legacy storage migration enters through JSON decoding. No speculative compatibility for candidate-only formats.
  • Current /2 controls: preserve valid asks/private authorization, exact effective argument bytes, ordered deferred calls, retry and enrollment metadata. Stale approval/retry/post-authorization entry rejects before effects or saves.
  • Complete events: preserve provable legacy controls; abandon unprovable continuations before later canonical activity with an unknown-effects notice. Close only proven outstanding calls; preserve completed results and readable salvage/provenance without guessed attachments. Do not imply nonexecution, rollback or authorization revocation. Fold-time synthetic interruption outcomes do not become invented durable events. Later valid asks/retry metadata survive another fold; canonical open authorization without private state returns ErrPrivateStateRequired.
  • Ownership: eventcodec owns storage schema/versioning, private legacy parsing/provenance, narrow DecodeLegacyRecord facts and sparse ProjectRecord. Eventsource owns complete-source Project/Projection, pairing/continuation proof, segmentation and private history normalization. Old codec reconstruction exports/wrappers are removed. Sparse records are display-only, never continuation authority.

Normal resume loads snapshots, not event folds. Learning's exact-run evidence and debugger history inspection use folds; copied history or stored IDs cannot establish execution, learning or promotion authority. No broader persistence, compaction or learning-storage redesign is included.

The approved ACP boundary remains unchanged: passive hydration is effect-free, while explicit editor reconnect may attach placement, mount supplied MCP servers, discover commands/capabilities, and report setup errors. It cannot execute tools, resume abandoned work, create permission grants, or publish learning; a separate prompt uses normal admission. ACP implementation and handler tests belong to #2407.

Additional behavior fixes worth reviewing

These were identified against the original base f9a3297c1, rather than claimed as defects introduced by recent main changes:

  • Restarted approval could target the wrong same-named call. Resume resolves the exact unanswered canonical occurrence. See engine/agent/continuation_identity_test.go.
  • A stale/wrong approval request could destroy a valid pending ask. Rejection leaves legitimate pending state and history unchanged, without execution, persistence, or learned permission effects. See the same continuation regressions.
  • Provider-reported error/cancellation stops could admit accumulated calls. ChunkDone(StopError/StopCancelled) discards tentative calls/private replay while retaining usage and display content. Iterator errors and context cancellation already rejected tentative calls at the original base. See engine/agent/tool_call_identity_test.go.
  • Cancellation during serial preparation or approval resume could leave accepted calls without final results. Record and publish cancellation results immediately; this is not a cross-crash exactly-once guarantee. See engine/agent/canonical_results_test.go.
  • Snapshots could alias mutable live history. Capture and restore independently own nested arguments, result/media storage, replay, and provenance. See engine/adapter/sessnap/replay_test.go.
  • Memstore metadata/lineage returned a stored relationship pointer. Clone BranchIndex and private provenance. This is a source-verified ownership fix, not a claim of a dedicated new BranchIndex mutation regression.

Whole-batch validation, compaction projection admission, eventsource-owned abandonment, original-source coordinates, verified reflection bindings, and private-ID redaction are identity/replay/evidence hardening, not additional historical-bug claims. Host replay/inspection fixes belong to the follow-on PR.

The cache investigation added comments and an offline snapshot/projection regression, not a runtime serialization fix. Historical probes retained cache hits after whitespace/escaping changes and reloads; argument-key reordering missed. No general cross-provider cache guarantee is claimed, and no paid probes were rerun for this rebase.

API compatibility

Assessment: approved intentional breaking pre-v1 Go API minor. This is not additive-only, a v1 release, or a /v2 import migration. engine/CHANGELOG.md classifies exported removals/signature/type changes; generated API snapshots capture the surface. Provider/host callers need coordinated source updates for LLM DTOs, replay chunks, removed reasoning/item fields, and canonical typed references.

Wire: engine semantics count separately from protobuf syntax. This engine PR does not change the advertised host major or protobuf fields. The coordinated host/client change uses the already-approved Harness API major 2, including removal/reservation of public opaque archive reasoning fields while retaining readable reasoning/text/media/phase. SDK versioning is independent. No new wire break or additional major bump is introduced by this rebase. Do not deploy an intermediate engine/host mixture.

Stored data: writers use sessnap-json/2 and eventlog-json/2; readers support /1 and /2 subject to the snapshot rejection and complete-event proof policies above. Legacy parked snapshots do not resume after upgrade; resolve them beforehand or start fresh. Drain old readers/writers and upgrade decoding snapshot/event drivers before harness/clients. No mixed writers or downgrade after new-format writes. Exact release versions remain unselected. The normalization/ownership follow-ups introduce no additional stored-format, wire field, feature or major change; reference-adapter source APIs are excluded from the stable core API.

Verification

Recorded qualification for published engine head 80ddd21af:

  • PASS standalone engine build/all tests with GOWORK=off, full engine race (-count=1), API snapshot freshness, pinned engine-only lint and whitespace checks.
  • PASS focused codec/eventsource/sessnap/memstore/learning and agent-abandonment checks.
  • Full panel clear after fixes/rechecks, including diagnostic ownership labels, native tuple source keys and the exact deterministic helper hash regression.
  • Earlier snapshot normalization/rejection checkpoints also passed standalone engine tests, engine race, API freshness and pinned engine lint, with targeted migration/current-control regressions and review rechecks.
  • FAIL full task test, task lint and offline demo at unchanged root/provider migration boundaries. The root hook still expects provider token c1. These failures are recorded, not waived.
  • Earlier docs failed during server-help compilation at the downstream API boundary. No full-stack/all-module race, current docs, browser, image or deployment pass is claimed. No module pins, generated artifacts or downstream source branches changed in these engine follow-ups.

Earlier assembled-source all-module lint, workspace race, docs/site, metadata and offline tool/result plus ask/approval/result demo receipts are historical, not qualification of this engine plus a refreshed downstream stack. No tests were rerun for this description-only update.

Intermediate integration failures and coordinated delivery

This remains an engine-only review draft. Unchanged root/provider consumers use the old API; current compilation failures include []session.Message versus []port.LLMMessage, removed ChunkReasoningItem/reasoning/item fields, and typed canonical references. Standalone provider tests against published older engine pins do not prove integration with this candidate.

Review stack: #2404 engine → #2405 providers → #2406 persistence adapters → #2407 host/contracts/clients. Providers and persistence adapters both depend directly on engine; their release dependency is not linear.

The original delivery-plan archive still contains the superseded hold policy. Use this description and current source for migration behavior; the archive was not edited in this update.

Before merge/release:

  1. Refresh the dependent source on the rebased engine and qualify the assembled current-main stack, including the host's unresolved browser/control-flow checks.
  2. Obtain human merge and separate release authorization; publish a main-reachable engine release.
  3. Update provider/adapter pins to the real engine tag, verify their standalone closures, and publish those independent modules.
  4. Finish real host dependency pins and full standalone, lint/race, demo, browser/image qualification.

No invented PR-head versions, permanent replacements, disabled checks, or implicit merge waiver. Any intermediate merge with failing required checks requires an explicit maintainer decision. No release, deployment, or merge performed. Draft, not merge/release-ready.

@jbeda

jbeda commented Oct 18, 2026

Copy link
Copy Markdown
Contributor Author

Approved engine-first delivery plan

Archived: 2026-10-18. Applies to review stack #2404 → #2405 → #2406 → #2407; exact branch/head/baseline identities below. Local plan paths are provenance only, not remote links. This is the full selected split/qualification plan; verification tasks below describe intended checks, not implicit pass claims. Actual results and remaining gates are recorded separately in the PR bodies.

Engine-first identity delivery

Slug: 2117-engine-identity
Plan path: .scratch/2117-engine-split/plan.md
Original implementation specification: .scratch/2117-tool-call-identity-plan.md (already implemented and reviewed; not reimplementation scope).

Immutable delivery baseline and branches

Base branch: main; recorded start commit f9a3297. Remote main observed at c4a043f (#2402, TUI/docs only); no automatic rebase. Host requires current-main merge qualification before merge.

Review branch Base branch Source head
impl/2117-engine-identity main 5d830f8
impl/2117-identity-providers impl/2117-engine-identity c669ec8
impl/2117-identity-adapters impl/2117-identity-providers 20f60fd
impl/2117-identity-host impl/2117-identity-adapters da27200

These bases organize review, not new module dependencies: providers and adapters both depend directly on engine. The host source tree exactly equals the retained assembled qualification candidate (tree 3763ffe56e1ba21bab363d3ad416c5e4a50077d7). The root checkout remains on the engine review branch; assembled/preparation worktrees stay under .scratch/2117-engine-split/.

Approved outcome and authority

Joe approved on 2026-10-18: prepare, commit, push and open four draft PRs: engine; changed providers; persistence adapters; host/contracts/required client integration. Humans alone merge. No tags/releases, deployment, invented dependency pins, CI bypasses or new TUI presentation work. Preserve the assembled candidate in a local worktree under .scratch and keep the engine review branch at the main checkout root.

Source boundary and contracts

  • Engine PR: engine/** and the necessary .golangci.yml depguard allowance, including engine-owned tests, migration fixtures, generated API snapshots, changelog and compatibility documentation.
  • Provider PR: existing changes under provider/openai, provider/anthropic and provider/openaichat, plus narrow offline real-engine/provider HTTP composition regressions. These modules release independently.
  • Persistence PR: adapters/**, including its changelog and driver format/skew coverage.
  • Host PR: remaining existing coordinated source, generated wire/SDK/Studio contracts, required client propagation and owning documentation. No new presentation work and no changes to mecatl-3.
  • Engine-minted canonical ToolCallID is distinct from provider pairing/item IDs; whole-batch validation before effects; copied history preserves occurrence IDs. Session owns opaque replay and bindings. Ambiguous legacy state is preserved but inspection-only before authority consumption/execution/learning.
  • Preserve existing provider fidelity, not a fuller replay redesign. Preserve allow/deny/ask, isolation, scrubbing and argument ownership behavior.

API compatibility

This is the already-approved breaking pre-v1 Go API minor, not additive-only and not /v2 import migration. Engine APIs change invocation identity, provider-neutral DTOs and private replay ownership. The engine PR does not itself change protobuf fields/server major, but carries engine-originated observable semantics that the final host PR exposes as approved Harness API major 2. Public opaque archive reasoning fields are removed/reserved in that final coordinated host/client change; readable reasoning/text/media/phase remain. Snapshot/event writers advance to sessnap-json/2 and eventlog-json/2; upgraded readers accept /1 and /2. Drain old readers/writers, upgrade both decoding drivers, then harness/clients; no mixed writers or downgrade after new writes. Exact module release versions remain unselected and publication requires separate authority.

Tasks and evidence

  1. Preserve assembled checkpoint; extract engine-only source in the root checkout and document exact base/HEAD identities.
  2. Verify existing engine standalone API/layering/migration tests. Capture engine-only root/provider compilation failures rather than guessing expected failures. Do not suppress unrelated defects.
  3. Reuse provider HTTP/SSE fixtures in one narrow real engine + real adapter + trivial tool test per changed provider. Assert canonical call/result ownership and next-request provider pairing/replay including completed provider-key reuse. No server, UI, deployment or paid/live calls. Keep engine dependencies inward.
  4. Run focused normal/race composition checks and assembled workspace/race, lint, docs and offline demo at the owning candidate; preserve original historical reviews and perform incremental review of new tests and split boundaries, not another full historical panel.
  5. Prepare stacked draft branches in dependency order, with exact source boundaries and matching assembled engine tree. Preserve a fully assembled stack tip. No fabricated PR-head module versions or permanent replacements.
  6. Publish four draft PRs with API compatibility statements, exact evidence, expected intermediate CI failures, dependency/release checklist and plan archive comments. Read back published bodies/comments and record their URLs/heads.

Merge/release readiness

Reviewable source is not merge/release-ready. Before the engine merge, dependent source must be reviewable, assembled behavior qualified and release/pin order explicit. Real engine publication precedes provider/adapter pins and independent releases, then final host/integration pins and standalone/publication qualification. Existing required checks are not waived: Joe accepted review drafts with accounted-for consumer failures; any intermediate merge still needs a separate explicit maintainer decision. Release latency cannot be eliminated. Browser/image qualification remains unclaimed unless actually completed; final host draft must disclose inherited limitations.

Verification ownership

The directing agent owns split, integration, PR publication and evidence. A focused writable worker adds provider composition tests only in the retained assembled worktree and runs narrow checks. New tests get independent incremental panel review. Full Go gates use 1800-second outer limits; other full gates 600 seconds. Heavy suites run serially. No HANDOFF update.

jbeda and others added 4 commits October 27, 2026 11:18
Separate provider pairing from engine invocation identity; preserve occurrence-bound replay, exact continuations and legacy evidence migration.

Co-authored-by: Mecatl <noreply@mecatl.dev>
…aries

Add snapshot round-trip coverage for provider pairing, replay, and argument key order. Explain the cache implications and the acceptance, continuation, migration, and evidence boundaries used in source review.

Co-authored-by: Mecatl <noreply@mecatl.dev>
Normalize legacy snapshots and complete event sources at codec seams, preserve provable controls and historical evidence, and remove persistent migration holds. Reject stale controls without effects and preserve later canonical lifecycle across migrated event segments.

Co-authored-by: Mecatl <noreply@mecatl.dev>
Keep latest-main permission-mode and yolo-review regressions meaningful after the identity rebase by resolving accepted call IDs and returned child handles. Assert completed background denial and non-vacuous canonical results.

Co-authored-by: Mecatl <noreply@mecatl.dev>
@jbeda
jbeda force-pushed the impl/2117-engine-identity branch from 701df1f to 1928b18 Compare October 27, 2026 18:31
jbeda and others added 6 commits October 27, 2026 14:12
Remove legacy pending approval and authorization preservation, unshipped candidate-hold normalization, and old direct Snapshot construction compatibility. Keep nonpending stored-history migration and current-schema continuations, with boundary regression coverage and clarified upgrade policy.

Co-authored-by: Mecatl <noreply@mecatl.dev>
Decode legacy storage through dedicated DTOs and normalize snapshots before reconstructing sessions. Remove legacy staging and old-format marshaling, preserving deterministic identity, history migration, and current continuation validation.

Co-authored-by: Mecatl <noreply@mecatl.dev>
Keep eventcodec focused on versioned records and sparse historical projection. Move pairing, segmentation, and abandonment into eventsource with a narrow codec-owned legacy decoding seam. Relocate tests with their responsibilities and remove obsolete codec reconstruction APIs.

Rename the snapshot legacy ToolCallID helper without changing derivation, and pin its output with a fixed-vector regression.

Co-authored-by: Mecatl <noreply@mecatl.dev>
…sources

Separate legacy and current event decoders without changing storage formats. Restore incremental folding with shared legacy conversion, preserve straightforward provider-reference reuse, and reject unsupported reconstruction instead of synthesizing repair history.

Co-authored-by: Mecatl <noreply@mecatl.dev>
…ning

Keep straightforward snapshot conversion, omit unsupported legacy events, and remove historical identity carriers and obsolete learning evidence compatibility APIs. Persist irreversible learning eligibility and preserve current replay and continuation validation.

Co-authored-by: Mecatl <noreply@mecatl.dev>
… conversion

Enforce current evidence protocols before ordinary or validated skill activation while retaining active history and rollback. Remove unused archive migration plumbing, preserve snapshot identity derivation, and correct replay documentation.

Co-authored-by: Mecatl <noreply@mecatl.dev>

This branch has not been deployed

No deployments
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