Uh oh!
There was an error while loading. Please reload this page.
docs: correct stale runtime-core architecture facts - #3523
Conversation
Chapter 1 still described a file-backed run store and the removed per-call permission model. Both languages carried the same drift, so `translation_status: synced` was no longer true. Corrected against the code: - `PermissionEngine` and its allow/block/prompt evaluation no longer exist; a tool that crosses the sandbox boundary returns `sandbox_boundary_required` with a concrete expansion, and the model raises the request through `request_sandbox_boundary`. - The decision is routed by `RuntimeKernel.respondToSandboxBoundary()`, not `respondToPermission()`. - Durable state is SQLite, not `sessions/<id>/runs/<id>/*.jsonl`. RuntimeEvents land in `runtime_events`, ordered by `event_seq` under a `(invocation_id, event_seq)` uniqueness constraint. - `maxSteps` is `number | undefined` and `undefined` is unbounded; the documented default of 50 appears nowhere in the repository. - `packages/storage/src/__tests__/agent-run-store.test.ts` no longer exists; the ledger repair coverage is `packages/runtime/src/__tests__/runtime-ledger-repair.test.ts`. - Startup recovery and continuation are now stated as the two distinct paths they are, naming `safe_boundary_continuation`, `continuationSource` and `runAdmittedRuntimeContinuation`. Also records that `AiSdkFlow` rejects the four legacy permission events at the mapping boundary, and that the boundary decision fact carries `role=system` with `author=user`. No new sections or examples: this change only brings existing statements back in line with the implementation. Refs apache#3522 Generated-by: Claude Opus 5 via Claude Code
jackwener
left a comment
There was a problem hiding this comment.
Checked the claims against current main, not the old prose.
PermissionEngine/respondToPermission()are gone;respondToSandboxBoundary()exists.- Durable runtime events are SQLite
runtime_eventswithUNIQUE (invocation_id, event_seq), not per-run JSONL. maxStepsisnumber | undefined; no default 50 inpackages/.- Ledger repair test path is
runtime-ledger-repair.test.ts. AiSdkFlowthrows on the four legacy permission event types.sandbox_boundary_decision_ackisrole=system/author=userin the mapping comments.- Continuation is a separate path (
safe_boundary_continuation,continuationSource,runAdmittedRuntimeContinuation). - EN/ZH stay in lockstep;
translation_status: syncedholds.
This only un-drifts Chapter 1. Approve.
Astro-Han
left a comment
There was a problem hiding this comment.
Thanks — correcting a stale architecture document is unglamorous and it is exactly the kind of drift that costs the next reader an hour. Net −20 lines is the right shape for it: fewer claims, each of them true.
Reviewed at exact head 1d014d481c484caa6d6c953df0ca247a970aa71e. test is green on this head.
We spot-checked three of the load-bearing claims against current main rather than taking the prose at its word:
PermissionEngineandrespondToPermission()return no hits anywhere underpackages/orapps/, andrespondToSandboxBoundary()is what exists in their place.maxStepsis declaredmaxSteps?: numberand held asnumber | undefined, resolved throughinput.maxSteps ?? this.maxSteps— there is no default of 50.- Durable runtime events live in the SQLite
runtime_eventstable, not per-run JSONL.
Each matches what this PR now says. @jackwener had already worked through the full list against main, which is the review that carries the weight here; ours agrees with it on the points we re-derived independently.
One thing worth naming for whoever reads this later: a documentation change that deletes claims is harder to review than one that rewrites them, because a wrongly-deleted line leaves nothing behind to notice. The deletions here are removals of superseded mechanisms, not of still-true statements.
No findings — no P0, P1, P2 or P3. Approving.
This review was AI-assisted. Findings were verified against the exact head listed above; any mistakes are ours to correct — please push back where we got it wrong.
Uh oh!
There was an error while loading. Please reload this page.
Summary
Chapter 1 of the runtime-core architecture documents still described a file-backed run store and the removed per-call permission model. Both language versions carried the same drift, so
translation_status: syncedwas no longer true; this change updates them together.Corrected against the code:
PermissionEngineand its allow/block/prompt evaluation no longer exist. A tool that crosses the sandbox boundary returnssandbox_boundary_requiredwith a concrete expansion, and the model raises the request throughrequest_sandbox_boundary.RuntimeKernel.respondToSandboxBoundary(), notrespondToPermission().sessions/<id>/runs/<id>/{run.json,events.jsonl,runtime-events.jsonl}. RuntimeEvents land inruntime_events, ordered byevent_sequnder a(invocation_id, event_seq)uniqueness constraint.maxStepsisnumber | undefinedandundefinedis unbounded; the documented default of 50 appears nowhere in the repository.packages/storage/src/__tests__/agent-run-store.test.tsno longer exists. The ledger repair coverage ispackages/runtime/src/__tests__/runtime-ledger-repair.test.ts.safe_boundary_continuation,continuationSourceandrunAdmittedRuntimeContinuation.Two facts are added because the corrected text would otherwise be incomplete:
AiSdkFlowrejectspermission_request/permission_answer_ack/permission_closure_ack/permission_decision_ackat the mapping boundary, and the boundary decision fact carriesrole=systemwithauthor=user.No new sections or examples. This change only brings existing statements back in line with the implementation, and the net diff is shorter than the original.
Refs #3522
Verification
Every corrected statement was checked against the code rather than inferred from the wording:
PermissionEngineremovedgrep -rn PermissionEngine packages/*/srcrespondToSandboxBoundary()packages/runtime/src/runtime-kernel.ts:149,2339respondToPermissiongrep -rln 'runtime-events.jsonl|events.jsonl' packages/storage/srcmaxStepsunboundedpackages/runtime/src/ai-sdk-backend.ts:739,1053maxSteps?: number/number | undefinedgrep -rn 'maxSteps.*50' packages/ls packages/storage/src/__tests__/agent-run-store.test.tsls packages/runtime/src/__tests__/runtime-ledger-repair.test.tspackages/core/src/agent-run.ts:71,167,packages/runtime/src/runtime-runner.ts:421packages/runtime/src/ai-sdk-flow.ts:148-170Repository checks:
No test covers prose, and this PR changes no code, so no suite was added.
npm testwas not run.AI use
Select exactly one:
Tool(s) and scope: Claude Opus 5 via Claude Code — ran the documentation audit against the code, drafted the corrected prose in both languages, and wrote the commit message. Every claim in the table above was verified by running the listed command. The commit carries a
Generated-bytrailer.Checklist
Does this PR entail a change in behavior?