fix(pi): recheck supervision outcomes before delivery - #2993
zachlandes wants to merge 7 commits into
Conversation
* A branch outcome was recorded the instant its claim was true and handed straight to Pi's own message queue, so a note reported inside the captain's running turn rendered after that turn saying whatever it had said at creation. A PR that merged in between still surfaced as review-ready, because nothing re-read the claim on the way out. * Held a note reported mid-turn in the extension instead, carrying the task's claim anchor, and re-checked it at main's idle boundary. Once Pi owns a message it cannot be inspected again, so the hold is what makes any re-check possible at all. * Dropped a stale routine note, which is noise by definition and stays readable in the durable store, and refreshed a stale captain one rather than suppressing it, so a real terminal outcome still opens its turn and reports the current truth. * Applied the same anchor to the session-start replay, which carries the longer of the two delivery windows: those rows never reached a handoff at all and can be arbitrarily old. * Anchored on the records that actually falsify a task-local claim, the task metadata and its armed merge poll, so ordinary progress on a task does not invalidate a note about it. An anchor that cannot be computed never reads as stale, so uncertainty never drops an outcome.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains. Reviews (6): Last reviewed commit: "no-mistakes(document): Correct Pi freshn..." | Re-trigger Greptile |
|
Speaking as Kun's firstmate: scheduled 7:10pm PT 8/24 pass (FM-FMOSS-CRON). First look on current class=corrective. This is a bugfix on the already-default-on Pi supervision delivery path, not a new grant. Inspected the DIFF, not the title: Does THIS DIFF fix mark-read-at-enqueue (#2984) vs a different path? It primarily fixes a different duplicate/stale-summary path: a review-ready note held across a captain turn was handed to Pi with the claim frozen at enqueue. The new hold queue re-checks the task claim-anchor (meta presence, On #2984 itself: #2977 (dup/displace captain outcomes): not covering. VISION.md (those files):
This HEAD: Security: none. Overlap/holds: none of spawn-freshen, #2804, herdr pair, or lock PRs. Files are Pi supervision + outcome store. Land-eligible: NO. Captain-flag NOW: no. waiting-on-author for a HEAD-matching attestation (and then green CI). Preferred later: this PR first among this batch once attestation+CI match, still not auto until those are green. Did not squash. |
|
Speaking as Kun's firstmate: first look on current class=corrective. Enqueue no longer calls Related #2977 (duplicate / displaced captain outcomes): partially covering — the delivery-boundary re-check and VISION.md (inspected
This HEAD: Security: none. Remaining documented non-atomic send-to-render window is the architecture's leftover, not a new hole. Overlap: none with spawn-freshen / pool / herdr / lock holds. Land-eligible rec: NO. Captain-flag NOW: no (author/CI: matching attestation + green CI). This is waiting-on-author. Not a captain-decision. Not a merge I will recommend. |
0a63e72 to
06264a5
Compare
|
Speaking as Kun's firstmate: correcting my earlier note on this thread. #2993 does not close #2984. The author's body is right, and I over-read the eligible-fetch closing link. This PR is the stale-claim / freshness path (queued review-ready note re-checked at idle delivery). #2977 stays open too (no A-then-B captain-outcome dedup). class=corrective and waiting-on-author (HEAD-matching attestation) are unchanged. Not a merge I will recommend. |
* Upstream moved 39 commits ahead while this reviewed change waited, and eight files conflicted; each was resolved as a union so both accepted intents survive rather than either side being dropped. * The captain outcome's typed operational envelope from upstream now wraps the note body chosen at this branch's delivery boundary, so a superseded captain outcome is self-describing too. Upstream's own stated invariant is that an unwrapped captain note can be mistaken for main's earlier answer and lost, and a refreshed outcome is no more self-describing than the original it replaces. * Two upstream tests asserted the mid-turn nextTurn handoff this change deliberately removed. Their assertions were kept intact and only the delivery timing moved to the idle boundary, because a note Pi already owns can no longer be re-checked before it is rendered. * Re-ran the real-SDK live guard and the strict typecheck against the installed Pi 0.84.4 and recorded the dated result, because this change alters the guard's own recorded output line.
zachlandes
changed the title
|
Closing this one. Main rewrote the outcome delivery path after I opened it ( The problem it was after is still there as far as I can tell. An outcome can still show up after the task it is about has moved on, for example a routine note queued while main is busy, or unread rows replayed after a crash. I have not proven those with a test yet. If I pick it back up it will be a fresh, narrower PR on the new path. |
2 participants
What Changed
Risk Assessment
🚨 High: The durable freshness fix remains bypassable by the repository's supported same-ID relaunch transition, allowing a stale outcome from a superseded task incarnation to be presented as current.
Testing
Targeted store, extension, and real Pi SDK checks demonstrated that stale task claims are marked superseded or dropped, current claims still arrive, routine notes explicitly disable turn triggering, and captain outcomes retain their typed handoff; the strengthened extension regression also passed on rerun, with the complete CLI transcript captured as evidence.
Evidence: Targeted supervision freshness and real-SDK validation transcript
Source: Targeted supervision freshness and real-SDK validation transcript
Pipeline
Updates from git push no-mistakes
⏭️ **intent** - skipped
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
bin/fm-branch-outcome.sh:138- The claim anchor omits the task's durablespawn_genincarnation. Concrete path: an outcome is held during a captain turn,fm-controlrelaunches the same task ID, and the replacement metadata retains the samepr=and poll presence; the anchor remains identical, so the old incarnation's outcome is delivered as current. Include the validated incarnation token in the anchor at this owning boundary, treating missing or ambiguous tokens as unverifiable for legacy records.✅ **Test** - passed
✅ No issues found.
Inspectedgit diff 4ad8cbaeafc109a17c1af3911867b7fe9e04e801..78f90f2312ecee93c57fee4e7254c471f18e6e1fto identify the freshness and delivery behavior.bin/fm-test-run.sh tests/fm-branch-supervision.test.shbin/fm-test-run.sh tests/fm-pi-branch-extension.test.shFM_PI_BRANCH_LIVE_E2E=1 bin/fm-test-run.sh tests/fm-pi-branch-live-e2e.test.shStrengthenedtests/fm-pi-branch-extension.test.shto requiretriggerTurn === falsefor immediate and held routine-note delivery, then reranbin/fm-test-run.sh tests/fm-pi-branch-extension.test.sh.Verified the evidence transcript directly and rangit diff --check.✅ **Document** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.