Re: [PR] refactor: make Runtime Host the sole Message admission authority [maka]

["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