feat(runtime-host): bridge event waits to Goal continuation

[chinawch007]

Summary

PR2 persists event-wait observations but does not resume Goal execution. This implements the PR3 execution-bridge plan: a Turn registers a wait, ends normally and releases execution resources; bounded observation makes no work-model or Goal-evaluator calls; a result or timeout can then admit at most one effective successor through existing Root admission.

• Atomically link wait registration, source settlement and the existing Goal continuation in SQLite and the independent Memory provider. Freeze successor IDs and input across busy admission and recovery, and acknowledge delivery in the Root admission transaction.
• Gate ordinary Goal evaluation, timers and recovery while delivery is outstanding, preserving normal accounting, permission and budget checks, user-input priority, and control-generation fencing.
• Carry typed result provenance through execution and recovery. WaitForEvent remains internal and absent from production catalogs; production adapters, the ShellRun entry point and UI remain in PR4.

Refs #5926 and #5920. Builds on #6057 and #6113.

Verification

Final source: 7fe6963f6da7fd9964536978dbf7056c74efbe29. The latest follow-up changes only recovery documentation and protocol comments; executable behavior and tests are unchanged from 5386b1dfc.

• Final-source checks passed with Node 24.18.0 / npm 11.19.0: root build, lint, typecheck, Desktop/UI Knip, git diff --check, and the protocol epoch guard against main 21f1cc1c2 (224 → 225). Normal commit hooks passed.
• Formatting passed on a complete snapshot of the tracked source. The checkout-wide command flags pre-existing, untracked investigation scripts, which were preserved.
• After the final build, the protocol suite passed 93/93 on Node 22.23.2. Immediately before this documentation follow-up, the Goal Root, protocol, event-wait manager, storage bridge and storage authority suites passed 483/483 serially on the same Node version. This includes both lost-admission-acknowledgement cases and the reported Retry-After case; it does not establish the cause of the reviewer's intermittent failure.
• Earlier validation at 5386b1dfc passed 1,053/1,053 focused Node 24 tests and 47/47 Goal Root tests on Node 22. CI 39218667755 passed on that earlier head; this is not a claim about CI on the new head.

The first sandboxed 483-test run could not initialize storage authority locks; the unchanged command passed outside the sandbox. No fresh full-workspace test run, full Electron run, production-adapter test or exhaustive Host-process crash test was performed for this follow-up. SQLite kill/reopen and consistent-snapshot tests establish their respective storage/recovery boundaries, not full Host-process crash coverage.

Migration and recovery

• Workflow schema 14 → 15 updates active-wait and delivery indexes. Same-root backups retain linkage; portable Session copies discard it and pause linked Goals.
• Protocol compatibility epoch 224 → 225 covers typed event-wait provenance rejected by older strict decoders. Public turn.start permissions remain unchanged. Re-check the epoch against main before merging if another incompatible protocol change lands first.
• Queued results retain their frozen identities across recovery. If admission commits but its acknowledgement is lost before Root-owner adoption, the Host drains and fences the Session. Restart closes an admitted Goal Turn without a Run as app_restarted and pauses the Goal; explicit user resume starts a fresh continuation without redelivering the old wait. This is not transparent retry of an uncertain execution. The internal bridge README now documents this procedure.

AI use

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

Tool(s) and scope: OpenAI Codex assisted with implementation, regression tests, review and documentation. Both commits retain Generated-by: OpenAI Codex; retain attribution in the final squash commit.

Checklist

• Tests cover the change and fail without it
• Lint, format, typecheck and the affected suites pass locally (format scope and test snapshots are stated above)

Does this PR entail a change in behavior?

• Yes — described under Summary above
• No

[Astro-Han]

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

First review of 30c291f96bcc0aaed730a36a6a3e40cd4983e951, one commit on merge-base db455a1661ec (current main 368f5c9a3d4a). I read the diff against the PR3 plan in #5926 and checked the storage bridge, both providers, schema v15, the Root admission seam, the Host bridge and Goal gating, and the Runtime yield. No prior review findings.

Verdict: I found no verified P0–P2 defect. Atomicity, provider parity, single-successor and migration behaviour hold up. Below are three P3 items: an epoch collision, a test that hangs on Node 22, and an untested lost-acknowledgement replay path.

