You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maka.apache.org by "me2seeks (via GitHub)" <gi...@apache.org> on 2026/08/29 05:20:10 UTC
[PR] feat(runtime-host): separate queued successor turns [maka]
me2seeks opened a new pull request, #4130:
URL: https://github.com/apache/maka/pull/4130
## Summary
- run each accepted `next_turn` message as its own FIFO successor Turn
- keep unconsumed steering messages together ahead of explicit successor work
- preserve queued messages across successor transitions and Runtime Host recovery
- apply the same per-successor capacity contract to submit, update, and promote operations
Fixes #4129
## Verification
- `npm run lint`
- `npm run format:check`
- `npm --workspace @maka/runtime-host run typecheck`
- `npm --workspace @maka/runtime-host run build`
- 133 affected Runtime Host tests passed with a worktree-local `TMPDIR`
Full-workspace `npm run typecheck` reaches and passes Runtime Host, but currently fails in unrelated UI/Desktop files due to existing interface/build-output mismatches.
## Review
Two independent reviews covered lifecycle/recovery correctness and design/simplicity. Both reviewers reported no blocking findings after fixes and re-review.
## AI use
Select exactly one:
- [ ] No generative tool made a substantive contribution
- [x] Generative tooling made a substantive contribution
Tool(s) and scope: OpenAI Codex assisted with code investigation, implementation, tests, and review preparation.
## Checklist
- [x] Tests cover the change and fail without it
- [x] Lint, format, typecheck and the affected suites pass locally
Does this PR entail a change in behavior?
- [x] Yes — described under Summary above
- [ ] No
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] feat(runtime-host): separate queued successor turns [maka]
Posted by "hqhq1025 (via GitHub)" <gi...@apache.org>.
hqhq1025 commented on code in PR #4130:
URL: https://github.com/apache/maka/pull/4130#discussion_r3888478558
##########
packages/storage/src/sqlite-session-metadata-store.ts:
##########
@@ -2031,6 +2062,13 @@ export class SqliteSessionMetadataStore {
if (message.turnId !== input.turnId) {
throw new SessionMetadataConflictError('Message admission transcript Turn conflict');
}
+ if (
+ steeringProof !== undefined &&
+ (message.ts !== steeringProof.admittedAt ||
Review Comment:
**[P1] Validate the transcript against the steering event timestamp, not the admission timestamp.** `HostMessageCoordinator` populates this proof's `admittedAt` from the queue admission (`message-coordinator.ts:690-697`), while an already-materialized steering transcript row is projected from the immutable runtime event with `ts = event.ts` (`runtime-event-read-model.ts:667-679`). A normal steering event is emitted after admission, so these timestamps are not required to match. This makes `markMessagesHandedOff()` throw `SessionMetadataConflictError: Proven steering transcript identity conflict` after an ordinary mid-turn steer; the root then fails cleanup, leaves `operation.turn.start` resident, and the Host exits uncleanly at the shutdown deadline. I reproduced the failing `subscribed Clients receive the durable steering echo as a session event` integration test locally; removing only this timestamp equality makes it pass. The hosted `test` job reports the same test plus two rel
ated message tests failing. Carry the immutable event id/timestamp in the proof and compare against those values, or otherwise stop equating event time with admission time.
##########
packages/runtime-host/src/server/message-coordinator.ts:
##########
@@ -2496,6 +2540,35 @@ function canonicalFollowupBatch(entries: readonly LiveEntry[]): {
}
}
+/**
+ * One explicit next-turn Message owns one successor root Turn. Steering that
+ * missed the final provider boundary is different: those entries all targeted
+ * the finishing Turn, so keep their correction context together in the first
+ * successor rather than turning each interjection into unrelated future work.
+ */
+function nextSuccessorItems<
+ T extends { readonly disposition: 'steering' | 'followup' | 'turn_started' },
+>(entries: readonly T[]): T[] {
+ if (entries.length === 0) return [];
+ if (entries[0]!.disposition !== 'steering') return [entries[0]!];
+ const steering: T[] = [];
+ for (const entry of entries) {
+ if (entry.disposition !== 'steering') break;
+ steering.push(entry);
+ }
+ return steering;
+}
+
+function nextRecoveredSuccessorItems(
+ pending: readonly PendingMessageAdmission[],
+): PendingMessageAdmission[] {
+ const steeringIntent = pending.filter(
Review Comment:
**[P1] This still rejects the production-valid form of the preassigned recovery batch.** WorkHub persists an idle-target assignment as `submittedPlacement: current_turn`, `placement: current_turn`, `disposition: steering`, with its own deterministic `wht_*`/`whr_*` identity (`execution-composition.ts:1337-1372`). Two such valid durable records are both selected here, then `pendingSteeringRootIdentity()` throws because their turn/run identities differ, so startup recovery cannot start either message. The new regression at `message-coordinator.test.ts:148-198` passes only because it constructs `placement: current_turn` with `disposition: followup`; `normalizePendingMessageAdmission()` rejects that combination. I reproduced the failure by seeding two records through `SessionStore.assignWorkHubMessage()` and calling `consumePendingAdmissions()`: it throws `Pending steering admissions disagree on their root identity`. Recover one preassigned root and restore the remainder (or explicitl
y define a safe rebinding rule), and run this regression through the real validating store.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] feat(runtime-host): separate queued successor turns [maka]
Posted by "Astro-Han (via GitHub)" <gi...@apache.org>.
Astro-Han commented on code in PR #4130:
URL: https://github.com/apache/maka/pull/4130#discussion_r3885892182
##########
packages/runtime-host/src/server/message-coordinator.ts:
##########
@@ -744,8 +767,8 @@ export class HostMessageCoordinator implements RuntimeMessageAuthority {
const entry: LiveEntry = {
entryId: this.#createId(),
messageId: admission.messageId,
- turnId: admission.turnId,
- runId: admission.runId,
+ turnId: rootState.turnId,
Review Comment:
**[P2] Preserve each queued admission’s durable identity during recovery**
These remaining admissions are still stored with their original `turnId` and `runId`, but this restores their `LiveEntry` objects using the newly active successor’s identity. A later `queue.entry.update` or `queue.entry.promote` therefore sends the rewritten identity to `updateMessageAdmission()`, where the durable store rejects it as an identity conflict.
This is reachable after a restart with two queued next-turn messages: the first becomes the recovered successor, while editing or promoting the second fails. Keeping `admission.turnId` / `admission.runId` here would preserve the admission provenance; `reservedRoot` already owns the active-root identity. A recovery regression that performs an update or promotion on the remaining entry would cover the composition boundary.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] feat(runtime-host): separate queued successor turns [maka]
Posted by "me2seeks (via GitHub)" <gi...@apache.org>.
me2seeks merged PR #4130:
URL: https://github.com/apache/maka/pull/4130
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] feat(runtime-host): separate queued successor turns [maka]
Posted by "Astro-Han (via GitHub)" <gi...@apache.org>.
Astro-Han commented on PR #4130:
URL: https://github.com/apache/maka/pull/4130#issuecomment-5463788808
This is a synthesis of the independent review by @Haoqing_Reviewer at exact head `fe5a94ee2fb6bb17a5e28437d9b5b598d52867b9` (base `16d06f217de5d57e6825b58fdb7a0ffe668cf092`, previous head `ba41d1c`). Delta `ba41d1c..fe5a94ee` is 2 files; I verified drift and exact-head checks myself — the file:line findings below are from Haoqing's sealed review.
What I checked myself:
- Confirmed drift `ba41d1c → fe5a94ee` (`message-coordinator.ts` contention with `main 8aed381e`), state OPEN, `mergeable UNKNOWN` / `CONFLICTING`, `mergeStateStatus` UNKNOWN, `reviewDecision` APPROVED still bound to `adcd42b6` (stale, does not cover `fe5a94ee`).
- Checked exact-head checks: `gh pr checks` reports *no checks* on this branch; no terminal `test`/`windows_recovery` for `fe5a94ee`; `statusCheckRollup=[]`.
Findings from Haoqing's sealed review:
**Overall: NO-GO — 0×P1 open (previous P1 closed) but merge blocked by CONFLICTING + missing checks**
- **[Closed] Previous P1 — cross-successor ownership rebinding (`message-coordinator.ts:518-545`, `:1399-1412`, `:1903-1921`, `:622-629`, `:686-699`) is fixed in `fe5a94ee`.** The immutable submission identity is now separated from mutable execution generation and generation is updated on promote/successor handoff. Two new one-off regressions were added and both pass after the fix:
- nack path: carried follow-up with old generation is now correctly re-queued instead of `release`d
- restart path: `startRecoveredMessages` no longer re-delivers a message already settled with an immutable steering proof
The fix was verified with the expanded `message-coordinator` tests and a local reproduction that previously showed `nack → empty` and `ack+proof → duplicate` now passes. No schema change or migration was found, and the broader `ba41d1c..fe5a94ee` diff remains limited to the coordinator, admission storage uniqueness/cancellation, immutable proof and successor handoff.
- **[Gate still blocked] Content conflict with `main`.** `fe5a94ee` conflicts with latest `main 8aed381e` in `packages/runtime-host/src/server/message-coordinator.ts` (also reported by GitHub as `CONFLICTING`). The old merge ref `d3937c1d` (for `ba41d1c`) cannot be used. Until the conflict is resolved and a new head is frozen, correctness of the final merged code (this fix + `main`'s new message execution/proven-root handoff) cannot be verified.
**Verification reported by reviewer**
- Builds core/storage/runtime/runtime-host: PASS
- `message-coordinator.test.js` incremental two files + full targeted checks: PASS; `git diff --check` / `commit --check`: PASS; worktree clean
- `runtime-host` full: 1350 passed / 1 failed / 9 skipped — the one failure is an unrelated `execution-model-composition.test.ts` managed-sandbox assertion, reproducible in isolation and not on PR path (local suite not fully green but not blocking this PR)
- GitHub checks: none on `fe5a94ee` (see self-check above)
**Undetermined**
- Final post-rebase correctness after resolving the `message-coordinator.ts` conflict
- Hosted CI for the new head (`test`/`windows_recovery`)
- Windows/macOS local behavior (hosted checks missing)
Gate: Previous P1 is considered closed based on the file:line evidence and two regressions, but head `fe5a94ee` is **not merge-ready** — it is `CONFLICTING` and has no terminal required checks. Author must rebase onto latest `main`, produce a new head, and re-freeze for review; task #400 stays `in_review`.
---
> **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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] feat(runtime-host): separate queued successor turns [maka]
Posted by "Astro-Han (via GitHub)" <gi...@apache.org>.
Astro-Han commented on PR #4130:
URL: https://github.com/apache/maka/pull/4130#issuecomment-5463626476
This is a synthesis of the independent review by @Haoqing_Reviewer at exact head `ba41d1ccd2863dd264856b89a786df7e9d14bdbb` (base `16d06f217de5d57e6825b58fdb7a0ffe668cf092`, merge-ref `d3937c1dc2641eb648ca2ff661d8d39fc72c2e2d` clean). I verified the drift and the exact-head CI myself; the file:line findings below are from Haoqing's sealed review.
What I checked myself:
- Confirmed head `ba41d1c` is still current (`base 16d06f2`, state OPEN/MERGEABLE, `reviewDecision` APPROVED but that approve is bound to older `adcd42b6` and does not cover this head — stale, not terminal).
- Checked exact-head CI: `test` SUCCESS (33263264872), `windows_recovery` SUCCESS (33263264968), `label` implied PASS; 2 required checks terminal green.
Findings from Haoqing's review:
**Overall: NO-GO — 1×P1 blocker (worst P1)**
- **[P1] `packages/runtime-host/src/server/message-coordinator.ts:518-545` — queued entries retained across successor are not rebound to current execution ownership, causing lost retry or duplicate execution after restart.** `beginTerminalTransition()` consumes only the queue head successor while `commitNextRoot()` increments `generation` without updating remaining entries. When the user promotes a remaining follow-up in a new Turn, the code writes the old `turnId/runId` back to durable admission and moves the old `generation` unchanged into steering (`:1399-1412`).
Two reachable consequences with file:line证据:
- *nack path*: provider nacks before persistence, ` :1903-1921` requires `entry.generation === state.generation` to re-queue; the carried entry belongs to the previous generation, so it is `release`d directly and the user message disappears from the current queue.
- *restart duplication*: provider has acked and the current Turn's immutable steering proof exists, but settlement only scans admissions whose durable identity equals the current run (`:622-629`) and recovery requires proof's turn/run to equal the old identity in admission (`:686-699`). The admission is therefore not deleted; the next Host restart re-delivers the already-delivered message as root, risking duplicate side effects.
Both paths were reproduced with one-off regression tests: nack → expected re-queue but actual empty; ack + steering proof of current run → `startRecoveredMessages` receives the same message again after recovery. Tests were removed and the worktree has no source changes after verification. Fix direction: separate immutable submission identity from current execution ownership and update current `generation` on cross-successor/promote; add the two carried-follow-up promote regression tests (nack and restart).
**Change scope checked**
- The PR splits each explicit `next_turn` into its own root, keeps steering prefix aggregation for the same finishing Turn, and adjusts recovery/capacity and related tests/docs. Review covered 6 changed files, full diff, and the `root-turn-coordinator` successor admission/handoff, Runtime `ai-sdk-backend` pull/ack/nack, and SQLite message admission identity/update/handoff paths.
**Verification reported**
- Builds: core, storage, runtime, runtime-host — PASS.
- `message-coordinator.test.js`: 51/51 PASS.
- `runtime-host` full suite: 1348 passed / 1 failed / 9 skipped — the single failure is an unrelated `managed-sandbox` tool-result path check, reproducible in isolation and not on the PR path.
- GitHub `test` / `windows_recovery`: PASS (exact-head, see above).
- `git diff --check` and `git show --check` for both commits: PASS.
**Self-check gaps noted by reviewer**
- Current plan is not yet correct, therefore not optimal; no deletable production code was found that would be safe to remove without losing the goal.
- Missing the two cross-successor promote regression tests; other new tests for FIFO/recovery/capacity are valuable.
- Ownership model needs a small deepening — rewriting durable turn/run to the current run would reintroduce the identity conflict fixed in the previous version.
- Current head is therefore **not merge-ready**.
- Undetermined: Windows/macOS behavior not verified locally (Windows hosted check passed); no schema change or migration was found.
Gate: Despite green CI and a stale APPROVED on `adcd42b6`, the exact head `ba41d1c` has an open P1 blocker (cross-successor ownership rebinding + two regression tests) and must not merge until fixed and re-reviewed at a new head.
---
> **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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org