Skip to content

feat(workhub): project delegated execution status - #4115

Merged
Astro-Han merged 4 commits into
apache:mainfrom
ARE404:feat/workhub-execution-feedback
Aug 29, 2026
Merged

feat(workhub): project delegated execution status#4115
Astro-Han merged 4 commits into
apache:mainfrom
ARE404:feat/workhub-execution-feedback

Conversation

@ARE404

@ARE404 ARE404 commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

Summary

Deliver Slice 5-B execution feedback on top of the durable delegation linkage from #3935:

  • acknowledge the atomic delegation_assigned record immediately as accepted
  • project accepted, running, waiting, completed, failed, aborted, and recovering from the exact target Session/Turn
  • refresh from Session change notifications with generation fencing so stale reads cannot overwrite newer state
  • rebuild the projection after navigation or restart without persisting a second WorkHub lifecycle
  • preserve the preassigned WorkHub root identity through idle recovery while keeping ordinary queued successors on fresh identities
  • make the projected delegation result navigate to its authoritative target Session

The target ordinary Session remains the only execution authority. This PR does not add replace, Stop ownership, supersession, or destructive correction.

Refs #3492
Proposal: #3286

Design choice: hybrid first response

The durable assignment is the immediate acknowledgement; WorkHub does not wait for target completion before confirming acceptance. It then joins the immutable delegation link to the exact target Turn and Session live state. Terminal Turn facts win, exact live membership projects running/waiting, unreadable authority projects recovering, and a not-yet-observed Turn remains accepted.

Execution status stays a rebuildable read model. Session change notifications invalidate it, opening WorkHub reconstructs it, and a generation fence drops stale asynchronous reads. No mutable coordination-status record or second recovery state machine is introduced.

Runtime recovery preserves a preassigned future root identity only for current-Turn steering admissions. Ordinary next-Turn follow-ups carry their predecessor identity, so they continue to receive a fresh successor rather than becoming self-referential.

Verification

  • npm run lint — PASS
  • npm run format:check — PASS
  • npm run build — PASS
  • npm run typecheck — PASS
  • npx knip --workspace apps/desktop — PASS
  • npx knip --workspace packages/ui — PASS
  • npm --workspace @maka/runtime-host test — PASS (1357 tests; 1348 passed, 9 skipped, 0 failed)
  • npm --workspace @maka/desktop test — PASS (1644/1644)
  • WorkHub Electron E2E — PASS (2/2):
    • WorkHub rebuilds delegated execution feedback after navigating away and back
    • WorkHub defers destructive correction until linked delegation exists
  • git diff --check upstream/main..HEAD — PASS

The WorkHub Electron E2E output is the user-visible behavior evidence for this change. It exercises delegation feedback reconstruction after leaving and reopening WorkHub.

Exact verified head: 179ba06afbf05d0648c400f7bd17a12cbc031af3.

AI use

Select exactly one:

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

Tool(s) and scope: Codex implemented the production and test changes, updated the ADR, investigated identity recovery behavior, and ran the listed verification. The commit includes the required Generated-by: Codex trailer.

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

@github-actions github-actions Bot added the effort/L Under 1000 readable lines label Aug 29, 2026
@M4n5ter

M4n5ter commented Aug 29, 2026

Copy link
Copy Markdown
Member
English

Reviewed at exact head 179ba06afbf05d0648c400f7bd17a12cbc031af3.

Blocking concern: the projection follows the admission Turn, not the delegated Message

The durable assignment records targetTurnId from the target's root state at admission time. The new projection then reads the lifecycle of exactly that Turn.

However, the existing Message authority explicitly allows an unconsumed steering Message to be folded into the follow-up queue when its original Turn ends, then executed by a fresh successor Turn. This is already covered by the existing “lone folded steering entry” regression test.

A supported path is therefore:

  1. WorkHub delegates to active Turn A and persists targetTurnId = A.
  2. Turn A finishes before consuming the steering Message.
  3. The Message is folded into successor Turn B.
  4. Turn A records completed.
  5. Turn B is still running, waiting, or may later fail.
  6. WorkHub projects Turn A and reports the delegation as completed.

The displayed lifecycle would describe the admission Turn, not the execution of the delegated work. The same mismatch would make future linked correction or Stop ownership target the wrong Turn.

The recovery change exposes the same underlying issue for concurrent pending admissions: if multiple pending steering admissions carry different preassigned identities, sharedPendingRootIdentity returns undefined and recovery aggregates them under a newly generated Turn. Their immutable targetTurnId links then no longer identify the actual execution.

Suggested direction

The stable identity of a delegation should be the target Message, not its initial Turn.

delegation_assigned already persists targetMessageId. I suggest resolving that Message through the target Message authority—pending admission, immutable steering proof, and root source receipt—to find its current or terminal owning Turn, then projecting that Turn's lifecycle.

In that model:

  • targetTurnId may remain the admission location, but is not treated as immutable execution ownership;
  • steering rollover and recovery remain valid Runtime behavior;
  • concurrent Messages may correctly resolve to the same aggregated successor Turn;
  • future correction/Stop can bind to the Turn that actually executes the delegated Message.

Alternatively, if the product contract requires one immutable target Turn, admission must guarantee that the delegated Message can never migrate to a successor. Preserving selected root identities only during one recovery path is not sufficient.

The hybrid acknowledgement, rebuildable projection, generation fence, and UI navigation all look reasonable, but I think this identity boundary needs to be corrected before the execution-status contract is reliable.

简体中文

审查的精确 head 为 179ba06afbf05d0648c400f7bd17a12cbc031af3

阻塞问题:投影跟随的是 admission Turn,而不是被委托的 Message

持久 assignment 会记录 admission 时目标 root state 中的 targetTurnId,新的 projection 随后读取这个确切 Turn 的生命周期。

但现有 Message authority 明确允许尚未被消费的 steering Message 在原 Turn 结束时折叠到 follow-up queue,并由新的 successor Turn 执行。仓库现有的 “lone folded steering entry” 回归测试也确认了这是正式支持的行为。

因此存在以下受支持路径:

  1. WorkHub 将任务委托给正在运行的 Turn A,并持久化 targetTurnId = A
  2. Turn A 在消费该 steering Message 之前结束。
  3. Message 被折叠到 successor Turn B。
  4. Turn A 记录为 completed
  5. Turn B 仍在运行、等待用户,或者之后执行失败。
  6. WorkHub 投影 Turn A,并把这次 delegation 显示为已完成。

此时界面展示的是 admission Turn 的生命周期,而不是被委托工作的实际执行状态。未来实现 linked correction 或 Stop ownership 时,同样可能控制错误的 Turn。

并发 pending admission 的恢复也暴露了相同根因:当多个 pending steering admission 携带不同的预分配 identity 时,sharedPendingRootIdentity 会返回 undefined,恢复逻辑将它们聚合到一个新生成的 Turn。原有不可变 targetTurnId 链接随后都不再指向实际执行。

建议方向

delegation 的稳定身份应该是目标 Message,而不是它最初进入的 Turn。

delegation_assigned 已经持久化了 targetMessageId。建议通过目标 Message authority——pending admission、immutable steering proof 和 root source receipt——解析该 Message 当前或最终归属的 Turn,再投影这个 Turn 的生命周期。

在这个模型中:

  • targetTurnId 可以继续表示 admission location,但不再被视为不可变的 execution ownership;
  • steering rollover 和 recovery 仍然是合法的 Runtime 行为;
  • 多个并发 Message 可以正确解析到同一个聚合后的 successor Turn;
  • 未来 correction/Stop 可以绑定真正执行该 delegation 的 Turn。

另一种选择是:如果产品契约要求目标 Turn 永远不可变,那么 admission 必须保证 delegated Message 永远不会迁移到 successor。只在某一条 recovery 路径中保留预选 root identity,并不足以建立这个保证。

Hybrid acknowledgement、可重建 projection、generation fence 和 UI navigation 本身都比较合理,但在 execution-status contract 可靠之前,我认为需要先修正这个 identity 边界。

@ARE404
ARE404 force-pushed the feat/workhub-execution-feedback branch from c988829 to bf55aab Compare August 29, 2026 03:21
@github-actions github-actions Bot added effort/XL Over 1000 readable lines and removed effort/L Under 1000 readable lines labels Aug 29, 2026
@ARE404
ARE404 force-pushed the feat/workhub-execution-feedback branch from bf55aab to 1cfa4d6 Compare August 29, 2026 05:52
@M4n5ter

M4n5ter commented Aug 29, 2026

Copy link
Copy Markdown
Member
English

Re-reviewed at exact head 1cfa4d6854bfc1ec4b21498df80b0fad3347a5cc.

The previous blocking issue is resolved: the delegated Message is now the stable identity, and the Message authority resolves its actual owning Turn across steering rollover and recovery aggregation.

Blocking: a cancelled delegation remains recovering forever

A supported path is still missing from the new execution-resolution contract:

  1. A delegated Message is pending in the target Session queue.
  2. The user deletes that queued Message through the existing Session UI.
  3. Runtime persists a cancellation tombstone and removes the pending admission.
  4. turn.message.execution.query checks only root source receipts, immutable steering proof, and pending admission. It cannot return the already-known cancellation fact, so it omits the Message entirely.
  5. WorkHub interprets a missing resolution as recovering.

The result is a delegation whose authoritative cancellation is durable across restart, but whose WorkHub status remains recovering indefinitely instead of reaching aborted. This violates the execution-feedback contract on an ordinary, user-supported queue operation.

Please add a cancelled execution resolution backed by the existing hasCancelledMessageAdmission tombstone and project it to aborted in WorkHub. A focused regression should exercise the real boundary: queue.entry.retract → execution query → aborted projection.

The Message-based ownership model otherwise looks correct, and both current CI checks pass.

简体中文

在精确 head 1cfa4d6854bfc1ec4b21498df80b0fad3347a5cc 上完成复审。

之前的阻塞问题已经解决:现在以 delegated Message 作为稳定身份,并由 Message authority 在 steering rollover 和 recovery 聚合后解析实际 owning Turn。

阻塞问题:被取消的 delegation 会永久停留在 recovering

新的 execution-resolution contract 仍遗漏了一条受支持路径:

  1. Delegated Message 尚在目标 Session 队列中等待执行。
  2. 用户通过现有 Session UI 删除该队列 Message。
  3. Runtime 持久化 cancellation tombstone,并删除 pending admission。
  4. turn.message.execution.query 只检查 root source receipt、immutable steering proof 和 pending admission,无法返回已经确定的 cancellation fact,因此会完全省略该 Message。
  5. WorkHub 将缺失的 resolution 解释为 recovering

结果是:delegation 的权威取消事实已经持久化并能跨重启保留,但 WorkHub 会无限期显示 recovering,而不是进入 aborted。这是普通且受支持的队列操作,因此会破坏本 PR 的 execution-feedback contract。

建议在 execution resolution 中增加由现有 hasCancelledMessageAdmission tombstone 驱动的 cancelled 状态,并在 WorkHub 中投影为 aborted。定向回归测试应覆盖真实边界:queue.entry.retract → execution query → aborted projection。

除此之外,Message-based ownership 模型是正确的,当前两项 CI 也均已通过。

@ARE404
ARE404 force-pushed the feat/workhub-execution-feedback branch from 1cfa4d6 to 4343a96 Compare August 29, 2026 08:31

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working through the Message-ownership and cancellation cases. The resulting authority direction now looks right: the delegated Message is the stable identity, Runtime resolves its actual owning Turn, and WorkHub rebuilds a read-only projection from target-owned facts.

My remaining concern is the shape of the PR as a whole. The raw diff size is not itself the problem, but the current change adds 1,164 lines while removing 33, and several mechanisms introduced under the earlier immutable-targetTurnId premise remain after the later correction replaced that premise with Message-based ownership.

For this slice, I think we should converge on one authority and remove what the new design makes redundant:

  • targetMessageId should be the sole stable delegation identity. Recovery can choose the actual successor Turn normally, and the Runtime Message authority can resolve that ownership.
  • recorded terminal facts should own completed / failed / aborted, while exact live membership should own running / waiting_for_user.
  • production and controller tests should exercise one Action Gate execution authority. The useful R2.4 routing and correction specifications can remain, but they should live at a pure policy seam or in the gated harness rather than preserving a parallel direct create / submit / stop executor.
  • fields and recovery branches that became unconsumed after the ownership correction should disappear from the final diff.

The feature can still require meaningful new code. The concern is that the replacement currently coexists with concepts and paths it superseded, so the slice has not yet reduced the codebase's authority or maintenance entropy. Since the PR already needs a rebase, could we use that pass to re-derive the minimum coherent design and remove those leftovers here rather than deferring the cleanup?

AI-assisted review disclosure: Codex performed a read-only review of the exact-head diff and traced the ownership, recovery, liveness, and controller-composition paths. Astro-Han reviewed and approved this text before posting. No product tests were run as part of this review.

