Skip to content

fix(runtime): preserve sandbox negotiation across continuations - #4308

Open
testikun wants to merge 8 commits into
apache:mainfrom
testikun:codex/issue-3731-sandbox-negotiation
Open

fix(runtime): preserve sandbox negotiation across continuations#4308
testikun wants to merge 8 commits into
apache:mainfrom
testikun:codex/issue-3731-sandbox-negotiation

Conversation

@testikun

@testikun testikun commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Summary

Safe continuations and Runtime restart recovery now preserve the minimal sandbox-boundary negotiation control state for the same logical Turn. The implementation derives denial, bounded invalid/unresolved correction rounds, and finalization state from digest-validated RuntimeEvent lineage plus the authoritative SQLite boundary log when an event/row crash gap exists. Restored state never grants authority; the live ExecutionBoundary remains the sole authority.

Fixes #3731

What changed and why

Before this change, negotiation state lived primarily in the in-memory ToolRuntime. A safe continuation or recovered Runtime segment creates a new ToolRuntime, so a Turn that had already been denied or had consumed correction attempts could start over and request the same boundary again.

This change:

  • Adds a Core-level SandboxBoundaryNegotiationState and one projection function shared by the continuation planner, Runtime kernel, backend, and ToolRuntime.
  • Rebuilds state only from canonical, digest-validated RuntimeEvent facts: boundary requests, decisions, structured failures, and matching direct or hidden Code Mode tool calls.
  • Reads the durable SQLite sandbox-boundary request log as well, covering the crash window where the request row commits before its RuntimeEvent is appended.
  • Carries the projected state into a continuation, then re-reads and revalidates the complete immutable lineage immediately before execution so caller-provided state cannot become authority.
  • Restores denial and correction budgets in the new ToolRuntime. A denied request cannot be reopened, and an exhausted budget enters tool-free finalization instead of repeatedly asking for permission.
  • Keeps approved capabilities usable through the current live ExecutionBoundary; restored negotiation state can never widen filesystem or network authority.
  • Resets negotiation state for a genuinely new user Turn, so old Turn denials and correction counts do not leak into new work.
  • Persists invalid_boundary_declaration as a structured failure reason and rejects malformed, legacy, duplicate, or identity-mismatched boundary facts fail-closed.
  • Bumps the Runtime Host compatibility epoch from the current main value 92 to 93 because Session continuity now carries the authenticated boundary-negotiation contract. This PR is the first half of the epoch repair with feat(runtime-host): bind ScheduledTasks to Connection identity #4321, which reserves epoch 94 for its separate ScheduledTask Connection-identity contract.

The important separation is:

negotiation state  -> remembers whether negotiation may continue
ExecutionBoundary -> remains the only authority that grants execution capability

This is a convergence and recovery fix, not a new permission grant.

Verification

  • npm --workspace @maka/core test — 738 passed.
  • npm --workspace @maka/storage test — passed.
  • npm --workspace @maka/runtime-host test — 1,429 passed, 12 skipped.
  • Runtime continuation and sandbox-convergence focused tests — 44/44 passed, including direct tools, hidden Code Mode, durable request-row recovery, malformed lineage, and new-Turn reset.
  • Runtime/core/storage/runtime-host builds, affected typechecks, protocol epoch check, Biome check, and git diff --check passed.
  • The full Runtime suite reports 9 unrelated pre-existing platform/concurrency failures (model-factory tool-call index and Unix node-pty lifecycle tests); no affected test failed.

AI use

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

Tool(s) and scope: OpenAI Codex analyzed issue #3731, designed and implemented the bounded sandbox negotiation restoration, added regression coverage, and ran the verification listed above. The human contributor remains responsible for review and submission.

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 — safe continuations no longer reopen a denied or exhausted sandbox negotiation
  • No

中文摘要

之前 sandbox 协商状态主要保存在当前 ToolRuntime 内存中,因此同一个逻辑 Turn 在 safe continuation、崩溃恢复或 Runtime 重启后创建新的运行段时,可能丢失“已拒绝”和修正次数状态,重新发起权限请求。这个 PR 从经过 digest 校验的 RuntimeEvent lineage 和权威 SQLite boundary log 恢复最小控制状态,并在执行前再次认证。恢复的数据只控制是否继续协商,不会扩大真实 sandbox 权限;达到修正上限或历史异常时会安全进入无工具终止流程;真正的新用户 Turn 会重新开始。

@github-actions github-actions Bot added the effort/XL Over 1000 readable lines label Aug 31, 2026
@testikun
testikun force-pushed the codex/issue-3731-sandbox-negotiation branch from 272523c to 73e2cef Compare August 31, 2026 03:37

@me2seeks me2seeks 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.

Blocking compatibility issue: this PR declares epoch 84 for the sandbox-continuation wire contract, while PR #4321 independently declares the same global epoch 84 for the ScheduledTask Connection-identity wire contract. RUNTIME_HOST_COMPATIBILITY_EPOCH is a single Host/Client interoperability boundary, not a per-feature version. Both branches are based on the old 9249bf3 base and are currently conflicting with main (which is at epoch 83). Please rebase and either compose both closed-shape changes under one epoch-84 ledger entry if they are intended to ship together, or land one at 84 and bump the other to 85 after the first. The stale 78→79 explanation should be updated as part of the same repair. Until this is resolved, the meaning of epoch 84 depends on merge order and clients cannot be given a deterministic compatibility contract.

