Skip to content

feat(orchestration): retain native outcome purpose - #388

Merged
lukemaj merged 5 commits into
mainfrom
t3/native-outcome-purpose
Oct 19, 2026
Merged

lukemaj merged 5 commits into
mainfrom
t3/native-outcome-purpose

Conversation

@lukemaj

@lukemaj lukemaj commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

What: Record immutable Issue/PR purpose before run execution and retain supported native causal evidence across clients.
Why: A thread's changing selection and delegated job identity cannot establish which outcome a run serves.
So what: All review findings are fixed at 45eaaf2b74. Fable passed through 08877e706e and Luna passed the final delta; merging needs the developer, and two behaviour questions below are the user's to answer.

Implements #377 under the approved policy and design review in toolboxmd/agent-observer#58. Canonical state and handoff: #377.

Ordinary messages pin the visible selection and queue separately when it differs from active work. Steer and Restart retain their resolved target purpose. Versioned intent distinguishes explicit choice from warned legacy omission. Local and remote delegation, source continuations, schedules, watches and mobile outbox records carry that choice through existing owners. Human child follow-ups retain delegated job binding and parent delivery while owning their selected purpose.

Native input witnesses and child/background inventories retain exact observed causes. The contract advertises each adapter's supported evidence and keeps unresolved resume boundaries and unreconciled usage explicit. This PR does not claim universal native finality or implement Observer accounting. No package version changed.

Port onto current main

Main was replaced by the upstream absorption (#419). The four WIP checkpoints (987f5ad, 3d24575, f54a894, 62a127b on old base 118f89e) are now one commit on main.

CI fixes (old Test Server 4 and 5 failures)

  • Test Server 5 (ClaudeAutomaticDelivery "agent auto"): this PR's. Agent auto sends now carry deliveryIntent: "auto" so they are classified as ordinary sends that follow the selection. The new restart-capable steer branch then steered Claude while tools ran, and decideSteeringExecution rejected it. It now also requires activeSteeringInterruptsTools !== true, so the send queues as it does on main.
  • Test Server 4 (ClaudeAdapterV2 "never answers" steer timeout): a test race this branch's timing exposed. The repeated steer reads skills from the real filesystem before it waits on the test clock, and the fixed 100-tick budget could run out first, which left the fiber hanging. The test now advances the clock until the steer settles. It failed in 3 of about 12 full-file runs before the fix, passed 6 of 6 after, and passed 7 of 7 on main.
  • Test Web (first CI run of the port): the new ExecutionPurposeControl used try/finally, which the React Compiler coverage gate rejects. It now catches a failed save and shows the save error.
  • New main test from fix(scheduler): preserve bare read grants through task decorator #467 (ScheduledTaskChecks bare-grant decorator) saved a schedule without a purpose, which Track explicit Issue and PR purpose across runs and continuations #377 rejects for new configuration. The fixture now chooses unscoped, like this PR's other schedule fixtures.

Review round 1 fixes

Round 1 at 94945e31c7 (Luna, Grok) found two blocking defects. Both are fixed in 1eac47a278, each with a test that failed before the fix:

  1. Inheriting an unscoped parent failed. Orchestrator.ts, ThreadLaunchService.ts and RemoteDelegation.ts rejected inherit unless the parent scope was single, so a child of the default Unscoped run never started. inherit now carries the parent run's exact scope with parent_run provenance, every unscoped reason included. I kept no refusal for mixed or unknown_cause. Copying an unscoped scope assigns no outcome: the child stays unscoped with the same reason, and Observer does not allocate unscoped work. Refusing would only block the child's start, which contradicts "children inherit unless explicitly assigned otherwise". Tests:

    • ExecutionPurpose.integration.test.ts "a delegated child inherits a deliberately unscoped parent run exactly" (local delegated task)
    • ThreadLaunchService.test.ts "a launch inheriting a deliberately unscoped parent run keeps that exact scope" (agent thread launch)
    • RemoteDelegation.test.ts "...launched with inherit intent starts and continues" (remote)

    Before the fix these failed with inherit_source_missing locally and "requires the exact scoped parent run" remotely.

  2. A remote child launched without purposeIntent could not be continued. The launch records no purposeLaunch when intent is omitted, so that old peers without v1 keep working, and PeerForwarding then refused the follow-up. An omitted-intent follow-up of such a child now forwards with no continuation evidence and stays warned legacy unscoped there, as a local one does. Explicit inherit, or a recorded launch from another thread, still requires validated evidence. Test: RemoteDelegation.test.ts "...launched with omitted intent starts and continues", which failed before with "no validated purpose evidence".

  3. Non-blocking: Muse and OpenCode v2 now advertise nativeUsageFinality: "unavailable", matching their unsupported evidence.

Left for the developer, as the parent asked: purposeDeliveries kept after a PR unlink, and mobile Steer rejecting and restoring the draft when the run ended before the outbox drained.

Review round 2 fixes

Round 2 (Fable) confirmed the round 1 fixes and found two blocking items at the head. Both are fixed in 68b561d2f1, each with a test that fails before the fix.

  1. Run group cancel (touches feat(server): one delegation runs several Colors of a role #512's code). After the rebase, feat(server): one delegation runs several Colors of a role #512's own test showed a cancelled group waking the parent. task_cancel on a group stops each member and only then disposes its delivery. A member stopped that way can end before its own disposal lands; the other members are terminal and disposed, so it released the group wake. My first fix disposed before stopping, but that lost the wake of a member whose stop is refused (for example a child raised above the parent's modes). Now main's order is kept in OrchestratorMcpService.ts, and RunGroups.runGroupWake holds a member whose run ended on an explicit stop request while a sibling's delivery is disposed. Only a group cancel disposes a member's delivery, and its disposal follows. A refused member is never stopped, so its result still wakes the parent. Tests in RunGroups.integration.test.ts: the existing "cancelling the group stops every member and gives up its wake" fails without the rule (one parent offer), and the new "a member that refuses the group cancel keeps its result's wake" fails with the dispose-first order (the refused member's delivery is disposed).
  2. PR watches without purpose intent were rejected. Mobile has no watch purpose control and sends none, and agents may call watch_pull_request without purposeIntent, so starting a watch failed there. Following Track explicit Issue and PR purpose across runs and continuations #377's rule for omitted intent, a watch without intent now starts as warned legacy unscoped, and its wakes fire unscoped. A later re-watch without intent keeps a purpose saved earlier and its watch generation. The web PR panel still sends the thread's selected purpose. I chose this over adding a mobile watch choice because it is smaller and meets Track explicit Issue and PR purpose across runs and continuations #377: old clients and agents keep working, warned unscoped. The now-unused saved_intent_missing admission reason is removed. Test: ExecutionPurpose.integration.test.ts "watch wakes retain saved purpose..." now starts a watch without intent, and re-watches without intent after saving A. It failed before with "Choose an Issue, PR or deliberately unscoped purpose before saving this trigger."

Round 2 proof on 68b561d2f1, one file at a time: RunGroups.integration.test.ts 10, ExecutionPurpose.integration.test.ts 11, mcp/toolkits/pullRequests/handlers.test.ts 25, all passing. Server tsc-rs --noEmit alone shows no errors.

Review round 3 fixes

Round 3 (Luna, Fable) showed the round 2 group-cancel rule was order-dependent: a sibling ending before its disposal still woke the parent, a later user Stop of a refused member held its wake forever, and some loop orders left the gap open. It also found Test Server 8 red, and that mobile watches still recorded no choice. Fixed in 08877e706e, replacing round 2 items 1 and 2:

  1. Group cancel by design (touches feat(server): one delegation runs several Colors of a role #512's code). task_cancel on a Run group first commits delegated_task.group-cancel, recording its command id as groupCancel on every member before any member is stopped. While a member carries it, the group delivery planner suppresses its wake (a quiet disposed delivery), whenever and in whatever order it ends. Each member is then stopped as on main, and disposed after a successful stop. A member whose stop is refused gets delegated_task.group-cancel.release under the same id. That clears the mark and re-plans a result the member finished meanwhile, so its result, or a later user Stop, wakes the parent normally. Suppression keys only on that group-cancel id, never on a generic stop. The round 2 hold rule in runGroupWake is removed, and RunGroups.ts matches main again. feat(server): one delegation runs several Colors of a role #512's single-task and Run group contracts are otherwise unchanged. Tests in RunGroups.integration.test.ts:

    • "members ending after the group cancel is recorded never wake the parent": both members end after the mark, with no disposal.
    • "a member already finished when the group is cancelled stays quiet": a held finished member, later in the loop order.
    • "a refused member that the user later stops still wakes the parent".
    • "a single task stopped by the user still wakes the parent": unchanged behavior.
    • Kept from earlier rounds: "cancelling the group stops every member and gives up its wake" and "a member that refuses the group cancel keeps its result's wake".

    Against the round 2 server code, the refused-then-Stop case and the recorded-cancel case fail. The finished-member case passed there in this run, because that gap depends on run timing the MCP loop does not let a test force. The recorded-cancel test is its deterministic proof.

  2. SelectionRestart.integration.test.ts (Test Server 8). A receipt replay now also returns admittedRun. The assertion now checks the requirement: the same sequence and stored events as the first dispatch, and the same admitted run unless the message id changed.

  3. Mobile watches send the thread's choice. The shared createThreadPullRequestWatchAction in client-runtime now takes the thread's purpose selection when the environment advertises purpose v1, and saves it on a new watch. Web and mobile both pass it, and the web-only wrapper is gone. An agent calling watch_pull_request without intent stays warned legacy unscoped, as in round 2. Test: pullRequestLinkInput.test.ts "saves the thread's purpose choice on a new watch" (reference and default unscoped), which fails against the old action.

Left, non-blocking: t3_thread_send with inherit to a thread that is not the caller's child still requires a single-scoped caller run. Round 1 aligned children, agent launches and remote launches; this send path is unchanged.

Round 3 proof on 08877e706e, one file at a time, all passing:

  • RunGroups.integration.test.ts 14, SelectionRestart.integration.test.ts 13, DelegatedCompletionDelivery.test.ts 29, client-runtime pullRequestLinkInput.test.ts 39.
  • tsc-rs --noEmit shows no errors for server, client-runtime, web and mobile.
  • fork-check.sh is OK.

Review rounds 4 and 5

Round 4 at 08877e706e (Fable PASS, Luna FAIL) found one lost-wake case in the group-cancel design. Fixed in 45eaaf2b74; Luna passed that delta.

  • Group-cancel mark (design from round 3). task_cancel on a Run group first records its command id as groupCancel on every member, in one command. A marked member ends quietly in any order. A member whose stop is refused is released from the mark under the same id.
  • Last marked member releases the held ones. A refused member that had already ended when released is held for the group. When the last running member then ends under the mark, it stays quiet but plans the group's one wake for those held members. The wake commits with that command's receipt, so a replay plans nothing new.
  • Refused-already-finished re-plan. The release step also re-plans a refused member that ended before the cancel and was still held with no delivery, so its wake is not lost when every other member ends first.
  • SelectionRestart's messageId replay case now asserts admittedRun is undefined.

Test: RunGroups.integration.test.ts "a refused member that already ended wakes the parent once the last member stops" (the mark, the release while the sibling runs, then the sibling's stop; one parent wake). It fails against 08877e706e with no wake. Proof on 45eaaf2b74, one file at a time, all passing: RunGroups.integration.test.ts 15, SelectionRestart.integration.test.ts 13, DelegatedCompletionDelivery.test.ts 29. Server tsc-rs --noEmit shows no errors.

Proof on the head above

All runs are local node_modules/.bin/vp test run <files> with isolated fixtures. No dev server, browser, device or live userdata. The first table is from 94945e31c7; the round 1 reruns below it ran one file at a time on 1eac47a278.

Scope Result
Every changed server test file plus Orchestrator.test.ts, EffectWorker.test.ts, issues/featureMap.test.ts (23 files) 1020 passed
Untouched server schedule/MCP/peer files that use scheduled tasks (10 files) 128 passed
contracts full suite 750 passed
client-runtime commands, provider-opencode adapter, mobile outbox, web schedule logic 28, 75, 46, 22 passed
scripts/fork-maintenance.test.ts, issues/featureMap.test.ts 41 passed
web reactCompilerCoverage.test.ts 1079 passed
tsc-rs --noEmit for server, web, mobile, contracts, client-runtime, provider-core/opencode/cursor/muse/pi/acp no errors
vp fmt --check and vp lint on changed files clean; lint warnings per file identical to main
bash scripts/fork-check.sh OK (50 features)

Round 1 reruns on 1eac47a278, one file at a time: RemoteDelegation.test.ts 39, PeerForwarding.test.ts 4, ExecutionPurpose.integration.test.ts 11, ThreadLaunchService.test.ts 94, runtimeLayer.test.ts 81, ClaudeAutomaticDelivery.integration.test.ts 21, DelegatedCompletionDelivery.test.ts 25, OrchestratorMcpToolkit.integration.test.ts 3, mcp/toolkits/core.test.ts 17, OpenCode2AdapterV2.test.ts 120, provider-opencode adapter 75, provider-muse adapter 28, contracts executionPurpose.test.ts 3, fork-maintenance.test.ts 19, all passing. tsc-rs --noEmit shows no errors for contracts, provider-muse, provider-opencode, client-runtime, server, web and mobile. fork-check.sh is OK.

After the #492 rebase, run one file at a time on b72b01ef6a: DelegatedCompletionDelivery.test.ts (#492's own file) 29, ExecutionPurpose.integration.test.ts 11, ThreadStop.test.ts 49, Orchestrator.control-reads.test.ts 5, RestartContinuation.test.ts 51, SteeringCompletion.integration.test.ts 23, childThreads/requestWake.test.ts 9, all passing. Server tsc-rs --noEmit alone shows no errors, and fork-check.sh is OK.
CI: run 39209438398 on head 45eaaf2b74 was in progress when this body was written. Earlier green heads: b72b01ef6a run 39192217825, 1eac47a278 run 39188747888.

Visible UI changes

Yes, on web/desktop and mobile:

  • Web/desktop chat: a Purpose: <label> row above the composer when the environment advertises purpose v1. It expands to an Issue/PR URL field with Select and Clear, plus each run's purpose and provenance. When the running work has a different purpose, it shows a one-line note that Send queues and Steer keeps the running purpose.
  • Web settings → Scheduled tasks: a purpose control in the task form. New tasks default to deliberately unscoped. Saving an edit to a legacy task without a purpose prompts for a choice.
  • Mobile: a matching purpose control on the thread screen, a picker on the new-task draft, and one in Settings → Scheduled tasks.
  • New PR watches from the web PR panel and from mobile save the thread's selected purpose. An agent's watch_pull_request without intent starts warned legacy unscoped. Nothing visible changes there.

Before/after images are not attached: no browser or device verification was authorized for this pass. The parent owns the integrated UI pass.

Remaining limits

  • Reviews: Fable passed through 08877e706e; Luna passed the 45eaaf2b74 delta.
  • No integrated web/mobile UI pass ran, and no screenshots are attached.
  • Two behaviour questions are left for the user: mobile Steer after the run has ended rejects and restores the draft, and purposeDeliveries stay recorded after a PR is unlinked.
  • t3_thread_send with inherit to a thread that is not the caller's child still requires a caller run scoped to a single Issue or PR.
  • If the server crashes between recording a group cancel's mark and stopping its members, the members stay marked until task_cancel is retried with the same request, which repeats the stops and releases any refusals.
  • Native limits are unchanged from the canonical handoff: resumed native response boundaries, late usage/finality, and unsupported adapters (OpenCode v2, Muse, Cursor, Pi, ACP, Grok, Antigravity) remain unproved.
  • Observer Flaky mobile test: shikiReviewHighlighter highlightSourceFile warmup #105 stays blocked until this exact source candidate is reviewed; Show accurate child status in Lineage and thread tools #106 depends on it.

Elon record

Requirements and who asked: The user accepted the multiple-Issue purpose policy in Observer #58 and authorized the dedicated #377 implementation, focused proof, commits, push and one reviewable PR. The parent asked for this port onto the replaced main, made green and review-ready.
Deleted: Opaque outcome registry, purpose repair, prose classification, PR splitting, new transport/services, price work, version bumps and production changes. The four WIP checkpoints are collapsed into one commit. No mobile watch purpose control (mobile reuses the thread's selection), and no order-dependent hold rule.
Bottleneck: The developer's merge decision and the two user questions.
Checked myself: Line-level old vs new patch comparison, typechecks, a reproduction of both CI failures on this branch with main passing the same tests, the fixes, focused suites, lint/format parity with main, and fork-check. For rounds 1 and 2: a test that fails before each fix, and #512's own Run group tests against the rebased head.

Ported and finished by Claude Opus 5.5 (claude-opus-5-5) through Claude Code in T3 Code; original implementation by gpt-6.1-sol, Codex through T3 Code.

🤖 Generated with Claude Code

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:XXL labels Oct 16, 2026
@github-actions

github-actions Bot commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

Provider Metric Main baseline This PR Impact PR ceiling
Codex Total thread wire 5.0 KiB 4.9 KiB −23 B (−0.5%) 6.8 KiB ✅
Codex Thread snapshot wire 3.8 KiB 3.8 KiB 0 B (0.0%) 4.9 KiB ✅
Codex Live turn WebSocket wire 1.2 KiB 1.2 KiB −23 B (−1.9%) 2.0 KiB ✅
Codex Live turn WebSocket decoded 20.9 KiB 20.8 KiB −41 B (−0.2%) 29.3 KiB ✅
Codex Live turn messages 2 1 −1 (−50.0%) 8 ✅
Claude Total thread wire 5.0 KiB 5.0 KiB 0 B (0.0%) 6.8 KiB ✅
Claude Thread snapshot wire 3.8 KiB 3.8 KiB 0 B (0.0%) 4.9 KiB ✅
Claude Live turn WebSocket wire 1.2 KiB 1.2 KiB 0 B (0.0%) 2.0 KiB ✅
Claude Live turn WebSocket decoded 21.2 KiB 21.2 KiB 0 B (0.0%) 29.3 KiB ✅
Claude Live turn messages 2 2 0 (0.0%) 8 ✅

Baseline: 15d99fa · PR result: 45eaaf2 · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

  • Codex decoded thread snapshot: 108.5 KiB
  • Claude decoded thread snapshot: 108.8 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

lukemaj added a commit that referenced this pull request Oct 18, 2026
Pin a typed Issue or PR purpose on every admitted run, carry it through
children, continuations, remote launches, schedules, watches and the mobile
outbox, and retain supported native causal evidence per adapter (#377).

Ports PR #388's four checkpoints onto current main. Agent auto sends now
queue, as on main, when steering would cancel running Claude tools, and the
repeated Claude steer test waits for its real skill discovery instead of a
fixed tick budget.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@lukemaj
lukemaj force-pushed the t3/native-outcome-purpose branch from 62a127b to a948563 Compare October 18, 2026 22:20
lukemaj added a commit that referenced this pull request Oct 18, 2026
Pin a typed Issue or PR purpose on every admitted run, carry it through
children, continuations, remote launches, schedules, watches and the mobile
outbox, and retain supported native causal evidence per adapter (#377).

Ports PR #388's four checkpoints onto current main. Agent auto sends now
queue, as on main, when steering would cancel running Claude tools, and the
repeated Claude steer test waits for its real skill discovery instead of a
fixed tick budget.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@lukemaj
lukemaj force-pushed the t3/native-outcome-purpose branch from a948563 to e10b793 Compare October 18, 2026 22:29
lukemaj added a commit that referenced this pull request Oct 18, 2026
Pin a typed Issue or PR purpose on every admitted run, carry it through
children, continuations, remote launches, schedules, watches and the mobile
outbox, and retain supported native causal evidence per adapter (#377).

Ports PR #388's four checkpoints onto current main. Agent auto sends now
queue, as on main, when steering would cancel running Claude tools, and the
repeated Claude steer test waits for its real skill discovery instead of a
fixed tick budget.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@lukemaj
lukemaj force-pushed the t3/native-outcome-purpose branch from e10b793 to 94945e3 Compare October 18, 2026 23:05
@lukemaj
lukemaj marked this pull request as ready for review October 18, 2026 23:27
@lukemaj

lukemaj commented Oct 18, 2026

Copy link
Copy Markdown
Contributor Author

What: One blocking child inheritance gap remains in PR #388.
Why: Explicitly unscoped parent runs cannot be inherited by local child launches.
So what: Allow exact inheritance of unscoped purpose, then add focused coverage before approval.

Finding

No tests or typechecks were run, as requested.

Verdict: failure (blocking finding).

@lukemaj

lukemaj commented Oct 18, 2026

Copy link
Copy Markdown
Contributor Author

What: Exact-head review of 94945e31 fails. A remote child started without a purpose intent cannot be continued, and inheriting the default unscoped run does not start a child.
Why: Omitted intent is specified as legacy unscoped so existing agents keep working, but the remote path treats that omission as missing evidence and refuses the send.
So what: Fix those two admission paths and re-review this head. Merging stays with the developer. Verdict: FAIL.

Reviewed read-only at head 94945e31c704be5c42fad2d283166fcc052b72af, base 7f300491d3604af50c37001f0cfd1fea45114dc2, against #377 and Observer #58. No tests or typecheck were run.

Blocking

  1. apps/server/src/peer/PeerForwarding.ts:186 and apps/server/src/peer/RemoteDelegation.ts:296. An agent calls delegate_task on a linked environment and omits purposeIntent, which is optional and defined as legacy unscoped. RemoteDelegation then stores delegatedFrom with no purposeLaunch. The child still starts. The next t3_thread_send with omitted intent or inherit, and mode auto or queue, finds that one remote child and returns invalid_request: "The original remote delegated launch has no validated purpose evidence." Nothing is forwarded. The same omitted follow-up on a local child inherits the original run and continues (Orchestrator.ts same-job follow-up). Track explicit Issue and PR purpose across runs and continuations #377 says an absent protocol warns unscoped so running agents continue. RemoteDelegation.test.ts only locks the failure for a launch that had purposeLaunch and then had it removed. That does not cover a launch that never recorded one, which is every current agent. Pre-upgrade remote children fail the same way after this build.

  2. apps/server/src/orchestration-v2/Orchestrator.ts:7386 and apps/server/src/peer/RemoteDelegation.ts:253. The composer default is Unscoped. A parent run admitted with that scope, or with explicit unscoped intent, is a real run. delegate_task with purposeIntent inherit (the versioned way the schema describes as inheriting the caller run) fails before the child exists: local inherit_source_missing ("Purpose inheritance requires the exact owning execution."), remote "Purpose inheritance requires the exact scoped parent run." The parent run is present. Its scope is just not single. A scoped parent inherits. The default chip does not.

Non-blocking

  1. packages/provider-muse/src/server/adapter.ts:98 and packages/provider-opencode/src/server/v2/adapter.ts:81. Both advertise nativeUsageFinality: "main_agent_only" while native inputs, child segments, and background inventory are unsupported. ACP, Cursor, Pi, Grok, and Antigravity say unavailable. A consumer can treat Muse or OpenCode v2 main-agent usage as a finality class this PR does not record. Grok and Antigravity do advertise unsupported evidence, via the ACP capabilities spread.

  2. apps/server/src/orchestration-v2/Orchestrator.ts:3421. purposeDeliveries is appended on PR link and kept on unlink, and ProjectionStore copies it onto every thread shell. The shell list is the hot websocket payload. The array grows by one object per distinct PR and purpose for the life of the thread. The new Purpose row is a collapsed button over data ChatView already holds. Its render cost is not material.

Checked, not a defect

CommandPolicy.ts:254 keeps the Test Server 5 fix. Claude's activeSteeringInterruptsTools is true, so an agent auto send queues. ACP and Cursor still steer through the new interrupt-restart branch, which matches main, where those sends omitted deliveryIntent and the requested steer was kept. The 16 conflicted files do not drop main behavior apart from the mobile steer encoding below. Web and desktop share ChatView. Clear writes unscoped deliberate and the label updates. Admitted run purpose stays immutable. A late mobile rejection is OrchestrationDispatchCommandError, so the outbox restores the draft instead of retrying. New schedules require a choice. A null purpose_intent_json still fires as legacy, and enabling that legacy row requires a choice. Codex, Claude, and OpenCode v1 record the evidence their capability flags name. The provenance union and native segments are the #58 contract, not spare machinery. purposeDeliveries on the shell is the part no UI reads yet.

Decisions for you

  • Mobile Steer now sends dispatchMode: "steer" (apps/mobile/src/state/use-thread-composer-state.ts:683), and that change is not gated on purpose v1. If the run has ended before the outbox drains, the server rejects it and the draft comes back. Main degraded that send into the next message. Confirm the reject-and-restore behavior.
  • purposeDeliveries stay after the PR is unlinked. Confirm that is a permanent declaration, not navigation state.
  • If inherit of an unscoped run is supposed to be refused, say so and fix the error text. As written against Track explicit Issue and PR purpose across runs and continuations #377, that refusal is blocking finding 2.

Verdict: FAIL

lukemaj added a commit that referenced this pull request Oct 19, 2026
Pin a typed Issue or PR purpose on every admitted run, carry it through
children, continuations, remote launches, schedules, watches and the mobile
outbox, and retain supported native causal evidence per adapter (#377).

Ports PR #388's four checkpoints onto current main. Agent auto sends now
queue, as on main, when steering would cancel running Claude tools, and the
repeated Claude steer test waits for its real skill discovery instead of a
fixed tick budget.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
lukemaj added a commit that referenced this pull request Oct 19, 2026
…te children

Review round 1 on #388:
- `inherit` now carries the parent run's exact scope, unscoped included,
  locally, through thread launch and through remote delegation.
- A remote child launched without purpose intent records no launch
  evidence; its omitted-intent follow-up now continues as warned legacy
  unscoped, as a local one does, instead of being refused.
- Muse and OpenCode v2 advertise unavailable usage finality, matching
  their unsupported evidence.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@lukemaj
lukemaj force-pushed the t3/native-outcome-purpose branch 2 times, most recently from 1eac47a to b72b01e Compare October 19, 2026 02:55
lukemaj added a commit that referenced this pull request Oct 19, 2026
Pin a typed Issue or PR purpose on every admitted run, carry it through
children, continuations, remote launches, schedules, watches and the mobile
outbox, and retain supported native causal evidence per adapter (#377).

Ports PR #388's four checkpoints onto current main. Agent auto sends now
queue, as on main, when steering would cancel running Claude tools, and the
repeated Claude steer test waits for its real skill discovery instead of a
fixed tick budget.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
lukemaj added a commit that referenced this pull request Oct 19, 2026
…te children

Review round 1 on #388:
- `inherit` now carries the parent run's exact scope, unscoped included,
  locally, through thread launch and through remote delegation.
- A remote child launched without purpose intent records no launch
  evidence; its omitted-intent follow-up now continues as warned legacy
  unscoped, as a local one does, instead of being refused.
- Muse and OpenCode v2 advertise unavailable usage finality, matching
  their unsupported evidence.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@lukemaj

lukemaj commented Oct 19, 2026

Copy link
Copy Markdown
Contributor Author

FYI from the #393 planner: PR #512 (#402, Run groups) changes apps/server/src/orchestration-v2/Orchestrator.ts (about 159 lines: a delegated_task.group.request command case, member id seeding, and a wrapper around delegated completion wake planning, which gains a siblingIds field). If #512 lands first, expect a rebase overlap there.

@lukemaj
lukemaj force-pushed the t3/native-outcome-purpose branch from b72b01e to 0e8faab Compare October 19, 2026 04:11
lukemaj added a commit that referenced this pull request Oct 19, 2026
Pin a typed Issue or PR purpose on every admitted run, carry it through
children, continuations, remote launches, schedules, watches and the mobile
outbox, and retain supported native causal evidence per adapter (#377).

Ports PR #388's four checkpoints onto current main. Agent auto sends now
queue, as on main, when steering would cancel running Claude tools, and the
repeated Claude steer test waits for its real skill discovery instead of a
fixed tick budget.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
lukemaj added a commit that referenced this pull request Oct 19, 2026
…te children

Review round 1 on #388:
- `inherit` now carries the parent run's exact scope, unscoped included,
  locally, through thread launch and through remote delegation.
- A remote child launched without purpose intent records no launch
  evidence; its omitted-intent follow-up now continues as warned legacy
  unscoped, as a local one does, instead of being refused.
- Muse and OpenCode v2 advertise unavailable usage finality, matching
  their unsupported evidence.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@lukemaj

lukemaj commented Oct 19, 2026

Copy link
Copy Markdown
Contributor Author

What: Round 2 review of 8cc6563bce437ba35e820189fdb5646b32a65737 found no blocking defects.
Why: The two round-1 admission failures are fixed with focused regressions, and inheritance provenance remains validated.
So what: The reviewed candidate has no review blocker; no tests or typechecks were run, as requested.

Reviewed exact SHA 8cc6563bce437ba35e820189fdb5646b32a65737 against origin/main 15d99fa1dae44f8de557d8c766ae68c472c55fdb. Read-only review; no checkout changes. I did not run tests, typechecks, or commit statuses.

Findings

None.

Round-1 blockers checked

  • Unscoped inheritance: fixed. apps/server/src/orchestration-v2/Orchestrator.ts now resolves inherit directly from the exact parent run without restricting its scope to single. ThreadLaunchService.ts likewise accepts the exact recorded parent run and copies its scope with parent_run provenance. Added regressions cover the local delegated-task path, launch-service path, and remote inherited launch; each expects the parent’s exact deliberate-unscoped reason and provenance. These trigger the former guards directly.
  • Remote legacy continuation: fixed without weakening explicit validation. apps/server/src/peer/PeerForwarding.ts:186-198 skips continuation evidence only when the matching origin-recorded remote child has no purposeLaunch and the follow-up omits purposeIntent. If evidence exists, or intent is explicit (including inherit), it still requires recorded evidence whose origin thread matches. Destination validation remains in place. RemoteDelegation.test.ts:1017-1087 covers omitted-intent legacy launch and explicit inherit launch of an unscoped parent, then continues both; the existing evidence-removal case still expects refusal. No incorrect parent-run provenance was found.
  • Usage finality: fixed. Muse and OpenCode v2 now advertise nativeUsageFinality: "unavailable", consistent with their unsupported native evidence.

Rebase checks

The exact candidate retains main’s stop-disposed follow-up wake behavior in Orchestrator.ts (including disposedBy: "stop" reopening and resultTask handling), and RemoteDelegation.ts retains Prism role admission before recording/forwarding. The PR panel’s existing unlink and watch actions remain present around its purpose-intent addition. scripts/fork-upstream-edits.txt retains the prior entries and appends the execution-purpose block. The added regressions use direct assertions and TestClock.adjust; no sleeps or polling were added.

Verdict: SUCCESS (no blocking finding).

lukemaj and others added 2 commits October 19, 2026 07:27
Pin a typed Issue or PR purpose on every admitted run, carry it through
children, continuations, remote launches, schedules, watches and the mobile
outbox, and retain supported native causal evidence per adapter (#377).

Ports PR #388's four checkpoints onto current main. Agent auto sends now
queue, as on main, when steering would cancel running Claude tools, and the
repeated Claude steer test waits for its real skill discovery instead of a
fixed tick budget.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…te children

Review round 1 on #388:
- `inherit` now carries the parent run's exact scope, unscoped included,
  locally, through thread launch and through remote delegation.
- A remote child launched without purpose intent records no launch
  evidence; its omitted-intent follow-up now continues as warned legacy
  unscoped, as a local one does, instead of being refused.
- Muse and OpenCode v2 advertise unavailable usage finality, matching
  their unsupported evidence.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@lukemaj
lukemaj force-pushed the t3/native-outcome-purpose branch from 0e8faab to 7bf9c74 Compare October 19, 2026 05:27
@lukemaj

lukemaj commented Oct 19, 2026

Copy link
Copy Markdown
Contributor Author

What: Round 2 review of exact SHA 8cc6563bce437ba35e820189fdb5646b32a65737 fails: the rebase against #512 dropped a closing brace in Orchestrator.ts, so the server at this SHA does not parse.
Why: The round-1 fixes themselves are correct, but the reviewed SHA is not the PR head; the head 7bf9c7484e restores the brace and also changes main's Run group cancel order without proof.
So what: Review the head as a new candidate after the cancel-order change is justified or reverted; merging stays with the developer. Verdict: FAIL.

Reviewed read-only against origin/main 15d99fa1da, #377 and Observer #58. No checkout, tests or typecheck. The only tool run was a single-file esbuild syntax parse of Orchestrator.ts at both SHAs. This replaces the Grok roast that could not run.

Blocking at 8cc6563bce

  1. apps/server/src/orchestration-v2/Orchestrator.ts:7721. The }); that closes main's dispatchDelegatedTaskGroupRequest (feat(server): one delegation runs several Colors of a role #512) is missing; resolveSubagentFollowup starts inside its body and the file never closes. esbuild: Unexpected "export" at line 13836. The server does not build, so no proof in the PR body applies to this SHA. git diff 7bf9c7484e 8cc6563bce shows the head restores exactly that });, and the head's Build and Typecheck job passed.
  2. apps/server/src/mcp/RunGroups.integration.test.ts:825. Main's feat(server): one delegation runs several Colors of a role #512 test expects a fresh delegate_task result to carry every OrchestratorMcpDelegateTaskResult field except remoteChild. This PR adds optional latestTerminalPurpose, absent until a later run ends, so the test fails at this SHA. The head filters that key. Same root cause: this SHA was never run against main's feat(server): one delegation runs several Colors of a role #512 tests.

Blocking at the head 7bf9c7484e (the candidate actually on the PR)

  1. apps/server/src/mcp/OrchestratorMcpService.ts:2047. The head reorders main's cancelTaskOrRunGroup to dispose each member's completion delivery before single.cancelTask; main cancels first and disposes on success. Scenario: a Run group of two, and member B's child thread was raised above the parent's modes, so assertStoppable refuses the cancel. On main the refusal leaves B's delivery intact and B's result still wakes the group. At the head B's delivery is disposed before the refusal, B keeps running, and its result never wakes the parent. This is outside Track explicit Issue and PR purpose across runs and continuations #377, has no test, and the PR body does not mention it. Either restore main's order and fix the cause in the orchestrator, or justify it with a failing-before test.

Round-1 blockers, checked independently (code as at the head, where it builds)

  • Unscoped inherit: fixed. Orchestrator.ts and RemoteDelegation.ts:263 drop the scope.type !== "single" guards; resolvePurposeIntent copies the parent run's exact scope with parent_run provenance. ThreadLaunchService.ts:1051 only refuses a missing run. The three added tests each drive the former guard (local delegate, agent launch, remote launch of a deliberate-unscoped parent) and assert the exact scope and provenance, so they fail before the fix by construction.
  • Legacy remote continuation: fixed without a bypass. PeerForwarding.ts:186-198 skips evidence only when the origin's own task record for that child has no purposeLaunch and the follow-up omits intent. Checked: a caller-supplied delegatedContinuation is refused at the origin and, at the destination, requires the link session that launched the thread plus an exact launch match; a follow-up from a thread that did not launch the child has no task record, forwards plain, and is admitted legacy unscoped there, never with the original purpose; explicit inherit without evidence is refused at both ends. Non-blocking asymmetry: an explicit inherit follow-up to a legacy remote child is refused, while the same follow-up to a legacy local child continues with delegated_launch provenance.
  • Muse and OpenCode v2 advertise nativeUsageFinality: "unavailable".

Rebase and rule checks

  • Orchestrator.ts keeps fix(server): a follow-up wakes the parent after a Stop caught its task running #492's stop-disposed wake logic (explicitlyStopped, observedDelivery) under the resultTask rename, and feat(server): one delegation runs several Colors of a role #512's group request and siblingIds apart from finding 1. RemoteDelegation.ts keeps Prism role admission before purpose resolution. ThreadPullRequestsPanel.tsx only wraps watch; unlink and watch actions are unchanged. scripts/fork-upstream-edits.txt appends one block.
  • Decide stays pure: the new admission code reads projections and the clock only. No sleeps or polling in the added test lines.
  • Every surface: web and mobile expose select and clear, schedules and watches require a choice, MCP carries intent, legacy omission warns. Reverse states: Clear writes deliberate unscoped; unwatch exists; the delivery declaration has no removal, which is the developer's open question.

Non-blocking

  • The run-purpose immutability check runs twice per run.updated (a projection read in emit, then a SQL read in the projector), and subagent and provider-thread upserts now always read the prior row. Point reads inside the same transaction; worth collapsing later, not now.
  • The PR body still names head b72b01ef6a. On the head, CI's Test job failed in packages/ssh/src/tunnel.test.ts:1188, which this PR does not touch, and the server shards were still pending at review time, so "CI green" cannot be claimed for the head yet.

Verdict for 8cc6563bce437ba35e820189fdb5646b32a65737: FAIL.

Review round 2 on #388:
- Group cancel keeps main's order (stop, then dispose on success). A member
  stopped by that cancel and ending before its own disposal no longer
  releases the cancelled group's wake; a member that refuses the cancel is
  never stopped, so its result still wakes the parent.
- A PR watch started without purpose intent (mobile, older agents) is saved
  as warned legacy unscoped instead of being rejected; re-watching without
  intent keeps a purpose saved earlier.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@lukemaj

lukemaj commented Oct 19, 2026

Copy link
Copy Markdown
Contributor Author

What: Reviewed PR #388 at 68b561d2f18d1934eeb67d2157c44e018764e359 and found one blocking cancellation race.
Why: A group member can release the parent wake before whole-group cancellation records sibling wake disposal.
So what: Hold or otherwise suppress that wake across this ordering before merging.

  • Blocking — apps/server/src/orchestration-v2/RunGroups.ts:72-80. During cancelTaskOrRunGroup, members are stopped and disposed sequentially. If the currently stopped member ends while another member has independently just become terminal, but that sibling's completionDelivery.dispose has not run yet, stopRequested is true while no sibling has completionDelivery.state === "disposed". runGroupWake therefore returns release, and the parent receives a wake despite group cancellation giving up the group's wake. The later disposal cannot retract the accepted delivery. The current guard covers disposal-before-end, but not the requested sibling-disposal-after-end ordering.

Verdict: FAILURE

@lukemaj

lukemaj commented Oct 19, 2026

Copy link
Copy Markdown
Contributor Author

What: Round 3 review of exact SHA 68b561d2f18d1934eeb67d2157c44e018764e359 fails: the head's own CI run has Test Server 8 red on a main test that this PR's receipt-replay admittedRun breaks, and the PR-watch relaxation does not meet #377's "new/edited configuration requires choice" for mobile.
Why: Both round-2 blockers are fixed as claimed (main's cancel order is back and the refused member keeps its wake), but the candidate was never run against SelectionRestart.integration.test.ts from #478, and mobile still sends no watch purpose although it has the selection.
So what: Fix the replay test for the requirement it protects, make mobile send the thread's selection like web (or get the developer's explicit waiver of the #377 line), rerun the flaky Test job, then re-review the new head. Merging stays with the developer. Verdict: FAIL.

Reviewed read-only against origin/main 15d99fa1da (gh pr diff, git diff origin/main 68b561d2f1, git show 68b561d2f1:<path>, job logs). No checkout, tests or typecheck. The five changed server files parse with a single-file esbuild run. gh pr view 388 --json headRefOid returned this SHA before and after the review.

Blocking

  1. CI Test Server 8 fails on this head: apps/server/src/orchestration-v2/SelectionRestart.integration.test.ts:533. Three of the four replays an accepted restart while replacement startup is held cases (unchanged, mode, legacy) fail; main 15d99fa1da passes them (run 39201584901). Cause: this PR's Orchestrator.ts:12766 admissionResult attaches admittedRun to a receipt replay when the stored message.updated for command.messageId resolves a run, so replay.dispatch is { sequence, storedEvents, admittedRun } while the test expects exactly { sequence, storedEvents }. The messageId case passes only because a changed message id finds no stored message. The replay still stores no new events and the receipt is unchanged, so the behavior is consistent with the first dispatch; the test froze the result shape. Per the verification rule, rewrite the assertion to the requirement (same sequence, same stored events, and admittedRun equal to the original run's purpose) rather than only pasting the new output. This test landed on main in fix(server): replay accepted sends during replacement startup #478, which the PR body lists among the rebases, but the round-2 proof table does not include the file.
  2. PR watches without intent do not meet Track explicit Issue and PR purpose across runs and continuations #377 for mobile: apps/mobile/src/features/threads/useLinkedPullRequestActions.ts:156. Track explicit Issue and PR purpose across runs and continuations #377 acceptance: "absent protocol warns unscoped so running agents continue. Old schedules/watches fire unscoped; new/edited configuration requires choice." Mobile speaks purpose v1 (it has the thread purpose control), yet its watch action sends no purposeIntent, and since Orchestrator.ts no longer rejects that, a new mobile watch is new configuration recorded without a choice. The same PR still enforces the choice for schedules (ScheduledTaskService.ts:1071 and :1225), so watches and schedules now disagree. Smallest fix that fits Track explicit Issue and PR purpose across runs and continuations #377: attach savedPurposeIntentForScope(purposeSelectionOf(thread).scope) in the shared createThreadPullRequestWatchAction (packages/client-runtime) when the environment advertises v1, which also removes the web-only wrapper in ThreadPullRequestsPanel.tsx:453. Whether an agent's watch_pull_request call without the optional field counts as "absent protocol" (warned legacy, as the PR does) or should default to the caller thread's selection is the developer's call; the PR body states the relaxation openly, so if the developer waives this line, say so on Track explicit Issue and PR purpose across runs and continuations #377 and this item clears.

Non-blocking

  1. apps/server/src/orchestration-v2/RunGroups.ts:72. The hold rule is keyed on "the stop came from a group cancel" but can only observe "a stop was requested". After a partially refused group cancel (stoppable member disposed with disposedBy undefined, refused member still running), a user pressing Stop on the refused member's thread, or the parent calling task_cancel on that member after lowering its modes, ends its run with a run_interrupt_request, so runGroupWake holds forever and the parent is never woken. On main the result would release. The new test covers the refused member completing, not being stopped later. Small consequence (the parent can still task_status), but it contradicts the comment's "a refused member ... still wakes the parent".
  2. apps/server/src/mcp/OrchestratorMcpService.ts:2047. The gap race is closed only when the sibling is already disposed. With a two-member group where the second member already completed and is held (status terminal, no delivery) and the first is running, the loop cancels the running member first; if its run ends before its own dispose lands, runGroupWake sees a terminal undisposed sibling and releases, so the cancelled group still wakes the parent. Pre-existing on main, narrowed here, but the PR body's "closed" is overstated. Handling already-terminal members before running ones in the loop closes this ordering.
  3. apps/server/src/mcp/OrchestratorMcpService.ts:3397. t3_thread_send with inherit to a thread that is not the caller's subagent still refuses unless the caller's active run is single, while delegated children, agent launches and remote launches now inherit an unscoped parent exactly (round 1). Inconsistent with "children inherit unless explicitly assigned otherwise"; not a round-1 regression because that fix was scoped to children.
  4. CI Test job: packages/ssh/src/tunnel.test.ts replaces a managed server that exited but was not reaped fails (expected '' to include 'Z'). The PR does not touch packages/ssh; main and the earlier head b72b01ef6a pass it. Looks environmental; rerun and confirm before claiming green.

Verified at this SHA

  • Round-2 item 3 fixed. OrchestratorMcpService.ts:2044-2080 has no diff against main: each member is stopped, and only a successful stop disposes its delivery under Effect.tap. A refused stop (assertStoppable or the dispatch-mode limit) leaves the member's delivery intact, and the new test a member that refuses the group cancel keeps its result's wake would fail at 7bf9c7484e's dispose-first order by construction (it asserts the refused member's delivery is not disposed and one parent offer).
  • Gap race for two stopped members closed. Cancel loop is sequential: A stop, A dispose, B stop, B dispose. If B's run ends before B's dispose, A is disposed with disposedBy undefined and B carries a run_interrupt_request, so runGroupWake holds; B's dispose then lands. The existing cancel test asserts both disposed and zero offers.
  • What the rule does not touch. A non-group task returns single first. A parent Stop cascade disposes the cohort with disposedBy: "stop" (Orchestrator.ts:2478, :11434) and does not match. Archive (:3824) and a cancelled queued delivery (:9189) dispose with disposedBy undefined, but planDelegatedCompletionDelivery would dispose that result anyway, so no wake is lost. No web or mobile code dispatches delegated_task.completion-delivery.dispose. Remote children never carry runGroupId, and the remote and wake-policy callers of planRunGroupCompletionDelivery pass no stopRequested, so they behave as on main. stopRequested derives from persisted turn items, so replay is deterministic. runGroupWake stays pure; the turn-item read sits in the effect-side follow-up resolver as on main.
  • Watches. Orchestrator.ts:3573 starts a new watch generation only when an intent is supplied and differs; an omitted intent spreads existing.watch, so a saved purpose and startedAt survive a re-watch, and :2713 keeps it through host sync. Web always sends the thread's selection, so an edited web watch cannot omit the choice. saved_intent_missing has no remaining reference. The rewritten test fails before the fix because the omitted-intent dispatch previously failed.
  • Round-1 blockers still fixed. The third commit touches six files, none of them ThreadLaunchService.ts, RemoteDelegation.ts or PeerForwarding.ts; PeerForwarding.ts:186-198 still forwards an omitted-intent follow-up of a legacy remote child without evidence and refuses explicit inherit without it; ExecutionPurpose.ts:100 still copies the inherited purpose exactly.
  • Rules. No sleeps or polling in the added tests (only TestClock.adjust in the schedule test). Conflict resolutions from round 2 are untouched by the third commit. The PR body is complete and names this head.

Out of scope as agreed: mobile Steer after run end, purposeDeliveries after unlink.

Verdict for 68b561d2f18d1934eeb67d2157c44e018764e359: FAIL.

Review round 3 on #388:
- A whole-group task_cancel first records its id on every member in one
  command. While a member carries it, its wake is suppressed whenever and in
  whatever order it ends. A member whose stop is refused has the mark
  cleared under the same id, and a result it finished meanwhile is
  re-planned, so its result and any later user Stop wake the parent. The
  order-dependent hold rule in runGroupWake is removed.
- The shared client-runtime watch action saves the thread's purpose choice
  on a new watch, so mobile and web both send it; the web-only wrapper is
  gone. Agents calling watch_pull_request without intent stay warned legacy
  unscoped.
- SelectionRestart's replay assertion checks the replayed sequence, events
  and admitted run instead of the exact result shape.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@lukemaj

lukemaj commented Oct 19, 2026

Copy link
Copy Markdown
Contributor Author

What: At 08877e706ee8fbe0ba74a5b210c8a89477b323bf, group cancellation can still lose the only parent wake when a refused member finishes before release and other members are still running.
Why: dispatchRunGroupCancelRelease clears that member’s groupCancel marker at apps/server/src/orchestration-v2/Orchestrator.ts:8520-8542; if its suppressed terminal result is detected, it replans immediately, but planRunGroupCompletionDelivery sees nonterminal siblings and returns a hold. The marker is already gone, while later marked siblings finish quietly, leaving no member to release the group.
So what: Please preserve/retry the pending wake until the group can release, and cover the interleaving where the refused member is already terminal while a sibling is still running.

I did not run tests, as requested. The release/wake path above is traced from the pushed source.

Verdict: FAILURE

@lukemaj

lukemaj commented Oct 19, 2026

Copy link
Copy Markdown
Contributor Author

What: Round 4 review of exact SHA 08877e706ee8fbe0ba74a5b210c8a89477b323bf passes: both round 3 blockers are fixed as claimed, and the group-cancel redesign suppresses a cancelled member's wake in every ordering I could construct.
Why: The mark is committed in its own transaction before any stop, the planner keys suppression only on that mark, a refused member is released under the same id, and CI is green on this head (Test, Test Server 1-8, Web, Lint, Build and Typecheck).
So what: Mergeable from my side; two non-blocking wake gaps below are pre-existing on main and can go to a follow-up Issue. Merging stays with the developer. Verdict: PASS.

Reviewed read-only against origin/main 15d99fa1da (gh pr diff, git show 08877e706e, git diff origin/main 08877e706e -- <path>, CI rollup). No checkout, tests or typecheck. The ten changed server, contracts, client-runtime, web and mobile files parse with a single-file esbuild run. gh pr view 388 --json headRefOid returned this SHA before and after the review.

Verified at this SHA

  • Mark before stop. OrchestratorMcpService.ts:2050-2070 dispatches delegated_task.group-cancel with a stable id (cancel-run-group + request key) and fails the call if it does not commit; only then does the loop run. The loop itself is main's code plus one Effect.tapError release (:2079-2101); the stop-then-dispose order and the first-refusal-wins result are unchanged against origin/main:2016-2062.
  • Suppression is order-free. Orchestrator.ts:11497-11510 returns a quiet disposed delivery for any member carrying groupCancel, before runGroupWake runs, so a member ending before or after its own dispose, before or after a sibling, stays quiet. RunGroups.ts has no diff against main (git diff --stat is empty). The three new orderings in RunGroups.integration.test.ts wait on the stored event stream and a Deferred, with no sleep.
  • Refused member release. Orchestrator.ts:8485-8585 clears the mark only when the stored id equals the command's groupCancelId; otherwise it re-emits the row unchanged (the same idempotent no-op pattern the dispose handler uses). A member quieted while marked (terminal + disposed + no disposedBy) is un-quieted and re-planned through planRunGroupCompletionDelivery, and :13445 now runs offerDelegatedCompletionDeliveries after the release commits, so the re-planned delivery is actually offered. A refused member later stopped by the user ends unmarked, so runGroupWake releases as on main (test "a refused member that the user later stops still wakes the parent", one offer).
  • Idempotent replay. Both commands carry request-key-derived ids. A retried task_cancel with the same clientRequestId hits the receipt path (Orchestrator.ts:12985-13010) and replays stored events for the mark, each stop, each dispose and each release; nothing is re-planned twice. A retry without clientRequestId gets a fresh key (OrchestratorMcpService.ts:1347), re-marks under the new id, and a refusal releases under that same new id. A second release for an id no longer on the row is the no-op branch.
  • User Stop of a marked member. explicitlyStopped (Orchestrator.ts:11895) still needs followUp, so a first-result user Stop goes through the planner and stays quiet while marked. That is the cancelled-group intent; a refused member is unmarked inside the same task_cancel call, so the only way to Stop a marked member whose cancel was refused is the crash window in item 1 below. A parent Stop cascade (:2464-2486) spreads the task and disposes with disposedBy: "stop", so the mark survives and the planner still returns quiet.
  • Remote members. runGroupId appears only in OrchestratorMcpService.ts, RunGroups.ts and Orchestrator.ts; RemoteDelegation.ts and PeerForwarding.ts never set it, so no member of a group lives in a linked environment and cancelTask's childThreadId === null branch is unreachable for members.
  • Wire contract and older clients. OrchestrationV2Subagent.groupCancel is Schema.optional; the two commands sit in OrchestrationV2InternalCommand, so clients cannot send them. Subagent rows persist as whole payloads (no column references to runGroupId or completionDelivery outside the orchestrator), and the installed effect-smol decoder strips unknown keys by default (.repos/effect-smol/packages/effect/src/SchemaAST.ts:463), so an older web or mobile build ignores the field. Against the previous head, packages/contracts/src/orchestrationV2.ts changes only by the field and the two commands.
  • Purity. Both new dispatchers do what the other delegated-task dispatchers do: projection reads through projectionStore and DateTime.now, then events into the Ref. The release's re-plan reuses planDelegatedCompletionDelivery, whose only effects are projection reads in completionWakeDetail. Lock resolution lists both commands under parentThreadId (ThreadManagementService.ts:117-118).
  • Client-runtime watch. pullRequestLinkInput.ts:141-170 attaches purposeIntent only when watching is true and the caller supplied purpose; web (ThreadPullRequestsPanel.tsx:445-452) and mobile (useLinkedPullRequestActions.ts:43,160) supply it only when executionPurposeVersions includes 1, so non-v1 environments receive the main-shaped input. These two are the only client callers of the watch RPC. presentThreadShell now carries purposeSelection (models.ts:86,233), so the mobile selection is real, not undefined-by-construction. The new pullRequestLinkInput.test.ts cases assert both the reference and the default-unscoped intent and the bare unwatch input.
  • SelectionRestart assertion. :533-537 checks sequence, stored events and, except for a changed message id, the admitted run. Round 3 item 1 is met.
  • Earlier fixes intact. Commit 4 touches 11 files, none of them ThreadLaunchService.ts, RemoteDelegation.ts, PeerForwarding.ts or ExecutionPurpose.ts, and its Orchestrator.ts hunks (lines 160, 571, 8436, 11343, 11737, 11806, 12713, 13279) do not reach the watch handler, so round 1's unscoped inherit and legacy remote continuation and round 2's watch re-save survive unchanged. Round 3 item 3 (refused member stopped later) is fixed by construction and tested; item 4 (finished member later in loop order) is covered by "a member already finished when the group is cancelled stays quiet".
  • CI. All checks on this head are green, including Test (the flaky packages/ssh job from round 3) and Test Server 8.

Non-blocking

  1. Held unmarked sibling is never released when the last member to end is marked. Orchestrator.ts:11497-11510 returns siblings: [] for a quiet member, and runGroupWake (RunGroups.ts:62) returns hold for a disposed task, so neither path releases a terminal sibling that is still waiting. Deterministic scenario, no timing race: group {A running, B completed and held}, B's thread raised above the parent's modes. task_cancel marks both; A is stopped and disposed; B's assertStoppable refuses, the release clears B's mark but B is not suppressed (its delivery was never set), so it stays held. A ends quietly and nothing releases B. The parent never receives B's result wake, though task_status still shows it. The same loss happens on main (A's disposed delivery makes runGroupWake hold), so this is pre-existing, and the same class as round 3 item 3. Smallest fix: in the quiet branch, when the quiet member is the last non-terminal member, run runGroupWake for the held unmarked siblings and plan their delivery; the release handler then needs no suppressed special case. One test: refusing member completes before the cancel while the other runs, expect one offer. The PR body's "its result ... wakes the parent normally" is overstated for this ordering.
  2. Mark survives a crash between the mark commit and the stops. delegated_task.group-cancel commits in its own transaction before any member is stopped. If the server dies before the loop (or the agent's call is cut off and never retried), every member keeps running, marked, and ends quietly: the parent gets no wake and the cancel never happened. On main the members would still wake the parent. A retry with the same clientRequestId repairs it through receipt replay, and task_status still reports the members, so this is a narrow window; worth a line in the task_cancel tool description (retry with the same clientRequestId after a failure) rather than more machinery.
  3. SelectionRestart.integration.test.ts:536. The messageId case asserts nothing about admittedRun; the requirement is that a changed message id finds no stored message, so asserting replay.dispatch.admittedRun === undefined there would pin it.
  4. Carried from round 3: t3_thread_send with inherit to a thread that is not the caller's child still requires a single-scoped caller run (OrchestratorMcpService.ts, unchanged). The PR body records it as left open.
  5. Proof limit, as the PR body states: "a member already finished when the group is cancelled stays quiet" also passed on the round 2 code; the recorded-cancel test is the deterministic proof for that ordering.

Out of scope as agreed: mobile Steer after run end, purposeDeliveries after unlink.

Verdict for 08877e706ee8fbe0ba74a5b210c8a89477b323bf: PASS.

…r ends last

Review round 4 on #388: a refused member released from the group cancel
after it had already ended is held for the group. When the last running
member then ends under the cancel's mark, it is quieted and now plans the
group's one wake for those held members. The release step also re-plans a
refused member that ended before the cancel and was still held.
SelectionRestart's messageId replay pins admittedRun to undefined.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@lukemaj

lukemaj commented Oct 19, 2026

Copy link
Copy Markdown
Contributor Author

What: Review of commit 45eaaf2 finds the held group member is delivered once when the last marked member ends.
Why: Release leaves an ended refused member held while siblings run; the last marked completion can plan its wake, while release plans it itself if that completion came first. Persisted completion delivery prevents a second plan, and all-marked successful cancellation has no eligible held member.
So what: No code change is requested; single-task Stop and non-cancelled group paths remain on their existing planner branches.
Findings: None. The added integration case drains effects without sleeps; no tests or status checks were run per instruction. SelectionRestart changed-message-ID replay now admits no run.
Verdict: success.

@lukemaj
lukemaj merged commit b2b9a4d into main Oct 19, 2026
28 checks passed
@lukemaj
lukemaj deleted the t3/native-outcome-purpose branch October 19, 2026 06:32
lukemaj added a commit that referenced this pull request Oct 29, 2026
* feat(orchestration): retain native outcome purpose

Pin a typed Issue or PR purpose on every admitted run, carry it through
children, continuations, remote launches, schedules, watches and the mobile
outbox, and retain supported native causal evidence per adapter (#377).

Ports PR #388's four checkpoints onto current main. Agent auto sends now
queue, as on main, when steering would cancel running Claude tools, and the
repeated Claude steer test waits for its real skill discovery instead of a
fixed tick budget.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(orchestration): inherit unscoped parents and continue legacy remote children

Review round 1 on #388:
- `inherit` now carries the parent run's exact scope, unscoped included,
  locally, through thread launch and through remote delegation.
- A remote child launched without purpose intent records no launch
  evidence; its omitted-intent follow-up now continues as warned legacy
  unscoped, as a local one does, instead of being refused.
- Muse and OpenCode v2 advertise unavailable usage finality, matching
  their unsupported evidence.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(orchestration): keep group cancel and legacy PR watches correct

Review round 2 on #388:
- Group cancel keeps main's order (stop, then dispose on success). A member
  stopped by that cancel and ending before its own disposal no longer
  releases the cancelled group's wake; a member that refuses the cancel is
  never stopped, so its result still wakes the parent.
- A PR watch started without purpose intent (mobile, older agents) is saved
  as warned legacy unscoped instead of being rejected; re-watching without
  intent keeps a purpose saved earlier.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(orchestration): record group cancel before stopping members

Review round 3 on #388:
- A whole-group task_cancel first records its id on every member in one
  command. While a member carries it, its wake is suppressed whenever and in
  whatever order it ends. A member whose stop is refused has the mark
  cleared under the same id, and a result it finished meanwhile is
  re-planned, so its result and any later user Stop wake the parent. The
  order-dependent hold rule in runGroupWake is removed.
- The shared client-runtime watch action saves the thread's purpose choice
  on a new watch, so mobile and web both send it; the web-only wrapper is
  gone. Agents calling watch_pull_request without intent stay warned legacy
  unscoped.
- SelectionRestart's replay assertion checks the replayed sequence, events
  and admitted run instead of the exact result shape.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(orchestration): release held group members when a cancelled member ends last

Review round 4 on #388: a refused member released from the group cancel
after it had already ended is held for the group. When the last running
member then ends under the cancel's mark, it is quieted and now plans the
group's one wake for those held members. The release step also re-plans a
refused member that ended before the cancel and was still held.
SelectionRestart's messageId replay pins admittedRun to undefined.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XXL vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant