Skip to content

feat(peer): ship reviewed cross-machine parity work (#379) - #584

Merged
lukemaj merged 32 commits into
mainfrom
feat/379-final
Oct 20, 2026
Merged

lukemaj merged 32 commits into
mainfrom
feat/379-final

Conversation

@lukemaj

@lukemaj lukemaj commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

What: Ships the reviewed #379 cross-machine parity work that is already integrated on feat/379-cross-machine-parity to main.
Why: Remote children could not return results, continue jobs, answer requests or use several app-owned tools across linked machines; those fixes have sat unreleased on the integration branch.
So what: Both review families approved exact head 750c60db7b (#584 (comment)); the planner merges on green CI; the remaining #380-#386 work continues as follow-up PRs against main.

Problem

Delegated work across linked T3 environments hit location-only fences: results and follow-ups did not return to the parent, remote requests could not be answered from the source, and attachments, devices, browsers and destination automations assumed local resources. See #379.

Change

Rebases the integration branch's 19 commits (head 02343e5fbd) onto current origin/main (9378189f41, which includes #581); the fork stack must stay linear, so there is no merge commit. Component PRs, each already independently reviewed into the integration branch:

Review fixes on top (round 1, Sol findings on fcfcdadba2, #584 (comment)):

  • fix(scheduled-tasks): a keyed manual run refused before admission is recorded as a terminal failure and releases the schedule's running marker (was stuck running forever).

  • fix(orchestration): remote answer admission skips a blank optional question, like the message-response path.

  • chore(fork): one owner for shardWeights.json after fix(prism): preserve purpose intent in local Run groups #581 registered a second one.

  • Round 2-3 fixes: reopened parents follow their remote tasks again; threads read and answer their own questions again (own approvals still refused); remote request cards keep multiline editor answers (web, mobile); keyed pause/resume replay their receipt; post-admission and final recheck refusals release keyed manual runs without failing a concurrent same-key dispatch; stop-disposed remote children stay pending background work; finished remote jobs long-poll on their own semaphore so they never hold back Stops or live jobs.

  • test(server): weight typed peer effects by their CI time (Test Server 8 exceeded its 10-minute limit).

#558 and #559 were aligned into the integration branch but are already on main.

The only rebase conflict was docs/map.md (Prism routing refs); resolved as the union of both sides (#579 from main, PR #539 from the integration).

Not shipping, stays open: #380, #381, #382, #383, #384, #385, #386 (each keeps its remainder; none is closed by this PR), #369, and PR #583 (feat/385-remote-secret-channel, unresolved lifecycle findings; to be retargeted to main later). No new component work was added, per the user's 2026-10-20 reduced-scope decision.

Refs #379

Scope and approval

Maintainer-owned outcome #379; final merge to main authorized by the user on 2026-10-20, conditional on two independent reviews and green CI on this exact head. No deployment or install is part of this PR.

Verification

On head 7a6a08fe2e (Bestia):

  • vp test run apps/server/src/mcp/RunGroups.integration.test.ts apps/server/src/peer/PeerAuthority.pendingRequests.workflow.test.ts apps/server/src/peer/PeerAuthority.typedEffects.workflow.test.ts apps/server/src/scheduledTasks/ScheduledTaskService.test.ts apps/server/src/issues/featureMap.test.ts scripts/fork-maintenance.test.ts: 6 files, 269 passed (before the shard-weight owner commit; that commit only edits fork metadata, after which scripts/fork-maintenance.test.ts passed 19/19).
  • New tests manual584 and pending584 failed before the fixes (outcome_unknown; "The answer does not match the current pending request.") and pass after; the worker's 9 nearby files passed 305/305 on f5e4240a5d.
  • bash scripts/fork-check.sh --upstream upstream: fork-check: OK.; tsc-rs --noEmit in apps/server: no errors.

Earlier,
On df8039e8fb (Bestia), whose tree is identical to the current head fcfcdadba2 (git diff df8039e8fb fcfcdadba2 is empty):

Website impact: none; Chromeria is a private fork with no public website for these peer features.

Elon record

Requirements and who asked: User (2026-10-20): ship only what is integrated and reviewed on the integration branch, through one final PR; no new component work.
Deleted: Further component work (#583, #380-#386 remainders) and the shipment step from this delivery.
Bottleneck: Rocky is down, so the reviewed work could not reach main; this PR moves it from Bestia without a second writer on the integration branch.
Checked myself: The rebase showed one conflict (docs/map.md); ran the targeted tests above on the merge commit.

Model and harness: Claude Opus 5.5 via Claude Code in T3 Code.

🤖 Generated with Claude Code

@github-actions github-actions Bot added the vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. label Oct 20, 2026
@lukemaj

lukemaj commented Oct 20, 2026

Copy link
Copy Markdown
Contributor Author

GPT-6.1 Sol review · fcfcdad

Verdict: REQUEST_CHANGES. This is a partial review with supported findings, not completion of the requested whole-diff review.

  1. P2 · A refused keyed manual run can leave a schedule permanently marked running. Blocks merge. apps/server/src/scheduledTasks/ScheduledTaskService.ts:1817

    Concrete scenario: save a normal interval schedule that launches new threads using provider X, disable X for its project, and call run_scheduled_task_now with a clientRequestId from an active caller using another enabled provider. Admission commits last_run_status='running' and active_request_id (lines 1744-1747), then changes the manual request to dispatching. ThreadLaunchService.launch refuses the disabled provider before creating a thread or command receipt (ThreadLaunchService.ts:1107). dispatchOwnedTask wraps that failure in Effect.exit, but this call ignores the returned failed Exit and only checks for an accepted receipt. It returns outcome_unknown without releasing the running marker.

    Re-enabling X does not repair it: the same key encounters the dispatching recovery guard without a thread-create receipt (lines 1798-1808), and a new key is refused as already running (lines 1693-1697). Scheduled execution excludes running rows, and restart recovery explicitly excludes these unfinished keyed requests (lines 1163-1175). A separate native manual run can intervene, but ordinary keyed retries and scheduled execution remain stuck. Preserve known pre-admission failures as terminal failures and release the schedule reservation while retaining deduplication; keep genuinely interrupted outcomes distinct.

  2. P2 · Remote answers cannot skip an optional choice-only question. Blocks merge. apps/server/src/orchestration-v2/Orchestrator.ts:9219

    Concrete scenario: a pending canonical request, including a persisted/imported request, contains a required question plus an optional single-choice question with required:false, allowCustomAnswer:false, and options without an empty-string value. Leave the optional question blank and answer the required one in the source web or mobile remote-request UI. Both clients consider the optional question complete and serialize its unanswered value as "". With the required expectedRevision, the new admission path turns that into [""] and rejects it as an unoffered option. Omitting the optional answer also fails the preceding values === undefined check. The request stays pending unless the person supplies a supposedly optional answer. The existing message-response path explicitly skips blank optional answers; remote admission needs to preserve that behavior before validating selected option values.

Checked evidence and limits

  • Reviewed head and live PR head both remain fcfcdadba236f281e7926ccfcf3310411dff9576; inspected base is origin/main at c3557ef338fce173285b40149fa3c19e49b52e86.
  • Read Issue spec: native cross-machine control across linked environments #379's Outcome and applied the requested reduced scope. Missing feat(peer): remote children return results and delegate onward within durable grants #380-feat(peer): complete all-tool cross-machine workflows across web desktop and mobile #386 follow-up work and PR feat(secrets): relay destination-owned webhook requests #583 are not findings.
  • Verified git diff df8039e8fb fcfcdadba2 is empty. Main's fix(web): preserve Escape focus over closing menus #578 BaseUI patch, dependency pin/lock entries and focus test are unchanged in this candidate; test(web): isolate panel keyboard tests from inherited pointer #575's pointer isolation survives in RightPanelTabs.keyboard.browser.test.tsx.
  • Inspected authority enrollment/validation, grants and resource predicates, MCP routing/access, forwarding, request receipts, peer task observation/results/stops, pending-request ingress/admission, event-sink authority propagation, schedule admission/recovery, selected asset/viewer authentication paths, and selected web/mobile consumers and contracts.
  • node_modules/.bin/vp test run apps/server/src/peer/PeerAuthority.pendingRequests.workflow.test.ts: 18 passed.
  • node_modules/.bin/vp test run apps/server/src/peer/PeerAuthority.typedEffects.workflow.test.ts -t manual382: 1 passed, 178 skipped. These existing tests do not reproduce the two reported edge cases; findings above are source-traced.
  • git diff --check origin/main...fcfcdadba2 passed. A focused added-line scan found no conflict markers, debugger statements, console.log calls, or obvious credential/private-key patterns; this is not an exhaustive secret audit.
  • Whole-review limit: the 287-file diff adds 73,201 lines. I did not finish its complete line-by-line review within this run's available review context. Remaining review includes the complete browser/device implementations, PeerMcpClient transport, AttachmentStaging, destination automation certificate storage/disable recovery, all remaining toolkit and client diffs, and the full changed test corpus. Do not use this comment as the final independent whole-candidate gate.

No repository edits, commits, pushes, branch changes, or commit statuses were made. Only this requested PR comment was posted.

@lukemaj

lukemaj commented Oct 20, 2026

Copy link
Copy Markdown
Contributor Author

grok-4.7 independent review of fcfcdadba236f281e7926ccfcf3310411dff9576 (feat/379-final vs origin/main c3557ef338).

Verdict

APPROVE. No blocking finding in the shipped scope.

Findings

None.

What was checked

  • Tree identity: git diff df8039e8fb fcfcdadba2 is empty. HEAD is the requested SHA, 19 commits ahead of origin/main, worktree clean.
  • Rebase: docs/map.md keeps main's Prism line, including #579, and adds the integration's cross-machine parts. No conflict markers. Main's fix(web): preserve Escape focus over closing menus #578 Escape-focus patch, pin, and menu.focus test are unchanged (git diff origin/main HEAD on those paths is empty) and the menu-escape-focus entry is still in docs/fork-features.md and scripts/fork-upstream-edits.txt. test(web): isolate panel keyboard tests from inherited pointer #575 pointer isolation is present in apps/web/src/components/RightPanelTabs.keyboard.browser.test.tsx.
  • Authority: PeerAuthority enroll defaults to created-work with onward, preview, and device off. accept intersects the caller's validated ceiling with the destination grant. process confirms a validate/pair/return frame only against the stored record, peer, and generation. setGrant updates an enrolled session only. PeerForwarding.forwardingCaller still refuses onward use unless the child record's ceiling has onward, except the pending-request relay, which is limited to a recorded remote-child hop. Vault fill and end-login refuse any caller with a link origin. Idle html_render rechecks the original invocation before publication.
  • Contracts: new remote-task, grant, and viewer RPCs are on WsRpcGroup, and RPC_REQUIRED_SCOPES covers every group method. Control vs view is raised in clientRpcRequiredScopes. Targeted run: ./node_modules/.bin/vp test run apps/server/src/auth/RpcAuthorization.test.ts (28 passed). Extra mobile grant fields decode by being stripped from PeerGrant.
  • Persistence: new peer tables are CREATE TABLE IF NOT EXISTS plus checked ADD COLUMN. The control-frame rebuild keeps old cursors. Session expiry is capped at 9999-12-31 so ISO strings stay sortable. No numbered migration rewrite of main's history.
  • Diff scan of non-test sources: no conflict markers, console.log, debugger, or FIXME. History subjects that say wip: do not leave debug content in the tree.

Not treated as defects

Missing #380-#386 and #583 behavior, per the reduced scope. Repo-wide vp check / test / typecheck were not run; CI owns those. This pass did not execute the multi-environment workflow suites.

@github-actions

github-actions Bot commented Oct 20, 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 5.0 KiB 0 B (0.0%) 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 0 B (0.0%) 2.0 KiB ✅
Codex Live turn WebSocket decoded 20.9 KiB 20.9 KiB 0 B (0.0%) 29.3 KiB ✅
Codex Live turn messages 2 2 0 (0.0%) 8 ✅
Claude Total thread wire 5.0 KiB 5.0 KiB −24 B (−0.5%) 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 −24 B (−2.0%) 2.0 KiB ✅
Claude Live turn WebSocket decoded 21.2 KiB 21.2 KiB −41 B (−0.2%) 29.3 KiB ✅
Claude Live turn messages 2 1 −1 (−50.0%) 8 ✅

Baseline: 9378189 · PR result: 750c60d · 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 and others added 22 commits October 20, 2026 16:56
…#391)

* feat(peer): return notices and delegate onward within durable authority

* fix(peer): retain authority through queued execution and recovery

* fix(peer): preserve active run authority across native operations

* fix(peer): retain authority for every accepted run input

* fix(peer): check every contributor against the actual target

* fix(peer): validate joint work against its actual resource

* docs(peer): inventory retained consent seams

* fix(peer): remove unused authority helper re-export

* fix(peer): bound private output by effective caller modes

* fix(ci): weight the measured peer authority workflow

* fix(peer): retain effective caller modes during task cancellation
* feat(peer): retain remote delegated job lifecycle and durable Stop

* docs(map): link remote lifecycle component PR

* fix(peer): preserve Stop replay and source task navigation

* fix(peer): reconcile accepted attachment and bound session expiry

* fix(server): bind delegated lifecycle routes to production runtime
)

* fix(peer): preserve attachment bytes and native request acceptance

* docs(peer): link request recovery component review

* docs(peer): register attachment recovery review provenance

* fix(peer): declare device receipt owner in fork inventory

* fix(server): preserve scoped retries and scheduler acceptance

* fix(peer): preserve authorized manual admission and saved snapshot replay

* fix(peer): preserve native preview capability refusal

* fix(attachments): recover exact native discard acceptance

* fix(attachments): retain bytes after keyed discard acceptance

* fix(preview): reconcile native recording claims before replay
**What**: Exact #369 candidate42051c passed real source-only web and Codex peer request acceptance, including committed replay after both full CLI servers restarted.
**Why**: Independent tests cannot establish that the production composition, real provider callback and source-only client work together.
**So what**: The planner accepts this bounded internal component for integration into #379 after a fresh head/base/gate check; combined multi-machine/client acceptance and final shipment remain pending.

Requirements and who asked: User #379 mandates native delegated lifecycle and all permitted tools with two MEDIUM reviewers and real isolated client/provider proof.
Deleted: No fake provider, direct SQL mutation, injected browser RPC, capability expansion, alternate harness or duplicate callback.
Bottleneck: Current candidate integration; #383 shared device authority/discovery and #384/#385/#386 remain outstanding.
Checked myself: Primary planner drove actual source A browser, inspected native B events/provider results, restarted both CLI products, replayed exact original request, and reopened the same source-only result.

Identity: HEAD42051c47813c354682b80677009b199d86fef1a0, treef9efef2d7df1b8b91ac64b09018386e79ccc1d45, baseab1bf53640e2a6541a2ba9f316839cedf81d4f46. Clean author workspace /Users/rocky/.t3/worktrees/chromeria/feat-369-remote-pending-requests. Both full production servers ran that source, with meaningful copied task-owned history in NEW /tmp/chromeria-369-acceptance-a and -b; old382 homes/evidence remained untouched. Node24.18, A server14247/web7663, B14248, source incognito Browser connected ONLY to A950a38f2-e2a5-4242-9171-c17e0cb8412c. Actual B is ef047b58-1edf-4ad6-a1ba-44320188325c. No live userdata write, new grant, physical device or standalone worker server.

Human source-only flow: A actual Codex gpt-6.1-sol/MEDIUM delegated real async B task369-parent-native-question-v1 in existing B project/root. B actual Codex asked request_user_input_async, persisted pending at B sequence288. Primary opened source task viewer from transcript; browser URL stayed A, no B profile. The actual card displayed369 REAL REMOTE QUESTION, ACCEPT_369/DECLINE_369 and disabled Send answer until selection. Primary selected ACCEPT_369 and sent once. B durable request resolved at sequence290, command mcp:request:385bcb2f7e0a7415a3c0d1cda8a0cabb0b7204f83949b505a9f4fe4cb6482210:0, 2026-10-18T10:10:47.347Z. Real provider consumed the answer and completed369_REMOTE_ANSWER=ACCEPT_369. Source returned exact original result and zero pending requests. Not inferred from dispatch acceptance.

Agent tool flow: Separate deliberate B task369-parent-native-agent-answer-v1, same explicit Color. Native delegate mode wait returned on real pending question with waitTimedOut:false, requestCount1, actual qualified target/revision334. Source local list{} correctly returned[]; explicit remote descendant list returned its actual request. Native qualified read returned real369 REAL AGENT QUESTION/options/revision334. Source called t3_pending_request_respond with original target/route/expectedRevision334, answers{0:AGENT_ACCEPT_369}, stable key369-parent-agent-response-v1. Initial AND exact retry both returned sequence340. B request has exactly one pending334 and one resolved336 event; real provider completed369_AGENT_RESULT=AGENT_ACCEPT_369. Two B tasks intentionally created, each ordinal1 completed, no duplicate B run.

Restart proof: Read-only query found zero preparing/queued/starting/running/waiting runs in A/B. Primary stopped only captured own sessions47643/49402 (exit130), restarted same source and same NEW homes in sessions86505/9639. A actual Codex ordinal34 called ONLY the exact original respond input/key after reconstruction: sequence340 again. Postrestart B has no additional request resolution or provider run. Source previous-agent viewer reopened after both restarts with original human result, real accepted answer and zero pending requests. Actual client screenshot inspected before upload.

Evidence: local immutable/raw read-only oracles /tmp/chromeria-369-parent-actual-question-proof.json, /tmp/chromeria-369-parent-native-agent-tool-proof.json, /tmp/chromeria-369-parent-postrestart-proof.json. Full CLI logs /tmp/chromeria-369-acceptance-{a,b}-server.log and -restart.log retained. Native Browser screenshots in existing UNPUBLISHED draft evidence-379, never a public release:

![Real source-only pending question](https://github.com/toolboxmd/chromeria/releases/download/untagged-cf149e2fe74d54191ae2/browser-screenshot-localhost-mvdnzzmy-b8c7d28b.png)

![Real source-only completed result after reconstruction](https://github.com/toolboxmd/chromeria/releases/download/untagged-cf149e2fe74d54191ae2/browser-screenshot-localhost-mvdo5woi-469c18e9.png)

Both fresh exact-candidate independent MEDIUM reviews APPROVE: Sol #416 (comment) and installed Grok4.7 #416 (comment). Full aggregate #369 (comment). Current420 CI39116640008, Fork39116640150 and final fingerprint39116844636 SUCCESS; original canceled fingerprint retained separately. Authenticated newest review/independent SUCCESS by lukemaj. Neither review substitutes for the actual proof above. Full author/R1/R2 packet #369 (comment).

Limits: These are TWO isolated Rocky environments, web/source-only browser and actual Codex provider. No physical cross-machine/tunnel/mobile/native desktop/all-provider/real live approval, durable-dismiss or secret acceptance claim. Registered native tests retain their own broader and S1/S2 race/empty-choice proof. Earlier native initialization404 remains UNKNOWN; later tests/CI/actual positives do not diagnose it. Operational verification earlier used invalid exact=true locator syntax and a stale assumed web port; corrected to supported selectors/actual dev-runner7663 without alternate browser or product changes. Cost supported publish #416 (comment) is partial task379 coverage, not complete cost; old unplaced/unpriced usage remains unknown. Background supported sync still pending at this record, not presumed complete.

States: implemented YES; current focused/native tests YES under their exact records; BOTH reviewed YES; current CI YES; bounded primary product proof YES; internal integration NOT YET at this record; main merged/build/shipped/activated/live combined acceptance NO. No package bump. All rollback/evidence/workspaces retained under explicit #379 instruction until final acceptance.
* fix(client): make remote request choices readable and accessible

* docs(peer): track the remote request client repair

* fix(peer): count initial remote work once in sparse shells
* feat(device): execute typed commands across peer links

* docs(map): link the typed device core component

* fix(device): preserve native authority and typed command semantics

* fix(device): recheck authority before native shutdown
* fix(peer): preserve current-main request answering policy

* fix(compat): restore inherited CI consumers

Reuse baseline-only repairs from the preserved #383 checkpoint d892050cb97854e08ca1ca427fc762caebbd69f7 under the #379 ownership transfer. Adapt the two web test fixtures for actual Chromium collection without changing production web behavior.

* test(prism): absorb native recovery observer fixture

Transfer the exact test blob authored in cdd4780 and preserved by PR537 at fcd3162. Await the native observer marker before measuring the live batch; preserve the duplicate assertion and production source.
* wip: hide remote children from default navigation

* wip: register destination navigation component PR

* wip: retain user forks with remote provenance

* wip: cover phone and alternate sidebar navigation

* wip: declare phone and legacy carried upstream edits
* feat(peer): retain destination automation authority offline

* docs(peer): link destination authority component PR

* wip: repair destination authority recovery and acceptance findings

* wip: fence schedule acceptance and preserve live birth permissions

* wip: preserve birth permission purpose and split native proof

* wip: preserve recovery routes and birth dependency lifetime

* wip: retain paired recovery routes through repeated unlink

* wip: wake exact automation disable recovery at known expiry
…#534)

* wip: combine qualified resources with destination automation

* wip: bound mobile returned-resource row height

* wip: respect native top safe area for tool resources
* fix(server): prepare HTML in idle peer resource contexts

* docs(map): track the open idle HTML component
* fix(orchestration): wake parents for remote pending requests

* fix(orchestration): deliver held remote requests when parent waits end

* fix(peer): wait for request content before notifying parents

* fix(orchestration): catch up requests after remote original completion
* fix(peer): attribute remote follow-ups to the completing run

* docs(map): link remote completion provenance repair

* fix(peer): use observed purpose for first remote completion
…ilures

A keyed run_scheduled_task_now whose dispatch failed with a typed error and
left no native receipt returned outcome_unknown and kept the schedule marked
running forever. Record it as the request's failed outcome, release the
running reservation, and replay the same failure for the same key.
Interrupted or defective dispatches stay outcome_unknown.

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

Remote pending-request answers with expectedRevision rejected a blank or
omitted optional choice-only question as an unoffered option. Skip blank
optional answers before validating options, matching the message-response
path.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
#581 registered shardWeights.json under run-groups while the integration
already owns it under fork-maintenance; fork-features requires one owner.

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

lukemaj commented Oct 20, 2026

Copy link
Copy Markdown
Contributor Author

GPT-6.1 Sol review · slice S4 · f421f50

What: S4 verdict is APPROVE, with no supported blocking or non-blocking defect findings.
Why: The inspected peer tests preserve current authority, durable operation identity, result ownership and lifecycle behavior across the shipped paths.
So what: The coordinator can use this as S4's exact-head review, alongside the other slices and required candidate gates.

Findings

None. The timeout recorded below is a validation caveat, not a source-confirmed product defect.

Coverage and prior fixes

I read every changed line of every one of the 15 files in /tmp/pr584-r3/S4.txt, comparing base 9378189f41e8cfa3e8e4ffe8a30f92e17b22bbc1 with head f421f5049ab79eba9dcf97e2667c8c6d16cbc17d, plus relevant implementation and caller code. Files not finished: none. Local HEAD and the live PR head still matched this candidate after verification.

  • The archive/reopen regression exercises an archived follower after reconstruction, then unarchives the parent and observes the later remote result. The implementation takes its event cursor before reading archive state and waits for unarchive/deletion without dropping follower ownership.
  • The keyed pause/resume regression uses authenticated peer calls, resumes with a new key, then replays the earlier disable and verifies both the newer certificate and enabled schedule survive.
  • The own-question correction retains local own-question read/answer while the related unit suite and peer workflows retain approval, linked-caller and remote-delegated-child restrictions.
  • The round-1 manual-refusal and optional-blank-answer regressions check native/persisted outcomes, including no second launch on replay, a fresh-key successful run after recovery, and refusal of required blanks or invalid choices.
  • Inspected the fixture boundaries and positive/negative observations for authenticated routing, grant narrowing/revocation, queued effects, continuation/Stop, receipt reconstruction, attachment ownership and exact bytes, and source-owned viewer access. Browser/device leaves are deterministic fixtures; this is not live browser, physical-device or cross-machine acceptance.

The adjudicated paused authority-stamped manual-run behavior was not re-raised. Remaining #380-#386 work and PR #583 were not treated as missing scope. The web/mobile multiline correction is outside S4 and is not claimed verified here.

Executed proof

From the repository root:

node_modules/.bin/vp test run apps/server/src/peer/PeerAuthority.test.ts apps/server/src/peer/PeerForwarding.bare.test.ts apps/server/src/peer/PeerForwarding.promachos.test.ts apps/server/src/peer/PeerLinks.testkit.ts apps/server/src/peer/PeerToolRouting.test.ts apps/server/src/peer/RemoteDelegation.test.ts apps/server/src/peer/environmentCli.test.ts

6 test files, 88 tests passed. The testkit is a fixture, not a collected test file.

node_modules/.bin/vp test run apps/server/src/peer/PeerAuthority.automationContributors.workflow.test.ts apps/server/src/peer/PeerAuthority.automationSchedules.workflow.test.ts apps/server/src/peer/PeerAuthority.automationSpectrum.workflow.test.ts apps/server/src/peer/PeerAuthority.lifecycle.workflow.test.ts apps/server/src/peer/PeerAuthority.pendingRequests.workflow.test.ts apps/server/src/peer/PeerAuthority.typedEffects.workflow.test.ts apps/server/src/peer/PeerAuthority.workflow.test.ts

341 passed, 1 failed by timeout across 7 files. The council case at PeerAuthority.automationSpectrum.workflow.test.ts:180 exceeded its explicit 30-second limit. This root invocation uses the root test configuration rather than the server's serialized-file configuration; load sensitivity is an inference, not an established cause.

Reran only that failed case from apps/server with its own configuration:

../../node_modules/.bin/vp test run src/peer/PeerAuthority.automationSpectrum.workflow.test.ts -t 'council round and lost native acceptance replay'

1 passed, 18 skipped, exit 0. Also ran:

../../node_modules/.bin/vp test run src/childThreads/pendingRequests.test.ts

32 passed. git diff --check origin/main...f421f5049a -- $(cat /tmp/pr584-r3/S4.txt) passed. These focused runs do not constitute a complete repository gate, and the original timeout is retained above rather than reported as an all-green combined run.

The worktree remained clean. No source edits, commits, pushes, branch changes, commit statuses, browser/computer use or repository-wide checks were made; this requested review comment is the only GitHub content written.

Requirements and who asked: The supplied final-round brief requests exact-head S4 correctness, authority, lifecycle, persistence, compatibility and test-proof review for #379's shipped subset.
Deleted: No implementation changes, follow-up-scope demands, broad checks or speculative findings.
Bottleneck: Distinguishing actual persisted behavior and authority restrictions from fixture-only claims, while checking the prior fixes.
Checked myself: All S4 changed lines and relevant consumers; focused runs and the isolated timeout rerun above; clean diff check and exact local/live candidate identity.

Model/harness: GPT-6.1 Sol, medium reasoning, Codex.

…anual runs

A typed refusal from the recheck after admission, or after the dispatch
claim, now records the key as failed and frees the schedule instead of
leaving it running. The stage-guarded claim keeps uncertain dispatches
outcome_unknown.

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

lukemaj commented Oct 20, 2026

Copy link
Copy Markdown
Contributor Author

Grok review · slice S23t · f421f50

Verdict: APPROVE

Read every changed line of all 66 files in this slice. Unfinished files: none.

Findings: none.

The round-2 fixes that this slice pins still match the restrictions they were meant to keep. Own-thread question read and answer stays limited to a non-linked caller on a thread that was not delegated from another environment, and own approvals stay refused. Web and mobile remote request editors send a multiline answer unchanged. Keyed receipt replay stays fail-closed for an ambiguous external effect, a legacy imported key, and an invocation whose authority retired before the effect. No new defect in the shipped test scope.

@lukemaj

lukemaj commented Oct 20, 2026

Copy link
Copy Markdown
Contributor Author

Grok review · slice S2p · f421f50

Verdict: REQUEST_CHANGES

Read every changed line of every file in slice S2p. No file left unfinished. This verdict is for f421f5049ab79eba9dcf97e2667c8c6d16cbc17d only.

P2, blocking

apps/server/src/childThreads/delegatedFollowupQueries.ts:189 drops a remote continuation that the in-memory predicate keeps.

The local arm of pendingDelegatedChildQuery keeps a completion disposed by stop (completionDelivery.state IS NOT 'disposed' OR disposedBy = 'stop', lines 155–156). The remote arm requires completionDelivery.state IS NOT 'disposed' and does not allow disposedBy = 'stop'. pendingDelegatedChildTasks (delegatedFollowups.ts:160–168) keeps that same task when remoteChild.stop is unset and snapshot.workState is working or waiting_for_children.

A normal run.interrupt (holdQueue not set) reaches disposeDelegatedCompletionCohort with disposition: "stopped" (Orchestrator.ts:11266–11272 and 13244–13286). That rewrite spreads the task and sets completionDelivery to { state: "disposed", disposedBy: "stop" } (Orchestrator.ts:2478–2494). It does not set remoteChild.stop. delegated-tasks.stop, which is what records remoteChild.stop, is enqueued only when holdQueue is true. The destination therefore keeps running, and later observations can still report workState: "working" with remoteChild.stop absent.

pendingDelegatedChildQuery is the production read for shell snapshots and settlement candidates (ProjectionStore.ts:5985, 6314, 6437). Those shells feed ThreadSettlementService.isAutoSettlementCandidate (pendingBackgroundTasks empty means the hold is skipped, line 139) and storageCleanupThreadIdle (line 119). After the interrupt commits, the parent looks idle. Auto-settle can settle it, and worktree cleanup can treat its worktree as idle, while the remote continuation is still working. Replay tests of pendingDelegatedChildTasks do not execute this SQL.

The fix is to match the local arm: AND (json_extract(..., '$.completionDelivery.state') IS NOT 'disposed' OR json_extract(..., '$.completionDelivery.disposedBy') = 'stop').

Checked, not re-opened

Own-thread user-input read and answer is restored and still refuses own approvals, dismiss, linked callers, and delegatedFrom children. Blank optional answers are accepted only on the expectedRevision path; required blanks still fail. Keyed pause and resume take the enable-only shortcut and replay a mutation receipt. A refused keyed manual run is stored as stage failed and is not left running. Run-now on a paused certificate-backed schedule stays the adjudicated non-blocking case.

@lukemaj

lukemaj commented Oct 20, 2026

Copy link
Copy Markdown
Contributor Author

grok-4.7 review · slice S3p · f421f50

Verdict: APPROVE

Coverage: every changed line of all 108 files in S3p was read. No file was left unfinished. The worktree HEAD is now 3583d3a, one commit after this head, and that commit does not touch any S3p file. This review is of f421f50.

Prior-round fix in this slice: remote request cards on web and mobile keep multiline editor answers. Both treat initialAnswer !== undefined as the editor signal, the same signal as the local composer. Web uses a textarea; mobile sets multiline and newline submit. The tests submit an edited newline and expect it unchanged. The cards still send clientRequestId plus expectedRevision, and dismiss stays tied to responseType === "message". Own-thread, linked-client, and remote-delegated refusals stay in PendingRequestService and are not loosened by this UI.

Non-blocking

P3. A failed device record or trace start can stick until the session closes. apps/server/src/device/DeviceService.ts:1572 stores commandCaptures before AgentDeviceCommand.run. The key is removed only after a successful stop publishes artifacts (1754) or the session is forgotten (538). The pre-check at 1324 refuses another start while the key exists ("Capture sequence does not belong to this exact opening."). If the start CLI fails, the map still says a capture is open, so the next start is refused, and a stop that also fails does not clear the key. Closing the device session clears it. Command ownership is still checked, so this does not hand the session to another caller. Not blocking.

No authority bypass in this slice. Viewer tickets bind environment, thread, tab or device, epoch or tool session, and view versus control. View-only device input accepts only reset-video. Preview input is dropped unless the viewer can operate. Destination grant loss is rechecked on viewer renew. Attachment acquire and read recompute the digest and refuse a mismatch. Qualified tool resources are dropped as a set when any candidate fails the schema or the budget check.

@lukemaj

lukemaj commented Oct 20, 2026

Copy link
Copy Markdown
Contributor Author

GPT-6.1 Sol delta review · 3583d3a

What: REQUEST_CHANGES, with one blocking P2 concurrency finding in this delta.
Why: The final-recheck cleanup can record a terminal refusal while a concurrent same-key retry is already dispatching to an existing thread.
So what: Preserve uncertain concurrent dispatch outcomes, add a regression for this overlap, and re-review the affected candidate.

P2 · A dispatch claim does not exclude a concurrent recovery dispatcher. Blocking.

apps/server/src/scheduledTasks/ScheduledTaskService.ts:1876-1878, through refuseBeforeDispatch at lines 1815-1818 and failManual at line 1686.

Concrete execution for a keyed schedule bound to an existing thread (snapshot.task.threadId !== null):

  1. Call A admits the key, changes its stage from prepared to dispatching, and waits in its third/final live recheck.
  2. Same-key call B reads stage='dispatching' and no native receipt. The thread-create recovery check at line 1858 does not apply to an existing thread. Its stage update returns no rows, but the line-1874 guard only refuses callers whose original snapshot was prepared, so B passes its own recheck and enters dispatchTask/sendToThread.
  3. Before B's dispatch resolves, A's recheck returns a typed refusal. The new catch invokes failManual(..., 'dispatching'). B has not changed that stage or stored result_json, so the SQL predicate matches: the manual request becomes failed, the schedule becomes failed, run_count increments, and the reservation is cleared.
  4. B can then commit the native message. Its completion reads the refusal text as fresh.result_json and fails decoding it as a successful manual result. Same-key retries permanently report a refusal even though the message was accepted, and another key can now dispatch again.

The prepared stage condition correctly protects against a different caller already moving that row to dispatching. The dispatching condition is insufficient here because existing-thread recovery deliberately permits dispatch without acquiring that transition. Terminal failure needs proof that no concurrent dispatcher can still accept the command; otherwise this remains an uncertain outcome.

I reproduced this with the actual ScheduledTaskService.layer, migrated in-memory SQLite, and Deferred barriers, using an isolated node --input-type=module stdin probe from apps/server. A's final recheck was held; B reached the mocked ThreadManagementService.sendToThread; then A was refused before allowing B's native acceptance to finish. Output:

A refused: Failure
Before B native acceptance: stage='failed', result_json='refused by live check'
B native accepted: true, B return: Failure
Final manual: stage='failed', result_json='refused by live check'
Final schedule: last_run_status='failed', run_count=1

The schedule owner and SQL transitions were real. Native dispatch/receipt ports and the live-check callback were controlled mocks; this establishes the service ordering defect, not live provider or cross-machine verification. Source tracing through McpRouting.invoke and the native handler found no outer same-key serialization that prevents this overlap.

Cleared paths and proof

  • Read the complete two-file commit diff and the whole runManualNow / failManual implementation, including recovery, native receipt checks, failure bookkeeping and relevant callers.
  • manual584r3 exercises both refusal windows using recheck calls 2 and 3, asserts terminal failure and stable replay, and proves a fresh key can run. It only exercises a new-thread schedule sequentially, so it does not cover the overlap above. I did not independently run its failing-before comparison.
  • Ordinary sequential failure bookkeeping uses a transaction and a stage/result guard, so replay does not increment run_count again. next_run_at derives from the current schedule and completion time, preserving paused behavior. Interruptions and defects bypass the new typed-error catches; previously dispatching retries do not acquire the new cleanup handler.
  • node_modules/.bin/vp test run apps/server/src/peer/PeerAuthority.typedEffects.workflow.test.ts -t 'manual584r3:|manual584:|manual382:': 3 passed, 178 skipped. This includes the prior typed launch-refusal regression and native acceptance recovery regression.
  • git diff --check f421f5049a 3583d3ad606de3ec3d7ca1136b1f1cb3c720ae9e: passed.
  • Local HEAD and live PR head both verified as 3583d3ad606de3ec3d7ca1136b1f1cb3c720ae9e; worktree clean. Base: f421f5049a.

No repository edits, commits, pushes, branch changes, commit statuses, browser/device use, or repo-wide checks. This verdict covers only this single-commit delta, not the complete merge gate. No additional non-blocking findings.

Requirements and who asked: The complete delta brief requests read-only review of terminal pre-dispatch refusals, same-key races, replay and bookkeeping for PR #584's exact head.
Deleted: Whole-slice re-review, implementation changes, broad checks and unrelated follow-up requirements.
Bottleneck: Establishing whether the dispatch-stage predicate proves exclusive dispatch ownership before terminal cleanup.
Checked myself: Complete delta and surrounding functions/callers, three focused passing regressions, deterministic actual-service concurrency probe, clean diff and live head identity.

Model: GPT-6.1 Sol, medium reasoning. Harness: Codex.

@lukemaj

lukemaj commented Oct 20, 2026

Copy link
Copy Markdown
Contributor Author

grok-4.7 review · slice S4c · f421f50

Verdict: REQUEST_CHANGES

Every changed line of every file in S4c was read. No file left unfinished. The worktree later moved to 3583d3ad (one commit outside this slice). The eleven slice files are identical at both commits, and the production lines cited below are the same at f421f5049a.

P2. Finished lifecycle-v1 remote tasks keep polling forever and fill the eight peer slots

Blocking.

getOpenRemoteDelegatedTasks now treats every lifecycle-v1 task that has a destination thread as open, including after a result and after the stop is confirmed:

apps/server/src/orchestration-v2/ProjectionStore.ts:6057

The first OR branch (threadId text AND launch.lifecycleVersion = 1) is sufficient by itself. The later branches (no result, stop pending/unconfirmed/acknowledged, cancelled or interrupted without stoppedThere) never get a chance to retire that row.

RemoteDelegation.start follows every row that query returns (apps/server/src/peer/RemoteDelegation.ts:1390). follow repeats while followOnce returns false (:814). followOnce says it returns true once the task here is done (:1214). For lifecycle v1 it calls modernFollow (:1245), and modernFollow returns true only when stop.state is not_started (:904). After it records a result and after it confirms a stop, it returns false (:1104).

Each look is observe or snapshot with wait: true (:994). That call takes one of eight networkSlots permits for the whole HTTP call (:805, :856). On the destination, an idle wait ends at Effect.timeoutOption("20 seconds") (apps/server/src/peer/PeerTaskLifecycle.ts:758), then the follower loops immediately.

Failure: a source that has finished N lifecycle-v1 remote tasks keeps N followers for the life of the process. Eight of them sit in idle 20s polls, so a later Stop, cancel, or live observation waits about (N/8)*20s for a permit. Restart calls start again and reselects the same history. This is a capacity stall, not a deadlock.

Staying open so a later parent Stop can still record stop.state = pending is intentional (apps/server/src/orchestration-v2/RemoteDelegatedTask.test.ts:719 keeps a completed task with stoppedThere: "ran-out" in the open set). The slice assertions that changed toEqual([]) to toContainEqual for an acknowledged or unconfirmed stop (RemoteDelegation.test.ts) match that obligation. They do not require a confirmed, settled job to long-poll. The legacy follower returns true once the result is in and the stop is recorded (RemoteDelegation.ts:1264, :1344). Lifecycle v1 never does.

Checked in this slice and left as they are: the reopened-parent follower waits on archive and then continues; a remote answer may leave an optional choice blank and still rejects a blank required choice; a paused schedule's Run now refusal stays the adjudicated non-blocking case.

…und work

A parent interrupt disposes a delegated completion with disposedBy "stop"
without stopping the remote child. The sparse SQL remote arm dropped that
row while the local arm and pendingDelegatedChildTasks kept it, so shells,
auto-settlement and storage cleanup treated the parent as idle.

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

lukemaj commented Oct 20, 2026

Copy link
Copy Markdown
Contributor Author

grok-4.7 delta review · 3583d3a

What: APPROVE. No findings.
Why: A typed post-admission recheck refusal now fails the keyed manual request and releases the schedule, and the stage predicate keeps that write off any request that has already left the claimed stage.
So what: This delta is clear to merge from the review side. No code change requested.

Findings

None.

Checked

failManual (ScheduledTaskService.ts:1686) now claims with stage=<fromStage> AND result_json IS NULL. A lost compare-and-set returns before run_count, last_run_status, last_run_error, next_run_at, or active_request_id change, so those schedule writes happen once. The prepared refusal (:1820) and the post-claim refusal (:1876) both run only before dispatchTask. Effect.catch recovers a Fail only, so a pure interruption or defect still propagates and leaves the reservation. A same-key caller that does not win prepared -> dispatching uses the bare recheck and cannot call failManual(fromStage="dispatching"). If that caller still sees prepared and the winner already moved the row, the prepared update matches nothing. Replay of stage=failed returns manualFailed before the success bookkeeping (:1845). The round-1 dispatchTask refusal still calls failManual(..., "dispatching") (:1914).

manual584r3 counts recheck calls: 2 is the post-admission window, 3 is the post-claim window. Both expect stage=failed, the refusal text, lastRunStatus=failed, runCount=1, stable same-key replay, and a new key that reaches succeeded / runCount=2 with one new user message. That last step is the release check. On f421f5049a those rechecks were bare yield* recheck, which could not persist stage=failed.

Command: node_modules/.bin/vp test run apps/server/src/peer/PeerAuthority.typedEffects.workflow.test.ts -t 'manual584'
Result: 2 passed, 179 skipped (manual584 and manual584r3).

Worktree clean at 3583d3ad606de3ec3d7ca1136b1f1cb3c720ae9e. No edits, commits, pushes, or repo-wide checks.

Requirements and who asked: Delta review brief for commit 3583d3a on PR 584.
Deleted: Re-review of lines outside this commit; implementation changes.
Bottleneck: Stage-conditioned claim versus a concurrent dispatch claim, and one-time schedule release.
Checked myself: Full commit diff, runManualNow / failManual, Effect.catch Fail-only behavior, and the two focused tests above.

@lukemaj

lukemaj commented Oct 20, 2026

Copy link
Copy Markdown
Contributor Author

grok-4.7 review · slice S1 · f421f50

Verdict: APPROVE

No findings.

Read every changed line of every file in S1 (31 files). None unfinished. Reviewed exact head f421f5049ab79eba9dcf97e2667c8c6d16cbc17d. The worktree later moved to 3583d3ad; git diff f421f5049a HEAD -- <S1 paths> is empty, so the slice bytes match the named head.

Round-2 Sol S1 holds. followOnce waits on the bounded live event tail for thread.unarchived or thread.deleted, and follow retries until the parent is deleted. A deleted parent ends the follower on the next pass.

Also checked inside this slice, with no defect: ceilings and dispatch mode limits fail closed; viewer tokens are not ordinary session or websocket credentials, and control is scope-checked in nativeViewerSession; return notices and lifecycle pairs require the stored pair, the same generation, and a live child; attachment reads authorize before bytes, and the bearer byte fetches use redirect: "manual"; keyed HTML publish replays the receipt before wx; automation recovery sends the retired generation's own secret only when unacked obligations exist and describe() matches that environment; a resource-websocket prefix only selects the parser, and an oversize frame closes 1009 before dispatch.

lukemaj and others added 2 commits October 20, 2026 18:20
…ent same-key dispatch

The final live recheck ran after the prepared -> dispatching claim and
terminalized a refusal from the dispatching stage. For an existing-thread
schedule a same-key retry can read dispatching and send concurrently, so
the refusal failed the request and freed the schedule while that send was
accepted. The final recheck now precedes the claim, so a refusal is only
recorded through the prepared-stage compare-and-set.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A lifecycle-v1 follower keeps long-polling its job there after the result,
so a later Stop or follow-up is still observed. Each poll held one of the
eight shared peer permits for up to 20 s, so more than eight finished jobs
starved Stops and live observations of other jobs.

Followers whose job has nothing left to do there (result available, no
queued runs, questions or recovery wait, or a confirmed Stop) now wait on
their own semaphore. Stops, unconfirmed and acknowledged stops, live jobs
and follow-ups seen as working keep the existing eight permits.

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

lukemaj commented Oct 20, 2026

Copy link
Copy Markdown
Contributor Author

GPT-6.1 Sol delta review · f421f50..7dd6720

What: REQUEST_CHANGES, with one blocking P2 finding in the remote follower classification.
Why: A historical confirmed Stop overrides current evidence that the destination is working again, leaving that active follower in the settled semaphore.
So what: Classify resumed work from the current snapshot as live, cover this transition, and re-review the affected delta.

P2 · A confirmed Stop must not permanently classify subsequent active work as settled. Blocking.

apps/server/src/peer/RemoteDelegation.ts:974-982.

Concrete scenario: a delegated child is stopped and its source records remote.stop.state = "confirmed". A person subsequently sends fresh input directly to that destination child. The follower observes and persists a newer snapshot with workState = "working" (or queued/recovery/question work). The historical Stop remains confirmed: dispatchRemoteObservation spreads the previous remote state and replaces snapshot, without clearing stop (Orchestrator.ts:8596-8606), and remote result handling likewise retains it. On every subsequent iteration, the unconditional first arm of settledThere still selects settledSlots, regardless of that new live snapshot.

With eight idle historical followers holding the settled permits, this known-active follower queues behind their long-polls while the live pool can be free. Its next progress/result observation waits for idle historical requests to finish; with more historical jobs the delay accumulates through their 20-second waits (PeerTaskLifecycle.ts:758). The problem persists after the source has already learned that the job is active, and across restart because lifecycle-v1 jobs are reselected. New pending Stops still preempt through sourceChanged; this finding does not claim that Stop preemption is broken.

This is a source-traced reachable transition, not an independently executed reproduction of this exact post-confirmation overlap. Fresh direct input after an accepted Stop is explicitly supported by the existing lifecycle test around lines 1762-1784. The new classification must let current active work override historical confirmation, rather than treating confirmation as permanent proof of quiescence. Add coverage for a confirmed-stop child becoming active again with the settled pool occupied. This does not require retiring settled followers or changing the planner's #593 decision.

Cleared paths and proof limits

  • Read the whole seven-file delta git diff f421f5049a 7dd6720335, every changed line, and the complete requested functions plus relevant callers. No delta file remains unread. Local HEAD and live PR head both matched 7dd6720335488db9e462be0660799602f8c8b062.
  • Pre-dispatch refusals now terminalize only through stage='prepared' AND result_json IS NULL; the final check precedes the dispatch claim. A concurrent successful claimant defeats that cleanup, as manual584r4 verifies. Transactional bookkeeping and stored-result replay prevent repeated run_count/schedule updates in these paths. Interruptions and defects are not caught as typed pre-dispatch refusals and are not converted into terminal failure by the new handler.
  • The SQL remote pending arm now agrees with the in-memory predicate for all three contract work states, including disposedBy='stop'. The remote369 test checks sparse SQL, memory and reconstructed rosters, then verifies non-stop disposal removes the entry.
  • The new "never hold back" test establishes Stop progress with nine finished observations held indefinitely by the fixture. It covers a separate live job, not a confirmed-stop job that resumes. Ordinary no-stop jobs switch to the live pool after a working snapshot; pending/unconfirmed/acknowledged Stops retain the live pool.
  • Kept the two explicitly adjudicated manual-run cases and settled-poll retirement outside this verdict. No additional non-blocking findings.

Executed on the exact head:

  • node_modules/.bin/vp test run apps/server/src/peer/PeerAuthority.typedEffects.workflow.test.ts -t 'manual584r3:|manual584r4:|manual584:|manual382:': 4 passed, 178 skipped.
  • node_modules/.bin/vp test run apps/server/src/peer/PeerAuthority.lifecycle.workflow.test.ts -t 'lifecycle381:': 13 passed, 23 skipped.
  • node_modules/.bin/vp test run apps/server/src/orchestration-v2/ProjectionStore.test.ts -t 'remote': 1 passed, 42 skipped.
  • git diff --check f421f5049a 7dd6720335: passed; worktree remained clean.

No repository edits, commits, pushes, branch changes, commit statuses, browser/device use or repo-wide checks. This is a delta verdict, not the complete merge gate; no failing-before comparison was run.

Requirements and who asked: The complete delta brief requests read-only exact-head review of manual refusal races, pending-work parity and follower capacity, including jobs becoming active again.
Deleted: Whole-candidate re-review, implementation changes, broad checks and the explicitly adjudicated follow-ups.
Bottleneck: Distinguishing historical Stop completion from current destination activity when selecting observer permits.
Checked myself: Whole delta and requested functions/callers, historical review findings, #593 scope, 18 focused passing tests, clean diff/worktree and exact live PR head.

Model: GPT-6.1 Sol, medium reasoning. Harness: Codex.

A confirmed Stop counted a job as settled whatever its latest snapshot
said, so fresh direct input on the destination child after the Stop
left its follower queued behind idle settled long-polls, also after a
restart.

A job is now settled only while its latest snapshot is quiet (result
available, no queued runs, questions or recovery wait); a confirmed Stop
also needs that quiet snapshot. Anything showing activity uses the
shared network permits.

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

lukemaj commented Oct 20, 2026

Copy link
Copy Markdown
Contributor Author

GPT-6.1 Sol delta review · 7dd6720..750c60d

What: APPROVE this delta; no blocking findings, one non-blocking test-hardening finding.
Why: Current destination activity now overrides historical Stop confirmation when selecting observation permits.
So what: The prior blocking classification finding is cleared; the coordinator can integrate this verdict with the earlier slice and delta reviews.

Read the whole three-file delta git diff 7dd6720335 750c60db7b, every changed line, the complete modernFollow function, semaphore selection and acquisition, followOnce state refresh, and the relevant fixture and destination snapshot construction. Local HEAD and live PR head both match 750c60db7b489aee89b186a1f2d30a850f37fc03.

Non-blocking P3: make the saturation barrier identify current, distinct holders

apps/server/src/peer/PeerAuthority.lifecycle.workflow.test.ts:565-569, with the signal emitted in PeerAuthority.workflow.fixture.ts:869.

The test enables held waits before closing the original connection, and quietWaitsHeld survives that close. It then consumes eight messages without checking their task IDs or connection lifetime. If an original follower enters the hold between enabling it and completion of connection shutdown, its signal remains queued after its permit has been released. That stale signal plus seven current holders can satisfy the barrier without eight current settled permits being occupied. Under the old classifier, the restarted stopped job could then occupy the remaining permit, observe the subsequently finished result, and let the test pass despite the regression. This is a source-level proof weakness, not a false pass I independently reproduced.

Smallest hardening: close the old connection before enabling held waits, then wait for the distinct expected task IDs. No production defect follows from this finding, so it does not block this delta.

Cleared paths

  • settledThere requires result_available, zero queued runs, zero pending questions and no recovery wait for both arms. Missing snapshots and current working/waiting-for-children snapshots use networkSlots even with a historical confirmed Stop.
  • Pending, unconfirmed and acknowledged Stops cannot satisfy either Stop-state arm. A newly persisted active snapshot is reread by followOnce and selects the live pool on the next iteration.
  • sourceChanged remains outside watch(...).pipe(slots.withPermits(1)) through raceFirst, so a new pending Stop interrupts both permit waiting and an in-flight observation. Result continuation retains its corresponding race.
  • The new test reaches confirmed Stop, sends real fresh direct input, observes the resulting non-quiet snapshot, reconnects, and observes the later result. Its synchronization uses stored event streams and a queue, with no sleeps or polling. Apart from the barrier qualification above, the fixture emits its signal from inside the permit-protected effect before hanging indefinitely.
  • Settled observation retirement and polling cost remain outside scope under perf(peer): settled remote delegated jobs keep long-polling for the life of the process #593.

Executed proof and limits

  • node_modules/.bin/vp test run apps/server/src/peer/PeerAuthority.lifecycle.workflow.test.ts -t 'lifecycle381:': 14 passed, 23 skipped.
  • git diff --check 7dd6720335 750c60db7b: passed.
  • Worktree remained clean. No edits, commits, pushes, branch changes, commit statuses, browser/device use or repo-wide checks. No failing-before comparison was run. This is the requested delta verdict, not a complete merge gate.

Requirements and who asked: The delta brief requests exact-head read-only review of active follower classification, Stop preemption and the new regression proof.
Deleted: Whole-PR re-review, implementation work, broad checks and the adjudicated settled-poll retirement follow-up.
Bottleneck: Ensuring current destination activity wins over historical Stop state, while separating the production guarantee from the fixture's saturation evidence.
Checked myself: Whole delta and requested function, next-iteration refresh, permit/race boundaries, destination snapshot construction, fixture signal lifetime, 14 passing focused tests, clean diff/worktree and exact live head.

Model: GPT-6.1 Sol, medium reasoning. Harness: Codex.

@lukemaj

lukemaj commented Oct 20, 2026

Copy link
Copy Markdown
Contributor Author

grok-4.7 delta review · f421f50..7dd6720

Verdict: REQUEST_CHANGES for assigned head 7dd6720335.

What: That head has one blocking classification bug. Current branch head 750c60db already fixes it.
Why: A confirmed Stop forced the settled semaphore even when the latest snapshot showed new work.
So what: Do not open another fix for this finding. 750c60db is outside this assigned range, so it still needs its own exact-head pass. I read that commit and its new test passed; I found no remaining blocker in it.

I read the whole delta git diff f421f5049a 7dd6720335 (7 files) and the functions it touches. While I was reading, the worktree moved from 7dd6720335 to 750c60db (fix(peer): stopped remote jobs with new work there keep live permits). I read that commit so this comment does not ask for a fix that is already on the branch.

P2 · A confirmed Stop stays on the settled semaphore after the destination starts working again. Blocking at 7dd6720335. Fixed at 750c60db.

apps/server/src/peer/RemoteDelegation.ts:974 at 7dd6720335.

settledThere was stop.state === "confirmed" || (no stop && quiet snapshot). The first clause ignored workState, queuedRuns, pendingQuestions, and recoveryWaiting.

Concrete scenario: a lifecycle-v1 job's Stop is confirmed, then someone sends direct input on the destination child. The snapshot becomes working, or it gains a queued run, a question, or a recovery wait. The follower keeps long-polling on settledSlots. Eight idle finished jobs hold those permits for up to 20 seconds each, so this job's later observation waits behind them while the eight network permits are free. A pending, unconfirmed, or acknowledged Stop, and a quiet job with no Stop, were classified correctly. This path was not.

750c60db requires a quiet snapshot in every settled case. A confirmed Stop no longer overrides that. lifecycle381: a stopped job with new work there is not held back by finished jobs waits until eight finished waits are held, then the restarted job's later result is still recorded.

Cleared on the assigned delta

  • Prepared-stage compare-and-set is the only refusal terminalization before dispatch. A post-admission refusal and a pre-claim final refusal both release the key (manual584r3, recheck calls 2 and 3). Replay returns the stored failure and does not increment run_count again. A fresh key runs.
  • The final recheck is before prepared -> dispatching. A refusal whose row is already dispatching does not call failManual. manual584r4 holds the same key inside sendToThread, refuses the other caller, and leaves one success with run_count 1.
  • failManual updates run_count, last_run_status, and next_run_at only after the stage claim and active_request_id match, in one transaction. A second claim matches no row.
  • Interruptions and defects still return outcome_unknown. Effect.catch does not take them, and the dispatch failure path still requires no interrupt and no defect.
  • The remote arm of pendingDelegatedChildQuery now keeps disposedBy = 'stop' when workState is working or waiting_for_children and remoteChild.stop is absent, matching pendingDelegatedChildTasks. The remote369 test checks the SQL shell and the in-memory roster. A disposal without disposedBy drops the row.
  • Not re-raised: the paused certificate-backed Run now refusal (fix(scheduled-tasks): paused linked schedules offer Run now that always fails #586), and the existing-thread dispatchTask typed refusal that races another same-key caller inside sendToThread.

Tests on the assigned head, before the worktree moved: remote369, manual584r3, manual584r4, and finished jobs watching — 4 passed. After the move, those two lifecycle tests on 750c60db — 2 passed.

No repository edits, commits, pushes, or branch changes from this review.

Requirements and who asked: The delta brief asks for a read-only review of f421f5049a..7dd6720335.
Deleted: Re-raising adjudicated follow-ups, the remaining settled-poll cost (#593), and a new fix for a defect already committed.
Bottleneck: Whether a confirmed Stop could keep an active destination on the settled semaphore.
Checked myself: The seven-file delta, the surrounding functions, and the four assigned regressions; then the follow-up commit and its lifecycle test on 750c60db.

@lukemaj

lukemaj commented Oct 20, 2026

Copy link
Copy Markdown
Contributor Author

Grok 4.7 delta review · 7dd6720..750c60d

What: APPROVE. A confirmed Stop no longer keeps a job on the settled semaphore once its latest snapshot shows work.
Why: Settlement now requires a quiet snapshot (result_available, queuedRuns === 0, pendingQuestions === 0, !recoveryWaiting) and either a confirmed Stop or a recorded result with no Stop.
So what: No change requested. Settled jobs stay on their own semaphore; retiring those polls remains #593.

Read the whole delta git diff 7dd6720335 750c60db7b (3 files, every changed line) and the whole modernFollow function, including semaphore selection, followOnce reload, and sourceChanged. Local HEAD was 750c60db7b489aee89b186a1f2d30a850f37fc03.

Classification

settledThere in apps/server/src/peer/RemoteDelegation.ts (around 976-982) can select settledSlots only while the latest snapshot is quiet and the job is not in a live stop handshake.

  • Active. working and waiting_for_children fail workState === "result_available". queuedRuns > 0 fails its conjunct. Both use networkSlots. Snapshot construction already turns recovery, incomplete work, and pending children into those non-quiet states.
  • Stopping. not_started returns before selection. pending, and unconfirmed without a failed stop execution, return on the stop path before selection. acknowledged, and unconfirmed with a failed stop, fail the confirmed-or-no-stop conjunct, so a quiet snapshot still uses networkSlots.
  • Questioning. pendingQuestions === 0 is required.
  • Recovering. !recoveryWaiting is required.
  • Confirmed Stop with no snapshot. The workState conjunct is false, so the job uses networkSlots.
  • Becomes active again. followOnce reloads the task every iteration. After the follower persists a non-quiet snapshot, the next modernFollow selects networkSlots. An in-flight wait keeps the semaphore it already took until that wait ends; the brief asks for the next iteration, and that is what the code does.
  • Stop preemption. sourceChanged is unchanged. A new pending Stop, or a backpressured delivery-state change, still races the in-flight watch. The next iteration takes the stop path on networkSlots.

Test

lifecycle381: a stopped job with new work there is not held back by finished jobs records a confirmed Stop together with workState !== "result_available", then occupies all 8 settled permits. quietWaitsHeld is offered only after withPermits(1) and only for the finished task ids, then the fixture waits forever (Effect.never). The resumed result is written on the destination only after those 8 takes. latestResult on the source is set from the remote observation (Orchestrator.ts around 8325), so a job stuck on settledSlots cannot record restarted-later-result. The wait is an event stream plus queue takes: no sleeps and no poll loop. A wrong classification would sit behind the 8 unreleased permits until the 30s test deadline.

Executed on this exact head:

  • node_modules/.bin/vp test run apps/server/src/peer/PeerAuthority.lifecycle.workflow.test.ts -t 'lifecycle381: a stopped job with new work there is not held back by finished jobs': 1 passed, 36 skipped (1949ms).
  • git diff --check 7dd6720335 750c60db7b: clean. Worktree clean.

No other lifecycle tests, no repo-wide checks, no commit status. This is a delta verdict, not the complete merge gate.

Requirements and who asked: The delta brief asks for a read-only review of this one commit against Sol's confirmed-Stop finding.
Deleted: Re-review of the earlier delta, implementation, settled-poll retirement (#593), and repo-wide proof.
Bottleneck: Whether a resumed job can still take settledSlots after the source has seen a live snapshot.
Checked myself: Whole delta, modernFollow and its callers' reload path, snapshot and stop schemas, the fixture hold signal, and the new test on 750c60db7b.

@lukemaj

lukemaj commented Oct 20, 2026

Copy link
Copy Markdown
Contributor Author

What: Both independent review families approve exact head 750c60db7b489aee89b186a1f2d30a850f37fc03.
Why: Whole-candidate slice reviews of f421f5049a by GPT-6.1 Sol (4 slices) and Grok 4.7 (7 slices), plus delta reviews by both of every later commit, cover every changed line; every blocking finding was fixed and re-reviewed.
So what: The planner merges once CI is green on this head.

Coverage on the exact head:

  • Slice reviews of f421f5049a: Sol S1 6295192243, S2 6295239287 (finding fixed in 3583d3ad60), S3 6295221575, S4 6295317980; Grok S1 6295578700, S2p 6295444969 (finding fixed in e3c0e90af5), S3p 6295449194, S23t 6295422333, S4a 6295277984, S4b 6295277734, S4c 6295487055 (finding fixed in 7dd6720335 + 750c60db7b).
  • Delta f421f5049a..3583d3ad60: Sol 6295465302 (finding fixed in 6c7a7ea3c1), Grok 6295547403 approve.
  • Delta f421f5049a..7dd6720335: Sol 6295998527 and Grok 6296181368, same single finding, fixed in 750c60db7b.
  • Delta 7dd6720335..750c60db7b: Sol 6296154512 approve, Grok 6296231336 approve.
  • git diff --stat f421f5049a 750c60db7b touches only the files covered by those delta reviews.

Adjudicated non-blocking, filed as follow-ups: #586 (paused certificate-backed schedule Run now), #587, #588, #592 (review notes), #593 (settled remote jobs keep long-polling, isolated from active work here), #594 (narrow same-key dispatch race in the round-1 refusal path).

@lukemaj

lukemaj commented Oct 20, 2026

Copy link
Copy Markdown
Contributor Author

Agent work on this PR

Estimated cost at least $0.82 · 34 responses · 38 sessions · 2.7 h wall time

Model Responses Tokens Estimated cost
grok-4.7 (grok, medium) 15 95,749,479 unknown
gpt-6.1-sol (codex, medium) 9 2,335,207 $0.32
claude-opus-5-5 (claude, medium) 10 1,665,455 $0.49

Flags: 1 human correction · 18 large tool outputs · 336 repeated reads · 5 repeated skill loads · 1 session without usage records · 15 unpriced responses

Details: snapshot, prices, coverage, counters
  • Task toolboxmd/chromeria#584: outcome unknown (recorded acceptance only; a finished process never implies it).
  • Proof: feat(peer): ship reviewed cross-machine parity work (#379) #584
  • Snapshot a9bfa4a97b5ade9d939b00897d3cfd60770696f2932036cee9639d3d48a39d47, records up to 2026-10-20 16:51 UTC.
  • 38 sessions on claude, codex, grok; AgentsMD 17.4.6.
  • The total includes work shared with 3 other tasks, counted whole in each, so sums across PRs overlap.
  • Not counted: 1,126 responses (at least $52.96) in sessions shared with other PRs that worked in no single PR's checkout.
  • 5 responses in these sessions worked on other PRs and are counted there.
  • Totals reconcile with the measured sessions: yes. Evidence complete: no.
  • Prices: list-price estimate from T3 local rate table (path withheld) as of 2026-10-20, schedule 42ef195f9bb07476e283db469fbf2854941cef35991d58821602b020c403160c. Unknown prices stay unknown, never zero.
    • T3 LiteLLM rate table when present; bundled schedule covers the rest.
    • Model price sources: T3 local rate table (path withheld).
    • Unpriced: unknown model (15).
  • Native session usage or worker ownership is unavailable.
  • Harness-reported cost: none reported.
  • Usage totals are not billing. Subscription spending is separate and is never posted as spend.
  • Crashed runs are counted separately: 0.

Token counters by model (native counter semantics; never added across semantics):

  • grok-4.7 (grok, medium), grok counters: input includes cached, output includes reasoning: input 95,241,215; cache-read 90,695,552; cache-write 0; output 508,264; reasoning 405,463; harness total 95,749,479.
  • gpt-6.1-sol (codex, medium), codex counters: input includes cached, output includes reasoning: input 2,330,358; cache-read 2,307,968; cache-write 0; output 4,849; reasoning 504; harness total 2,335,207.
  • claude-opus-5-5 (claude, medium), claude counters: input excludes cache, output includes thinking: input 20; cache-read 1,649,586; cache-write 12,942; output 2,907; reasoning 563; harness total 1,665,455.

Generated tokens (reasoning and other output are disjoint; other output includes code, tool calls and replies):

Model Reasoning Other output Reasoning share Split known for
grok-4.7 (grok, medium) unknown unknown unknown 0 of 15
gpt-6.1-sol (codex, medium) 504 4,345 10.4% 9 of 9
claude-opus-5-5 (claude, medium) 563 2,344 19.4% 10 of 10

Usage by work phase (this task's own responses):

Phase Model Responses Input Cache read Cache write Output Reasoning Total Priced part
unclassified gpt-6.1-sol (codex, medium) 9 2,330,358 2,307,968 0 4,849 504 2,335,207 $0.324067
unclassified claude-opus-5-5 (claude, medium) 10 20 1,649,586 12,942 2,907 563 1,665,455 $0.491673

Recorded activity by phase (observations, not separate token bills):

Phase Tool calls MCP results Reads Skill invocations File changes Failed tool results
unclassified 251 13 95 unavailable 0 10

Selected rates (USD per million tokens). These rates value the report at the selected schedule date; they do not establish historical prices or subscription spending.

Model Input tier Input Cache read Cache write Other output Reasoning
claude-opus-5-5 all 4 0.2 5m: 5; 1h: 8 20 20
gpt-6.1-sol all 2 0.1 2.5 10 10
grok-4.7 all unknown unknown unknown unknown unknown

Other output and reasoning are priced without double counting inclusive native output. Missing rates remain unknown.

Local measurement from native records; usage totals are not billing. Updated in place by agent-observer publish.

@lukemaj
lukemaj merged commit f5f8604 into main Oct 20, 2026
26 checks passed
@lukemaj
lukemaj deleted the feat/379-final branch October 20, 2026 16:57
lukemaj added a commit that referenced this pull request Oct 20, 2026
* feat(secrets): relay destination-owned webhook requests

* docs(secrets): register component PR ownership

* fix(secrets): keep shared privacy text private

* fix(secrets): isolate retirement from question delivery

* test(secrets): retain failed retirement for recovery

* fix(secrets): preserve remote request lifecycle recovery

* fix(secrets): use the secret variant for scheduled pause and resume

Main's #584 pause/resume path still passed consumesSecret: false, which the
remote secret channel replaced with an explicit secret variant. The keyed
disable then crashed reading mutation.secret.type.

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

* docs(secrets): point component ownership at PR #599

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