feat(workhub): add direct stop coordination - #4439
Conversation
M4n5ter
left a comment
There was a problem hiding this comment.
English
I recommend addressing three correctness issues before merging. The review was performed against exact head fe7e267e.
-
P1 — one
actionIdcan stop two delegations. Stop claims are looked up and persisted bystopsDelegationId, without a durable global owner for the stop action ID. If the first request is persisted but retirement returnsrecovering, the rejection releases the in-memory action slot; the sameactionIdcan then claim and retire another delegation. I reproduced this on the exact head without a restart. This is also reachable through the normal UI lease behavior, which preserves the action ID across reload while allowing edited text. Please establish one durableactionId -> exact operation/fingerprintclaim and reject cross-delegation or cross-disposition reuse before any effect. (gate, durable request) -
P2 — target Session removal can permanently lose the stop outcome.
delegation_stop_requestedis committed before retirement and resolution. After a pending cancellation succeeds, a crash beforedelegation_stop_resolvedfollowed by target Session removal can cascade-delete the cancellation claim/Turn proof. Retry then finds no receipt, steering proof, tombstone, or admission and remainsrecoveringforever. Session removal must not erase proof needed by an unresolved stop claim; coordinate removal with the claim or retain the proof outside the target Session lifetime. (effect/resolution seam, fallback torecovering) -
P2 — replacement is persisted as direct-stop provenance. Replacement also calls
retireDelegation(source, replacement.actionId), but every owning-root retirement now goes throughstopOwnedWorkHubRoot(), which hardcodessource: 'workhub_direct_stop'. A normal route correction is therefore permanently recorded asworkhub.direct_stop.<replacementAction>despite having no direct-stop request oruser_stopconfirmation. Please separate the cancellation claim identity from the Stop operation/provenance; only direct stop should emitworkhub_direct_stop. (replacement path, provenance write)
Two non-blocking simplifications are worth considering in this PR:
- Make
cancellationClaimIdmandatory incancelMessageIfPending()and remove its unused production no-claim mode. - Represent stop parsing as a single
none | unusable | nameddiscriminated union and parse once, instead of maintainingcue + imperative + target?and matching twice.
Validation: the relevant Core, Storage, Runtime, Runtime Host, and Desktop builds passed, as did 448 targeted tests. The P1 path was independently reproduced. CI is green, but these boundary cases are not covered. The PR currently conflicts with main because both branches independently use compatibility epoch 87; after rebasing, the combined protocol epoch should be advanced rather than resolving the conflict by retaining 87.
中文
建议合并前修复以下三个 correctness 问题。本次审查基于精确 head fe7e267e。
-
P1 — 一个
actionId可以停止两个 delegation。 Stop claim 按stopsDelegationId查询和持久化,没有 durable 的全局 stop action ID owner。第一个 request 已持久化、但 retirement 返回recovering时,失败会释放内存 action slot;同一个actionId随即可以 claim 并退役另一条 delegation。我已在精确 head 上复现,而且无需重启。正常 UI lease 也会在 reload 后保留 action ID、同时允许修改文本,因此这是实际可达路径。请建立统一的 durableactionId -> exact operation/fingerprintclaim,并在任何 effect 前拒绝跨 delegation 或 disposition 的复用。(gate,durable request) -
P2 — 删除 target Session 会永久丢失 stop outcome。
delegation_stop_requested在 retirement 和 resolution 之前提交。Pending cancellation 已成功后,如果 Host 在写入delegation_stop_resolved前崩溃,随后 target Session 被删除,cancellation claim/Turn proof 会被级联清除。重试找不到 receipt、steering proof、tombstone 或 admission,只会永久停在recovering。Session removal 不得清除 unresolved stop claim 所需的证明;应协调 removal 与 claim,或将证明保存在 target Session 生命周期之外。(effect/resolution seam,fallback torecovering) -
P2 — replacement 被持久化成 direct-stop provenance。 Replacement 同样调用
retireDelegation(source, replacement.actionId),但 owning-root retirement 现在都会经过stopOwnedWorkHubRoot(),并硬编码source: 'workhub_direct_stop'。因此普通 route correction 会被永久记录为workhub.direct_stop.<replacementAction>,尽管不存在 direct-stop request 或user_stopconfirmation。请分离 cancellation claim identity 与 Stop operation/provenance;只有 direct stop 可以写入workhub_direct_stop。(replacement path,provenance write)
另有两项值得在本 PR 中考虑的非阻塞简化:
- 将
cancelMessageIfPending()的cancellationClaimId改为必填,删除生产环境已无调用方的 no-claim 模式。 - 将 stop parser 收敛成单一的
none | unusable | named判别联合并只解析一次,删除cue + imperative + target?组合及重复匹配。
验证结果:相关 Core、Storage、Runtime、Runtime Host 和 Desktop 构建通过,448 项定向测试通过;P1 路径已独立复现。CI 虽然全绿,但没有覆盖这些边界。PR 当前还与 main 冲突,因为两个分支分别使用了 compatibility epoch 87;rebase 后应推进合并协议的 epoch,而不是保留 87 来解决冲突。
Astro-Han
left a comment
There was a problem hiding this comment.
Reviewed exact head fe7e267eb3cd9a4053666557bf9583488d154011 (+2760/-152, 42 files; production ≈ +1206, tests ≈ +1512, docs +42). heavy, plan, test, label, windows_recovery all green; MERGEABLE; no prior review. Read-only against source. I executed the two stop regexes from this head to produce the table below; everything else I verified by reading. I did not run a live session.
First, the part I have no findings on. The durable claim → effect → resolution ordering, the action-derived abort source that lets a replay distinguish WorkHub's own delivery from a concurrent manual Stop, the runMany admission over the Coordination Session plus every active target lane, and the fail-closed re-proof inside that lease are correct as far as I can follow them. I traced the crash seam in #stop: a crash between retireDelegation and resolveStop replays to the same outcome on both the pending-cancel and the owning-root branch, which is exactly what the provenance binding buys. That is the dangerous half of this change and it is well built.
My findings are all on the other side — how ordinary text becomes a stop. None of them are about the destructive machinery being too complex; they are about a ten-line function in the renderer.
P1 — the stop route is one-way, and its clarification carries no options
resolveStop commits to the stop route on intent.stop.cue alone. Whether the named target resolves to an active delegation only selects between target and clarification — it never withdraws the routing decision. So a failed reference is reported to the user as a problem with their phrasing, which is the one thing that was fine.
flowchart TD
U["<b>Stop using the deprecated API</b><br/>an ordinary work request"]
G{"resolveStop diverts on<br/>intent.stop.cue alone"}
A["normal routing<br/><i>where this input belongs</i>"]
B["stop route<br/>target = 'using the deprecated API'"]
C["no active delegation<br/>has that name"]
D["clarification: 'Name the work explicitly'<br/>with an empty options list"]
U --> G
G -->|"not taken"| A
G -->|"taken: the sentence starts with a stop verb"| B
B --> C
C --> D
D -. "user restates it, identically" .-> U
D -. "missing: withdraw the route" .-> A
classDef dead fill:#FCEBEB,stroke:#A32D2D,color:#501313
classDef hot fill:#FAECE7,stroke:#993C1D,color:#4A1B0C
class D dead
class B,C hot
Two things have to be true at once for the loop to close, and both are new here.
It is one-way. Every other branch in workhub-route-policy.ts that fails to resolve a reference either falls through to the next hypothesis or degrades into a clarification that offers alternatives — resolve does this three times (affirmedCorrections, alternatives, related plus the rest). resolveStop is the first branch in this file that cannot be left, and control returns from createWorkHubController before coordination.candidates() is ever reached.
The clarification has nothing in it. The convention in this file is that a clarification carries the choices it is asking about. The one exception is ambiguous_command, and that exception is principled: it is about the sentence being unparseable, so there is nothing to list. stop_target_required copies that shape but is about a target, so it inherits emptiness exactly where the answer set is what the user needs — either "these are the delegations you can stop" or "none of them."
The reachable surface is wider than the stop feature. DIRECT_STOP_REQUEST captures everything after the leading verb as the target, so any work request beginning with stop/cancel/terminate/halt or 停止/取消/终止/中止 parses as a named direct stop. Running the two regexes and UNSAFE_STOP_TARGET from this head:
| input | parsed stop target |
|---|---|
Stop using the deprecated API in the parser |
using the deprecated API in the parser |
Cancel the pending invoices feature |
pending invoices feature |
Terminate the SSH connection when idle |
SSH connection when idle |
Halt the retry loop when the queue drains |
retry loop when the queue drains |
停止使用废弃的 API |
使用废弃的 API |
取消订单超时的重试逻辑 |
订单超时的重试逻辑 |
Each is imperative: true, matches no Session, and lands on the empty clarification. The copy then tells the user to name the work explicitly — which they did — so restating it reproduces the identical response. Nothing routes and nothing is created. The escape (We should stop using the deprecated API) exists only because the regex is start-anchored, which no user can be expected to discover.
Your own negative-case table shows the gap: it covers pronouns, pause/wait, questions, negation, and malformed literals, and Pause Payments correctly stays out of the stop branch and routes normally. There is no case where a stop-shaped sentence is ordinary work.
Smallest fix, at the same site: when a stop parses but names nothing stoppable, return not_requested and let normal routing have it. Keep clarification for cue && !imperative (a stop speech act with an unsafe target) and for a genuinely ambiguous matching.length > 1. Regression worth pinning: Stop using the deprecated API routes as ordinary work while Stop Payments still stops the Payments delegation.
P2 — after a not_owned outcome, that delegation can never be stopped again
not_owned deliberately writes no terminal state: activeWorkHubAssignments filters only outcome !== 'not_owned', and the controller keeps the action active with if (admitted.outcome !== 'not_owned') removeActiveAction(...). So the link stays active and the Session stays stoppable in the UI, and the user — who was just told "Open the Session to stop it" — will reasonably try again.
The second attempt carries a new requestId. In the gate, readStopRequest finds the existing claim and assertStopReplay rejects on its first comparison, request.actionId !== input.actionId, raising action_conflict — permanently, for every future attempt.
Neither test covers this. The controller test named "retryable" stubs act to return not_owned unconditionally, so it proves the renderer re-submits, not that the gate accepts. The gate test replays with the same actionId, which is the crash-replay case. The domain-language entry says a not_owned resolution releases the exclusion so a later route correction can proceed — it does not release it for a second stop, and I think that is the gap rather than the intent.
P2 — the same dead end for a real stop whose target is not exactly one active delegation
stoppable keeps only Sessions with exactly one active delegation, so naming a Session with zero (already finished, or never delegated by WorkHub) or with two produces the same empty clarification. Host-side the proof is consistent and fails closed, so nothing unsafe happens; the user simply cannot learn why, and repeating the name cannot help. The P1 fix covers this if the copy also distinguishes "WorkHub is not running work called X" from "X has more than one active delegation."
P3 — the stop authorization proof is written twice
WorkHubCoordinationActionGate.act()'s stop_work branch and HostWorkHubCoordinationCoordinator.#prepareStop's beforeAppend both call readWorkHubRequestIntent(userText), build sessionNameById from listHeaders(), filter active assignments through workHubStopTargetsSession, require exactly one that is the named source, and check supersession and replacement. Only the second runs inside the admission lease, so only the second is authoritative; the first is an early rejection whose policy can drift into something different without a test noticing, since the two raise different action_conflict messages for the same condition. Worth converging on one predicate both sites call, leaving the gate with what only it can do: the user_stop confirmation, the assignment lookup, and the replay assertions.
P3 — the correction path's Stop provenance changes without being mentioned
retireDelegation(source, replacement.actionId) now routes a correction that retires a running owned root through stopOwnedWorkHubRoot, stamping workhub.direct_stop.<digest> on the target Turn where main passed no source, and switching that path from cancelMessageAdmissions to claimMessageAdmissionCancellation. I checked both: the claim variant's body is the single-message body of cancelMessageAdmissions plus one column, and turnAbortStatusLabel only special-cases renderer.stop_button, so I do not believe anything user-visible changes. But the durable record now labels correction-initiated stops as direct_stop, on a path shipped in #4242, and the Summary describes only direct stop. Either name the source for both destructive paths or say so in the Summary.
P3 — the abort source hashes an identifier that is already stored in the clear
workHubDirectStopAbortSource writes workhub.direct_stop.<48 hex of sha256(actionId)>. That same actionId sits in plaintext in delegation_stop_requested.actionId, and it is already constrained to [A-Za-z0-9_-]{1,128}, so there is no secret and no length problem. The digest costs a node:crypto import in session-projection-helpers.ts and makes the durable abort source unreadable when diagnosing a stuck stop. workhub.direct_stop.<actionId> compares just as exactly.
P3 — cancelMessageIfPending's claim id is optional but never omitted
execution-composition.ts:1312 is the only production caller and always passes one, so if (!cancellationClaimId) { await cancelMessageAdmissions(...) } and the claimOutcome === undefined case are unreachable. Making the parameter required deletes both and leaves one cancellation write on this path. (cancelMessageAdmissions keeps its three other callers.)
Next step
Requesting changes for the P1 only; it is confined to resolveStop plus the clarification copy, and the second P2 falls out of the same fix. The not_owned P2 I would also like fixed here since the durable claim is what blocks the retry, but I will not hold the PR on it if you would rather resolve it in the follow-up that adds pause/resume — say which and I will re-review promptly either way. The P3s are yours to take or defer.
The ADR and domain-language entries are unusually good and are most of the reason this was reviewable at all. Thank you for writing them.
AI-assisted review: I used Maka to read the diff and the surrounding authorities on main, and to draft this comment. I executed the two stop regexes and UNSAFE_STOP_TARGET from this head to produce the P1 table, and verified every other claim against source myself. I own this assessment.
Dismissing my own blocking state — the findings above stand as review comments rather than a merge block. #4439 already carries independent findings from another reviewer, and the author should weigh both without two separate blocks to clear.
fe7e267 to
be83bef
Compare
hqhq1025
left a comment
There was a problem hiding this comment.
I found one P1 blocker on exact head be83bef50ac0a42f6077b919b62c769e6f501a04.
The change adds direct WorkHub stop coordination, global durable action ownership, target-removal convergence, replacement-specific cancellation provenance, and the associated Core/Storage/Runtime/Host/Desktop protocol and UI paths. I reviewed the complete 42-file diff, the current-main merge, migrations 37-38, action replay/assignment ordering, stop and replacement effects, renderer send-lease behavior, and the prior findings on the older head.
The previous current concerns are addressed for newly admitted actions, removed stop targets, replacement provenance, stop parsing, and not_owned retry behavior. However, an existing replacement prepared before the new action-claim table was introduced can still perform destructive retirement before its action identity is checked against a conflicting post-upgrade claim; see the inline P1.
Validation: clean npm ci, npm run build:test, full typecheck, Core 750/750, 458 affected exact-head tests, and 458 affected current-main synthetic-merge tests passed. Full Runtime Host completed 1,563 pass / 1 fail / 12 skip; the sole live-sandbox failure reproduced on the current-main synthetic merge and is environment-related. Full Desktop completed 1,837 pass / 0 fail / 8 cancelled in the unchanged MCP OAuth deadline group. git diff --check passed. Hosted plan and windows_recovery are green; heavy is still running.
Unable to validate a real interactive WorkHub/provider session in this Linux environment.
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.
Architecture discussion: resolve Sessions, not stop syntaxThis is a first-principles / Occam pass, not a claim that I already have the final design. I think direct stop belongs in WorkHub. At this point WorkHub is naturally becoming both a coordination control plane and a navigation layer. My concern is narrower: the current change makes target resolution a stop-specific text grammar. The current path is roughly: That gives Stop its own target resolver in Starting from the underlying model:
A possible direction to discuss is: For an initial deterministic baseline, Session retrieval could use BM25 over permitted raw-message chunks and Session metadata, aggregate message hits by Visibility also fits this model cleanly. Hidden, archived, or logically deleted Sessions can be excluded from the recall projection according to explicit policy while their underlying records follow the existing retention/recovery contract. The search index remains rebuildable rather than becoming lifecycle authority. From this perspective, the valuable part of this PR is the latter half: durable action identity, exact Message ownership, pending cancellation, owning-root Stop, crash replay, and stop/replacement arbitration. The questionable part is permanently binding that control protocol to a bespoke direct-stop parser and exact Session-name uniqueness proof. My current Occam conclusion is:
BM25 is only one plausible first baseline, not the final answer. I would like to discuss whether there are repository or product constraints that rule out Session-level relevance recall, and whether this PR can keep the durable stop execution work while avoiding the regex/exact-name path becoming the canonical target-resolution contract. |
|
@likun666661 I agree with this boundary. The direction I propose is: The Resolver is shared by continue, inspect, stop, resume, delegation, and future control actions. It may combine structured references, visible Session metadata, focus/recency/state signals, and later lexical retrieval such as BM25. It only proposes identities; it never grants execution authority.
For this PR, the durable half remains valuable and should stay: action identity, exact ownership, cancellation, owning-root Stop, replay, and stop/replacement arbitration. The current direct-stop parser and exact-name lookup should be treated as a compatibility adapter, not the canonical architecture. Follow-up work can introduce the shared Resolver contract first, move stop onto it, then add deterministic retrieval/BM25 behind the same contract. |
|
Thanks, I think we are aligned on the long-term boundary. My preference is to do the shared Session Resolver properly rather than knowingly establish a second target-resolution path. If delivery timing requires this PR to keep the deterministic regex/exact-name behavior temporarily, I think the minimum acceptable shape is to introduce the shared Without that port, the current code is not really a compatibility adapter: there is no previous direct-stop contract to preserve. It becomes the canonical implementation by construction, and its regex, display-name matching, clarification behavior, tests, and domain language will all become migration constraints for the future Resolver. Putting the boundary in place now would let this PR retain its valuable durable stop execution work while making the temporary resolver visibly replaceable. I would also avoid documenting the exact-name grammar as a long-term product contract, and link a concrete follow-up with removal criteria for the temporary resolver. One smaller modeling question: I think a Session Resolver should probably return ranked existing Sessions, If there is no timing constraint, I would rather introduce the real shared Resolver path in this change. If there is one, the interface plus temporary implementation seems like the smallest step that does not harden the wrong boundary. |
|
@likun666661 Agreed on both points. I will treat the shared I also agree that I added a proposed design document that records the original experiment branches and R2.4/R3 plan, the production slices already delivered, the new pipeline and contracts, the reframed experiment arms, and the remaining PR-sized work:
The WorkHub domain language now also names |
af0c519 to
16d15de
Compare
There was a problem hiding this comment.
Reviewed 16d15dec (+4845/−172, 47 files: production 1822, tests 2583, docs 440). Built the workspaces and ran test:dist for core, storage, runtime-host and the desktop WorkHub suites, plus desktop typecheck and the renderer architecture check; all green. No live session.
First, the good news, because there is a lot of it. Every point from my last round is closed for real except one: the stop-shaped ordinary work now falls back to normal routing, not_owned converges instead of conflicting forever, correction no longer writes direct-stop provenance, cancelMessageIfPending lost its optional claim, and admission is now proven from opaque delegation identity rather than display names. M4n5ter's one-actionId-two-delegations case is closed by the durable claim table, hqhq1025's prepared-replacement ordering is closed by claiming before replay. The claim → effect → resolution ordering and the crash replay through same_claim are the most solid part of the PR.
Why it is this big. Since my last review the PR grew by 2085 lines: 52% tests, 19% docs, 29% production. It is big in redundancy, not in scope, and I do not think it should be split; there is no merge constraint between the halves, and the port without a consumer or the schema without a claimant would each leave main in a state only the next PR explains. What I would like instead is the list at the end.
One thing that blocks merge (P1, normal user path). Delete any Session that was ever a WorkHub target, and every direct stop in the system fails from then on. workhub-coordination-action-gate.ts:385-392 rejects the proposal if any active assignment points at a Session that is no longer listed, and the coordinator repeats the check under the lease. Nothing ever retires that assignment: activeWorkHubAssignments only knows superseded, aborted and stop-resolved, Session deletion writes nothing to the coordination log, and a stop or correction aimed at the dangling delegation is refused by the same visibility check before it can resolve. The existing test rejects a stop when its target is removed before stop admission ends in exactly this state; append one more stop against a different Session to it and it fails. The fix is to scope the visibility proof to the delegation being stopped (currentTargetName and the coordinator's visibleSessionIds.has(input.targetSessionId) already do that) and delete the two global some(...) checks; if you want dangling delegations out of the active set, probeSessionRemoval is the tombstone-backed proof to use.
Two P2s, same root.
stopOwnedWorkHubRoot(execution-composition.ts:1978-1996) decides "active" fromreadRootState, which is in-memory. After a Host restart, before recovery re-registers the execution, it skipsstopRoot, reads a snapshot that can still berunning, and returnsalready_terminal, which#stopthen commits as an immutabledelegation_stop_resolved. The user sees "stopped"; the Turn keeps running.isHostedExecutionTerminalfromhosted-execution-authority.tsis the seam; returnrecoveringwhen the stop was not delivered and the snapshot is not terminal. I could not confirm whethercoordination.actis admitted before recovery completes, so the window may be narrower than it looks, but the committed fact is still wrong when it happens.- No fact records that a delegation's work finished. Delegate to Session S twice, let the first complete normally, and S has two "active" delegations forever: renderer answers
stop_target_not_unique, Gate and coordinator refuselength !== 1, correction takes.at(-1). S can never be direct-stopped again. This is the death end from my last round with better wording; the wording is now also untrue ("more than one active delegation" when one is done). This PR is the first to let "active delegation" carry destructive authority, so it needs to say which concept that is: a link that ends only by supersession, or an execution that ends when the Turn does. If the completion fact belongs to a later slice, the body should state the constraint explicitly.
Two more P2s on the resolver work.
- The two stop clarifications
stop_target_not_activeandstop_target_not_unique(workhub-route-policy.ts:197-203) are decided from the renderer'sactiveActionIdsBySessionIdmirror alone; nocoordination.actis sent, so the Host never checks them. A second window or a reload before the coordination stream has refilled gives a zero-option clarification that contradicts Host state. Either read the Host before answering, or submit and let the Gate's conflict reason come back as the copy; the second also removes the duplicated sole-delegation predicate. - The port went in but the path it was meant to replace stayed.
workHubSessionReferenceNamesSessiondelegates tostopTargetMatchesSession(workhub-creation-intent.ts:444), whilecorrectionTargetMatchesSessionat:397is a second ~25-line copy of the same rule, andresolve()in route-policy still runs its ownrankExactSessions/rankRelatedSessionswithout touching the port. So the exact-name grammar is still canonical by construction, which is what likun666661's thread set out to avoid, and the port is a third path rather than the shared one. I tried both ways to close this on a scratch branch. Folding the two matchers into one strict core with per-action tails is a net −3 lines and makes the rule single-sourced; the tails are genuinely different (correction needs the withdrawal guard and accepts a trailing executable clause, stop accepts punctuation only), so the core is the real seam. Routing correction through the port instead costs +37 lines and forces ascopeon the reference, so the port starts choosing the action, which its own doc says it never does. Removing the port entirely is −394 lines with every core and desktop test green and behaviour unchanged. likun666661's ask was one of two shapes: the real shared resolver now, or the port as the first replaceable implementation. Pick one in this PR: either bringresolve()and correction onto the resolver too, or drop the port and let the folded matcher be the shared seam until a ranked resolver has a second real consumer. What is there now is neither.
What I would cut, in this PR. I tried each of these on a scratch branch from 16d15dec before asking (build, runtime-host and storage suites, and knocking out each guard one at a time to see which test still catches it), so the numbers below are measured, and two items I had first drafted turned out wrong and are corrected here. Roughly 1400 lines below this head in total, with the P1 fixed and one new regression test, all suites green:
expects.activeActionIds(protocol, Gate checks at:401-421,sameActiveDelegationSet,listActiveAssignments, renderer proposal: −192 lines). The coordinator proves the same thing from durable state under the lease; with the field gone, no guard mutation survives, so the client field cannot reject anything the proof would admit. It is arequireExactRecordmember on epoch 95 now, so removing it later costs another epoch. The Gate'sreadSupersession/readReplacementre-reads before the lease and the coordinator's secondvisibleSessionIdslookup are the same shape: no test kills them even today, because the same proof is made again later under the lease.- Most of the multi-lane admission in
#prepareStop. Every writer of a delegation fact already holds the Coordination lane (execution-composition.ts:1422and the three terminal facts), so that lane alone is exclusive over the active set, andinitiallyActive/admittedTargetSessionIds/ the "active set changed" check only pin the set seen at the advisory read; they also block unrelated Turns on every delegated Session and make a stop of A fail because someone delegated to B. The one window they do not close is Session deletion:session-retirement-coordinator.ts:437runs on the deleted Session's lanes without the Coordination lane, so the stop target can vanish betweenbeforeAppendand the append. One line closes it,runMany([WORKHUB_COORDINATION_SESSION_ID, input.targetSessionId]), which holds exactly the lane a concurrent delete would take. Keep that, drop the rest. readWorkHubActionClaimacrossSessionAuthorityStore,SqliteSessionStoreandexecution-stores: zero production readers. AssertclaimWorkHubAction'sclaimed → same_claim → conflictsequence instead.- Not the claim on
answer_hereandclarify. I first drafted "claim only for destructive operations"; removing those two#claimActioncalls leaves all 1611 runtime-host tests green, and then this passes:answer_herewith some actionId, Host restart (empties the replay map),stop_workwith the same actionId, admitted, delegation retired.readWorkHubAssignmentcannot catch it because an answer never creates an assignment, so the claim row is the only durable owner of that identity. Keep the calls and add that scenario as a test; nothing covers it today. What remains open is that the table is never deleted from andclaimed_athas no reader, so say what retention it implies. docs/architecture/workhub-action-resolution-design.md(326 lines). Its contract half is already in the ADR and the domain-language file this PR also edits; the rest is history, delivered-slice tables, plan and deferred decisions, whichdocs/README.mdassigns to issues and discussions. The nine branches it cites do not exist on apache/maka. Move the narrative to #3286 and delete the file.- Tests: the two runtime-host files carry 1170 lines against 559 production lines, largely the same fail-closed precondition spread over fixture tables in both. After the P1 fix and the cuts above, each stop guard is killed by exactly one test and those two files lose about 250 lines;
workhub-session-resolver-port.test.tskeeps its first case (the described-resolver swap is the one independent obligation) and drops the other four, which re-prove controller and core tests one layer down. One test is missing:stoppedin thedata-link-statetable ofworkhub-surface-flow.test.ts.
Smaller: workHubDirectStopAbortSource still hashes an actionId that is stored in clear two facts earlier; WorkHubResolverSession.projectName / updatedAt have no reader; stop_target_required remains a zero-option clarification. Epoch 95 is correctly bumped here, but #4386 also claims 95, so whichever merges second must take 96, not resolve the conflict to 95. The body has no evidence for the eight new copy strings and the stop turn rendering; there is no WorkHub story, so a recording is fine.
Evidence boundary: static read of 16d15dec against main 92fa5281; core 772/772, storage 1089 pass, runtime-host 1599 pass, desktop WorkHub suites 110/110, typecheck, architecture check, Biome on touched files; no live session; cross-Host execution ownership not checked.
AI-assisted review: drafted with Maka across three passes (Host, core/storage, desktop/docs); I verified the global visibility check, the missing completion fact, the restart path in stopOwnedWorkHubRoot and the size split myself.
简体中文
上轮的意见除 abort source 哈希外都真正关了,claim → effect → resolution 的顺序和崩溃重放是全 PR 最扎实的部分。体量:上轮之后新增 2085 行,测试 52%、文档 19%、生产 29%,大在冗余不在范围,不建议拆。
阻塞一条:删掉任何一个曾被 WorkHub 委派过的 Session,之后全局所有 direct stop 永久失败。Gate 第 385 行对全部 active assignment 做可见性检查,而 delegation 没有完成事实、Session 删除也不写 coordination log,悬空的那条永远清不掉。修法是把可见性证明收敛到被停的那一条,删掉两处全局 some。
四条 P2:stopOwnedWorkHubRoot 用内存态判 active,Host 重启后会把仍在运行的 Turn 记成 already_terminal 并落成不可变事实;同一 Session 被派活两次后永久不可 stop,因为没有完成事实,本 PR 第一次让「active delegation」承担破坏性权威,必须说清它是链接还是执行概念;stop 的两条 clarification 只信 renderer 本地镜像,Host 不参与;port 只接了 stop,correction 的名字匹配和 resolve() 的检索仍是独立实现,exact-name 文法仍按构造是规范实现,port 成了第三条路。实测:两个 matcher 合一净 −3 行且规则单源;correction 接上 port 要 +37 行并让 port 知道 action;整个去掉 port −394 行行为不变。李坤要的是两种形状之一,本 PR 二选一,别留半截。
可删清单我在实验分支上逐条试过,实测约 −1400 行、全绿,其中两条修正:answer/clarify 的 claim 不能删,去掉后同 actionId 先 answer、重启、再 stop 会被放行并真的执行 retire,请保留并补这条回归;多通道 admission 保护的只有「Session 删除不持 Coordination 通道」这一个窗口,一行 runMany([COORDINATION, targetSessionId]) 才能关上,其余那套可删。成立的:expects.activeActionIds(−192,无 mutant 幸存)、Gate 的两处 lease 前重读和 coordinator 的第二份可见性查找(今天就没有测试能杀死它们)、readWorkHubActionClaim 四层、326 行设计文档、测试矩阵收敛后每条守卫恰一条测试。epoch 95 与 #4386 撞号,后合的改 96。
|
@ARE404 Two things outside this PR that surfaced on the scratch branch, in case you want to file them:
Happy to share the branches if you open something for either. 简体中文两件本 PR 之外的事,供你决定要不要开 issue:main 上带引号的命名请求会被判不可执行;系列里有几条已无生产者或消费者的路径,删掉后测试全绿。需要的话我把实验分支给你。 |
Stop admission proved that *every* active delegation still had a visible target Session. Nothing ever retires a delegation whose Session the user deleted: `activeWorkHubAssignments` ends a delegation only on supersession, replacement abort, or a resolved stop, and Session removal writes nothing to the coordination log. One deleted target therefore left a permanently active delegation that failed the global check, and from that point every direct stop in the system was refused — including stops aimed at healthy, unrelated Sessions. Prove visibility only for the delegation being stopped, in both the Gate and the coordinator's under-lock recheck. A dangling delegation still fails closed when it is itself the target, which is correct: its Message proof is gone. Reported by Astro-Han in review of apache#4439. Generated-by: Claude Opus
The stop proposal carried the active delegation set the policy observed, and the Gate compared it against current state. That proof was already made from durable facts under the admission lease, where it is authoritative; the client-supplied copy could not reject anything the durable proof would admit, and as a member of a closed protocol shape it would have cost another compatibility epoch to remove later. Drop it. The proposal keeps the one precondition it can meaningfully assert — the Session it resolved the delegation against — and the Gate proves sole-active-delegation from the assignments it just read. Reported by Astro-Han in review of apache#4439. Generated-by: Claude Opus
Root registration is in-memory. Between a Host restart and execution recovery a still-running root looks inactive, so `stopOwnedWorkHubRoot` skipped `stopRoot`, read a snapshot that was still `running`, and reported `already_terminal` — which the Gate then commits as an immutable `delegation_stop_resolved`. The user is told the work stopped while the Turn keeps running. `already_terminal` is an observation that can never be revised, so only a durably terminal snapshot may claim it. Anything else is `recovering`, which admission already handles by leaving the claim unresolved for a later attempt. `readDelegationRetirement` proved retirement the same way and gains the same rule: an unregistered root is not evidence that its work ended. Reported by Astro-Han in review of apache#4439. Generated-by: Claude Opus
Nothing records that a delegation's work is done. A link ends only by supersession or a resolved stop, so delegating to one Session twice and letting the first complete normally left it with two permanently active delegations: the renderer answered `stop_target_not_unique`, the Gate and coordinator refused on the count, and that Session could never be direct-stopped again. The two senses of "active" were being conflated. The link is still active — WorkHub still owns it, and correction still works through it — but a delegation whose work has finished is not a competing stop target, because there is nothing left in it to stop. Only work that could still be stopped makes a Session's stop target ambiguous. Prove that from the target Session, which owns execution state, through the existing non-destructive `readDelegationRetirement`. The renderer applies the same rule to its own delegation mirror using the read-only execution projection it already receives. Execution state that cannot be read counts as competing, never as finished, so the stop still fails closed while the owner is unresolved. This deliberately does not add a completion fact to the coordination log: target execution state stays an ordinary Session fact that WorkHub reads as a rebuildable projection. Reported by Astro-Han in review of apache#4439. Generated-by: Claude Opus
`stop_target_not_active` and `stop_target_not_unique` were decided from the renderer's own delegation mirror, and both returned without ever calling `coordination.act` — so the Host never saw the request and never got to disagree. That mirror is filled only by the Coordination stream, which means it is empty in a second window, after a reload, and across a reconnect. In that window WorkHub told the user, specifically and confidently, that a Session had no running work while it did. Add a read-only `workhub.coordination.delegations` query and ask it once a reference has resolved to one Session, so an ordinary message never pays for the round trip. The Host answers with its active links and, for each, whether it still holds work a stop could reach — the same judgement admission makes, from the same durable state. That also removes the client-side settled-delegation mirror added for the completion-fact fix: which delegations are stoppable now has one owner instead of a renderer projection that had to agree with the Host. Reported by Astro-Han in review of apache#4439. Generated-by: Claude Opus
f133e26 to
70955be
Compare
Stop admission proved that *every* active delegation still had a visible target Session. Nothing ever retires a delegation whose Session the user deleted: `activeWorkHubAssignments` ends a delegation only on supersession, replacement abort, or a resolved stop, and Session removal writes nothing to the coordination log. One deleted target therefore left a permanently active delegation that failed the global check, and from that point every direct stop in the system was refused — including stops aimed at healthy, unrelated Sessions. Prove visibility only for the delegation being stopped, in both the Gate and the coordinator's under-lock recheck. A dangling delegation still fails closed when it is itself the target, which is correct: its Message proof is gone. Reported by Astro-Han in review of apache#4439. Generated-by: Claude Opus
The stop proposal carried the active delegation set the policy observed, and the Gate compared it against current state. That proof was already made from durable facts under the admission lease, where it is authoritative; the client-supplied copy could not reject anything the durable proof would admit, and as a member of a closed protocol shape it would have cost another compatibility epoch to remove later. Drop it. The proposal keeps the one precondition it can meaningfully assert — the Session it resolved the delegation against — and the Gate proves sole-active-delegation from the assignments it just read. Reported by Astro-Han in review of apache#4439. Generated-by: Claude Opus
Root registration is in-memory. Between a Host restart and execution recovery a still-running root looks inactive, so `stopOwnedWorkHubRoot` skipped `stopRoot`, read a snapshot that was still `running`, and reported `already_terminal` — which the Gate then commits as an immutable `delegation_stop_resolved`. The user is told the work stopped while the Turn keeps running. `already_terminal` is an observation that can never be revised, so only a durably terminal snapshot may claim it. Anything else is `recovering`, which admission already handles by leaving the claim unresolved for a later attempt. `readDelegationRetirement` proved retirement the same way and gains the same rule: an unregistered root is not evidence that its work ended. Reported by Astro-Han in review of apache#4439. Generated-by: Claude Opus
Nothing records that a delegation's work is done. A link ends only by supersession or a resolved stop, so delegating to one Session twice and letting the first complete normally left it with two permanently active delegations: the renderer answered `stop_target_not_unique`, the Gate and coordinator refused on the count, and that Session could never be direct-stopped again. The two senses of "active" were being conflated. The link is still active — WorkHub still owns it, and correction still works through it — but a delegation whose work has finished is not a competing stop target, because there is nothing left in it to stop. Only work that could still be stopped makes a Session's stop target ambiguous. Prove that from the target Session, which owns execution state, through the existing non-destructive `readDelegationRetirement`. The renderer applies the same rule to its own delegation mirror using the read-only execution projection it already receives. Execution state that cannot be read counts as competing, never as finished, so the stop still fails closed while the owner is unresolved. This deliberately does not add a completion fact to the coordination log: target execution state stays an ordinary Session fact that WorkHub reads as a rebuildable projection. Reported by Astro-Han in review of apache#4439. Generated-by: Claude Opus
`stop_target_not_active` and `stop_target_not_unique` were decided from the renderer's own delegation mirror, and both returned without ever calling `coordination.act` — so the Host never saw the request and never got to disagree. That mirror is filled only by the Coordination stream, which means it is empty in a second window, after a reload, and across a reconnect. In that window WorkHub told the user, specifically and confidently, that a Session had no running work while it did. Add a read-only `workhub.coordination.delegations` query and ask it once a reference has resolved to one Session, so an ordinary message never pays for the round trip. The Host answers with its active links and, for each, whether it still holds work a stop could reach — the same judgement admission makes, from the same durable state. That also removes the client-side settled-delegation mirror added for the completion-fact fix: which delegations are stoppable now has one owner instead of a renderer projection that had to agree with the Host. Reported by Astro-Han in review of apache#4439. Generated-by: Claude Opus
`stopsActionId` made the client name the delegation to stop, so the policy had to learn one before it could submit. It did that by reading `workhub.coordination.delegations`, which reads the whole global Coordination transcript and parses every record — once per stop-shaped message, and twice more inside `#prepareStop`. The client cannot prove which link is live anyway. The Gate already resolved the assignment, checked it against the named Session, and proved sole-active-delegation from durable state under the admission lease, so the field was a weaker copy of an answer the Host was making regardless. Drop it. The proposal names only the Session it resolved, and the Gate resolves the delegation from its own active links: one link is the answer whatever state its work is in, and only several need separating, by the rule competition already used. Resolving again on replay would fail, because a resolved stop takes its delegation out of the active set — the second attempt would find nothing where the first found one. `workhub_action_claims` already records the delegation each action bound itself to, is written before any effect, is never deleted, and outlives removal of the target Session; it is read here for the first time. The claim-then-request order leaves one seam, where an action owns a stop with no request behind it, and nothing destructive has happened there, so it resolves like a first attempt. That retires the `delegations` query with its transcript scan, and with it the renderer's last stop-state judgement: `stop_target_not_active` and `stop_target_not_unique` were the two answers a client mirror could give while contradicting the Host, and the Gate's refusal now carries them. Reported by Astro-Han in review of apache#4439. Generated-by: Claude Opus
Astro-Han
left a comment
There was a problem hiding this comment.
Reviewed exact head f4e60a97a779075d42733e024ffee750ea667faf (+5050/−204, 46 files; production ≈ 1800). This is an AI-assisted review: I read the source at this head and ran the checks below locally; I did not run a live session.
The stop-target resolution question from the last round is settled well, and settled in a better shape than either option we were choosing between. The renderer now submits nothing but expects.targetSessionId, and the Host names the delegation itself. #stopSource reads the durable claim first so a replay converges on whatever the first attempt bound itself to, and falls back to the active links when there is no claim to converge on. That removes the client-supplied delegation identity, removes the renderer's active-delegation pre-read, and puts the durable claim table to work instead of leaving it write-only. The Session-name rule is genuinely single-sourced now too: matchWorkHubSessionName is the one matcher, correctionTargetMatchesSession is a one-line wrapper over it, and stop and correction each apply their own tail rule to the same recall. The bridge surface that appeared in 70955be81 is gone again in f4e60a97a, so the renderer ratchet is untouched. All four earlier findings check out against this head: scoped visibility proof, unrecovered roots reported as recovering, finished delegations no longer competing for stop, and Host-decided stop refusals.
Blocking: the branch no longer merges. mergeStateStatus is DIRTY, with one content conflict in packages/runtime-host/src/protocol/index.ts (a mechanical export-list collision, and the only conflicting file). CI on this head is green (test, audit). Nothing else here blocks; the two P2s below are cleanups, not correctness holes.
P2: the Gate proves the same thing twice on the first attempt. Reachability: ① normal path. Whenever the block at :385 runs, #stopSource has just derived source from listActiveAssignments() using exactly the rule "one delegation on this Session still holds work", and the replay branch returns at :380 before reaching it. The block then re-fetches the active set and re-runs that rule (:407 plus #hasCompetingWork), and #prepareStop proves it a third time under the lease, which is the copy that actually decides. So a stop now walks the append-only coordination transcript four times, and the retirement rule exists in two places that can drift. listSessions()/currentTargetName and the supersession/replacement reads are still needed there; the active-set half looks like what the resolver refactor made redundant.
P2: stop admission still takes a system-wide lease and a global set check. Reachability: ② concurrency. #prepareStop passes admissionSessionIds: [coordination, ...every Session with an active delegation] and then aborts if the lease-held set contains any assignment whose target was not in the pre-read set. Two effects: admitting one stop serializes delegation traffic across every Session WorkHub currently delegates to, and a delegation created concurrently to an unrelated Session refuses this stop with "active delegation set changed during stop admission". The proof that matters is the scoped one right below it (target Session visible, source present in targetActive, every competitor retired), and that needs only the target Session's lane plus the coordination Session. It fails closed and is retryable, so it is not a blocker.
P3: the comment at :568 promises more convergence than the code gives. Reachability: ② crash plus concurrency. It says a crash between claim and stop request "resolves from the active links below like a first attempt", which holds only while the single active link on that Session is unchanged. If that delegation leaves the active set and another takes its place before the retry, #stopSource re-derives the new one, the stop fingerprint changes with it, and claimAction returns conflict, permanently, since claims are never deleted. Nothing destructive happened and a fresh message gets a fresh action id, so it costs one refused attempt. Either narrow the comment or compare the re-derived delegation against claim.subject and return the stable refusal.
P3: stopReplacedWorkHubRoot still reads in-memory registration as proof of termination. Reachability: ② restart. It is the one reader left in this file that decides already_terminal without consulting a durable snapshot, and it is the same window the sibling readDelegationRetirement names in its own comment. It preserves what main already did on the replacement path, so it is not a regression from this change. Worth a follow-up rather than a fix here, since threading recovering through the replacement outcome is its own change.
On size: at effort/XL this is close to purely additive, and the one thing it does delete, the duplicated Session-name matcher, is the right kind of deletion. The two P2s above are the concrete leftovers the resolver refactor created and did not collect.
Evidence boundary: everything above comes from reading the source at this head and from the local runs listed next. I did not exercise a live stop, a real Host restart, or the crash windows in P3 and P4; those are read from the code and from the tests in this branch.
Verification I ran locally at this head, after building the affected workspaces: test:dist green for @maka/core, @maka/storage, @maka/runtime and @maka/runtime-host; @maka/desktop test:dist 1964/1967 with the three failures all ERR_MODULE_NOT_FOUND from workspaces I had not built (computer-use-host, mcp-oauth-controller, mcp-runtime-e2e) and every WorkHub and stop assertion passing; desktop typecheck clean; npm run format and npm run lint clean with no working-tree changes; check-renderer-architecture.mjs passing against the merge base 898b86d6b.
简体中文
在 f4e60a97a 这个 head 上复核。上一轮悬着的 stop 目标解析问题解决了,而且比我们当时在权衡的两个方案都好:渲染层现在只提交 expects.targetSessionId,由 Host 自己认领 delegation。#stopSource 先读持久化的 claim,重放就能收敛到第一次尝试绑定的那个 delegation,没有 claim 时才退回活跃链接去推。这样既去掉了客户端自带的 delegation 身份,也去掉了渲染层对活跃集的预读,还让原本只写不读的 claim 表真正用了起来。Session 名字的匹配规则也确实单一化了:matchWorkHubSessionName 是唯一的匹配器,correctionTargetMatchesSession 只剩一行包装,stop 和 correction 各自对同一个召回结果套自己的尾部规则。70955be81 一度加进来的 bridge 面在 f4e60a97a 又撤掉了,渲染层的架构棘轮没有被动。上一轮的四条意见对着这个 head 逐条核过,都成立。
阻塞项只有一个:分支已经合不上了。 mergeStateStatus 是 DIRTY,冲突只有 packages/runtime-host/src/protocol/index.ts 一个文件,是导出列表的机械冲突。这个 head 的 CI 是绿的。下面两条 P2 是可以清理的冗余,不是正确性问题。
P2:首次尝试时 Gate 把同一件事证了两遍。 走到 :385 那段的时候,#stopSource 刚刚用同一条规则从活跃集推出了 source(重放分支在 :380 就返回了)。这段又把活跃集取一遍、规则跑一遍,而 #prepareStop 在租约下还会证第三遍,那一遍才是真正算数的。结果是一次 stop 要把只增不删的协调 transcript 全量扫四次,退休规则也散在两处,容易走偏。那里真正还需要的是 listSessions() 拿名字,以及 supersession / replacement 两个读。
P2:stop 的准入仍然拿着一把全局锁和一个全局集合校验。 #prepareStop 把「所有当前有活跃 delegation 的 Session」都放进 admissionSessionIds,然后只要租约下的集合里冒出一个预读时不存在的目标就整体退掉。两个后果:一次 stop 准入会把 WorkHub 当前所有委派目标的通道都串起来;另外一个 Session 上并发建的委派会把这次 stop 顶掉。真正管用的证明是下面那段限定在目标 Session 上的,只需要目标 Session 加协调 Session 两条通道。它是 fail closed 且可重试的,所以不阻塞。
P3::568 的注释承诺的收敛性比代码强。 claim 写完、stop request 没写就崩,这时候只有目标 Session 上那条活跃链接没变,才真的「和首次尝试一样」。如果那条链接退出活跃集、另一条顶上来,重推会推到新的 delegation,指纹跟着变,claimAction 直接 conflict,而且因为 claim 从不删除,这个 action id 就永久废了。没有破坏性后果,重新发一条消息会拿到新 id,代价是一次被拒。要么把注释收窄,要么把重推结果和 claim.subject 比一下,给一个稳定的拒绝。
P3:stopReplacedWorkHubRoot 还在拿内存里的注册状态当「已经结束」的证据。 这是本文件里最后一个不查持久快照就下 already_terminal 的地方,而旁边 readDelegationRetirement 的注释恰好点名了这个重启窗口。它保持的是 main 上替换路径原本的行为,不算这次引入的回归,适合另开一个改动处理,把 recovering 接进替换路径本身就是一次独立的改动。
关于体量:effort/XL 这个量级下,这个 PR 基本是纯加法,唯一删掉的东西(重复的 Session 名匹配器)删得对。上面两条 P2 就是解析器重构造成、但还没回收的残留。
证据边界:以上都来自读这个 head 的源码和下面这些本地运行。我没有真的跑一次 stop,也没有构造真实的 Host 重启和 P3 里那两个崩溃切面,那部分是从代码和分支自带的测试读出来的。
本地验证:@maka/core、@maka/storage、@maka/runtime、@maka/runtime-host 的 test:dist 全绿;@maka/desktop 的 test:dist 1964/1967,三个失败全是我没有构建的 workspace 导致的 ERR_MODULE_NOT_FOUND,WorkHub 和 stop 相关断言全部通过;desktop typecheck 干净;npm run format 和 npm run lint 干净且没有产生改动;check-renderer-architecture.mjs 以 merge base 898b86d6b 为基准通过。
| // competing stop target though — there is nothing left in it to stop — | ||
| // and counting it would make a Session that was delegated to twice | ||
| // permanently unstoppable once the first delegation completed. | ||
| if (!activeAssignments.some((assignment) => assignment.actionId === source.actionId)) { |
There was a problem hiding this comment.
P2. #stopSource derived source from listActiveAssignments() moments ago using this same rule, and the replay branch returns at :380 before reaching here, so on every path that reaches this line the check has just been made. #prepareStop makes it a third time under the lease, which is the copy that decides. Dropping this and the #hasCompetingWork call below it removes one full scan of the coordination transcript per stop and one of the two places the retirement rule lives; listSessions()/currentTargetName and the supersession/replacement reads above still earn their place.
| // The request records which delegation this action bound itself to. It is | ||
| // written after the claim, so a crash between the two leaves a claim with | ||
| // nothing to converge on — and nothing destructive happened either, so | ||
| // that case resolves from the active links below like a first attempt. |
There was a problem hiding this comment.
P3. This holds only while the single active link on that Session is unchanged. If that delegation leaves the active set and another takes its place before the retry, the fallback below re-derives the new one, stopActionFingerprint changes with it, and claimAction returns conflict, permanently, since claims are never deleted. Nothing destructive happened and a fresh message gets a fresh action id, so it costs one refused attempt rather than anything worse. Either narrow the sentence, or compare the re-derived delegation against claim.subject and return the stable refusal.
| } | ||
| const visibleSessionIds = new Set(headers.map((header) => header.id)); | ||
| const activeAssignments = activeWorkHubAssignments(messages); | ||
| if ( |
There was a problem hiding this comment.
P2. This whole-set comparison is what forces admissionSessionIds to cover every Session with an active delegation, so admitting one stop serializes delegation traffic system-wide, and a delegation created concurrently to an unrelated Session refuses this stop. The scoped proof below (target Session visible, source found in targetActive, every competitor retired) is what actually establishes the one-target property, and it needs only the target Session lane plus the coordination Session. It fails closed and is retryable, so not a blocker.
| readonly outcome: 'stop_delivered' | 'already_terminal'; | ||
| readonly targetTurnId: string; | ||
| }> { | ||
| if (!isActiveWorkHubRoot(coordinator, identity)) { |
There was a problem hiding this comment.
P3. The last reader in this file that treats missing in-memory registration as proof the work ended, the same restart window readDelegationRetirement names in its own comment, and the one stopOwnedWorkHubRoot now guards. It preserves what main did on the replacement path, so not a regression here; threading a recovering outcome through replacement is its own change and reads like a follow-up.
Generated-by: Codex
Generated-by: Codex
Generated-by: Claude Opus
…lver Direct stop owned its own target resolver: it recognized stop-specific text and then matched Session display names itself, ahead of the ordinary routing path. Every future control verb would have grown one the same way. Introduce the shared `SessionResolver` port. It answers one question — which visible existing Sessions a trusted user reference recalls — as ranked candidates, nothing, or ambiguity. Candidates are opaque Runtime-issued references bounded by the caller's visible set, so a resolver can neither invent an identity nor widen its own visibility. `create_new` is absent from the result type: creation is an Action Policy decision, never a retrieval result. Exact display-name matching becomes the first implementation behind that port rather than the stop path's own rule, and stop becomes an Action Policy: Action Intent supplies the reference, the Resolver recalls Sessions, and the policy alone decides destructive sufficiency. The stop decision now carries the resolved delegation identity, so the renderer no longer re-derives it from its own active-delegation bookkeeping. Exact-name syntax is not the long-term product boundary. It can be removed once every target-bearing WorkHub action resolves through this port, the replacement resolver passes the common routing evaluation, and its rollout retains a tested rollback path. Generated-by: Claude Opus
Stop admission re-derived its target from the request text: the Action Gate and the coordinator's under-lock recheck both parsed the user's words again and proved uniqueness by matching Session display names. That made display names the destructive authority and coupled the durable stop protocol to one resolver's grammar, so replacing the resolver could not have kept the protocol intact. The stop proposal now carries what the Action Policy resolved: the opaque delegation identity, the Session it belongs to, and every active WorkHub delegation the policy observed for that Session. The Gate revalidates exactly that immediately before any effect — the assignment exists, it still belongs to the proposed Session, and that Session's current active delegations are still the set the policy saw, which for stop must be the one delegation being stopped. A stale resolution fails closed; a rename between resolution and admission correctly no longer invalidates a claim. Trusted user text must still carry a direct stop imperative, and `user_stop` confirmation stays outside strategy output, so neither model output nor a display name can select what gets stopped. The Gate no longer proves that the text names the target: that binding now rests on the proposal being produced by trusted policy, which is what lets a ranked resolver replace exact naming without touching durable stop execution, replay, ownership, or arbitration. `stop_work` shapes are introduced by this compatibility epoch, so refining the proposal needs no further epoch. Generated-by: Claude Opus
Describe direct stop as it now behaves: its target comes from the shared Session Resolver, its proposal carries opaque identities and the expected active-delegation state, and admission revalidates that state rather than re-deriving a display-name match. Say plainly that a rename between resolution and admission is irrelevant while a concurrent delegation to the same Session is not, and that the trusted-text binding is now the stop imperative and the out-of-band confirmation rather than a name. Record in the design doc why admission by expected state is what makes the port real, and note that correction still resolves its own target and joins the port with continue, inspect, and resume under item 2. Generated-by: Claude Opus
Stop admission proved that *every* active delegation still had a visible target Session. Nothing ever retires a delegation whose Session the user deleted: `activeWorkHubAssignments` ends a delegation only on supersession, replacement abort, or a resolved stop, and Session removal writes nothing to the coordination log. One deleted target therefore left a permanently active delegation that failed the global check, and from that point every direct stop in the system was refused — including stops aimed at healthy, unrelated Sessions. Prove visibility only for the delegation being stopped, in both the Gate and the coordinator's under-lock recheck. A dangling delegation still fails closed when it is itself the target, which is correct: its Message proof is gone. Reported by Astro-Han in review of apache#4439. Generated-by: Claude Opus
The port went in but the paths it was meant to replace stayed: stop resolved through it, correction kept a second near-identical copy of the same name-matching rule, and the exact-name grammar was still canonical by construction. That is a third path, not a shared one. The two matchers differed only in what they allowed *after* the name — a stop reference may carry punctuation and nothing else, a correction may name its target and go on to say what to do with it. That difference is an action's rule, not a retrieval rule, and it is why folding the two matchers into the Resolver would have forced the Resolver to know which action it was serving, contradicting its own contract. So the Resolver reports where the name matched and what text was left over, and says nothing about whether that leftover is acceptable. One `matchWorkHubSessionName` now owns the matching rule for both actions. Stop's tail rule moves into the stop Action Policy; correction's stays in its own predicate, which now takes a match so a caller that already resolved candidates applies the rule to exactly that recall instead of matching names a second time. Correction resolves through the port too. The Resolver stays action-agnostic: reporting a remainder is a neutral fact about a match, not a decision about what to do with it. Generated-by: Claude Opus
The stop proposal carried the active delegation set the policy observed, and the Gate compared it against current state. That proof was already made from durable facts under the admission lease, where it is authoritative; the client-supplied copy could not reject anything the durable proof would admit, and as a member of a closed protocol shape it would have cost another compatibility epoch to remove later. Drop it. The proposal keeps the one precondition it can meaningfully assert — the Session it resolved the delegation against — and the Gate proves sole-active-delegation from the assignments it just read. Reported by Astro-Han in review of apache#4439. Generated-by: Claude Opus
Root registration is in-memory. Between a Host restart and execution recovery a still-running root looks inactive, so `stopOwnedWorkHubRoot` skipped `stopRoot`, read a snapshot that was still `running`, and reported `already_terminal` — which the Gate then commits as an immutable `delegation_stop_resolved`. The user is told the work stopped while the Turn keeps running. `already_terminal` is an observation that can never be revised, so only a durably terminal snapshot may claim it. Anything else is `recovering`, which admission already handles by leaving the claim unresolved for a later attempt. `readDelegationRetirement` proved retirement the same way and gains the same rule: an unregistered root is not evidence that its work ended. Reported by Astro-Han in review of apache#4439. Generated-by: Claude Opus
Nothing records that a delegation's work is done. A link ends only by supersession or a resolved stop, so delegating to one Session twice and letting the first complete normally left it with two permanently active delegations: the renderer answered `stop_target_not_unique`, the Gate and coordinator refused on the count, and that Session could never be direct-stopped again. The two senses of "active" were being conflated. The link is still active — WorkHub still owns it, and correction still works through it — but a delegation whose work has finished is not a competing stop target, because there is nothing left in it to stop. Only work that could still be stopped makes a Session's stop target ambiguous. Prove that from the target Session, which owns execution state, through the existing non-destructive `readDelegationRetirement`. The renderer applies the same rule to its own delegation mirror using the read-only execution projection it already receives. Execution state that cannot be read counts as competing, never as finished, so the stop still fails closed while the owner is unresolved. This deliberately does not add a completion fact to the coordination log: target execution state stays an ordinary Session fact that WorkHub reads as a rebuildable projection. Reported by Astro-Han in review of apache#4439. Generated-by: Claude Opus
The design document duplicated the contract half of the ADR and the domain-language file, and its remaining half — history, delivered-slice tables, plan and deferred decisions — is what docs/README.md assigns to issues and discussions. It also cited branches that do not exist on this repository. Delete it; the narrative belongs in discussion apache#3286. Record what this slice actually settled in the two files that keep contracts: which sense of "active delegation" carries destructive authority, why visibility is proved for one delegation rather than the whole set, and why an unrecovered root can never be reported terminal. Generated-by: Claude Opus
`stop_target_not_active` and `stop_target_not_unique` were decided from the renderer's own delegation mirror, and both returned without ever calling `coordination.act` — so the Host never saw the request and never got to disagree. That mirror is filled only by the Coordination stream, which means it is empty in a second window, after a reload, and across a reconnect. In that window WorkHub told the user, specifically and confidently, that a Session had no running work while it did. Add a read-only `workhub.coordination.delegations` query and ask it once a reference has resolved to one Session, so an ordinary message never pays for the round trip. The Host answers with its active links and, for each, whether it still holds work a stop could reach — the same judgement admission makes, from the same durable state. That also removes the client-side settled-delegation mirror added for the completion-fact fix: which delegations are stoppable now has one owner instead of a renderer projection that had to agree with the Host. Reported by Astro-Han in review of apache#4439. Generated-by: Claude Opus
`stopsActionId` made the client name the delegation to stop, so the policy had to learn one before it could submit. It did that by reading `workhub.coordination.delegations`, which reads the whole global Coordination transcript and parses every record — once per stop-shaped message, and twice more inside `#prepareStop`. The client cannot prove which link is live anyway. The Gate already resolved the assignment, checked it against the named Session, and proved sole-active-delegation from durable state under the admission lease, so the field was a weaker copy of an answer the Host was making regardless. Drop it. The proposal names only the Session it resolved, and the Gate resolves the delegation from its own active links: one link is the answer whatever state its work is in, and only several need separating, by the rule competition already used. Resolving again on replay would fail, because a resolved stop takes its delegation out of the active set — the second attempt would find nothing where the first found one. `workhub_action_claims` already records the delegation each action bound itself to, is written before any effect, is never deleted, and outlives removal of the target Session; it is read here for the first time. The claim-then-request order leaves one seam, where an action owns a stop with no request behind it, and nothing destructive has happened there, so it resolves like a first attempt. That retires the `delegations` query with its transcript scan, and with it the renderer's last stop-state judgement: `stop_target_not_active` and `stop_target_not_unique` were the two answers a client mirror could give while contradicting the Host, and the Gate's refusal now carries them. Reported by Astro-Han in review of apache#4439. Generated-by: Claude Opus
Moving delegation resolution into the Gate left both of its predecessors standing. A stop now derives its delegation from the active links, then proves the same rule twice more. The Gate's second pass is unreachable as a decision. Every path that gets there took `source` from `#stopSource`, which had just applied that rule to `listActiveAssignments()`; the replay branch returns before this line. So re-reading the transcript to check that `source` is active, and to ask whether anything competes with it, restates the answer the stop arrived with. `listSessions` and the supersession and replacement reads stay: those are facts `#stopSource` never established. Admission's set-equivalence check is the same duplication with a cost. It requires `admissionSessionIds` to name every Session holding an active delegation, so admitting one stop serializes delegation traffic across all of them, and a delegation to an unrelated Session arriving mid-admission fails a stop that cannot touch it. What decides is the narrowed proof under the lease — target visible, `source` still linked, competitors retired — and it reads only the target's own delegations. Together they took one stop from four reads of the append-only Coordination transcript to two: one to derive, one to reprove under the lease. The retirement rule now lives in `#stopSource` alone rather than in two places that must agree. Reported by Astro-Han in review of apache#4439. Generated-by: Claude Opus
…gation A claim with no request behind it resolves from the active links like a first attempt. That is only true while those links still name the delegation the claim bound itself to. If it left and another took its place, re-deriving picks the successor, the fingerprint no longer matches the claim, and `claimAction` refuses — permanently, because claims are never deleted, and without saying why. Nothing destructive happens either way and a fresh message carries a fresh action id, so this costs one refused attempt. Refuse it by name instead: the identity is spent, and it says so. Reported by Astro-Han in review of apache#4439. Generated-by: Claude Opus
Pin a stop action to its original target Session, remove redundant tests, and leave target visibility to the gate while the coordinator owns the lease-held delegation proof. Generated-by: OpenAI Codex
b0097b5 to
8f723e0
Compare
Astro-Han
left a comment
There was a problem hiding this comment.
Re-approving on 8f723e072, since my previous approval was anchored to 7a53194e9.
I diffed this head against b0097b5aa relative to each merge base: the net change is identical line for line except the epoch moving from 102 to 104 and the matching comment number. The rebase carried nothing else in.
Everything from the last round stands. The replay test pins the same userText and the same actionId against a second Session and asserts only the first delegation ever gets a stop request, so the claim fingerprint is no longer the only thing holding that path shut. Dropping visibleSessionIds in the coordinator, and the listHeaders() fetch with it, is the better of the two options I offered: target visibility now has one owner in the Gate rather than two checks that could drift. The competitor fixture leaves the source not_retired so it can actually distinguish itself from the sole-delegation refusal.
test and audit both green on this head, mergeable and clean. Merging.
简体中文
在 8f723e072 上重新批准,上一个绿勾锚在 7a53194e9。
我把这个 head 和 b0097b5aa 各自相对 merge base 的净改动逐行比过:完全一致,只有 epoch 从 102 变成 104 以及注释里对应的编号。rebase 没有夹带别的东西。
上一轮的结论都成立。重放那条测试用同一个 userText、同一个 actionId 打向第二个 Session,断言只有第一条委派拿到过 stop request,claim 指纹不再是唯一防线。协调器里删掉 visibleSessionIds 连同 listHeaders() 那次取数,是我给的两条路里更好的一条:目标可见性现在只有 Gate 一个权威,而不是两处会漂的检查。竞争者 fixture 让源保持 not_retired,这样它才真的能和「没有唯一活跃委派」那条拒绝区分开。
这个 head 上 test 和 audit 都绿,可合并且干净,我合了。
The stop test from #4439 read the run's status and abort source off the header. Both are the terminal event's to state, and the test already asserts on it. Generated-by: Claude Code
The stop test from #4439 read the run's status and abort source off the header. Both are the terminal event's to state, and the test already asserts on it. Generated-by: Claude Code
The stop test from #4439 read the run's status and abort source off the header. Both are the terminal event's to state, and the test already asserts on it. Generated-by: Claude Code
Summary
Add WorkHub Slice 5D: direct stop for one resolved, active durable delegation.
SessionResolverport that returns ranked candidates, nothing, or ambiguity — nevercreate_new, never an identity outside the caller's visible candidate setalready_terminalfor a Turn that is still runningdelegation_stop_requestedanddelegation_stop_resolvedfacts and project their outcomes into WorkHubnot_ownedasked to stop) without claiming execution completionArchitecture: the shared Session Resolver port
This addresses the architecture discussion on this PR. The pipeline is now:
SessionResolveris action-agnostic and injectable.createExactNameSessionResolveris a temporary deterministic baseline, removable once the replacement resolver passes the common routing evaluation and its rollout retains a tested rollback path.Stop and correction previously carried two near-identical copies of the same name-matching rule. They differed only in what may follow the name — a stop reference may carry punctuation and nothing else, a correction may name its target and go on to say what to do with it. That is an action's rule, not a retrieval rule, which is why folding both into the Resolver would have forced it to know which action it was serving. Instead one
matchWorkHubSessionNameowns matching and reports what text was left over; each action's policy judges that leftover. Both actions now resolve through the port.resolve()'s broader lexical retrieval is a different algorithm and joins the port with continue, inspect, and resume in the follow-up slice.Making the port real required moving admission off display names. Previously the Action Gate and the coordinator's under-lock recheck each re-parsed
userTextand proved uniqueness by matching Session names, which made presentation metadata the destructive authority and would have rejected any resolver that recalled a Session by anything else. Stop now proposes{ stopsActionId, expects: { targetSessionId } }and asserts no proof of its own; the Host proves from durable state that the assignment exists, still belongs to that Session, and that no other delegation there still holds stoppable work. A stale resolution fails closed; a rename between resolution and admission correctly no longer invalidates a claim.Destructive answers do not come from projections
WorkHub projections are rebuildable and can be empty when a window opens. Two stop refusals were decided from the renderer's delegation mirror and returned without ever calling the Action Gate, so a fresh window, a reload, or a reconnect produced a confident, specific, and false "there is nothing to stop". A new read-only
workhub.coordination.delegationsquery is asked once a reference resolves — an ordinary message never pays for it — and the Host answers which links still hold stoppable work, using the same judgement admission makes.Which sense of "active delegation" carries destructive authority
A delegation link ends only by supersession or a resolved stop, so work that simply finished stays linked. This slice is the first to let "active delegation" gate a destructive action, and it settles the question narrowly: the link stays active, but a delegation whose work has finished is not a competing stop target, because there is nothing left in it to stop. Execution state that cannot be read counts as competing, never as finished. No completion fact is added to the coordination log — target execution state remains an ordinary Session fact that WorkHub reads as a rebuildable projection.
Boundary change reviewers should weigh
The Action Gate no longer proves that trusted user text names the target.
Stop Loginpaired with a proposal pointing at the Payments delegation was previously rejected at the Gate; it now depends on the proposal being correct. What still holds:user_stopconfirmation andstopsActionIdare both outside strategy output, so no model proposal can authorize or retarget a Stop.This is the boundary the discussion proposed: the Gate owns existence, Host scope, ownership, active delegation identity, and the cancellation itself, while recall quality is the Resolver's concern. It is called out here because it is a deliberate trade, not a side effect.
stop_workproposal shapes are introduced by this PR's compatibility epoch (95), so refining the proposal needs no further epoch. Epochs through 94 remain assigned to changes already onmain.Verification
git diff --check, ASF license headers, knip at baseline, renderer architecture checkrechecks sole-delegation stop preconditions after the advisory active-link readworkhub-session-resolver.test.tsfor the port contract, andworkhub-session-resolver-port.test.ts, which swaps in a resolver that recalls by description instead of display name and shows stop producing the same typed opaque decision with no change to the durable protocolrecoveringrather thanalready_terminal; a stop answered with an unfilled delegation mirror still reaches the Action Gate; and thestoppedlink state joins the surface-flow tablenon-serving Runtime Host kernel,deleted account identity survives same-slug reuse until explicit recovery,two Clients share exact retryable Session branch and revision authority) pass in a smaller batch, andowned Host exits promptly after its first connection closesis intermittent even alone — its own comment notes its 8s election deadline and 500ms settle window are too tight under load. None touch WorkHub code.Review focus
The destructive authority remains delegation- and Message-centered, and is now identity-centered end to end: the renderer proposes only opaque identities and the state it resolved against; Session, Message, Turn, and Run identities are resolved and verified by Host-owned durable state. Pending cancellation and owning-root Stop each retain action-bound durable provenance across the request/resolution crash seam.
The most valuable review targets are the boundary change above, and the "active delegation" definition this slice settles.
docs/architecture/workhub-action-resolution-design.mdhas been deleted: its contract half duplicated the ADR and the domain-language file, and its narrative half belongs in discussion #3286.AI use
Select exactly one:
Tool(s) and scope: Codex implemented, tested, documented, rebased, and adversarially reviewed the original direct-stop change. Claude Opus was consulted during design planning, and implemented, tested, and documented the shared Session Resolver port and the identity-based stop admission.
Checklist
Does this PR entail a change in behavior?