Repository navigation
fix(tasks): carry the unattended stamp onto a role checklist - #2234
Merged
Merged
Conversation
A role checklist's contract inherits the run's frozen authority from the parent — the claim-review gate, the publication-review flag, the evidence-contract era — and did not inherit unattendedCompletion. Since loadTerminalContract prefers the scoped role file where one exists, a review, review-fix or ci-fix checklist on an unattended trial lost the stamp, so a refused completion there printed advice no operator will read instead of ending the run. That is the stall #2208 exists to prevent, reappearing one checklist along. Whether anyone attends the run is a property of the run, like the gates beside it: a role checklist on an unattended trial has no reviewer either. Spread conditionally so an attended parent hands down nothing — the worker tests `=== true`, and writing `false` would assert something the parent never said. Found by Grok in #2208 r7 and parked there rather than widening that PR. Pre-existing and not from this change: the "a removed slot preserves legacy refresh" case in the same file sits at a 5 s subprocess budget and flaps on a loaded machine. It failed once here at 5012 ms and passed at 3434 ms and 3499 ms with this fix in place, and passed at 3300 ms without it.
Both reviewers found the inheritance alone was inert — Grok graded it a blocker and traced the whole chain: an eval trial is autonomous (methods/eval.ts), the writer puts mode and the stamp on the root contract, a CI failure syncs a ci-fix role contract, the role manifest points `mark complete` at CI-FIX.md, and loadTerminalContract prefers that scoped file. The exit hook that writes the blocked signal was gated on that same file having mode === 'autonomous' as well, and role sync passes no mode, so the stamp the role now inherits could never be read. The stamp is sufficient by itself. The gateway writes it only where the run declares `review: false`, which already says nobody attends, and a role checklist's contract legitimately has no dispatch mode. Inheriting mode instead would change what resolveWorkerTerminalContract produces for that role, which this fix has no business touching. My first test deserved the criticism it got: it asserted the field in the synced JSON and claimed a behaviour. The regression now drives the shape the gateway actually writes — a scoped role contract with the stamp and no mode — through a refused completion, and asserts REVIEW-SIGNAL.json ends blocked with the refusal as its reason. Reverting the gate alone makes it fail on "the role refusal ends the run". Pre-existing, measured again here: the "a removed slot preserves legacy refresh" case sits on a 5 s subprocess budget and flaps — 5016 ms and 5012 ms failing, 3300 ms, 3434 ms and 3499 ms passing, with and without this change.
deeeed
pushed a commit
that referenced
this pull request
Nov 15, 2026
…op-log and mark fixes (#2237) ## Why The operation-log fix (#2228) has never been published. `@farmslot/recipe-cli` 0.35.12 went to npm at 07:34Z on 15 November; #2228 merged at 18:10Z the same day. Harness 0.96.24 pins 0.35.11, so a worker is two releases behind the fix — which is why GLM 759e0f5a still showed 20 of 23 operation logs empty. The same gap holds for the checklist-mark work: published `agent-runtime` 0.48.1 predates #2232 (18:48Z) and the harness pins 0.48.0. ## What Both groups, cut with `yarn release:cut`. hosted-cc goes first because the release tool refuses to leave unreleased protocol bullets behind an npm cut; those hosted bullets are other lanes' work and go out as written. | Package | New version | Carries | | --- | --- | --- | | `@farmslot/command-center-ui` | 0.36.5 | other lanes | | `@farmslot/command-center` | 0.20.4 | other lanes | | `@farmslot/gateway` | 0.39.5 | other lanes | | `@farmslot/protocol` | 0.86.1 | the checklist-event declaration the mark engine writes | | `@farmslot/recipe-runner` | 0.46.19 | #2228's 2 MiB cap and truncation marker, and harness's 0.96.25 consumer note | | `@farmslot/agent-runtime` | 0.48.2 | #2220, #2222, #2232, #2234 | | `@farmslot/recipe-cli` | 0.35.13 | #2228's tee, and the `farmslot_mark` schema | | `@farmslot/adapter-node` | 0.8.20 | pin alignment | | `@farmslot/adapter-rn` | 0.31.21 | pin alignment | | `@farmslot/adapter-sdk` | 0.27.10 | pin alignment | | `@farmslot/adapter-web` | 0.19.17 | pin alignment | | `@farmslot/capabilities` | 0.7.4 | pin alignment | | `@farmslot/handoff` | 0.4.6 | pin alignment | | `@farmslot/review-dispatch` | 0.25.6 | pin alignment | | `@farmslot/skills` | 0.13.10 | pin alignment | A published package resolves its workspace dependencies to exact versions, so every package depending on the four engine packages still pins the previous cohort until it is republished — which would make mm-harness install two copies of the engine. All eight exact pinners are in, not only the two found by hand: `adapter-node`, `adapter-rn`, `adapter-sdk`, `adapter-web`, `capabilities`, `handoff`, `review-dispatch` and `skills` each depend on at least one of the four. Two editorial calls, both stated so they can be reversed: - Dropped recipe-runner's unreleased consumer note for the mm-harness 0.96.23 cut. It told readers to keep recipe-runner 0.46.17 and protocol 0.85.0 unchanged, which is no longer true and would have misdirected the very harness bump this cut exists to enable. - Put harness's 0.96.25 consumer note into 0.46.19 beside the cap it describes. A release of its own would have left the cohort with two runner versions and made every alignment bullet wrong. ## Follow-on, for the harness lane `metamask-harness` bumps `@farmslot/protocol` → 0.86.1, `@farmslot/recipe-runner` → 0.46.19, `@farmslot/agent-runtime` → 0.48.2, `@farmslot/recipe-cli` → 0.35.13, `@farmslot/adapter-node` → 0.8.20, `@farmslot/adapter-rn` → 0.31.21, `@farmslot/adapter-sdk` → 0.27.10, `@farmslot/adapter-web` → 0.19.17, `@farmslot/handoff` → 0.4.6, then releases 0.96.25, then the skill path pin follows. `yarn quality:changelogs` passes. No implementation changes here.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The gap
A role checklist gets its own terminal contract, and that contract inherits the run's frozen authority from the parent — the claim-review gate, the publication-review flag, the evidence-contract era, the acceptance contract version (
services/gateway/src/tasks/worker-terminal-contract.ts:460-474).It did not inherit
unattendedCompletion. BecauseloadTerminalContractprefers the scoped role file where one exists (packages/agent-runtime/scripts/mark-checklist-step.cjs), a review, review-fix or ci-fix checklist on an unattended trial lost the stamp — so a refused completion there printed completion advice no operator will ever read, instead of ending the run with the refusal as its reason.That is exactly the stall #2208 was built to prevent, reappearing one checklist along. Grok found it in #2208 r7; the lead parked it rather than widen that PR, and this is it.
The fix
One conditional spread beside the four projections already there:
Whether anyone attends the run is a property of the run, like the gates next to it: a role checklist on an unattended trial has no reviewer either.
Conditional rather than
unattendedCompletion: parent.unattendedCompletion, so an attended parent hands down nothing. The worker tests=== true, and writingfalsewould assert something the parent never said — the same reasoning that keepsacceptanceContractVersionconditional two lines below.Tests
Failing first, in
services/gateway/src/tasks/deterministic-checks.test.tswhere the role-sync cases already live:unattendedCompletion: true→ the syncedREVIEW.mdcontract carries it (red before the fix)falsedeterministic-checks.test.ts17/17 andwriter.test.ts46/46 with the fix.Pre-existing, not from this change
The "a removed slot preserves legacy refresh but refuses retained deterministic declarations" case in that same file sits on a 5 s subprocess budget and flaps under load. Measured here: 5012 ms (fail) once, then 3434 ms and 3499 ms (pass) with this fix applied, and 3300 ms (pass) without it. The variance is the machine, not the diff — but the budget is tight enough that it will keep flapping in CI.
Proof
Live, after deploy: an unattended run whose review checklist refuses completion ends the run terminally instead of parking. That is the behaviour this restores, and it is the same shape as #2208's own proof.