[Sun-GLiang]

Summary

Preserve the Host-produced SkillInvocationResult for every admitted message disposition instead of dropping it when a Skill message is steered or queued behind a running Turn.

• require and validate skillInvocation on steering, followup, turn_started, and blocked submit results
• keep partial Skill preparation outcomes through queue admission while blocking messages whose Skill requests all fail
• persist each admission and root source outcome through SQLite recovery, retries, and durable proofs
• forward the Host outcome through the existing Desktop and CLI Skill feedback paths
• include Skill outcomes in durable admission capacity checks, including queued-message updates

Fixes #4026

Verification

• npm test -w @maka/storage — 981 passed, 0 failed, 16 platform skips
• npm test -w @maka/runtime-host — 1315 passed, 0 failed, 9 platform skips
• npm test -w @maka/desktop — 1624 passed, 0 failed
• npm test --prefix packages/cli — 545 passed, 0 failed
• npm run lint — passed
• npm run typecheck — passed
• git diff --check — passed
• npm test — all workspaces passed except the pre-existing packages/runtime macOS Bash executable-root assertion, which expects /usr/local even when that root is already represented by the current /usr/local/bin/node; the same failure was reproduced before this change

Migration and compatibility

• bumps the Runtime Host protocol compatibility epoch from 56 to 57 because TurnMessageSubmitResult.skillInvocation is now required for every disposition
• bumps SQLite session metadata schema from 33 to 34 and migrates historical pending admissions to an empty Skill outcome
• Runtime Host, Desktop, and CLI are upgraded together; no legacy protocol compatibility branch is added

AI use

Select exactly one:

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

Tool(s) and scope:

OpenAI Codex implemented the protocol, Runtime Host, storage migration, Desktop/CLI forwarding, tests, documentation, and verification described above.

Checklist

• Tests cover the change and fail without it
• Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

• Yes — described under Summary above
• No

[Astro-Han]

This is a synthesis of the independent blind review by @Sol-404ARE at exact head 32097d1543789453dde678e321d1005d7710cd25 (base b6b647cf6369d94ca1a38691e8d1a332b4926662, 24 files +1037/-125). I verified the diff and the exact-head CI myself; the file:line findings below are from Sol's sealed review.

What I checked myself:

• Read gh pr diff 4054 (fix: retain queued skill-invocation results, 24 files) and confirmed the versioned contract propagation touching PendingMessageAdmission, SQLite migration 33→34, Host prepare/queue/recovery, and Desktop/CLI projections.
• Checked exact-head CI: test run 33139071828 SUCCESS, windows_recovery run 33139071842 SUCCESS, label run 33139071761 SUCCESS, OPEN/MERGEABLE/BLOCKED/REVIEW_REQUIRED.

Findings from Sol's review (file:line anchored):

Standards — GO (0 P0–P3)

• Protocol migration is explicit/fail-closed: every submit disposition now requires skillInvocation, exact codecs enforce it, and compatibility epoch 56→57 records the break.
• Durable boundary is complete: required decode/equality in PendingMessageAdmission, SQLite 33→34 NOT NULL empty-outcome migration plus convergence guard, and all insert/read/update paths carry the column.
• Host preserves the exact result across prepare, queue retry/recovery/update, and root-source handoff/proof; full-record capacity preflight uses storage's actual normalize/serialized-size authority.
• Desktop/CLI forward Host authority, docs update the existing queue contract, and title/branch/template/AI trailer hygiene pass.
• The 24-file breadth is required propagation of one versioned contract, not Shotgun Surgery — repeated empty outcomes are boundary fixtures, not duplicated logic. Entropy is neutral-to-reduced.
• Validation: merge-base 2b5dd8a87, git diff --check PASS, Biome across 23 TS/Markdown files PASS, ASF header audit PASS, worktree clean.

Spec — GO (0 P0–P3)

• Busy-turn partial resolution returns the same failed entries on steering/followup; total resolution failure returns structured blocked before queue mutation, and Desktop/CLI reach existing Skill-specific feedback.
• The exact outcome is persisted, decoded, equality-checked, restored into live entries, reused for same queued identity and idle takeover, copied into root-source receipts, and counted in complete root-admission capacity checks. Ordering is durable admission commit before in-memory visibility.
• Focused tests cover partial/total busy paths, persistence/retry, active and idle recovery, large-result capacity, protocol codecs, migration, Desktop, and CLI — storage 981, Runtime Host 1315, Desktop 1624, CLI 545 passing (as reported).

Explicit exclusions noted: blocked outcomes intentionally admit nothing and can succeed later; legacy rows migrate to empty because historical failures were never recorded; a consumed queued receipt may still answer outcome_unknown because its original queueRevision was never durable (pre-existing limit), while this PR durably retains the Skill outcome itself.

What I did not judge: a full live Host recovery with a queued Skill failure across a real restart was not executed beyond unit/integration coverage — verification was by code inspection and the suites noted above.

Gate: Standards 0 and Spec 0 — no P0–P2, test/windows_recovery/label green, head 32097d15 is ready to be approved and merged once a write holder approves. Seal: notes/pr-4054-provisional.md.

---

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

[Astro-Han]

