Skip to content

refactor(runtime): persist durable Tool Result projections - #4287

Open
Astro-Han wants to merge 29 commits into
apache:mainfrom
Astro-Han:refactor/4283-durable-projection
Open

refactor(runtime): persist durable Tool Result projections#4287
Astro-Han wants to merge 29 commits into
apache:mainfrom
Astro-Han:refactor/4283-durable-projection

Conversation

@Astro-Han

@Astro-Han Astro-Han commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

Summary

Tool Results previously had no single durable, provider-neutral model-visible representation. Live continuation, later replay, and restart recovery could therefore reconstruct different output, while a transient post-T2 model-output path competed with the RuntimeEvent ledger.

This PR adds the PR 1 foundation from #4283:

  • a closed, versioned, bounded, provider-neutral projection schema with one deterministic failure sentinel;
  • content fidelity for arbitrary tool-authored text and JSON, without introducing a secret-detection or DLP promise;
  • Core as the single schema/admission and Artifact source-policy authority, Storage as the exact artifact-ref persistence authority, and Runtime as the T1/publish/T2 lifecycle owner;
  • one compatibility codec and effective-history reducer input;
  • operation admission and complete projection admission before observable publication;
  • atomic T2 commit of the completed outcome and projection;
  • the same durable projection for live continuation, next-Turn replay, and cold restart;
  • protected content-addressed image artifacts and ref remapping through the existing Runtime Host exact-copy owner; and
  • removal of the replaced transient settlement output, raw settlement lane, synthetic writer surface, retired SessionManager conversation-copy owner, and superseded seam-only tests.

The final diff is 43 files (+2779/-1578): production and other source is +1225/-463, while tests are +1554/-1115. The added volume defines and verifies the durable protocol; the deletions remove competing authorities and tests replaced by stronger production-composition coverage.

Refs #4283

Lifecycle

flowchart TD
    A[Provider tool call] --> B{RuntimeStore T1 claim}
    B -- rejected --> R[Stop before call publication or execution]
    B -- accepted --> C[Runtime publishes the admitted call and executes once]
    C --> D{Storage plans exact artifact refs without writes}
    D -- planning fails --> F[Core-defined deterministic failure projection]
    D -- planned --> E{Core admits one closed and bounded projection}
    E -- rejected --> F
    E -- admitted --> G{Storage persists protected projection artifacts}
    G -- persistence fails --> F
    G -- persisted --> H[Canonical durable projection]
    F --> I{RuntimeStore T2 atomic commit}
    H --> I
    I -- fails --> J[Fail-stop; T1 prevents repeated effects]
    I -- committed --> K[Effective-history reducer]
    K --> L[Live continuation]
    K --> M[Next-Turn replay]
    K --> N[Cold restart]
Loading

Core is the only compatibility/schema authority; Runtime does not recalculate projection semantics during replay. Session copy also does not create another projection path: the existing Runtime Host exact-copy owner only remaps persisted session_file refs.

Projection bounding is a structural and resource contract, not arbitrary-content redaction. Tool-authored text and JSON remain faithful, consistent with Discussion #4119; credential stores and omission-based credential APIs remain separate security boundaries.

Verification

  • npm --workspace @maka/core run test:dist — 739 passed
  • npm --workspace @maka/storage run test:dist — 1015 passed, 7 skipped
  • npm --workspace @maka/runtime run test:dist — 3093 passed, 13 skipped
  • npm --workspace @maka/runtime-host run test:dist — 1444 passed, 12 skipped
  • focused projection, T1/T2 rollback, user-delete protection, live continuation, next-Turn replay, real RuntimeStore/ArtifactStore close-reopen, recovery, artifact, and Runtime Host copy suites passed
  • the full repository build and test build passed, including Desktop
  • Biome, ASF license-header audit, model-metadata check, and git diff --check passed

Review focus

This remains the durable projection foundation. Budgeting/compaction policy, projection transitions/pruning/copy-artifact protocols, and overflow omission remain outside this PR.

Schema/admission failures are zero-write: all parts and exact artifact refs are planned and decoded before any artifact is published. Projection-owned artifacts use a protected lifetime, so ordinary user deletion cannot invalidate committed history. Once multi-artifact publication begins, a later artifact failure or T2 failure can still leave a hidden, content-addressed orphan without committing a dangling projection ref. Repeat settlement is rejected at T1 before tool execution, projection work, or call-side publication can run again. Making publication atomic across ArtifactStore and RuntimeStore requires a batch staging/publish or garbage-collection lifecycle protocol, so that bounded P2 remains explicit follow-up scope rather than a local PR 1 patch.

The main invariants are that admission precedes effects, projection is computed before T2, outcome and projection commit atomically, and one durable authority feeds live continuation, next-Turn replay, and restart.

AI use

Select exactly one:

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: Codex implemented the schema, codec, Runtime and Host integration, tests, validation, simplification, and review-driven corrections.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

@github-actions github-actions Bot added the effort/XL Over 1000 readable lines label Aug 30, 2026
@Astro-Han
Astro-Han force-pushed the refactor/4283-durable-projection branch from 9d41dd3 to 4d7007a Compare August 30, 2026 23:57
@Astro-Han
Astro-Han marked this pull request as ready for review August 31, 2026 04:03
@Astro-Han
Astro-Han force-pushed the refactor/4283-durable-projection branch from 484f4e9 to b2b2ae8 Compare August 31, 2026 07:20

@jackwener jackwener left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Reviewed the latest head b2b2ae86d. No P0 or P1 — approving. I found nothing worth filing at P2 or P3, so there are no inline comments. audit, ubuntu-latest, macos-latest, windows-latest and windows_recovery are green; package and test were still running.

This is 43 files and 2,779 additions, so rather than claim I read all of it evenly, here is what I went after and what I found.

The allocation bound is done properly. 3d423b8c3 checks byteLength before copying, and the test is the part worth praising: it subclasses ArrayBuffer to count slice() calls and asserts data.copies === 0, plus passes a planning callback that throws if reached. That proves rejection happens before allocation rather than merely that oversized input is rejected — two independent proofs of the property that actually matters.

The same discipline holds across the module. decodeBoundedImageData bounds the encoded base64 length before decoding, so an oversized string never allocates a decoded buffer, and it rejects non-canonical base64 — which also closes off two encodings of the same bytes. Structural limits on depth, node count and part count are in place, and unsafe media types fail closed in three separate paths.

The artifact protection is the right shape. The new tool_result_projection source carries userDeletable: false while tool_result is user-deletable. That matters because durable replay depends on these projections — a user-deletable projection would mean a tool result that cannot be reconstructed. sharedReadable: true keeps shared-session reads working. I also checked whether this new enum value crosses the Runtime Host protocol wire and would need an epoch bump: it does not appear in the protocol frames, so no bump is required.

The session-manager.ts removal is safe, and I checked the call chain rather than the symbol. Removing branchFromTurn, branchBeforeTurn and reviseBeforeTurn looks alarming at first, because the Desktop renderer, preload bridge and IPC main all still reference those names, and the implementation is gone from every package. It is fine: the live path is sessions:branchFromTurndeps.client.copySession("branch", ...) → Host → conversation-copy.ts, which never went through those SessionManager methods. They were a superseded in-process path with no remaining callers, and the 995 removed test lines tested exactly them.

And the copy path is correctly updated for the new format, which is the detail that would have been easy to miss: conversation-copy.ts now rewrites artifact references inside modelProjection when copying. Without that, a branched or revised session would carry projection refs pointing at the source session's artifacts. It has 22 tests.

Scope of this review. I examined the allocation bounds, the artifact source policy and its wire exposure, the session-manager removal and its full call chain, and the conversation-copy projection rewrite. I did not line-by-line review tool-runtime.ts (+266), ai-sdk-backend.ts (+143), or all 478 lines of the new projection module — I scanned the latter for bounds and failure modes rather than reading every branch. Saying so plainly since "approved" on a change this size should not be read as "every line verified".

Automated review notice: This comment was posted by an automated review agent operated by jackwener. It is not an independent human review and does not replace one.

@hqhq1025 hqhq1025 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reviewed the current head and found one P1 blocker in the durable tool-result settlement path. The change otherwise has substantial focused coverage, but a supported asynchronous model-output projector can now prevent T2 from ever being committed after the tool effect has already completed.


Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.

? Object.freeze(coerceResultContent(result))
: coerceResultContent(result);
const projected = this.projectToolResult(tool, turnId, toolUseId, executionArgs, result);
const modelProjection = isPromiseLike(projected) ? await projected : projected;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 — Bound or abort asynchronous projection before T2.

MakaTool.toModelOutput is explicitly allowed to return a PromiseLike, but this await has no timeout or abort race and now occurs before commitOutcome. A projector that never settles leaves an already-completed tool effect with only T1 durable: in an exact-head production-path probe, the implementation returned, commitToolPrepared ran once, commitToolOutcome stayed at zero, and both settleToolCall and endTurn("aborted") remained pending. Before this change, the raw settlement path committed T2 before model-output materialization.

Please make projection total at this boundary (for example, race async projection against turn abort or a bounded timeout and commit the deterministic failure sentinel), or narrow and enforce the contract to synchronous projectors, so a projector cannot prevent a completed outcome from reaching T2.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I independently reproduced this on exact head b2b2ae86dd2a00312e73dd1fb3090ff07365a4cc. A toModelOutput PromiseLike that never settles allowed the implementation to finish after one T1 commit, but T2 remained at zero; aborting the turn still left settleToolCall pending. This confirms the completed tool effect can be stranded indefinitely behind the projector. The projector therefore needs a bounded/abort-aware fallback to the deterministic failure projection, or the contract must be narrowed and enforced as synchronous.


Automated review notice: This comment was posted by an automated review agent operated by M4n5ter. It is not an independent human review and does not replace one.

@M4n5ter M4n5ter left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Review of exact head b2b2ae86dd2a00312e73dd1fb3090ff07365a4cc: changes are required. I independently reproduced the unresolved P1 on tool-runtime.ts:1610: an allowed asynchronous projector can leave a completed tool effect permanently at T1 with no T2 outcome, even after turn abort. Because that P1 blocks approval, I am not approving this head. I left the separate artifact-lifecycle P2 inline below. The closed schema, side-effect-free projection admission, atomic SQLite outcome/projection commit, live/next-turn/restart replay, and exact Session copy paths otherwise held in focused verification; I found no separate simplification issue.


Automated review notice: This comment was posted by an automated review agent operated by M4n5ter. It is not an independent human review and does not replace one.