Comment thread packages/runtime-host/src/protocol/index.ts Outdated

@Astro-Han Astro-Han 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.

Thanks — reviewed 1b986025 for substance. @me2seeks holds the open block on the epoch, so I've stayed off it apart from one factual note at the end.

The problem is real and well-stated: a safe continuation builds a new ToolRuntime, so a Turn that was already denied or had spent its correction budget could start the negotiation over. Rebuilding from digest-validated event lineage plus the durable request log, and keeping ExecutionBoundary as the only thing that grants capability, is the right shape.

P2 — the carried sandboxBoundaryNegotiationState never becomes authority, so it costs more than it earns.

In revalidateContinuationBoundary, the state is re-derived from the lineage and the durable rows, compared against continuation.sandboxBoundaryNegotiationState with isDeepStrictEqual, and on mismatch throws source_replay_changed — then the re-derived value is what's returned and used (runtime-kernel.ts:2871). A second equality check on the same pair sits at :3056.

Since the consumer has to derive it anyway to be safe, the carried copy is a second representation of a fact the consumer already owns. What it adds is a field on RuntimeContinuation, two deep comparisons, and a failure mode — and that failure mode fires precisely in the window this PR documents elsewhere: the request row commits before its RuntimeEvent is appended. A continuation planned before that event lands and revalidated after it lands derives two different states and throws, turning a recoverable timing skew into a hard failure of the Turn. I have not built that race, so treat the reachability as argued rather than demonstrated — but the two derivations are separated in time over an append-only log with a documented commit gap, which is enough to want the check gone rather than tuned.

Dropping the field takes both comparisons and source_replay_changed with it, and RuntimeContinuation stops growing.

If the intent is to catch a planner bug rather than a hostile caller, that is a reasonable thing to want — but then it belongs as an internal invariant assertion at the point of derivation, not as a field the caller supplies. As written the producer of the value and the party it is checked against are the same untrusted input.

Nothing else stood out. projectSandboxBoundaryNegotiation rejecting malformed, legacy, duplicate, and identity-mismatched facts fail-closed reads correctly, and the refusal to infer a correction count from older ledgers without the structured marker is the right call — inferring there would have been the easy mistake.

Evidence boundary: I read the projection, the kernel's revalidation path, and the continuity contract; I did not run the suites and did not review the 534 lines of new tests in detail.

Factual note, not a verdict — that stays with @me2seeks: main is at 87 and this branch is at 88, so the "86 → 87" wording in the description has been overtaken again. Worth refreshing the body whenever you next rebase.


AI-assisted review: drafted with Maka; I verified the re-derivation ordering, both equality checks, and the field's provenance against the branch source myself.

@testikun
testikun force-pushed the codex/issue-3731-sandbox-negotiation branch 3 times, most recently from f1099d3 to 58f25c0 Compare September 1, 2026 14:29

@Astro-Han Astro-Han 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.

PR #4308620903d — follow-up review

Summary: Sandbox boundary durable settlement. Exact head 620903de6d6fbe441aeccedfe75931d958882e3b frozen, windows_recovery green, test/package pending, MERGEABLE/BLOCKED. This follows prior 73e2cef 1×P2 NO-GO; current head still exhibits same ordering gap (only typeof decision.revision gate added).

Finding (reproducible, decision-changing):

  • P2 — durable settlement applied without authoritative orderingpackages/core/src/sandbox-boundary.ts:241-405 tallies descendant failures by RuntimeEvent order, then 415-470 applies sqlite-session-metadata-store durable approved/denied settlement without a comparable sequence number. If Host persisted settlement (session-metadata-store.ts:720-815) before tool-runtime.ts:2863-2880 decision ack is lost, continuation replays approved then later descendant invalid/unresolved failure is reset at 452-466, clearing correction budget/finalizationRequested. Existing test 717-745 covers isolated denial only. Fix: unify on authoritative order or fail-closed when ordering unavailable; add interleaved approved→failure and denied→approval regression.

Gating: hosted windows_recovery SUCCESS, test QUEUED. No current-head formal review before this comment.

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


简体中文

本条结论来自 @Luna-Deep-Qronos 在 exact head 620903d 的独立复核,已核对 head 未漂移。编排仅同步发布,内容以技术线为准。

@testikun
testikun force-pushed the codex/issue-3731-sandbox-negotiation branch from 620903d to 6166099 Compare September 2, 2026 01:45
@testikun
testikun force-pushed the codex/issue-3731-sandbox-negotiation branch from 6166099 to 6fee964 Compare September 2, 2026 01:47
@testikun

testikun commented Sep 2, 2026

Copy link
Copy Markdown
Contributor Author

@Astro-Han @me2seeks Thanks for the follow-up review. The authoritative-ordering finding is addressed in 6fee964c5: durable settlements without a matching decision ack now fail closed whenever their ordering relative to other boundary decisions/failures is unavailable. I also added the approved→failure and denied→approval regressions, rebased onto main at 622d6fecd, and refreshed the epoch to 93. Local affected tests, builds, lint, and format pass; the new CI run is in progress.

@testikun
testikun requested a review from me2seeks September 2, 2026 02:17
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.

enhancement(runtime): preserve sandbox boundary negotiation across safe continuations

3 participants