What I verified

• Atomicity and provider parity. Register, settle, queue and admit are pure functions in event-wait-bridge.ts. SQLite runs each one inside a single operational-DB write transaction, and Root admission writes the Goal/wait linkage in the same transaction that inserts core_root_turn_admissions. The Memory provider runs them inside one authority.write. Ordering is identical in both: existing admission → rejection → source checks → joint write.
  • The only parity difference is how a second undelivered wait is rejected. Memory checks for it explicitly; SQLite relies on the partial unique index workflow_event_waits_one_active_session. Both reject, but the error type differs.
  • Rollback at each stage and SQLite SIGKILL at each window are tested.
• At most one effective successor.
  • Result vs timeout: resolving at or after the deadline is rejected, expired is terminal, and the PR2 CAS commit picks one winner.
  • Once queued, the pending entry is frozen: same turn/run/message IDs and provenance digest, and a conflicting re-queue throws.
  • Admission requires a settled gate, no currentExecution, no prior delivery, and a matching digest. In the same transaction it clears the gate and sets delivery.admitted.
  • A replayed admitRootTurn returns existing before reaching the bridge block.
  • The late-queue-acknowledgement race is covered, and #adoptWaitAuthority ignores stale snapshots.
  • Goal control shares the Session admission gate with Root admission, so a pause cannot interleave with the admitted() adoption.
• Gating and resources.
  • A source Turn that does not end with a durable event_wait yield ends up pausing the Goal, which suppresses the wait; it does not replay.
  • Observation authorization reuses PR2's flight and cooldown accounting.
  • The bridge's own timer is unref'd and stopped on drain.
• Schema. v14 → v15 runs only when the registered version is older; newer versions are refused. The target-schema check uses the same migration function. Indexes exclude delivered rows from active ownership and delivery scans. Portable copies pause linked Goals with a new generation; same-root backups keep the linkage.
• Protocol. turn-origin, the runtime invocation root authority and the root execution descriptor each gain an optional eventWait, and older strict decoders reject it. The epoch bump is therefore warranted.

Findings

1. P3: protocol epoch collision. This PR moves RUNTIME_HOST_COMPATIBILITY_EPOCH from 222 to 223. #6115, #6091 and #6124 also declare 223. Whichever merges later needs to rebase to the next free epoch and keep its comment line.

2. P3: test hangs deterministically on Node 22.

   • late queue acknowledgement cannot restore a gate after Root admission hangs on Node 22.23.2 (engines allows ^22.19.0). It fails 3/3 in isolation, and the runner then cancels the remaining 22 tests in goal-root-authority.test.js.
   • It stalls at await admissionAccess.promise. The second authorization depends on the bridge's 1 s rescan timer, which is unref()'d (goal-event-waits.ts:391), so Node 22's event loop empties first.
   • Adding a ref'd keep-alive around that await makes the whole file pass (45/45).
   • CI's test job runs only Node 24, where it passes in about 2 s.
   • Production is unaffected. A fix would be to drive the rescan through the fixture's injected clock, or to call reconcile() explicitly before awaiting.

3. P3 (unreproduced suspicion): in-process lost acknowledgement at Root admission.

   • Suppose admitRootTurn commits but its acknowledgement is lost, and the Goal lane retries admitTurn with the same frozen identity. The retry takes the existing-admission branch of RootTurnCoordinator, which runs the admitExecution gate after the fact.
   • By then the gate has been cleared and delivery.admitted is set. authorizeAdmission therefore returns false, and the retry fails with HostedRootAdmissionGateError instead of adopting the durable admission.
   • Nothing is duplicated. The likely outcome is an errored Goal Turn or a Host drain, then a restart that closes the admitted successor without a Run, so the wait result is never acted on.
   • Only the storage-level replay is tested (Memory, event-wait-bridge.test.ts).
   • Suggested fix: have authorizeAdmission accept a wait whose delivery is admitted with exactly the pending turn/run/message IDs, and add a Host-level test.

Tests run locally. Fresh worktree, Node 22.23.2, npm ci + build, then the PR's 14 focused files run serially: 766 tests, 743 pass, 0 fail, 23 cancelled. All 23 cancellations come from finding 2; with the keep-alive the affected file passes 45/45. I did not run the full suite.

