You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maka.apache.org by "Astro-Han (via GitHub)" <gi...@apache.org> on 2026/08/25 14:02:06 UTC
[PR] fix: reconcile CLI and Desktop messages by canonical identity [maka]
Astro-Han opened a new pull request, #3803:
URL: https://github.com/apache/maka/pull/3803
## Summary
Refs #3633
This is the replacement PR 2 after #3721 and #3716 landed on main.
- Interactive external Turn admission uses the submitted Turn identity as the canonical user-message identity, so CLI and Desktop can show a stable row immediately and reconcile the durable transcript by messageId.
- Desktop keeps transient presentation rows separate from durable history and replaces them deterministically on sparse transcript replacement, reconnect, terminal refresh, and remount-style state changes.
- CLI assigns the same stable identity to its transient TUI user entry, preserves it through sparse/reconnect transcript replacement, and removes pendingFallback, takePendingFollowup, client-side delivery retry, and duplicate queue rendering. Queue state remains a read-only Runtime Host projection.
- Waiting live-send arms no longer manufacture an empty renderer Turn; Follow Up remains Host-queued and immediately visible through the existing queue projection.
## Verification
- npm ci in the isolated worktree.
- npm --workspace @maka/ui run build; materialize.test.js: 17 passed.
- npm --workspace @maka/runtime-host run build; root-turn-coordinator.test.js: 53 passed, including canonical external user-message identity.
- npm --workspace maka-agent run build; pi-tui-runner.test.js and focused transcript/turn tests passed. The runner coverage includes active-turn steering, Follow Up queueing, old terminal events, interruption, and session replacement.
- Desktop main build and full desktop typecheck passed; transient-message-projection.test.js and app-shell-busy-race-settlement.test.js: 7 passed.
- Scoped Biome check and git diff --check passed.
Before: Desktop and TUI optimistic rows used client-only or renderer-owned identities, so durable refresh could duplicate or erase them; waiting arms could create an empty pseudo Turn. After: transient and canonical rows share messageId, durable transcript wins in place, and queue/lifecycle authority remains in Runtime Host.
## AI use
- [ ] No generative tool made a substantive contribution
- [x] Generative tooling made a substantive contribution
Tool(s) and scope: Codex authored the implementation and tests under human review; the commit includes the required Generated-by: Codex trailer.
## 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] refactor: make Runtime Host the sole Message admission authority [maka]
Posted by "Astro-Han (via GitHub)" <gi...@apache.org>.
Astro-Han commented on PR #3803:
URL: https://github.com/apache/maka/pull/3803#issuecomment-5422105959
Reply to [the review on `587a1da5a`](https://github.com/apache/maka/pull/3803#pullrequestreview-5027460742). All of it is now addressed on `3edcc4f25` except one item I am pushing back on and one I moved out of this PR. Hosted `test` is green on that head (17m24s), as is `windows_recovery`.
Before acting I traced each item against the cited code. The eight I accepted turned out to share one cause, which changed how I fixed them.
### Root cause
The PR moved the admission *decision* to Runtime Host but never gave the resulting window — client submit → Host answer — an owner. Inside it neither side holds the whole fact: the client knows the intent and not the outcome, the Host knows the outcome and not what the client is rendering. The findings are the three ways that window was unowned, so they are fixed as three seams rather than as eight local patches.
**The window was not serialized** — Blocking #1. Removing the TUI's submit hold left nothing enforcing arrival order, and `#ensureSession()` had no single-flight. `56e5e29b6` puts admission and configuration on one ordered tail in the driver and shares one in-flight Session creation. Session identity changes stay off the tail deliberately: `/session` and `/new` are how a user leaves a Session whose admission is stuck, and the existing generation assert already fences them. `runtime-host-session-driver.test.ts` now asserts one `session.create`, one Session identity and stable submit order under a held `session.create`, plus that a `/model` typed after a Message reaches the Host after it.
Worth noting the asymmetry that confirms the diagnosis: Desktop did not have this defect, not because it is written differently but because it still has `setNewTaskSendPending` serializing the first send. The side whose seam I removed is the side that broke.
**The window's two identities were never rebound** — #2 and the projection half of the simplify audit. `8570af26a` rebinds the arm on `turn_started`, but only while it is still the arm that send placed and still `unconfirmed`; an admission whose outcome cannot be proven releases the arm while keeping the Message row, since those are separate claims. The same dual identity was the fabricated `turnId: messageId`, so `TransientUserMessageProjection` now states the presentation fields it actually renders instead of borrowing `StoredMessage` (`8570af26a`, `3edcc4f25`).
**The four-value answer was collapsed to threw/did-not-throw** — #3, #4, #5, #7. `46f95013f`: `enqueueMessage` resolves with whether the Message was sent, so a refusal returns `false` and the composer keeps the draft, while `outcome_unknown` still counts as sent; a second dispatched interruption maps to `outcome_unknown` as the CLI driver already did; the cancellation-proof query chunks by `MESSAGE_QUEUE_MAX_ENTRIES`. `c86690368` projects `turn_started.skillInvocation` in the TUI.
Your reading of why the CLI Skill test passed is correct and worth stating plainly: the fake spread its prepared turn — the `turn.start` shape production no longer uses here — into the started-Turn push. It now answers the submit with `turn_started` and strips the receipt from the push, and the assertion fails without the fix. The same infidelity existed in the Runtime Host fixture, which rebuilt a source message from parts instead of storing the one the coordinator produced; fixed in `6e6fa1d68`.
### #1, first half — accepted
`6e6fa1d68`. `RootTurnSourceMessage` gains `submittedIntentDigest` over the Skill ids and orchestration, written at admission and compared on every retry; `PendingMessageAdmission` gains the orchestration value itself (schema 32, replay-guarded for the merge-convergence path) and `startRecoveredMessages` applies it to both the fresh graph epoch and the root admission. Two regressions: a graph→swarm retry under one Message identity is `operation_conflict`, and a Message recovered across the pre-root cut re-opens under the orchestration it asked for.
### #1, second half — pushing back
`blocked` not being persisted is deliberate and documented at `message-coordinator.ts:901-908`: not remembering it is what lets the same identity be submitted again once the Skill resolves. The ghost row needs `blocked` **and** a lost response, it is recoverable, and turning `blocked` into a keyed durable terminal result is a design decision about what a terminal Message outcome is — not something to settle inside this PR. Happy to open it separately if you think the trade is wrong.
### #6 — real, but not this PR
Confirmed and filed as #3881. `git log -S` puts both the unfiltered `messages: opened.messages` and the runner's transcript replacement before this PR's base commit; they arrived with #3759. #3760 is open against the same two files, so it should land after that.
### Simplify audit
Both done — projection narrowing above, and `c5833e52a` makes the submit IPC fail closed on a missing `messageId` and drops `command.messageId ?? newId()`.
### #8
Fixed in `f3418063c`, migrated to `sessions.submitMessage` and failing closed on a refused result. Agreed the prompt-rail failure is #3862/#3863.
--
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] refactor: make Runtime Host the sole Message admission authority [maka]
Posted by "Astro-Han (via GitHub)" <gi...@apache.org>.
Astro-Han merged PR #3803:
URL: https://github.com/apache/maka/pull/3803
--
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] refactor: make Runtime Host the sole Message admission authority [maka]
Posted by "Astro-Han (via GitHub)" <gi...@apache.org>.
Astro-Han commented on PR #3803:
URL: https://github.com/apache/maka/pull/3803#issuecomment-5422109563
Reply to the NO-GO on `587a1da5a`. The P1 is real, I reproduced it independently before fixing it, and it is closed on `3edcc4f25`.
Reproduction first: I wrote your control as a test against the message coordinator — one Message identity submitted with graph orchestration, then retried with the same content and placement under swarm — and it returned the earlier `turn_started` success instead of `operation_conflict`. Your reading of the cause is right: `skillIds` and `turnOrchestration` are new inputs to `turn.message.submit` in this PR, and neither `sameSourcePayload` nor `PendingMessageAdmission` was widened to match, so the exact-Turn intent never became part of what makes a submit the same submit.
`6e6fa1d68` binds it at both ends.
**Durable receipt.** `RootTurnSourceMessage` gains `submittedIntentDigest`, a digest over the ordered Skill ids and the orchestration override, written when the root admission records the source message and compared on every retry. A receipt that carries no digest was written for a submit that asked for no exact intent, so any intent now is a different request and fails closed — which is correct here because `turn.message.submit` has never shipped with these fields.
**Pre-root cut.** The fingerprint alone would not have fixed recovery, since recovery needs the value rather than a digest of it. `PendingMessageAdmission` now carries `turnOrchestration` (SQLite schema 32; the ALTER is guarded because the post-merge convergence path replays it onto a database that may already have the column), the recovery batch carries it when the batch is a lone Message — exact-Turn intent needs an idle Session, so it can never be one of several — and `startRecoveredMessages` applies it to both `prepareFreshAgentGraphEpoch` and `admitRootTurn`. Before this it admitted with no orchestration at all, which is exactly the silent downgrade you described.
Both regressions you asked for are in `message-coordinator.test.ts`: `a retry that changes exact-Turn intent is a conflict, not the earlier success` (and an unchanged retry still returns the original success, with one `startFromMessage` call), and `recovery re-opens a Turn under the orchestration the Message asked for`. I verified each fails without its fix.
One thing your review surfaced indirectly. The first version of the conflict test passed for the wrong reason: the fixture's `startFromMessage` rebuilt a receipt from parts instead of storing the source message the coordinator produced, so it dropped whatever the coordinator recorded about the submit — the very thing a retry is compared against. That is fixed in the same commit; without it the durable half of this would have looked covered while testing nothing.
The stale `sessions.steer` E2E consumer is fixed in `f3418063c`. Hosted `test` is green on `3edcc4f25` (17m24s), as is `windows_recovery`. Runtime Host 1219/1219 and Storage suites pass locally.
--
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] refactor: make Runtime Host the sole Message admission authority [maka]
Posted by "M4n5ter (via GitHub)" <gi...@apache.org>.
M4n5ter commented on code in PR #3803:
URL: https://github.com/apache/maka/pull/3803#discussion_r3864224700
##########
packages/runtime-host/src/server/message-coordinator.ts:
##########
@@ -662,6 +701,9 @@ export class HostMessageCoordinator implements RuntimeMessageAuthority {
content: aggregateMessageContents(pending.map((entry) => entry.content)),
submittedContent: aggregateMessageContents(pending.map((entry) => entry.content)),
sources: pending.map(pendingMessageSource),
+ ...(pending.length === 1 && pending[0]!.turnOrchestration
Review Comment:
**[P2] Preserve the full exact-Turn identity through pre-root recovery**
This recovery batch preserves `turnOrchestration`, but the pending admission and schema do not preserve the `submittedIntentDigest` that now defines sameness for both ordered Skill ids and orchestration. `pendingMessageSource()` therefore writes a recovered root source without that digest. If the Host stops after the pending Message commit but before root admission, then recovers the Turn, an unchanged retry with the same Message id, content, placement, and graph/swarm request reaches `sameSourcePayload()` and compares `undefined` with the newly computed digest. The Host returns `operation_conflict` even though it is already executing the same recovered request.
I reproduced this with a focused coordinator regression: the retry returned `{ ok: false, error: { code: 'operation_conflict', message: 'Durable message receipt has a different payload' } }` instead of the recovered `turn_started` success. The existing recovery test only checks that orchestration reaches `startRecoveredMessages`; it does not persist the reconstructed source receipt and retry the original submit, so it cannot catch this window.
Please persist the canonical submitted-intent digest in `PendingMessageAdmission`, copy it into the recovered source, and add same-intent-success plus changed-intent-conflict tests across this crash point.
---
_Posted by an automated review agent operated by @M4n5ter. This 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] refactor: make Runtime Host the sole Message admission authority [maka]
Posted by "Astro-Han (via GitHub)" <gi...@apache.org>.
Astro-Han commented on code in PR #3803:
URL: https://github.com/apache/maka/pull/3803#discussion_r3864591168
##########
packages/runtime-host/src/server/message-coordinator.ts:
##########
@@ -662,6 +701,9 @@ export class HostMessageCoordinator implements RuntimeMessageAuthority {
content: aggregateMessageContents(pending.map((entry) => entry.content)),
submittedContent: aggregateMessageContents(pending.map((entry) => entry.content)),
sources: pending.map(pendingMessageSource),
+ ...(pending.length === 1 && pending[0]!.turnOrchestration
Review Comment:
Fixed in `dc48df742`. You were right about the window, and the repair boundary turned out to be one concept wider than the missing field.
**Root cause.** The intent was bound into durable identity through two independent representations: the root source message carried a *digest* of it, the pending admission carried only the raw `turnOrchestration`, and `pendingMessageSource()` converted one shape into the other by hand. The digest was not among the fields that conversion could carry — and could not have been, because `skillIds` were never stored either. So the recovered source had no intent at all and an unchanged retry compared a real digest against `undefined`.
**Repair.** Rather than adding the digest as a third representation, the intent is now one value stored whole wherever it is durable:
- new `SubmittedTurnIntent { skillIds, turnOrchestration? }` in `@maka/storage` — one normalizer, one order-sensitive equality, sole authority for the concept;
- `PendingMessageAdmission.turnOrchestration?` → `submittedIntent?`, and `RootTurnSourceMessage.submittedIntentDigest?` → `submittedIntent?`; both records now hold the same shape, so `pendingMessageSource()` is a copy;
- `submittedIntentDigest()` and the `createHash` import are gone. Intent is a few ordered ids and a mode; hashing it bought nothing but the inability to rebuild it.
- Schema 32 is unreleased, so its column becomes `submitted_intent_json` in place rather than gaining a successor.
**One extra defect the same split was hiding:** the live pending-admission comparison at the former `message-coordinator.ts:875` compared `turnOrchestration` only and silently ignored `skillIds`, so a retry that changed just the Skill set under one Message identity aliased the earlier success without any crash involved. It is one comparison now, so it covers both.
**One deviation from what you asked for.** The unchanged retry after recovery answers `outcome_unknown`, not `turn_started`. `PendingMessageAdmission.disposition` is typed `steering | followup` and cannot express `turn_started`, so the recovered source keeps the queued disposition. The Host therefore declines to name a Turn it cannot prove, and the client keeps its transient row and reconciles from canonical transcript. Making it answer `turn_started` would mean *deriving* a disposition that was never stored — the same shape this commit removes — and it would also mislabel a lone recovered follow-up. I left it as a boundary rather than trading one derived fact for another; say so if you read that differently.
**Tests.** Three added: recovery preserves the whole intent; an unchanged retry after recovery is not a conflict; a changed retry after recovery still is. I verified the middle one red-then-green by dropping the intent copy out of `pendingMessageSource()` — it returns to `operation_conflict`. Your point about the fixture was the reason the window was untestable: `startRecoveredMessages` in the coordinator double now writes the receipt its production counterpart writes.
Local verification on this head: build, typecheck, lint, format, knip clean; Runtime Host 105/105 and Storage 58/58 on the affected files. Hosted `test` is running.
--
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