Comment thread packages/runtime-host/src/server/message-coordinator.ts Outdated
Comment thread apps/desktop/src/renderer/workhub-session-port.ts Outdated
Comment thread apps/desktop/src/renderer/workhub-controller.ts
@ARE404
ARE404 force-pushed the feat/workhub-execution-feedback branch 2 times, most recently from a1e0d76 to 3c9b4dc Compare August 29, 2026 13:58
@ARE404
ARE404 force-pushed the feat/workhub-execution-feedback branch from 3c9b4dc to 3a1208e Compare August 29, 2026 14:27
@Astro-Han
Astro-Han merged commit 1ae4541 into apache:main Aug 29, 2026
2 checks passed
@ARE404
ARE404 deleted the feat/workhub-execution-feedback branch August 29, 2026 14:53
@jackwener

Copy link
Copy Markdown
Member

Post-merge review of source head 3a1208ef036c16f66c89abe74987677ea1c35f7c, merged as 1ae45418b4affa8c4ba94aa68e79756e8c4cfee3.

The PR merged while this review was in progress. I found three high-priority correctness issues and one refresh-amplification issue that remain in current main at 00a15abdc5e1494e7c7c9d04629ec77644dcf301. I did not issue an approval before merge.

[P1] Render the durable delegation row during the same visit

visibleWorkHubConversation() in apps/desktop/src/renderer/workhub-surface.tsx removes a durable Coordination turn whenever a local row with the same request ID still exists. A successful submitted row remains in that local array, and it renders only the target Session's aggregate status and latest result; it never reads the assignment's exact feedbackState.

In a targeted test, a local successful submission plus its durable assignment with feedbackState: 'aborted' produced durableRows=0 and localRows=1. The exact aborted result becomes visible only after leaving WorkHub and reopening it, because remounting clears the local row. This means the normal same-visit path does not show the exact accepted/running/waiting/completed/failed/aborted lifecycle that this PR adds.

Please replace or merge the local placeholder as soon as the matching durable assignment arrives, and add a surface regression that observes a terminal feedback update without remounting WorkHub.

[P1] Do not strand an idle assignment behind a competing root Turn

The Action Gate atomically stores an idle assignment with preselected wht_* / whr_* identities, releases the admission leases, and then starts consumePendingAdmissions() in the background. If an ordinary direct root wins the Session lease before that background call, the consumer observes an active root with different Turn/Run IDs. HostMessageCoordinator.#consumePendingAdmissions() skips the stored admission in that case.

There is no terminal-settlement retry for this admission: the only production calls are the immediate fire-and-forget attempt and Host-start recovery. The assignment can therefore remain durably pending after the competing root completes, while WorkHub has already acknowledged it as accepted. The targeted coordinator test retained the pending admission with no steering entry both during and after the competing root.

The later #3923 change adds durable proof for legacy Root source Messages, but it does not add the missing retry. Re-running the competing-root test on current main still produced zero recovered batches after normal root settlement; only explicit Host-start recovery consumed the admission.

Please make normal root settlement consume still-pending canonical admissions, or otherwise schedule a durable retry after this race, and cover the exact assignment-commit -> competing-root -> terminal interleaving.

[P1] Fence candidates() and act() with the Coordination Host generation

The Session projection is wrapped by scopeWorkHubSessionsToCoordinationHost(), which checks isCurrent(), but the Coordination port's candidates() and act() calls in app-shell.tsx are not wrapped by the same generation authority. A controller can therefore pass its Session read, wait in candidates(), become obsolete after the default Host changes, and still commit act() against the old Coordination Session.

I delayed candidates(), revoked the old generation, and then released the read. The obsolete controller still returned submitted and called act() once. Please bind all Coordination operations to the same generation claim and revalidate it immediately before the side-effecting act() call.

[P2] Coalesce feedback refreshes instead of only discarding stale results

Every Session change starts another asynchronous refreshFeedback(). The generation counter prevents an older result from overwriting a newer one, but it does not cancel, coalesce, or single-flight the reads. On this head, holding one feedback read and emitting 100 Session changes produced calls=101 and maxActive=101.

This is expensive in production because each Desktop feedback refresh reads the full paginated Turn list for every referenced target Session and then queries Message execution ownership. Session notifications include status and message/tool-result changes, including changes unrelated to these delegations. A busy target can therefore create an unbounded backlog and continuously invalidate the result that would have moved its row out of accepted.

Please filter notifications to relevant target Sessions and use one in-flight refresh with at most one trailing rerun. Add a regression that holds the first read, emits a burst, and proves both bounded concurrency and delivery of the final state.

The earlier Message-identity, cancelled-delegation, stale-recorded-running, preassigned recovery-root, and duplicate-controller-authority concerns are otherwise closed. The three previous review threads are resolved and do not report these four defects.


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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/XL Over 1000 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants