Skip to content

fix(workhub): resolve WorkHub delegation linkage on demand - #4699

Merged
Astro-Han merged 1 commit into
mainfrom
fix/4647-bounded-coordination-replay
Sep 5, 2026
Merged

fix(workhub): resolve WorkHub delegation linkage on demand#4699
Astro-Han merged 1 commit into
mainfrom
fix/4647-bounded-coordination-replay

Conversation

@Astro-Han

@Astro-Han Astro-Han commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Summary

Opening or reconnecting WorkHub Coordination replayed the entire append-only Coordination transcript. The renderer used one append-only log for two unrelated jobs: showing recent activity, and rebuilding which delegations are currently live. Only the second job needed the whole history, so every open paged back to sequence zero.

The two jobs are now separated without adding a second authority. The Coordination ledger stays the only durable WorkHub fact. Current linkage is resolved on demand from the target Session: inside one SQLite read transaction, the target's own message_admissions, session_messages and cancelled_message_admissions rows give the Message identity, and the Coordination ledger is then point-queried for the matching assignment and terminal facts. Those are three lifecycle positions of the same Message, not three WorkHub authorities — no new table, no schema version, no migration, no Host-global active map.

The Desktop side keeps only a bounded resident transcript. When the 16KiB bootstrap cuts off the newest record, it performs a single loadAround capped at 512KiB; a revision/generation guard stops a pre-reset result from overwriting newer state.

workhub.coordination.candidates now carries latestDelegationActionId per candidate — the compare-and-swap handle the renderer previously derived from the full replay. It is never model-facing. The whole page resolves in one bounded store read, not one read per candidate. The candidate value is advisory: replacement and stop both re-prove the exact durable linkage inside the Coordination and target admission lanes before writing, and the assignment, target admission and optional supersession still commit in a single SQLite transaction.

Fixes #4647

Breaking change

Runtime Host compatibility epoch 113 → 114. The candidate shape changed, so mismatched Client-Host pairs are rejected at the handshake.

Upgrade

Terminal Coordination records first appear in v0.2.0-dev.12 (2026-09-01); v0.2.0-dev.11 and earlier define only delegation_assigned and delegation_intent and have no stop or supersede capability anywhere in the tree. Their assignments are shape-valid for the new reader, so a workspace last opened by one of those Nightlies reads every past delegation as still linked. This PR covers those workspaces and does not migrate them: the candidate list is advisory, stop and replacement re-prove the exact durable linkage before writing, newest-first ordering keeps an old row from displacing the current one, and a single stop writes whz_ and heals the target.

Verification

  • packages/storage: node --test dist/__tests__/workhub-*.test.js — 11 pass, including a terminal matrix that reddens under each of the four reverse mutations (drop the supersession, replacement-abort, or terminal stop filter, or drop the not_owned exception)
  • packages/runtime-host: node --test dist/__tests__/workhub-coordination-*.test.js dist/__tests__/message-coordinator.test.js dist/__tests__/execution-composition.test.js — 163 pass
  • apps/desktop: node --test dist/main/__tests__/workhub-*.test.js — 120 pass
  • npm run format, npm run lint, workspace builds, ASF header and protocol epoch guards
  • Not run: the full repository suite, and E2E (no renderer behavior is added — the Desktop change is a net deletion).

Review focus

Current-linkage resolution is O(A) in the target Session's own WorkHub history. maxAssignmentsPerTarget caps accepted rows, not the iterator, so it never engages on a target whose assignments are all terminal — every historical row still costs one assignment lookup plus up to three terminal point reads. Measured on a real store, 32 targets, all-terminal history: 100/1000/6000 assignments ≈ 3.3/32.4/193.6ms, synchronous inside the shared read transaction.

This is left as is, deliberately. Activity is decided by exclusion against terminal records whose ids are sha256(delegationId), which SQLite cannot compute, so the anti-join cannot be pushed into the query plan and LIMIT cannot apply after filtering. Bounding the scan instead would drop an older still-active link. Fixing it properly means changing either the terminal-record identity scheme or the stop protocol — stop_work is the only caller that needs the full set, and it needs it only because the proposal names a Session rather than the delegation, unlike replace, which names replacesActionId directly. That asymmetry is being written up separately.

Worth stating for scope: this is residual cost, not a regression. Before this change the renderer replayed the entire Coordination transcript across all targets on every open and reconnect.

AI use

Select exactly one:

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

Tool(s) and scope: Codex authored the initial implementation and tests. Claude Code reviewed the resulting diff for scope, removed changes that did not belong to this fix, and batched the per-candidate linkage read.

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 Astro-Han added the effort/L Under 1000 readable lines label Sep 4, 2026
@Astro-Han
Astro-Han force-pushed the fix/4647-bounded-coordination-replay branch from 5308791 to 240be81 Compare September 4, 2026 01:26

@jackwener jackwener left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I reviewed exact head 240be81dd26c9734b2a928bbf02200a68ca18f56. One P1 and two P2s.

The problem statement is right — one historical ledger serving both "the recent timeline" and "current active delegation linkage" is a real conflation, and separating them is the right move. The live path is also sound: the transcript write and the projection update happen in one SQLite transaction, supersession and replacement-abort behave as the old projector did, and migration rollback and retry are idempotent on their own.

The findings are about what the migration misses, and about a claim in the description that does not hold.

P1 — the migration skips chunked assignments, and the upgraded Host can no longer stop a running delegation

sqlite-session-metadata-schema.ts:1285-1319 backfills by running json_extract() over session_messages.record_json. But records above 64 KiB do not keep their JSON there: sqlite-session-metadata-store.ts:5475-5520 leaves a marker in that column and stores the real payload in the payload/chunk tables. So the backfill sees a marker where it expects a document, and the assignment is silently not projected.

This is reachable with supported input. The protocol allows a 48 KiB userText; a legitimate 40 KiB string with many backslashes escapes to an 82,608-byte delegation_assigned. Before migration both assignments are active. After a real schema-39 migration only the short one is projected, while the chunked ledger record remains complete and readable. Interrupting the transaction and retrying reproduces the same omission.

The consequence is not cosmetic. Feeding the migrated database to the production WorkHubCoordinationActionGate, an explicit Stop against that target is refused at workhub-coordination-action-gate.ts:567-573 with "WorkHub has no active durable delegation to stop on that Session", and no stop claim is written. The delegation is still running; the Host has lost the authority to stop it.

The existing migration test (workhub-message-assignment.test.ts:103-146) uses short inline records only, so it never reaches the chunk tables.

Fix: have the backfill go through the canonical inline+chunk reader rather than json_extract alone — or reassemble payload and chunks equivalently in SQL — and add a regression with a legitimate assignment over 64 KiB that asserts both the projection and the production Stop lookup.

P2 — a single-Session bundle keeps the projection and drops its source, which is the second authority the description rules out

The new table carries only target_session_id (sqlite-session-metadata-schema.ts:1277-1283). The generic bundle filter at session-bundle-policy.ts:204-231 therefore treats its rows as belonging to the target Session and keeps them, while correctly removing the reserved Coordination Session and its messages.

Measured against the production exportSessionBundleState(): the exported database contains only the target Session, yet listActiveWorkHubAssignments() still returns chunked-action. Provisioning a fresh, empty Coordination Session leaves that active projection in place while readWorkHubAssignment('chunked-action') finds no source record.

That is a projection outliving the ledger it is derived from, in a database a user can actually produce — which is precisely the "second execution authority" the summary says this change avoids. The claim is worth correcting or the behaviour is worth changing; either way the two should agree.

Fix: keep this global projection out of a target-only bundle, or rebuild it from whatever Coordination ledger survives inside the bundle, with a bundle-filter regression.

P2 — older history is retained but unreachable from WorkHub

workhub-coordination-port.ts:71-133 no longer calls loadBefore() or loadAround(), and a new test asserts historyLoads === 0 on both open and reset even when hasOlder is true (workhub-session-port.test.ts:437-568). What used to be a 40-turn projector is now whatever fits the 16 KiB bootstrap (session-transcript.ts:32). The records are not deleted, but WorkHub has no paging entry point and the reserved Coordination Session is excluded from the ordinary catalog, so there is no second route to them in the UI.

The description does say the timeline never pages backward automatically, so this is disclosed rather than hidden. Still, a performance fix does not have to remove access: a bounded "load older", or a page sized to fill the previous 40-turn cap, would keep the fix and the reach.

Evidence boundary: the migration, bundle and gate results above come from probes against this head, each reproduced twice and recorded with digests. build:test passes, Runtime Host is 1674 pass / 12 skip, Storage is 1098 pass / 8 skip with the one failure being a pre-existing child-process test that treats Node's experimental-SQLite warning on stderr as a failure. Hosted test is terminal green on this head; the PR is still a draft.

简体中文

我审的是 240be81dd26c9734b2a928bbf02200a68ca18f56一条 P1、两条 P2。

问题陈述是对的——让一份历史账本同时承担「最近的时间线」和「当前活跃委派链接」两种读,确实是一种职责混同,把它们分开是正确的方向。活跃路径也是稳的:transcript 写入与投影更新发生在同一个 SQLite 事务里;supersession 与替换中止的行为和旧 projector 一致;迁移的回滚与重试其自身是幂等的。

下面这些发现,针对的是迁移漏掉了什么,以及描述里一个不成立的主张

P1:迁移跳过分块存储的 assignment,升级后的 Host 再也无法停止一个仍在运行的委派

sqlite-session-metadata-schema.ts:1285-1319 的回填是对 session_messages.record_jsonjson_extract()。但超过 64 KiB 的记录并不把 JSON 放在那一列:sqlite-session-metadata-store.ts:5475-5520 只在该列留一个 marker,真正的载荷存在 payload/chunk 表里。于是回填在期待一份文档的地方看到的是 marker,该 assignment 被静默地漏掉了

这一点用受支持的输入就能触及。 协议允许 48 KiB 的 userText;一个合法的、含大量反斜杠的 40 KiB 字符串,转义后会生成 82,608 字节的 delegation_assigned。迁移前两条 assignment 都是 active;经过一次真实的 schema-39 迁移之后,只有短的那条被投影,而分块的账本记录本身仍然完整可读。中断事务再重试,会稳定地重现同一处遗漏。

后果不是画面问题。 把迁移后的数据库喂给生产的 WorkHubCoordinationActionGate,针对该 target 的明确 Stop 会在 workhub-coordination-action-gate.ts:567-573 被拒绝,理由是*「WorkHub has no active durable delegation to stop on that Session」*,而且不会写入 stop claim。那个委派仍在运行,而 Host 已经失去了停止它的权限。

现有的迁移测试(workhub-message-assignment.test.ts:103-146)只用短的 inline 记录,永远走不到 chunk 表

修法:让回填走 inline+chunk 的规范读取路径,而不是只靠 json_extract——或者在 SQL 里等价地把 payload 与 chunks 拼回来——并补一条回归用例:放一条超过 64 KiB 的合法 assignment,同时断言投影结果和生产的 Stop 查找

P2:单 Session 的 bundle 保留了投影却丢掉了它的来源,这正是描述所排除的那个「第二权威」

新表只带 target_session_id(sqlite-session-metadata-schema.ts:1277-1283)。于是 session-bundle-policy.ts:204-231 的通用 bundle 过滤器把它的行当作属于 target Session 而保留下来,同时正确地移除了保留的 Coordination Session 及其消息。

针对生产 exportSessionBundleState() 的实测:导出的数据库里只有 target Session,但 listActiveWorkHubAssignments() 仍然返回 chunked-action。再 provision 一个全新的空 Coordination Session,那条 active 投影依然在,而 readWorkHubAssignment('chunked-action') 找不到任何来源记录。

这就是一份投影活得比它所派生的账本更久,而且发生在用户真的能生产出来的数据库里——恰恰是摘要中声称这次改动所避免的那个「第二执行权威」。要么修正这个主张,要么改变这个行为;两者总得对上。

P2:更早的历史仍然保留,但在 WorkHub 里够不着

workhub-coordination-port.ts:71-133 不再调用 loadBefore()loadAround(),而且新增的测试明确断言:即使 hasOlder 为真,open 与 reset 两次的 historyLoads === 0(workhub-session-port.test.ts:437-568)。原先那个 40 turn 的 projector,现在变成了「16 KiB bootstrap 装得下多少就是多少」(session-transcript.ts:32)。记录没有被删除,但 WorkHub 没有分页入口,而保留的 Coordination Session 又被排除在普通目录之外,所以 UI 里没有第二条通往它们的路

描述里确实写明了时间线不再自动向后翻页,所以这是已披露而非隐瞒。但话说回来,一个性能修复并不需要连访问一起取消:一个有界的「加载更早」,或者一页恰好填满原先 40 turn 上限的分页,可以既保住修复又保住可达性。

证据边界:上面关于迁移、bundle 和 gate 的结果,都来自针对这个 head 的探针,各自复现两次并留有摘要。build:test 通过,Runtime Host 1674 通过 / 12 跳过,Storage 1098 通过 / 8 跳过——其中唯一的失败是一个既有的子进程测试,它把 Node 的 experimental-SQLite 警告输出到 stderr 当成了失败。这个 head 上托管 test 已终态通过;PR 仍是草稿。


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

@Astro-Han
Astro-Han marked this pull request as ready for review September 4, 2026 16:41
@Astro-Han
Astro-Han marked this pull request as draft September 4, 2026 17:31
@Astro-Han
Astro-Han force-pushed the fix/4647-bounded-coordination-replay branch from 240be81 to 591918e Compare September 4, 2026 17:52
@Astro-Han

Copy link
Copy Markdown
Contributor Author

Thanks — I reproduced the chunked-record migration failure and the bundle orphan you identified.

Rather than patching those two cases into the persisted projection, I removed that projection entirely in 591918e9df.

The Coordination ledger is now the only durable authority. Runtime Host lazily rebuilds one disposable in-memory active-delegation index through the canonical paged record reader, at a fixed watermark, once per Host lifetime. Fresh Host-owned writes update that index after their ledger commit. Desktop continues to receive active linkage through the existing candidates snapshot and never replays older history on open or reconnect.

This changes the findings as follows:

  1. The chunked-assignment migration failure no longer exists because there is no SQL backfill or schema-39 migration. The new regression places a supported >64 KiB chunked assignment after 256 historical records and proves that two fresh Hosts each reconstruct both pages exactly once, while repeated snapshots perform no additional scans.

  2. The bundle orphan no longer exists because workhub_active_delegations, its bundle-visible rows, its migration, and its Storage API have all been removed. A projection cannot outlive its ledger because no active projection is persisted.

  3. I did not add automatic or manual backward paging to WorkHub. Before this change the renderer loaded the complete transcript but still projected only the final 40 turns, with no user-facing older-history entry point. The full read existed to recover active linkage, not to provide historical navigation. The new path preserves that bounded presentation contract while removing the hidden replay. A user-facing history browser would be a separate product change.

Local verification on the new head:

  • Storage: 1121 passed, 8 skipped
  • Runtime Host: 1682 passed, 12 skipped
  • Desktop: 2160 passed
  • Renderer architecture: 98 passed
  • format and lint passed

The PR remains draft and the new CI run has been triggered.

@Astro-Han
Astro-Han marked this pull request as ready for review September 4, 2026 18:31

@jackwener jackwener left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Approving at exact head 591918e9df082a2f2878097229c0cee97f9a88d0. The [P1] I raised on 240be81d is closed — it did not reproduce. One P2 remains, found by a probe that was looking for something else. Both checks pass.

The original finding is resolved

The earlier review reported that a delegation left running across an upgrade could not be stopped. Probed again on this head with pre-upgrade state driving the new code, it does not reproduce. I am closing it rather than carrying it forward.

[P2] A concurrent stop can land in the window between lease release and index publication

The assignment commit works: the queue is flushed. But the lease is released before the index is published, and a stop arriving inside that window reads zero activity, returns a conflict, and asks the caller to retry. Work that is already committed or already running cannot be stopped during that interval.

It is recoverable — the caller is told to retry, and a retry after the window closes succeeds — which is why this is not blocking. What makes it worth fixing anyway is that the failure presents as "there is nothing to stop" rather than "try again in a moment", so a caller has no way to distinguish a genuine no-op from a lost stop.

Two directions, either sufficient: publish the index before releasing the lease, so the two are never both absent; or have stop read the commit-synchronised authoritative state rather than the index. A barrier regression around the window would keep it closed.

What else was checked

The tightened boundaries hold on their own terms: the old out-of-range replay path is no longer emitted, the paging watermark rejects explicitly rather than silently clamping, assignment happens in one transaction, the restart conflict path passes, and the protocol round-trip is aligned.

projectWorkHubActiveDelegations and its tests are gone, replaced by bounding the visible timeline independently of old delegation linkage — bounds the visible timeline independently of old delegation linkage. That is the right shape for the original problem: the timeline no longer inherits a bound from linkage that may not survive an upgrade, which is what made the earlier failure possible.

This is a fix, so the merge decision remains a human's — and with a P2 open on the stop path, worth a deliberate one.

Reviewed on this head by four independent seats across two teams. The concurrency finding above is from @Luna-Deep-Qronos.

简体中文

591918e9df082a2f2878097229c0cee97f9a88d0 上批准。我在 240be81d 上提的那条 [P1] 已关闭——它没有复现。 另有一条 P2,来自一个本来在找别的东西的探针。两项检查均通过。

原发现已解决

先前的评审报告过:一个跨升级仍在运行的委派无法被停止。在这个 head 上用升级前的状态驱动新代码重新探测,它不再复现。 我把它关闭,而不是继续挂着。

[P2] 并发的停止可能落在「释放租约」与「发布索引」之间的窗口里

分配提交这一步是好的:队列已被刷新。但租约在索引发布之前就被释放了,而一个在这个窗口内到达的停止操作会读到零活动、返回冲突、并要求调用方重试。在那段间隔里,已经提交或正在运行的工作停不掉。

它是可恢复的——调用方被告知重试,而窗口关闭后的重试会成功——所以它不阻塞。 之所以仍然值得修,是因为这个失败表现为「没有可停止的东西」,而不是「请稍后再试」,于是调用方无从区分「真的没有可停的」与「一次丢失的停止」。

两个方向,任一即可:在释放租约之前发布索引,让两者不会同时缺席;或者让停止去读与提交同步的权威状态,而不是读索引。围绕这个窗口加一条 barrier 回归可以把它钉住。

其余已核

那些被收紧的边界各自成立:旧的越界回放路径不再发出,分页水位是明确拒绝而不是静默钳制,分配发生在同一个事务里,重启冲突路径通过,协议往返对齐。

projectWorkHubActiveDelegations 及其测试已被移除,取而代之的是让可见时间线的边界独立于旧的委派链接——即 bounds the visible timeline independently of old delegation linkage对原问题而言这是正确的形状:时间线不再从「可能撑不过一次升级的链接」那里继承边界,而那正是先前那次失败得以发生的原因。

这是一个 fix,所以合并与否仍由人决定——而在 stop 路径上还开着一条 P2 的情况下,这个决定值得是审慎的。

本 head 由跨两个团队的四个独立席位评审。上述并发问题由 @Luna-Deep-Qronos 发现。


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

@Astro-Han

Copy link
Copy Markdown
Contributor Author

Fixed the automated P2 on exact head a5e1832283.

The root cause was broader than callback timing: the durable assignment commit, disposable Host-index publication, and active-link readers did not share one synchronization boundary. Moving publication earlier alone would still leave a stop able to read the index while the writer held the lease.

The assignment path no longer returns committedAssignment for a wrapper to publish after runMany() releases its lease. Production assignment now publishes the coordinator-owned disposable index inside the existing Coordination + target admission. Stop source resolution and candidate snapshots read that index through the same Coordination admission. The Coordination ledger remains the only durable authority; this adds no table, polling loop, lock, or second index.

Regression evidence:

  • Added a deterministic SQLite + real SessionAdmissionGate barrier test. Before the fix it failed with WorkHub has no active durable delegation to stop on that Session; now the concurrent stop succeeds.
  • Extended the production composition test through assignment -> active snapshot -> stop, so the real wiring is covered.
  • Full @maka/runtime-host suite: 1683 passed, 12 skipped, 0 failed.
  • Final focused suites: 42 passed, 0 failed.
  • npm run format and npm run lint passed.

@Astro-Han
Astro-Han force-pushed the fix/4647-bounded-coordination-replay branch from a5e1832 to 2edf744 Compare September 5, 2026 10:50
@Astro-Han
Astro-Han marked this pull request as draft September 5, 2026 10:50
@Astro-Han
Astro-Han force-pushed the fix/4647-bounded-coordination-replay branch from 2edf744 to 4a035a8 Compare September 5, 2026 14:36
@Astro-Han Astro-Han changed the title fix(workhub): bound Coordination history replay fix(workhub): resolve WorkHub delegation linkage on demand Sep 5, 2026
@Astro-Han
Astro-Han force-pushed the fix/4647-bounded-coordination-replay branch from 4a035a8 to 06472b7 Compare September 5, 2026 14:46
@Astro-Han
Astro-Han marked this pull request as ready for review September 5, 2026 15:08
@github-actions github-actions Bot added effort/XL Under 2500 readable lines and removed effort/L Under 1000 readable lines labels Sep 5, 2026

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Reviewed current head 06472b7d9adb5fdf831f7dc99f064dd552cb9b6f (OPEN, MERGEABLE). Technical GO — no P0–P3 found that would change the merge decision. Checks green on this head: test and label both success at final state.

Scope

Focused suites all green: storage 9/9, coordinator 21/21, action-gate 41/41, protocol 5/5, message-coordinator 74/74, desktop session-port 16/16, controller 72/72. A bounded simplify audit found no safely-removable complexity introduced by this PR. Old-head conclusions not carried over.

What I could not judge

Local execution-composition tests and the full desktop build are blocked by missing dependencies unrelated to this change — not counted as passing.


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.

简体中文

本条结论全部来自 @未开智选手 的审查。我自己没有读这份 diff;我核的是当前 head 有没有漂移、以及 exact-head 的门禁状态。当前 head 是 06472b7,未关闭。技术上无阻断问题,聚焦测试全绿。

@jackwener jackwener left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Approving at exact head 06472b7d. No P0 or P1 across four independent lanes — upgrade-state compatibility, terminal-set completeness, ownership and cost of the removal, and concurrency. Two non-overlapping [P2]s and one disclosure item are below.

Worth saying first: replacing the persisted projection and the disposable Host index with an on-demand read is the right shape. The earlier lease-release / index-publication window is not patched here — it is gone by construction, because assignment and target admission commit in one transaction and the lookup resolves assignment and terminal state inside a single SQLite snapshot. Reading an assignment as active while its terminal record is not yet committed is a normal snapshot state, not a stale-duplicate hole.

The terminal set is unchanged from both prior representations

Activity is decided by exclusion: an assignment is active unless delegation_superseded, delegation_replacement_aborted, or a delegation_stop_resolved whose outcome is not not_owned exists. A natural completion or failure writes none of these, so a completed delegation still reads as linked.

That looks alarming in isolation, so it was checked against the two representations this PR removes rather than argued from the current head:

  • the durable workhub_active_delegations projection at 240be81d DELETEs on exactly those three terminals; ordinary completed/failed execution feedback never reaches that projector;
  • the Host-side #activeAssignments map at 591918e9 retires entries on the same three Coordination terminals; a normal completion changes renderer feedback state only.

So "completion does not end linkage" is the existing contract, not a fourth terminal this PR forgot. A nine-assignment matrix on a real store returns exactly the four expected active rows (plain active, replacement requested, stop requested, not_owned) out of nine.

[P2] Two of the three terminal filters have no test that can fail

The production filtering is correct, but its guards are uneven. With the focused Storage and Runtime Host suites:

  • removing the delegation_replacement_aborted filter → 93/93 still green;
  • removing the terminal delegation_stop_resolved filter → 93/93 still green.

Supersession and the not_owned exception both already have an owner that reddens. These two branches can regress silently — a retired delegation would reappear as an active link with nothing reporting it. The minimal fix is to land the terminal matrix as a case in packages/storage/src/__tests__/workhub-message-assignment.test.ts; it discriminates, since each of the four reverse mutations reddens it.

[P2] The candidate hot path rescans terminal history

#candidates() calls the reader once per page for up to 32 target Sessions with maxAssignmentsPerTarget = 1. Batching per page rather than per candidate is already the right call, and the comment there says so.

The limit does not bound the work. It caps accepted rows, not the SQL iterator — and in the shape that matters it never engages at all: when a target's assignments are all terminal and none is active, accepted stays 0, so every historical row still performs its assignment lookup plus up to three terminal point reads.

Measured on a real store, 32 targets, all-terminal history:

assignments median p90
100 3.311 ms 4.010 ms
1,000 32.399 ms 33.197 ms
6,000 193.587 ms 195.656 ms

This runs synchronously inside a shared node:sqlite read transaction, so it blocks the Host event loop, and the history is append-only with no retention or SQL hard limit. The reachable case is specific rather than theoretical: a target Session that has accumulated a long run of finished delegations and has no active one pays the full scan every time the candidate list opens.

The direction that fixes it is pushing the terminal anti-join and newest-live-per-target selection into an indexed SQL plan, with a query-budget regression that grows terminal history. One caveat worth writing down: do not simply LIMIT before filtering terminals — that would drop older still-active links.

The other three call sites are single-target (:176 is advisory and re-proves before writing, :239 and :311 re-read under the same admissions), so this is the only hot path.

Disclosure: pre-terminal Nightly workspaces

Terminal records first appear in v0.2.0-dev.12 (2026-09-01). Every earlier tag writes none: v0.2.0-dev.11 and older define only delegation_assigned and delegation_intent, with no stop or supersede capability anywhere in the tree. Their assignments are shape-valid for the new reader — same 52-character ids, same suffix derivation — so a workspace last opened by an Aug-31 Nightly reads every past delegation as permanently active.

This does not deserve a grade on its own: the candidate list is advisory, stop and replacement re-prove before writing, ordering keeps an old row from displacing the current one, and a single stop writes whz_ and heals it. But the CHANGELOG already treats v0.2.0-dev workspaces as upgrade subjects, so if the PR intends to cover those Nightlies, one line saying so belongs in the description.

Gate state

Exact head 06472b7d, MERGEABLE, test and label green on this head, merge tree clean, no schema or migration change. INDEXED BY session_messages_by_identity is safe: that index has existed since schema migration 20 and the current version is 38.

One thing a human should notice: the existing APPROVED on this PR is bound to 591918e9, not to this head. This head carries only automated comments. Neither this review nor any other automated one is an independent human review, and the merge decision remains a human's.

简体中文

在 exact head 06472b7d 上批准。四条独立车道均无 P0/P1 —— 升级态兼容性、终态集合完备性、移除的所有权与代价、并发。下面是两条互不重叠的 [P2] 与一项披露。

先说要紧的:把持久投影与一次性 Host 索引换成按需读取,方向是对的。旧的 lease 释放 / 索引发布窗口在这里不是被打了补丁,而是按构造消失了 —— assignment 与 target admission 在同一事务里提交,查找在同一个 SQLite 快照内解析 assignment 与终态。「assignment 已提交、终态尚未提交时读到活跃」是正常的快照状态,不是陈旧重复操作的空洞。

终态集合与此前两种表示完全一致

活跃性用排除法判定:除非存在 delegation_supersededdelegation_replacement_aborted、或 outcome 非 not_owneddelegation_stop_resolved,否则 assignment 即为活跃。自然完成或失败不写这三者中的任何一条,因此一次已完成的委派仍读作「已链接」。

这句话孤立地看令人不安,所以它不是从当前 head 推理出来的,而是拿本 PR 删掉的那两种表示去核的:

  • 240be81d 的持久投影 workhub_active_delegations 只在上述三类终态时 DELETE;普通的 completed/failed 执行反馈根本不进入该 projector;
  • 591918e9 的 Host 内存 #activeAssignments 同样只按这三类 Coordination 终态退役条目;正常完成只改变 renderer 的反馈状态。

所以「完成不结束链接」是既有合同,而不是本 PR 漏掉的第四种终态。 真实 store 上九条 assignment 的矩阵,精确返回预期的四条活跃行(普通活跃、已请求取代、已请求停止、not_owned)。

[P2] 三条终态过滤里有两条没有任何能失败的测试

生产过滤是正确的,但守卫并不均匀。在聚焦的 Storage 与 Runtime Host 套件下:

  • 去掉 delegation_replacement_aborted 过滤 → 仍 93/93 绿;
  • 去掉终态 delegation_stop_resolved 过滤 → 仍 93/93 绿

supersession 与 not_owned 例外都已有能打红的 owner。这两条分支可以静默回归 —— 一次已退役的委派会重新显示为活跃链接,而没有任何东西会报警。 最小补法是把那份终态矩阵作为一个用例落进 packages/storage/src/__tests__/workhub-message-assignment.test.ts;它具备区分力,四种反向变异都会把它打红。

[P2] 候选热路径会重扫终态历史

#candidates() 每页调用一次读取器,最多 32 个目标 Session,maxAssignmentsPerTarget = 1按页批读而不是按候选逐个查,这个决定本身是对的,那里的注释也写明了。

但这个上限并不约束工作量。它限制的是已接受的行数,不是 SQL 迭代器 —— 而在真正要紧的形状里它根本不会生效:当某个目标的 assignment 全部已终态、没有一条活跃时,accepted 恒为 0,于是每一条历史行仍要做一次 assignment 查找加最多三次终态点查。

真实 store、32 targets、历史全终态的实测:

assignment 数 中位数 p90
100 3.311 ms 4.010 ms
1,000 32.399 ms 33.197 ms
6,000 193.587 ms 195.656 ms

它在共享的 node:sqlite 读事务里同步执行,会阻塞 Host event loop;而历史是 append-only 的,没有 retention 也没有 SQL 硬上限可及的情形是具体的而非理论的:某个目标 Session 积累了长长一串已结束的委派、且当前没有活跃那条,则每次打开候选列表都要付全量扫描。

修的方向是把终态 anti-join 与 newest-live-per-target 的选择下推到有索引的 SQL 计划,并补一条随终态历史增长的查询预算回归。有一点值得写下来:不要简单地在过滤终态之前 LIMIT —— 那会丢掉更旧但仍然活跃的链接。

另外三处调用都是单 target(:176 是 advisory 且写前重复证明,:239:311 在同一 admission 下重读),所以热路径只有这一处

披露:终态之前的 Nightly 工作区

终态记录最早出现在 v0.2.0-dev.12(2026-09-01)。更早的每一个 tag 都不写终态:v0.2.0-dev.11 及更早只定义 delegation_assigneddelegation_intent,全树没有停止或取代能力。它们的 assignment 对新读取器而言形状完全合法 —— 同样 52 字符 id、同样的后缀派生 —— 因此一个最后由 8/31 Nightly 打开过的工作区,会把过去每一次委派都读作永久活跃。

这本身不值得单独定级:候选列表是 advisory,停止与取代在写前重复证明,排序使旧行挤不掉当前那条,而一次停止写入 whz_ 即可自愈。但 CHANGELOG 已把 v0.2.0-dev 工作区当作升级对象,所以如果本 PR 意在覆盖那些 Nightly,描述里应当有一句话说明。

门禁状态

exact head 06472b7d,MERGEABLE,本 head 的 testlabel 均绿,merge tree clean,无 schema 或 migration 变更。INDEXED BY session_messages_by_identity 是安全的:该索引自 schema 迁移 20 起就存在,当前版本为 38。

有一点需要人类注意:本 PR 现有的 APPROVED 绑定的是 591918e9,不是这个 head。 这个 head 上只有自动化评论。本条评审与任何其他自动化评审都不是独立的人类审查,合并与否仍由人决定。


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

@Astro-Han
Astro-Han force-pushed the fix/4647-bounded-coordination-replay branch from 06472b7 to 7c01158 Compare September 5, 2026 18:03
@Astro-Han

Copy link
Copy Markdown
Contributor Author

Addressed on head 7c011585.

[P2] Two terminal filters had no failing test — fixed. I reproduced both mutations before writing anything: dropping the delegation_replacement_aborted filter left storage 10/10 and coordinator 67/67 green, and dropping the terminal delegation_stop_resolved filter did the same. packages/storage/src/__tests__/workhub-message-assignment.test.ts now carries one terminal matrix — five assignments on one target, one per terminal shape plus the not_owned exception, asserting the two survivors in newest-first order. Verified discriminating: each of the two mutations above now reddens it.

[P2] Candidate hot path — not fixed, and the reason is in the description now. The finding is right, including that the cap counts accepted rows rather than the iterator. I did not patch it, because every bounded variant is wrong and every correct variant is out of scope for this PR:

  • A scan budget instead of an accept budget drops an older still-active link, which is the failure your own caveat names.
  • Pushing the anti-join into SQL needs the terminal id to be derivable in the query. It is sha256(delegationId), so it is not.

What is left is changing the terminal-record identity scheme, or changing who names the delegation. On the second: stop_work is the only caller that needs the full set, and only because its proposal carries a targetSessionId and the Gate resolves the link itself. replace in the same protocol carries replacesActionId and the Gate validates it — same trust boundary, opposite answer. The stated reason ("a client cannot prove which link is live") no longer holds now that candidates carry latestDelegationActionId; what needs protecting is authorization, and that is already held by confirmation: { kind: 'user_stop' } outside strategy output. If stop named its delegation, the active set, the competitor-retirement loop and listActiveAssignments all disappear, and this hot path with them.

That is an abstraction change, not a fix to this diff, so it is being written up separately. The Review focus section now records the measurement, why bounding is unsafe, and where the real correction lives. Also worth stating: this is residual cost, not a regression — before this change the renderer replayed the entire Coordination transcript across all targets on every open and reconnect.

Disclosure — pre-terminal Nightly workspaces. Confirmed independently: v0.2.0-dev.11.20260831 defines none of the three terminal kinds. The PR does cover those workspaces without migrating them, and the description now has an Upgrade section saying so and why it is safe.

Head 7c011585: storage 11 pass, runtime-host 163 pass, desktop 120 pass, format, lint, ASF header and epoch guard all green.

Generated-by: Codex
Generated-by: Claude Code
@Astro-Han
Astro-Han force-pushed the fix/4647-bounded-coordination-replay branch from 7c01158 to 04825ce Compare September 5, 2026 18:10
@Astro-Han
Astro-Han merged commit eca7778 into main Sep 5, 2026
1 check passed
@Astro-Han
Astro-Han deleted the fix/4647-bounded-coordination-replay branch September 5, 2026 18:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/XL Under 2500 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

WorkHub Coordination transcript is replayed in full on every open and every reconnect

2 participants