Thanks for carrying queued Skill outcomes through the Host-owned admission and SQLite recovery paths. I reviewed exact head 1553a2149e5b752510031a3bb211fa05e1892f00; the normal steering/follow-up paths now preserve the structured result, and I left one P2 durable-proof recovery suggestion inline. The branch is currently conflicting and its exact-head required test is failing, so it will need rebase and fresh verification after the recovery case is addressed. Please do push back if the intended contract is explicitly ‘feedback only on the first response’ rather than the retry/durable-proof behavior described in the PR.

中文摘要

谢谢把 queued Skill outcome 接入 Host admission 和 SQLite 恢复。正常 steering/follow-up 已修复，但响应丢失后从 durable proof 重试时，已持久化的 outcome 仍被丢弃；Desktop/CLI 因 outcome_unknown 不再展示 Skill 反馈。若产品合同明确是“只在第一次响应报告一次”，欢迎 push back；否则建议补齐该恢复分支。当前还需重基并恢复 exact-head test。

AI-assisted review disclosure: Codex ran an independent analysis lane; Astro-Han independently verified the exact head, production path, and severity, and owns this review.

[hqhq1025]

I found one P2 in the durable retry path. The normal queued Skill outcome flow and the previously reported durable replay case are fixed, but a follow-up that is promoted before handoff can no longer be matched by the original request after a Host restart. The inline comment includes the reproducing sequence and the missing invariant. Local build, typecheck, focused Runtime Host/Storage/Desktop/CLI tests, full CLI, and exact-head hosted checks passed; unrelated baseline/environment failures remain in the full Runtime Host and Desktop suites.

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.

[hqhq1025]

Re-reviewed exact head 3409e15d20033f37ff0b5d7d79a4f594591fc214. I found no remaining P0-P3 issues.

The previous durable-retry issue is fixed by preserving the immutable submitted placement alongside the promoted live placement, carrying it through storage normalization and recovery, and comparing retries against submittedPlacement ?? placement. The new regression covers the exact queued next_turn -> promotion -> terminal handoff -> old-Epoch retry sequence and verifies that the original retry replays the stored Skill outcome while a current_turn retry still conflicts.

Local build, full typecheck, focused Runtime Host/Storage tests, changed-file formatting, diff checks, and merge-tree validation passed. The full Runtime Host and Storage suites each retained one environment/baseline failure that reproduces on the previous head. Exact-head hosted test and windows_recovery are green.

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.

[Astro-Han]

Thanks for the durable-retry update. I confirmed that submittedPlacement now stays immutable through admission, promotion, handoff, restart recovery, and retry matching; the earlier functional findings are fixed. I have one test-quality suggestion below. This was an AI-assisted review using Codex; I independently traced the storage/coordinator paths and exact-head CI. Please push back if this fixture exercises a persistence seam that I have overlooked.

[Astro-Han]

Thanks for adding the exact promotion/retry regression. [P3] This test manually manufactures the durable receipt in the fixture Map after beginTerminalTransition, so it verifies coordinator payload comparison but not the production RootTurnCoordinator / admission store / SQLite / reopen path that must actually preserve submittedPlacement. A future persistence omission could therefore leave this test green. Could we either rename/comment it as coordinator-level only, or replace it (rather than add another duplicate matrix) with one focused persistence round-trip through the real owner? Please push back if fixture.receipts is backed by the production durable store in this fixture.

[Sun-GLiang]

Replaced the coordinator-only fixture test in 7f9759b with a focused RootAdmissionOwner → AgentRunStore → SQLite close/reopen → recoverSession round-trip. The new test verifies that a promoted source recovers submittedPlacement=next_turn while its effective placement remains current_turn, and the old duplicate matrix was removed.

[Astro-Han]

Thanks for the durable-retry fixes. I rechecked exact head 3409e15: the original Skill outcome now survives admission, promotion, handoff, restart recovery, and replay, and the required checks are green. The remaining test-boundary P3 is non-blocking. This was an AI-assisted review using Codex; I independently verified the production ownership path, recovery behavior, exact head, and CI. Please push back if any invariant has been misread.

[Sun-GLiang]

@Astro-Han I've addressed the remaining non-blocking P3 by replacing the coordinator-only fixture with a focused production persistence round-trip test. This PR is ready to merge—could you please merge it when convenient? Thanks!

[hqhq1025]

Re-reviewed exact head 05ede86ffe95b78b7c10573a9687b38b4bc5af79. I found no remaining P0-P3 issues.

The three previously reviewed functional commits are unchanged. The new delta replaces the synthetic persistence fixture with a production SQLite close/reopen regression: root-admission-owner.test.ts now admits a promoted source through RootAdmissionOwner, closes and reopens the real store, recovers the session, and verifies both the immutable submittedPlacement and effective promoted placement. Together with the existing coordinator promotion/update coverage, this resolves the prior test-boundary concern. The merge from current main also resolves the protocol conflict correctly at compatibility epoch 79 while preserving the epoch 78 OAuth history.

Local clean install, build:test, full typecheck, focused coordinator/admission/protocol tests (74/74), lint, formatting, diff checks, and a clean merge-tree passed. The full Runtime Host and Storage suites each retained the same single environment/baseline failure recorded on the previous reviewed head; neither failing path is changed by this PR. Exact-head hosted test and windows_recovery are green.

The local review host used Node 22.22.1. The existing APPROVED review is bound to the older 3409e15 head and does not independently cover this commit.

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.