CI and mergeability. All checks on the head pass, including test (Node 24, 15m) and installed-CLI validation on Node 22.19/24. GitHub reports the PR as mergeable; BLOCKED is only the pending required review.

[Astro-Han]

P3: epoch 223 is also declared by #6115, #6091 and #6124 (main is at 222). Whichever PR merges later needs to move to the next free epoch and keep its comment line.

[Astro-Han]

P3: on Node 22.23.2 (allowed by engines ^22.19.0) this test hangs at await admissionAccess.promise every time. The second authorization depends on the bridge's unref'd 1 s rescan timer, so the event loop empties first, and the runner then cancels the remaining 22 tests in this file. Adding a ref'd keep-alive around that await makes the file pass 45/45. CI only runs it on Node 24. Consider driving the rescan through the fixture clock, or calling reconcile() explicitly.

[Astro-Han]

P3 (unreproduced): suppose a Root admission commits but its acknowledgement is lost, and admitTurn is retried with the same frozen identity. The existing-admission path runs this gate again, but the admission transaction has already cleared activeEventWait and set delivery.admitted. This check then returns false, the retry fails with HostedRootAdmissionGateError, and the durable admission is never adopted. Consider accepting delivery.kind === 'admitted' when its turn/run/message IDs match pending.execution/userMessageId, and add a Host-level test for this window.

[chinawch007]

Updated branch: 5386b1dfc. The focused Node 24 regression run passes 1,053/1,053; the Node 22 Goal Root suite passes 47/47.

Drafts for the three review threads; not posted to GitHub.

Protocol epoch

Original comment

Thanks for flagging the merge-order dependency. I rebased onto apache/maka main at 6772a3456, which uses epoch 224, and updated this PR to epoch 225, including the compatibility test and explanation. The protocol epoch guard passes with 224 -> 225. This is relative to that main snapshot; a later incompatible main change would require another check before merging.

Node 22 test lifetime

Original comment

Confirmed and fixed. On Node 22.23.2, the original test exits while awaiting admissionAccess because the bridge's rescan timer is unref'd. The test now uses the shared withTimeout helper for that signal: its referenced timer keeps the wait alive and fails after five seconds if authorization never starts. The production timer remains unref'd, the test's existing 15-second timeout is unchanged, and the original late-queue/admission barriers and assertions are preserved.

The original test still reproduces the cancellation against the rebased production code. The updated complete goal-root-authority file passes 47/47 on Node 22.23.2, including the two new acknowledgement-loss cases below.

Lost admission acknowledgement

Original comment

I added Host-level regression cases for both Memory and SQLite, injecting an error after the real admitRootTurn transaction commits but before RootAdmissionOwner receives its result.

These confirm the existing failure boundary: the Host requests drain, the delivery remains durably admitted, and no successor Run starts. An explicit retry with the same identity is rejected by assertKnownAdmission before the event-wait execution gate is called. Both cases also verify that there is only one successor admission and that stale Goal cleanup cannot overwrite the committed revision.

I therefore kept the event-wait gate unchanged: allowing delivery.admitted there would not recover this unacknowledged admission, and would not satisfy the existing poisoned/fencing contract. The existing restart test still covers closing an admitted Goal without a Run as app_restarted. The added cases cover the store-to-owner acknowledgement boundary, not a full Host process crash/restart or a later failure after the owner has accepted the result.

[Astro-Han]

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

Incremental re-review of 5386b1dfc0bb318ef90b3cac99c7294ccd8c74ed. The previous review was at 30c291f96bcc. The branch was rebased from merge-base db455a1661ec onto 6772a3456223, which brings in 24 main commits, and now has two commits: the rebased feature commit cdc7a68d6 and a new test-only commit 5386b1dfc.

What changed. git range-diff shows the feature commit differs only in rebase context: an import line in runtime-invocation.ts, surrounding lines in ai-sdk-turn.ts, and the protocol epoch moving from 223 to 225, placed above main's 224 comment block. No production logic changed. The new commit only touches goal-root-authority.test.ts. It wraps the late-queue barrier in withTimeout, adds an afterRootAdmission fixture hook, adds a Memory and a SQLite test for a lost Root-admission acknowledgement, and makes fixture cleanup run in finally.

Prior findings

1. P3, epoch collision at 223: superseded, but the collision is back at 225 (see below).
2. P3, the late-queue test hanging on Node 22: fixed. withTimeout uses a ref'd timer, which keeps the event loop alive while the unref'd bridge rescan reaches the barrier. On Node 22.23.2, goal-root-authority.test.js now passes 47/47, three runs in a row (before the fix: 23 cancelled).
3. P3, in-process lost acknowledgement at Root admission: resolved by design. The author chose to fail closed instead of adopting the durable admission. When the acknowledgement is lost, RootAdmissionOwner poisons the Session. A same-identity retry is rejected with "identity changed within one Host Epoch" before the execution gate runs, nothing starts, and the Host drains. This matches the existing crash-after-admission contract and is now tested for both Memory and SQLite. Both tests pass locally. What happens to the Goal after restart is still not tested end to end, but I'm not raising that as a finding.

New findings

• P3: protocol epoch collision at 225. Main is at 224, and this PR now claims 225. Open PRs #6163 and #6091 also declare RUNTIME_HOST_COMPATIBILITY_EPOCH = 225 (#6115 still declares the stale 223). Whichever merges later needs to move to the next free epoch and keep its comment line. Separately, the new 225 comment sits above main's 224 block, and that block still says "223 is used by ... open PRs #6115 and #6091". That note is now stale and could be cleaned up when the epoch is re-checked at merge time.

I found no P0–P2 issues in the increment.

Tests run locally. Fresh worktree, Node 22.23.2, npm ci plus builds of core through runtime-host.

• goal-root-authority, protocol and event-wait-manager run serially in one invocation: 395 tests, 394 pass, 1 fail. The failure was expire: peer Retry-After releases the dispatch reservation without consuming another attempt. That test already exists on main and is not changed by this PR, and it passed 3/3 when run alone, so I treat it as a load-related flake unrelated to this PR.
• Storage event-wait-bridge and event-wait-authority: 88/88 pass.

CI and mergeability. All checks on 5386b1df pass, including test (15m28s) and installed-CLI validation on Node 22.19 and 24. GitHub reports the PR as MERGEABLE; the BLOCKED state only reflects the pending required review.

[Astro-Han]

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

I reviewed the 50-file change at 5386b1dfc0bb318ef90b3cac99c7294ccd8c74ed, including the Goal continuation coordinator, Root admission/recovery, both storage providers, workflow migration, and typed transcript provenance. The change registers an explicit wait, settles its source Turn, freezes a successor, and consumes that successor in the Root admission transaction. WaitForEvent remains internal: production adapters, catalogs, ShellRun integration, and UI are not enabled here.

No verified P0–P2 findings. Registration/settlement/delivery preserve Goal control generations and budget checks. Ordinary evaluation and waiting timers remain gated while an explicit wait is outstanding; cancellation, revocation, and newer user work do not authorize a stale successor. I checked the joint transitions in packages/storage/src/event-wait-bridge.ts, their SQLite/Memory implementations, and the admission and recovery integration under packages/runtime-host/src/server/.

The lost-acknowledgement behavior deliberately favors avoiding duplicate work over automatic progress. The existing Root authority rejects another admission in the same Host epoch and requests Host drain after a committed admission loses its acknowledgement. I additionally reopened a SQLite snapshot into fresh Host/Root/Goal objects: an admitted successor without a Run was closed as app_restarted, the Goal became paused, and explicit user resume completed through a fresh ordinary Goal Turn without redelivering the old wait. This requires Host restart and user resume; it is not transparent automatic recovery. Separately, queued resolved and expired results survived reopening and executed exactly one successor with their original Turn/Run/message IDs and provenance. Documenting this recovery procedure alongside the internal bridge would help future adapters expose the behavior clearly.

Validation on Node 22.23.2: 770 tests passed across the 14 focused files, with no skips or cancellations. These exercise both providers, failed transactions, storage SIGKILL/reopen boundaries, bounded observation, permission/control races, accounting, existing Goal behavior, and real Root admission with fake adapters/models. Six additional checks covered the three fresh-Host recovery scenarios above and workflow 14 → 15 upgrades of existing waiting/resolved/expired records; Goal/wait facts and source cooldowns were preserved. Fresh dependency installation/patches, normal root build:test, workspace typechecks, production renderer build and output checks, lint, formatting, ASF headers, renderer architecture, surface inventory, and protocol guard passed.

P3 — coordinate compatibility epoch 225 before merging (packages/runtime-host/src/protocol/index.ts:111). Main is currently 224, but open #6091 (752020ce) and #6163 (591c08bc) also declare 225 for different protocol changes. The first merge may use 225; later changes must rebase and take the next unused epoch. The PR body's older 222 → 223 migration note also needs refreshing.

The exact head remains OPEN/MERGEABLE, and CI workflow 39218667755 succeeded. A trial merge with current main was clean; I did not build that merged tree. I did not run full Electron, production adapters/providers, the complete workspace test suite, or exhaustive Host-process crash interleavings. The additional recovery checks used durable SQLite snapshots, not process-kill testing of the full Host.

[Astro-Han]

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

Incremental review of 5386b1dfc0bb318ef90b3cac99c7294ccd8c74ed → 7fe6963f6da7fd9964536978dbf7056c74efbe29. No new verified P0–P2 findings. I compared the rewritten two-commit series with git range-diff and checked the actual file differences. Both heads retain the same merge base, 6772a3456; this update did not incorporate newer main commits. The second patch is identical. The first changes only recovery documentation and protocol comments: two files, 12 additions/two deletions. All executable Goal, event-wait, Root-admission and storage code, and all tests, remain byte-for-byte unchanged; the protocol module differs only in standalone comments.

The new recovery instructions in packages/runtime/README.md:79 match the existing behavior: queued successors retain frozen identities, while uncertain admission drains the Host and fences the Session. After restart, an admitted Goal Turn without a Run is closed as app_restarted and the Goal pauses; explicit user resume starts a fresh continuation without redelivering the old wait. Both Memory and SQLite lost-acknowledgement cases passed again. My earlier fresh-Host recovery and migration probes remain historical evidence at 5386b1dfc, rather than tests newly rerun here. Production event sources and the public WaitForEvent tool remain outside this PR's enabled surface.

P3 — epoch 225 still overlaps other open protocol changes. Current main 622a4cb045b067aa93c798a598fc124646199f3e still declares epoch 224, so 225 has not been consumed on main. This head uses 225 at packages/runtime-host/src/protocol/index.ts:111; open #6091 at 752020ce418d55a406943ebfe556ed019d142fb1 and #6163 at 591c08bc2bc365f85320eab3b7133486ae2448cb also declare 225 for their distinct changes. Re-check after the first incompatible change lands and give later changes the next unused epoch. The guard currently passes 224 → 225. The PR description now states that transition correctly; the previous stale epoch description is fixed.

Fresh dependency installation/patches, normal root build:test, all workspace typechecks including the four Desktop configurations, formatting/lint for the changed protocol file, and git diff --check passed. Five focused test files covering Goal/Root admission, protocol decoding, event-wait management and the storage bridge/authority passed 483/483, with no skips or cancellations. CI 39253103634, CLI package validation and dependency audit succeeded at this exact head.

The head is OPEN/MERGEABLE and a trial merge with current main was clean. I did not build that merged tree, repeat the complete local suite or earlier custom recovery probes, or test Electron, real production adapters/providers, or exhaustive Host-process crashes. This follow-up confirms the unchanged execution logic and the documentation update within those limits.

[chinawch007]

Thanks for the incremental review and the focused test rerun.

Agreed on coordinating the epoch at merge time. I checked main at 622a4cb04, which still uses 224, while this PR uses 225. If #6147 lands first, 225 remains appropriate; subsequent incompatible protocol changes will need to advance the epoch against the main they merge into. If another incompatible change lands first, I’ll update this PR’s epoch and rerun the guard before merging. There is no need to bump it further solely because other open PRs also use 225.

The update at 7fe6963f6 is limited to the recovery documentation and protocol comments; executable behavior and tests are unchanged. CI for this head has passed. The PR description retains the distinction between current validation and historical recovery probes, along with the remaining coverage limits.