You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maka.apache.org by "ARE404 (via GitHub)" <gi...@apache.org> on 2026/09/22 13:41:56 UTC
[PR] fix(workhub): preserve choice history and task control evidence [maka]
ARE404 opened a new pull request, #5592:
URL: https://github.com/apache/maka/pull/5592
## Summary
WorkHub lost settled selection details during transcript replay, made multi-target stop/resume actions hard to identify, and exposed admission receipts without a read-only way to verify the exact execution turn. Repeated stop/resume sequences could also reuse an earlier stop identity.
- Project canonical form/question outcomes into expandable history, including all options, selected answers and cancellation.
- Split multi-Session rails equally and retain target identity on stop/resume results, including failures.
- Preserve stopped assignment discovery and scope repeated stops to their action.
- Add bounded, Host-validated exact-turn status reads and explicit admission evidence. Omit absent evidence fields from control receipts so successful stops do not fail strict JSON decoding.
- Update routing guidance for contextual follow-ups, ambiguous stops and truthful execution reporting.
Protocol compatibility epoch is 178 after integrating current main; older strict transcript decoders do not accept the new interaction message projection. Safe-boundary resume remains disabled by default.
## Verification
Native Electron acceptance used an isolated profile and real DeepSeek V4 Flash conversations. Verified expandable selected/cancelled history, restart replay, equal two-Session rails and filtering, task navigation, stop/failed-resume identity, and completed/running/cancelled status queries without re-delegation. A newly discovered stop receipt failure was reproduced, fixed, and retested in the native app.
See [acceptance record](docs/workhub-fix-acceptance-2026-09-22.md) for exact natural-language inputs, scope and remaining limitations. Earlier combined affected regression: 429 passed. Latest pre-integration Desktop WorkHub regression: 119 passed. After integrating upstream main `0052f1cfd`: **286 affected tests passed**, `npm run build:test`, Desktop renderer build, Desktop typecheck, changed-file Biome checks, and renderer architecture checks passed (112 checker tests plus strict base comparison). Native acceptance above predates integration; it was not rerun on the merged tree.
The merge commit pre-commit ASF audit flags three unchanged upstream Codex executor example files (README.md, maka.composition.yml, and test/fixtures/fake-codex.mjs). The original fix commit passed all pre-commit checks. Only the merge commit bypasses hooks; the protocol epoch and diff checks were run explicitly.
## Remaining acceptance work
- [ ] F04: eliminate unsupported artifact-preservation claims and inference of an older Turn's completion from Session availability.
- [ ] Keep coordination replies concise and avoid unnecessary internal IDs/flags.
This is a draft because these model-response issues remain. The latest live run was targeted; attachments, floating drafts, forced termination and native successful-resume UI were not rerun.
## AI use
- [ ] No generative tool made a substantive contribution
- [x] Generative tooling made a substantive contribution
Tool(s) and scope: OpenAI Codex implemented the changes and regression tests, operated native acceptance, and prepared this PR. Commits carry Generated-by trailers.
## 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] fix(workhub): preserve choice history and task control evidence [maka]
Posted by "ARE404 (via GitHub)" <gi...@apache.org>.
ARE404 merged PR #5592:
URL: https://github.com/apache/maka/pull/5592
--
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] fix(workhub): preserve choice history and task control evidence [maka]
Posted by "Astro-Han (via GitHub)" <gi...@apache.org>.
Astro-Han commented on code in PR #5592:
URL: https://github.com/apache/maka/pull/5592#discussion_r4081982627
##########
packages/runtime-host/src/server/session-transcript-reader.ts:
##########
@@ -175,6 +176,42 @@ function createDurableLedgerTranscriptReader(input: {
if (projected.diagnostics.some(isHardRuntimeEventReadModelDiagnostic)) {
throw new Error('Durable RuntimeEvent transcript projection is incomplete');
}
+ const interactions = await input.stores.interactionStore.listTurnInteractions(
+ turn.invocation.sessionId,
+ turn.invocation.turnId,
+ );
+ interactions.sort(
+ (a, b) =>
+ (a.outcome?.outcome.committedAt ?? a.request.createdAt) -
+ (b.outcome?.outcome.committedAt ?? b.request.createdAt),
+ );
+ for (const record of interactions) {
Review Comment:
**P1 — trigger ② (handoff/resume path): settled `form_interaction` rows are emitted once per invocation run, so a multi-invocation Turn duplicates them.** `projectTurn` runs once per run (a run is one invocation's ordinal range), but `listTurnInteractions` is scoped by `(sessionId, turnId)` and never filters `record.request.runId`. When a `turnId` is shared by K invocations — `runtime_handoff_pause_v1` makes that a production path — each run emits the same settled interaction, anchored into its own ordinal range, all passing the window filter.
Downstream there's no dedup: the desktop replica stores by sequence, and `chat-view.tsx` renders one card per message id — the same `interaction:${requestId}` card appears K times (with React key collisions), and rows anchored into an already-consumed range stay invisible live but appear after reseed, breaking the live≡replay equivalence this PR is built on. The added tests only cover single-invocation turns, so this slips through.
Smallest fix: emit each settled record under exactly one invocation — e.g. filter to `record.request.runId === turn.invocation.runId` (owner run), with the commit-time anchor as fallback when the owner run isn't visible.
<details>
<summary>中文</summary>
`projectTurn` 按 run（单 invocation）调用，但 `listTurnInteractions` 按 (sessionId, turnId) 查、不按 `runId` 过滤——多 invocation 共享 turnId 时（runtime handoff 是生产路径）同一条 settled 交互会在每个 run 各发射一次：下游按 sequence 存、按消息 id 渲染卡片，同一张卡出现 K 次且 live 与 replay 不等价。修法：每条记录只归 owner invocation 发射（按 `record.request.runId` 过滤），owner 不可见时用提交时间锚定兜底。
</details>
##########
packages/runtime-host/src/server/session-transcript-reader.ts:
##########
@@ -175,6 +176,42 @@ function createDurableLedgerTranscriptReader(input: {
if (projected.diagnostics.some(isHardRuntimeEventReadModelDiagnostic)) {
throw new Error('Durable RuntimeEvent transcript projection is incomplete');
}
+ const interactions = await input.stores.interactionStore.listTurnInteractions(
+ turn.invocation.sessionId,
+ turn.invocation.turnId,
+ );
+ interactions.sort(
+ (a, b) =>
+ (a.outcome?.outcome.committedAt ?? a.request.createdAt) -
+ (b.outcome?.outcome.committedAt ?? b.request.createdAt),
+ );
+ for (const record of interactions) {
+ const { request, outcome } = record;
+ if (
+ (request.request.kind !== 'form' && request.request.kind !== 'question') ||
+ !outcome ||
+ (outcome.outcome.kind !== 'form_answer' &&
+ outcome.outcome.kind !== 'question_answer' &&
+ outcome.outcome.kind !== 'closure')
+ )
+ continue;
+ // Anchor after acceptance, so an incremental reader that already saw the
+ // pending request receives the settled history row as well.
+ const source =
+ turn.projection.anchors.find((event) => event.ts >= outcome.outcome.committedAt) ??
+ turn.projection.anchors.at(-1);
Review Comment:
**P3 — trigger ②: rows settling after the run's last event land on an already-covered sequence and stay invisible until reseed.** The `?? anchors.at(-1)` fallback anchors to the last event of *this* run; its sequence then sits ≤ the replica's `durableThrough`, and `transcript_advanced` only fires when the RuntimeEvent high-water moves — an interaction commit doesn't move it. So a closure committed after all run events (host_restarted / turn_terminal / producer_cancelled settle paths) never reaches a live replica until the next full reseed. Narrow and self-healing, but worth fixing together with the run-scoping comment above: prefer anchoring to the run that *contains* an event with `ts >= committedAt`, falling back to the owner run's tail only when none qualifies.
<details>
<summary>中文</summary>
晚于本 run 最后一个事件才 settle 的记录会锚到已覆盖的 sequence——interaction 提交不推高水位，`transcript_advanced` 不发帧，live 副本在 reseed 前永远收不到。窄场景可自愈，建议和上面的 run 归属过滤一起修：优先锚到"含 ts≥committedAt 事件"的那个 run。
</details>
##########
packages/runtime-host/src/server/shared-session-transcript.ts:
##########
@@ -66,6 +66,8 @@ export function projectSharedSessionTranscriptMessage(
: { coordinationActionId: message.coordinationActionId }),
};
}
+ case 'form_interaction':
Review Comment:
**P3 — worth an explicit decision: `form_interaction` passes through the shared-session projection verbatim**, while `permission_decision` and `workhub_coordination` rows are `return null`'d and tool args are sanitized via `projectToolActivityArgs`. If exposing a guest audience the full form/question request and selected answers is intended (it is the user's own choice history), fine — but given the neighboring "decision evidence" types are all hidden, the asymmetry reads like an omission. If not intended, `return null` here or project a redacted shape.
<details>
<summary>中文</summary>
`form_interaction` 在 shared-session 投影里原样透传（request 全文 + 所选答案），而隔壁 `permission_decision`/`workhub_coordination` 一律隐藏、tool args 脱敏——这个不对称像遗漏。若属有意（用户自己的选择史）则无碍；否则对该类型 `return null` 或投影成脱敏形状。
</details>
##########
packages/runtime-host/src/server/workhub-coordination-coordinator.ts:
##########
@@ -340,15 +346,20 @@ export class HostWorkHubCoordinationCoordinator {
async #prepareStop(
input: Parameters<WorkHubActionGateEffects['prepareStop']>[0],
): Promise<WorkHubDelegationStopRequestedMessage> {
- const suffix = workHubDestructiveClaimIdentitySuffix(input.stopsDelegationId);
+ const first = await this.#stores.readWorkHubStopRequest(input.stopsDelegationId);
Review Comment:
**P2 — trigger ② (concurrency): the stop-record slot is chosen from a stale pre-lane read, so two concurrent actions stopping the same delegation can overwrite each other.** `first` is read *outside* `#commitCoordinationFact`'s admission lane. If actions X1 and X2 both read `first === undefined` before either commits, both compute the delegation-keyed suffix; inside the lane, the scoped `read()` fallback can't see the mismatch (X2's read of `whq_<del>` finds X1's record, actionId differs, falls through to the empty `whq_<[del,X2]>` slot) — so `existing` is `undefined` and X2's `build` writes `whq_<del>`, silently replacing X1's request. X1's later resolution then reads back X2's fields and conflicts.
Smallest fix: inside the lane, read *both* keys (primary and the action-scoped slot) and let `build` pick the id from what was actually found — not from a pre-lane read. That keeps first-stop compatibility, makes the mismatch visible to the serialized section, and converges concurrent writers instead of clobbering.
<details>
<summary>中文</summary>
stop 记录的槽位由车道外的陈旧读决定：两个不同 action 并发 stop 同一 delegation 时都在 `first=undefined` 上算出 delegation 键；车道内的 scoped 读因 actionId 回退反而"看不到"对方已提交的 primary 记录 → 后者把 `whq_<del>` 整体覆盖，先者的 resolution 读到他人字段。修法：车道内同时读 primary 与 action 槽两个键，由 `build` 按实际读到的事实选 id。
</details>
##########
packages/runtime-host/src/server/workhub-coordination-action-gate.ts:
##########
@@ -390,7 +395,7 @@ export class WorkHubCoordinationActionGate {
const source = await this.#stopSource(input.actionId, proposal.expects.targetSessionId);
const stopFingerprint = stopActionFingerprint(input, source);
await this.#claimAction(input.actionId, 'stop', stopFingerprint, source.delegationId);
- const existing = await this.#effects.readStopRequest(source.delegationId);
+ const existing = await this.#effects.readStopRequest(source.delegationId, input.actionId);
if (existing) {
if (existing.actionId !== input.actionId) {
Review Comment:
**P3: this branch is unreachable against the real store — and the fixture that exercises it behaves differently from production.** `readWorkHubStopRequest(d, X)` only ever returns the primary record when its `actionId === X`, or the scoped `whq_<[d,X]>` record (which can only hold X's own request) — so `existing.actionId !== input.actionId` can't happen in production. The gate test's fake ignores `actionId` and returns the first request, driving a path that doesn't exist and asserting `retirements === 1`; production actually retries and converges on the same terminal outcome. Deleting the dead branch and re-anchoring the test to the real semantics would keep the fixture honest.
<details>
<summary>中文</summary>
这个分支在生产中不可达：scoped 读只会返回 actionId 相同的 primary 或 X 自己的 scoped 记录，恒等于入参。测试 fake 忽略 actionId 直接返首个请求，走了一条生产不存在的路径。删掉死分支、把断言改回真实语义（幂等重试收敛）即可。
</details>
##########
packages/runtime-host/src/server/workhub-coordination-coordinator.ts:
##########
@@ -415,9 +430,10 @@ export class HostWorkHubCoordinationCoordinator {
input: Parameters<WorkHubActionGateEffects['resolveStop']>[0],
): Promise<WorkHubDelegationStopResolvedMessage> {
const request = input.request;
- const suffix = workHubDestructiveClaimIdentitySuffix(request.stopsDelegationId);
+ const suffix = request.id.slice(4);
return this.#commitCoordinationFact({
- read: () => this.#stores.readWorkHubStopResolution(request.stopsDelegationId),
+ read: () =>
+ this.#stores.readWorkHubStopResolution(request.stopsDelegationId, request.actionId),
build: (existing) => ({
type: 'workhub_coordination',
id: `whz_${suffix}`,
Review Comment:
**P3 — trigger ② (narrow crash window): a terminal resolution written to the action-scoped `whz_<[d,X]>` slot is invisible to delegation-level reads.** `readActiveWorkHubAssignmentsByTarget` and the unscoped `readWorkHubStopResolution` only look at the primary `whz_<d>` — so if X1's request committed and the process died before its resolution, then X2's stop lands on the scoped slot, the primary slot never receives a terminal state and active-assignment listings keep reporting the stopped delegation as live (feeding `isActive` and `#prepareReplacement`'s latest-active check). Consider also writing the terminal outcome to the delegation-level slot — upgrading a primary `not_owned` (non-destructive) is safe — while keeping the scoped slot as the per-action receipt.
<details>
<summary>中文</summary>
写到 `whz_<[d,X]>` 的终态对 delegation 级读取不可见：X1 请求已提交、resolution 未提交时崩溃，随后 X2 的 stop 落到 scoped 槽 → primary `whz_<d>` 永远缺终态，活跃列表仍把已停 delegation 报为活跃。建议终态同时写 delegation 级槽位（把 primary 的非破坏性 `not_owned` 升级为终态是安全的），scoped 槽只留 per-action 回执。
</details>
--
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