[drewstone]

Post-merge review of #33 (the detached-execution fence), which merged on top of the failed-execution receipts work with no approval on record. I read the merged diff adversarially first, fixed what it turned up, then simplified the area and upgraded the first-party SDKs.

Five disjoint commits, one concern each.

Residuals cleared

tasks/get blocked for the whole agent turn. session.result() awaits a terminal result. #33 made handleTasksGet call reconcileTask(task, slug, allowActive = true) unconditionally, so a status poll fell through to result() and held the request open until the detached run finished — the exact coupling the fence had just removed from the stream path. Proven before fixing: a tasks/get against an in-flight run never returned inside 1.5s.

AssertionError: expected 'timeout' not to be 'timeout'
 ❯ tests/a2a-lifecycle.test.ts:488

The durable-session contract now carries runs(), and reconciliation skips result() while the fence is live and the run is still active. An expired fence still awaits the result, because no owner is left to settle the task.

The resume cursor was write-only. #33 added TaskIdParams.lastEventId and SandboxStreamEvent.id and passed the former to events({ since }), but the resubscribe stream never wrote an SSE id: field — a client had no way to learn the cursor it was being asked to send back, and nothing tested it. Every artifact-update frame now carries id:, skipping ids containing a line break so a runtime value cannot forge a frame boundary.

SandboxPromptResult.question had the wrong type. It declared question?: string; the runtime returns { questionId, questions }. A detached run that finished awaiting a question put that object straight onto the wire as a text part:

- "text": "Which environment should I deploy to?"
+ "text": { "questionId": "question-1", "questions": [ ... ] }

Found by the new compile-time SDK check (below), not by reading. Fixed by modelling the payload and reading a prompt out of it.

The pinned SDK. @tangle-network/sandbox was pinned at 0.37.0 as a devDependency that nothing imported, so the surface #33 adopted was never checked against it at all. See "Upgraded".

Simplified

That commit on its own is 52 insertions / 81 deletions (−29).

• dispatchDetachedSandboxStreamRich restated all thirteen parameters of dispatchSandboxStreamRich purely to forward them, then guarded at run time against an options argument both call sites always pass (the guard was unreachable, and options could not be made required while trailing an optional parameter). It now forwards the base function's own argument tuple with the sandbox factory swapped, and takes options as its first, required argument. A change to the base signature can no longer drift out of sync here.
• gatewayExecutionRecovery was one dead constant in handler.ts plus two hardcoded literals. One exported constant, two users.
• exactReference normalized the run control ref twice and carried a second failure branch that only changed an error message.
• withTaskExecution, clearTaskExecution and hasExpiredTaskExecution each detoured through a nested conditional spread, an IIFE, or a second marker read.
• hasDetachedSandbox had no caller outside its module.
• a2a.test.ts and a2a-long-horizon.test.ts each carried their own copy of the same agent, stub sandbox, demo key verifier, API key header, message builder and SSE parser — and the copies had already drifted, one textMessage taking positional arguments and the other an options object. One tests/a2a-harness.ts, one shape: −180 lines across the two suites for +85 shared.

Upgraded

package	from	to
@tangle-network/sandbox	0.37.0	0.43.0
@tangle-network/agent-app	0.47.6	0.48.3
@tangle-network/agent-interface	2.3.0	2.10.0
@tangle-network/agent-runtime	0.192.2	0.240.0

All at npm view <pkg> version latest. src/sandbox-sdk-contract.ts is a compile-time-only module — nothing imports it, and it is not a tsup entry, so it never reaches dist/ — asserting what the gateway actually reads out of the SDK: the dispatch result, the session result, the run listing and the interrupt outcome, each satisfies the gateway's own type. Event payloads are deliberately excluded: SandboxStreamEvent is a narrow read of an intentionally open event bag, not a mirror of it. An SDK that reshapes the adopted surface now fails pnpm typecheck instead of passing silently. It earned its keep immediately by finding the question defect above.

Public type changes and their consumers

Both are on SandboxBox, the structural type a host implements to supply its own sandbox. In this repo the only implementors are the test fixtures; external implementors of the detached path (added in #33, unreleased in a tagged version) need:

• session(id) must now also return runs(): Promise<Array<{ executionId: string; status: string }>>. The gateway validates it alongside events/result/interrupt and fails closed with A2A task execution session controls are unavailable.
• SandboxPromptResult.question is now { questionId: string; questions?: unknown } rather than string.

No behavioral change to any public API.

Not done and why

• The unmet peer ranges are left unmet. agent-app@0.48.3 declares peers for sandbox 0.40.x, agent-runtime 0.231.x and agent-eval 0.182.x, so "newest of everything" is internally inconsistent upstream and pnpm reports three unmet peers. Typecheck, the full suite and the build are green on the newest set, and agent-app is a devDependency exercised by a single composition test, so I kept the newest versions rather than pinning three packages backwards to satisfy a range nothing here depends on.
• Tests are still outside tsconfig.json's include. pnpm typecheck covers src only, so test type errors are invisible today. Adding tests surfaces 9 pre-existing type errors across 4 files unrelated to this work; fixing them is its own net-additive change. This is why the SDK conformance check lives in src/ — in tests/ it would never have run.
• An expired lease over a still-running execution still fails the task rather than reclaiming it. That is pre-existing behavior, unchanged here; reclaiming is a feature, not a fix.

Verification

Same commands before and after, /Users/drew/.nvm/versions/node/v24.11.1/bin/node, pnpm 10.28.0.

Before, at d2e45c7 (origin/main):

Test Files  31 passed (31)
     Tests  525 passed (525)
test_exit=0        tc_exit=0        build_exit=0

After:

Test Files  31 passed (31)
     Tests  528 passed (528)
test=0             tc=0             build=0

Both new correctness fixes were checked against their own absence:

• tasks/get: failed with expected 'timeout' not to be 'timeout' before the fix.
• resume cursor: failed with expected undefined to be truthy with the id: field suppressed.
• question payload: failed with the raw object in the text part with the extractor reverted.

Line delta excluding the lockfile: +443 / −273. Runtime source, excluding the compile-time-only contract module, is +89 / −88; the rest is tests and the conformance check. The simplification commit alone is −29.

🤖 Generated with Claude Code

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-09-20T14:51:25.066242Z	52995f7	Manual request

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ba1705a115

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Probe active runs even after the execution lease expires

When an execution owner crashes or misses heartbeats while its sandbox run continues, the task's fence is expired but session.result() still blocks until that run terminates. Because this guard calls isRunning() only for a live fence, recoverExpiredExecutionIfNeeded() reaches the blocking result() call during tasks/get, so a status poll can hang indefinitely for precisely the orphaned-run recovery case. Check the sandbox run state independently of lease freshness before awaiting the result.

Useful? React with 👍 / 👎.

[drewstone]

Fixed in 9ca2df3 and covered by a regression test.

You are right that the guard was one branch too narrow. recoverExpiredExecutionIfNeeded reached the blocking result() for exactly the orphaned-run case. Two changes:

• reconcileTaskExecution now returns early whenever isRunning() is true, with no lease-freshness condition, so no caller can reach a blocking result() on a live run.
• recoverExpiredExecutionIfNeeded probes the run before reconciling and leaves the task working when the sandbox still owns it. Without that second half the first half would have converted the hang into a wrongful failed, abandoning a live run.

tests/pr11-regressions.test.ts seeds an expired fence over an active run whose result() never resolves; tasks/get answers inside 1.5s, result() is never called, and the task stays working. Reverting the source change fails it with expected 'timeout' not to be 'timeout'.

[drewstone]

@codex review

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9ca2df3dde

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Recognize allowed nonterminal run statuses

After the 9ca2df3 fix, the fresh remaining issue is that the new public SandboxDurableSession.runs() contract permits any string status, but this probe recognizes only the literal active. A structurally valid custom sandbox returning a nonterminal value such as running or queued—values already treated as nonterminal sandbox-result states in task-execution-recovery.ts—will make isRunning() return false, after which tasks/get calls the documented-blocking session.result() and hangs until completion. Normalize all supported nonterminal statuses here or narrow the public status type so implementations cannot supply values the probe misclassifies.

Useful? React with 👍 / 👎.

[drewstone]

Fixed in 52995f7. Inverted the test rather than enumerating live states: completed, failed, cancelled and canceled are terminal, case-insensitively, and every other status means the sandbox still owns the run. An unrecognized status now costs one deferred poll instead of a hung request, which is the safe direction for this probe. The contract is documented on SandboxDurableSession.runs() so a host implementor sees it.

The lapsed-lease regression test is now parameterized over active, running, queued and ACTIVE; reverting the source change fails three of the four.

[drewstone]

@codex review

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. Can't wait for the next one!

Reviewed commit: 52995f7698

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[tangletools]

Approved after verifying the branch at 52995f7 locally with the absolute node path (v24.11.1) and pnpm 10.28.0.

Verified:

• pnpm typecheck exit 0, pnpm test 31 files / 532 tests passed exit 0, pnpm build exit 0. Baseline at origin/main d2e45c7 was 525 tests.
• Each of the four behavior fixes fails without its source change: tasks/get blocking (expected 'timeout' not to be 'timeout'), the missing SSE id: cursor (expected undefined to be truthy), the question payload landing raw in a text part, and the lapsed-lease probe (3 of 4 parameterized cases).
• src/sandbox-sdk-contract.ts is not a tsup entry and grep -rl '@tangle-network/sandbox' dist/ is empty, so the devDependency type import does not reach the published bundle.
• Both Codex findings were fixed and answered in thread; the re-review of 52995f7 reports no major issues. Both required checks green on Node 22.13.0 and 24.18.0.