const persisted = new Set<string>();
for (const artifact of prepared.artifacts) {
if (persisted.has(artifact.ref.relativePath)) continue;
await artifact.persist();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

P2 — Reclaim partially published projection artifacts. This loop publishes each protected artifact separately. If a later persist() rejects, the function returns the failure sentinel while earlier writes remain live, hidden, and non-user-deletable; the same orphan also remains if the later RuntimeStore T2 transaction fails. A two-artifact probe where the second write rejected produced persisted: [1] plus the failure projection, with no durable projection referencing artifact 1. Independent failing operations can therefore accumulate inaccessible disk usage until the whole Session is purged. Please add batch/staged publication, finalize ownership with T2, or persist enough ownership for deterministic garbage collection. This is non-blocking under the current P0/P1 approval gate, but it is a real lifecycle leak.


Automated review notice: This comment was posted by an automated review agent operated by M4n5ter. It is not an independent human review and does not replace one.

@zhiiw zhiiw left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reviewed at exact head b2b2ae86 (verified unchanged at review time; all checks completed/success on this head, including test 25m50s and package 23m41s).

Durable Tool Result projection foundation, verified rather than assumed:

  • The schema is closed, versioned, and bounded at the Core layer: 256 KiB / 64 parts / depth 32 / 20k nodes, exact-shape validation per kind, one frozen failure sentinel. sanitizeJsonValue rejects non-finite numbers, non-plain objects, and toJSON serializers, and writes keys via defineProperty, which makes __proto__ payload keys inert by construction. Encoding re-decodes its own output before returning, so the writer can never produce something the reader would reject — invalid output becomes the sentinel instead.
  • Artifact refs are session-scoped and image-only: parts must reference the owning session (ref.sessionId checked at both the encode and the decodeRuntimeEvent boundary, the latter via hasOwnedModelProjection — no cross-session ref can ride a persisted event), carry a canonicalized raster MIME, and inline image bytes must round-trip as canonical base64 under the existing MAX_READ_IMAGE_BYTES. The planner validates and freezes without writing; persist() runs only after admission, so a rejected projection never leaves an orphan write on the artifact path.
  • Admission precedes publication: the call message and tool_start event now land only after prepareDurableToolAttempt (T1) succeeds — a rejected claim publishes nothing. T2 adoption deep-compares the committed event including modelProjection, so a replayed outcome cannot silently carry a different projection than the one admitted.
  • Live, replay, and restart share one representation: the transient settledModelOutputs lane and the post-T2 model-output recomputation are gone; replay materializes from the durable projection, rehydrating image refs for vision-capable models with per-artifact budget charging (deterministic decision keys, so re-materialization does not double-charge) and degrading to explicit text notes otherwise. Legacy events flow through the one compatibility codec (decodeEffectiveToolResultProjection), including the retired-ExploreAgent and shell-result shapes; unprojectable legacy path images pass through as legacy output rather than failing.
  • Session copy does not open a second projection path: the copy collects session_file refs from exactly the sites rewriteStorageRef reaches (including the new projection parts), remaps through the existing exact-copy owner, and rewriteProjectionArtifactRef throws on an invalid rewrite instead of emitting a corrupt projection.
  • Source policy is now one Core table: ARTIFACT_SOURCE_POLICIES (userDeletable / userVisible / sharedReadable) replaces the desktop's local visibility switch and the coordinator's local shared-source set; tool_result_projection is not user-deletable and not user-visible, matching its role as ledger content.

Executed on a real Windows machine at this head: clean forced rebuild, then Core/Runtime projection suites 45/45, the four ToolRuntime boundary suites 76/77 (the one failure is the pre-existing symlink-EPERM environment case — this machine has no Developer Mode; the failing assertion creates a symlink fixture), storage artifact/runtime-store + ai-sdk-backend 274/274, and the runtime-host recovery pair 1 pass + 1 platform skip (UDS). One honest limitation: the rewritten execution-model-composition suite does not finish on this machine (pre-existing environmental slowness in the process-heavy composition fixtures); the hosted test job runs it green on this head.


Automated review notice: This comment was posted by an automated review agent operated by zhiiw. It is not an independent human review and does not replace one.

简体中文

durable Tool Result 投影地基,全部核实而非假设:Core 层 schema 封闭有界(256KiB/64 段/深度 32/2 万节点),sanitizeJson 拒绝非有限数/非纯对象/toJSON,defineProperty 逐键写使 proto 键天然失效;编码后自解码自检,非法输出收编到唯一 sentinel。artifact ref 限本 session(编码与 decodeRuntimeEvent 双边界都查 sessionId)、白名单 raster MIME、canonical base64 往返校验;plan 不落盘、persist 在准入之后。T1 先准入后发布(call 消息/事件只在 claim 成功后落地);T2 adopt 深比对含 modelProjection。实时/重放/重启共享同一表示:settledModelOutputs 瞬态车道删除,重放按 vision 能力+预算把 artifact ref 再水化成图片(确定性 decision key 不重复计费),失败降级为明确文本注记;legacy 事件走唯一兼容 codec。session copy 只 remap ref 不另开投影路径,非法 rewrite 直接抛。artifact 来源策略收成 Core 一张三轴表。本机真 Windows:投影套件 45/45、tool-runtime 边界 76/77(1 个既有 symlink EPERM 环境噪音)、storage+backend 274/274;重写的 composition 套件本机跑不完(既有环境性),hosted test 同 head 跑绿——如实声明。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/XL Over 1000 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants