fix(pi): keep supervision outcome delivery responsive - #3767
Merged
Conversation
The supervision branch runs inside the captain's own Pi process, and Pi runs extensions, their tools, and their event handlers on the single JavaScript thread that also draws the TUI and reads the keyboard. Every delivered outcome ran roughly five bash script invocations plus several `ps` calls through spawnSync on that thread, so the TUI could not repaint or echo a keystroke for the whole chain - the subsecond freeze the captain saw every time a routine or captain-facing outcome arrived. Convert the delivery path's subprocess calls to an awaited spawn behind a serializing queue. lib/fm-async-exec.ts is the single owner of the awaited-spawn replacement and returns the same capture shape and failure verdicts spawnSync returned. Awaiting yields the thread, so what the single thread used to guarantee for free is now an explicit queue: every delivery, acknowledgement, and turn-boundary reconciliation runs as one unit of it, preserving the durable append before anything visible, one delivery at a time in sequence order, the read cursor advanced before the next reader sees a row, and one ownership activation per generation. Cancellation is preserved by the generation and lock-ownership rechecks the awaits are placed around. Two reads stay synchronous because Pi's own API is synchronous there, not as an optimization: its bash spawn hook is typed as a plain function, and the watcher reads offer.accepted the moment its dispatch event returns, so a session that does not own the fleet lock must still refuse a wake without waiting. Both walk the lock's process ancestry in full every time, never cached, because reparenting and pid reuse can invalidate a remembered chain and that answer decides ownership rather than hinting at it. The store scripts and their durability contracts are unchanged. Measured through the real fm_branch_report tool and real bin/ scripts with a 1 ms interval timer, the largest block of the JS thread falls from 273 to 2.0 ms for a routine outcome, 286 to 2.0 ms for a captain outcome, and 134 to 1.9 ms for main's acknowledgement, against a 1.3-2.2 ms idle floor. In a real Pi 0.82.0 TUI the worst keystroke echo while two outcomes arrive falls from 676.9 ms to 36.8 ms, against a 22.6 ms extension-free floor. Regressions: a delivery must leave the event loop running (zero timer ticks before this change, in 250 ms), interleaved reports stay ordered and exactly once, a session replaced mid-delivery neither loses nor duplicates an outcome, and a failing store script surfaces without losing or doubling one. The real-TUI half is an opt-in live guard that types into an isolated Pi pane while outcomes are delivered and fails if echo leaves the class of the same machine's own floor. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013bzoWyr2EcJGBKuoUjVRSp
Confidence Score: 5/5The PR appears safe to merge because no unacknowledged blocking failure remains. No blocking failure remains. Reviews (3): Last reviewed commit: "no-mistakes(document): Clarify asynchron..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
…uence-keyed delivery receipt before awaiting cursor advancement, preventing duplicate delivery after mark-read failure. Corrected the session-replacement test to exercise an actual asynchronous ps ancestry lookup. Targeted behavioral tests, strict Pi typecheck, ShellCheck, and diff checks pass. The full extension test remains locally blocked by an unrelated stock-render assertion under the installed Pi runtime. The no-mistakes attestation failure is external pipeline state (test was previously skipped), not a source defect
…case below its Pi floor Four follow-ups on the same branch, plus one revert. Revert the routine-delivery receipt a CI auto-fix round added. It introduced a new persisted `fm-branch-routine-delivery` entry, written into the captain's transcript for every routine note, to deduplicate a note whose cursor write failed. That is a change to the delivery contract, which this task is not authorized to make: the approved work is the asynchronous conversion with the existing durability contract preserved. The ownership re-read and output bounding from the review round are kept - both are genuine asynchronous correctness, not contract changes - as is that round's use of a real parent pid so the replacement regression traverses an actual ps subprocess. Record the routine gap instead of closing it. A routine note is a plain message with no sequence-keyed record, so a mark-read failure after delivery makes the next reconciliation send it once more; a captain row cannot duplicate that way because its visible entry is found by store sequence. That asymmetry predates moving delivery off the render thread. It is now stated at the call site and in the delivery-contract docs, tracked as fm-pi-routine-delivery-idempotency-followup-r1, and pinned by a regression that proves the routine note is re-delivered exactly once more and never again, the captain entry stays single, and the store keeps both rows. Give the stock-renderer case a Pi version floor. It compares the extension's renderers against Pi's stock rendering, so its verdict only means anything against the contract those renderers target: since 0.84.4 the stock renderer no longer supplies an implicit reset at multiline boundaries and the extension emits that reset itself, so an older installed Pi differs legitimately. It now names the installed version and the floor and skips, while a package whose version cannot be read at all still fails. Make the responsiveness regression's second signal a fraction rather than a millisecond budget. A loaded machine that deschedules the process inflates an absolute stall budget into a false failure, but it inflates the delivery's own wall time too, so requiring the worst stall to be a minority of that wall time holds under load. Synchronous delivery sits near 1.0 there whatever the load, and the tick-count signal still reads zero on it. Replace the test-family mapping for the Pi extension libraries with per-script targeting. Routing them to whole families - or leaving them unmapped, which widens through the reference scan to each referencing suite's entire family - selected dozens of suites with nothing to do with Pi and pulled an unrelated flake into the run. The changed-file selection drops from 112 scripts to 61. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013bzoWyr2EcJGBKuoUjVRSp
kunchenguid
changed the title
lytv
pushed a commit
to lytv/mymate
that referenced
this pull request
Sep 8, 2026
* fix(pi): deliver supervision outcomes off Pi's render thread The supervision branch runs inside the captain's own Pi process, and Pi runs extensions, their tools, and their event handlers on the single JavaScript thread that also draws the TUI and reads the keyboard. Every delivered outcome ran roughly five bash script invocations plus several `ps` calls through spawnSync on that thread, so the TUI could not repaint or echo a keystroke for the whole chain - the subsecond freeze the captain saw every time a routine or captain-facing outcome arrived. Convert the delivery path's subprocess calls to an awaited spawn behind a serializing queue. lib/fm-async-exec.ts is the single owner of the awaited-spawn replacement and returns the same capture shape and failure verdicts spawnSync returned. Awaiting yields the thread, so what the single thread used to guarantee for free is now an explicit queue: every delivery, acknowledgement, and turn-boundary reconciliation runs as one unit of it, preserving the durable append before anything visible, one delivery at a time in sequence order, the read cursor advanced before the next reader sees a row, and one ownership activation per generation. Cancellation is preserved by the generation and lock-ownership rechecks the awaits are placed around. Two reads stay synchronous because Pi's own API is synchronous there, not as an optimization: its bash spawn hook is typed as a plain function, and the watcher reads offer.accepted the moment its dispatch event returns, so a session that does not own the fleet lock must still refuse a wake without waiting. Both walk the lock's process ancestry in full every time, never cached, because reparenting and pid reuse can invalidate a remembered chain and that answer decides ownership rather than hinting at it. The store scripts and their durability contracts are unchanged. Measured through the real fm_branch_report tool and real bin/ scripts with a 1 ms interval timer, the largest block of the JS thread falls from 273 to 2.0 ms for a routine outcome, 286 to 2.0 ms for a captain outcome, and 134 to 1.9 ms for main's acknowledgement, against a 1.3-2.2 ms idle floor. In a real Pi 0.82.0 TUI the worst keystroke echo while two outcomes arrive falls from 676.9 ms to 36.8 ms, against a 22.6 ms extension-free floor. Regressions: a delivery must leave the event loop running (zero timer ticks before this change, in 250 ms), interleaved reports stay ordered and exactly once, a session replaced mid-delivery neither loses nor duplicates an outcome, and a failing store script surfaces without losing or doubling one. The real-TUI half is an opt-in live guard that types into an isolated Pi pane while outcomes are delivered and fails if echo leaves the class of the same machine's own floor. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013bzoWyr2EcJGBKuoUjVRSp * no-mistakes(review): Revalidate ownership and bound asynchronous subprocess output * no-mistakes(ci): Fixed CI defects: routine outcomes now persist a sequence-keyed delivery receipt before awaiting cursor advancement, preventing duplicate delivery after mark-read failure. Corrected the session-replacement test to exercise an actual asynchronous ps ancestry lookup. Targeted behavioral tests, strict Pi typecheck, ShellCheck, and diff checks pass. The full extension test remains locally blocked by an unrelated stock-render assertion under the installed Pi runtime. The no-mistakes attestation failure is external pipeline state (test was previously skipped), not a source defect * fix(pi): keep the declined routine receipt out and skip the renderer case below its Pi floor Four follow-ups on the same branch, plus one revert. Revert the routine-delivery receipt a CI auto-fix round added. It introduced a new persisted `fm-branch-routine-delivery` entry, written into the captain's transcript for every routine note, to deduplicate a note whose cursor write failed. That is a change to the delivery contract, which this task is not authorized to make: the approved work is the asynchronous conversion with the existing durability contract preserved. The ownership re-read and output bounding from the review round are kept - both are genuine asynchronous correctness, not contract changes - as is that round's use of a real parent pid so the replacement regression traverses an actual ps subprocess. Record the routine gap instead of closing it. A routine note is a plain message with no sequence-keyed record, so a mark-read failure after delivery makes the next reconciliation send it once more; a captain row cannot duplicate that way because its visible entry is found by store sequence. That asymmetry predates moving delivery off the render thread. It is now stated at the call site and in the delivery-contract docs, tracked as fm-pi-routine-delivery-idempotency-followup-r1, and pinned by a regression that proves the routine note is re-delivered exactly once more and never again, the captain entry stays single, and the store keeps both rows. Give the stock-renderer case a Pi version floor. It compares the extension's renderers against Pi's stock rendering, so its verdict only means anything against the contract those renderers target: since 0.84.4 the stock renderer no longer supplies an implicit reset at multiline boundaries and the extension emits that reset itself, so an older installed Pi differs legitimately. It now names the installed version and the floor and skips, while a package whose version cannot be read at all still fails. Make the responsiveness regression's second signal a fraction rather than a millisecond budget. A loaded machine that deschedules the process inflates an absolute stall budget into a false failure, but it inflates the delivery's own wall time too, so requiring the worst stall to be a minority of that wall time holds under load. Synchronous delivery sits near 1.0 there whatever the load, and the tick-count signal still reads zero on it. Replace the test-family mapping for the Pi extension libraries with per-script targeting. Routing them to whole families - or leaving them unmapped, which widens through the reference scan to each referencing suite's entire family - selected dozens of suites with nothing to do with Pi and pulled an unrelated flake into the run. The changed-file selection drops from 112 scripts to 61. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013bzoWyr2EcJGBKuoUjVRSp * no-mistakes(document): Clarify asynchronous execution documentation --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
BenWilcox8
pushed a commit
to BenWilcox8/firstmate
that referenced
this pull request
Sep 12, 2026
* fix(pi): deliver supervision outcomes off Pi's render thread The supervision branch runs inside the captain's own Pi process, and Pi runs extensions, their tools, and their event handlers on the single JavaScript thread that also draws the TUI and reads the keyboard. Every delivered outcome ran roughly five bash script invocations plus several `ps` calls through spawnSync on that thread, so the TUI could not repaint or echo a keystroke for the whole chain - the subsecond freeze the captain saw every time a routine or captain-facing outcome arrived. Convert the delivery path's subprocess calls to an awaited spawn behind a serializing queue. lib/fm-async-exec.ts is the single owner of the awaited-spawn replacement and returns the same capture shape and failure verdicts spawnSync returned. Awaiting yields the thread, so what the single thread used to guarantee for free is now an explicit queue: every delivery, acknowledgement, and turn-boundary reconciliation runs as one unit of it, preserving the durable append before anything visible, one delivery at a time in sequence order, the read cursor advanced before the next reader sees a row, and one ownership activation per generation. Cancellation is preserved by the generation and lock-ownership rechecks the awaits are placed around. Two reads stay synchronous because Pi's own API is synchronous there, not as an optimization: its bash spawn hook is typed as a plain function, and the watcher reads offer.accepted the moment its dispatch event returns, so a session that does not own the fleet lock must still refuse a wake without waiting. Both walk the lock's process ancestry in full every time, never cached, because reparenting and pid reuse can invalidate a remembered chain and that answer decides ownership rather than hinting at it. The store scripts and their durability contracts are unchanged. Measured through the real fm_branch_report tool and real bin/ scripts with a 1 ms interval timer, the largest block of the JS thread falls from 273 to 2.0 ms for a routine outcome, 286 to 2.0 ms for a captain outcome, and 134 to 1.9 ms for main's acknowledgement, against a 1.3-2.2 ms idle floor. In a real Pi 0.82.0 TUI the worst keystroke echo while two outcomes arrive falls from 676.9 ms to 36.8 ms, against a 22.6 ms extension-free floor. Regressions: a delivery must leave the event loop running (zero timer ticks before this change, in 250 ms), interleaved reports stay ordered and exactly once, a session replaced mid-delivery neither loses nor duplicates an outcome, and a failing store script surfaces without losing or doubling one. The real-TUI half is an opt-in live guard that types into an isolated Pi pane while outcomes are delivered and fails if echo leaves the class of the same machine's own floor. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013bzoWyr2EcJGBKuoUjVRSp * no-mistakes(review): Revalidate ownership and bound asynchronous subprocess output * no-mistakes(ci): Fixed CI defects: routine outcomes now persist a sequence-keyed delivery receipt before awaiting cursor advancement, preventing duplicate delivery after mark-read failure. Corrected the session-replacement test to exercise an actual asynchronous ps ancestry lookup. Targeted behavioral tests, strict Pi typecheck, ShellCheck, and diff checks pass. The full extension test remains locally blocked by an unrelated stock-render assertion under the installed Pi runtime. The no-mistakes attestation failure is external pipeline state (test was previously skipped), not a source defect * fix(pi): keep the declined routine receipt out and skip the renderer case below its Pi floor Four follow-ups on the same branch, plus one revert. Revert the routine-delivery receipt a CI auto-fix round added. It introduced a new persisted `fm-branch-routine-delivery` entry, written into the captain's transcript for every routine note, to deduplicate a note whose cursor write failed. That is a change to the delivery contract, which this task is not authorized to make: the approved work is the asynchronous conversion with the existing durability contract preserved. The ownership re-read and output bounding from the review round are kept - both are genuine asynchronous correctness, not contract changes - as is that round's use of a real parent pid so the replacement regression traverses an actual ps subprocess. Record the routine gap instead of closing it. A routine note is a plain message with no sequence-keyed record, so a mark-read failure after delivery makes the next reconciliation send it once more; a captain row cannot duplicate that way because its visible entry is found by store sequence. That asymmetry predates moving delivery off the render thread. It is now stated at the call site and in the delivery-contract docs, tracked as fm-pi-routine-delivery-idempotency-followup-r1, and pinned by a regression that proves the routine note is re-delivered exactly once more and never again, the captain entry stays single, and the store keeps both rows. Give the stock-renderer case a Pi version floor. It compares the extension's renderers against Pi's stock rendering, so its verdict only means anything against the contract those renderers target: since 0.84.4 the stock renderer no longer supplies an implicit reset at multiline boundaries and the extension emits that reset itself, so an older installed Pi differs legitimately. It now names the installed version and the floor and skips, while a package whose version cannot be read at all still fails. Make the responsiveness regression's second signal a fraction rather than a millisecond budget. A loaded machine that deschedules the process inflates an absolute stall budget into a false failure, but it inflates the delivery's own wall time too, so requiring the worst stall to be a minority of that wall time holds under load. Synchronous delivery sits near 1.0 there whatever the load, and the tick-count signal still reads zero on it. Replace the test-family mapping for the Pi extension libraries with per-script targeting. Routing them to whole families - or leaving them unmapped, which widens through the reference scan to each referencing suite's entire family - selected dozens of suites with nothing to do with Pi and pulled an unrelated flake into the run. The changed-file selection drops from 112 scripts to 61. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013bzoWyr2EcJGBKuoUjVRSp * no-mistakes(document): Clarify asynchronous execution documentation --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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
1 participant
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.
Intent
The captain approved shipping a focused ASYNCHRONOUS conversion of the Pi supervision outcome-delivery path to eliminate the visible subsecond UI freeze that occurs whenever a routine (sailboat) or captain-facing (anchor) outcome arrives into his Pi main session. Captain's exact words: "I agree with the asynchronous fix."
THE PROBLEM (established diagnosis): the supervision branch runs inside the captain's own Pi process, and Pi runs extensions, their tools, and their event handlers on the single JavaScript thread that also draws the TUI and reads the keyboard. Each delivered outcome runs about five bash script invocations (bin/fm-branch-outcome.sh append, processed-init, unread, mark-read per row, unprocessed, plus bin/fm-lease.sh and bin/fm-operational-input.sh encode) plus four to six
pscalls for the lock-ownership ancestry walk, all through synchronous spawnSync on that thread, so the TUI cannot repaint or echo a keystroke for roughly 250-440 ms per outcome. It is not rendering, transcript size, store size, or the model: a silent heartbeat that paints nothing still blocked, and the same chain minus the append also runs at the session_start, turn_end, and agent_settled boundaries, so a turn boundary costs the same stall even when no outcome exists. The relevant code is .pi/extensions/fm-branch-supervision.ts: createReportTool/execute, lockOwnership, runOutcomeScript, reconcileUnreadOutcomes, presentUnprocessedOutcomes, and those three event boundaries. Those millisecond figures are host- and Pi-version-specific, not the captain's live-session latency; the code-path finding is what holds.THE FIX (the captain's chosen option): convert the delivery path's subprocess calls from spawnSync to asynchronous spawn plus await behind a serializing queue, so the TUI keeps painting while the store and script work happens off the render-blocking path.
INVARIANTS THAT MUST BE PRESERVED (the captain's explicit constraints):
Keep the existing durable store and script contracts (bin/fm-branch-outcome.sh and friends) unchanged. Do NOT add a new framework, do NOT batch away acknowledgement boundaries, and do NOT cache the ownership authority merely for speed: process ancestry is not immutable (reparenting and pid-identity reuse can invalidate a cached ancestor chain) and it feeds an ownership-authority check, so caching it is correctness-sensitive rather than trivial.
TESTS (behavioral, required): interleaved deliveries; session replacement/cancellation; failures that must not lose or duplicate outcomes; PLUS a real-Pi typing/repaint responsiveness check during the delivery path under the tested supported Pi version.
BOUNDARIES: preserve the MacBook/Pi-version limitations of the measurements rather than overclaiming them; do NOT alter the captain's live Pi session or global Pi settings, using an isolated Pi/session for testing. Report readiness with the full PR URL and the ACTUAL measured responsiveness, not a predicted latency target. Implementation is approved; merge is NOT (the captain merges).
What Changed
Risk Assessment
Testing
The successful changed-test baseline was supplemented with focused delivery regressions covering event-loop responsiveness, interleaving and serialization, session replacement/cancellation, durability, failure handling, and acknowledgement behavior. An isolated real Pi 0.82.0 TUI measured worst keystroke echo at 32.0 ms unloaded, 54.4 ms extension-idle, and 53.0 ms while delivering outcomes. No screenshot was captured because the reported defect is a temporal input/repaint freeze; the reviewer-visible timing transcript directly records the end-user behavior.
Evidence: Real Pi typing and repaint responsiveness measurement
Source: Real Pi typing and repaint responsiveness measurement
pi 0.82.0 keystroke echo, worst observed: floor 32.0 ms, extension idle 54.4 ms, extension delivering 53.0 msEvidence: Focused asynchronous delivery behavioral regressions
Source: Focused asynchronous delivery behavioral regressions
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
tests/fm-pi-branch-extension.test.sh:21- The new Pi renderer version floor, custom version comparator, and conditional skip are unrelated to the required asynchronous outcome-delivery conversion or its responsiveness tests. No stated intent requires changing this pre-existing renderer test; remove this component and handle renderer compatibility separately if desired.✅ **Test** - passed
✅ No issues found.
bin/fm-test-run.sh --changed --exclude-family real-herdr-gatedBaseline (already completed):bin/fm-test-run.sh --changed --exclude-family real-herdr-gatedbin/fm-test-run.sh tests/fm-pi-branch-extension.test.shFM_PI_BRANCH_RESPONSIVENESS_E2E=1 bash tests/fm-pi-branch-responsiveness-live-e2e.test.shgit status --shortconfirmed testing left the worktree clean✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.