fix(pi): preserve extension-owned tools in Calm mode - #1857
Open
alexdancer wants to merge 18 commits into
Open
Conversation
…guid#1860) * perf(session-start): run every network check off the blocking path The session-start digest runs on a session-open hook that blocks session initialization, and every external-network call it made was individually unbounded: `gh auth status`, secondmate liveness, secondmate convergence, pending remote handoff delivery, and the fleet-sync fetch. One unreachable remote secondmate could consume the whole FM_SESSION_START_TIMEOUT and truncate the digest, so a slow network could cost the work queue itself. Measured against a host hanging 25s per SSH connection, that startup took 1m18s. The digest is now composed from local reads alone. bin/fm-startup-network.sh runs the same checks concurrently in a bounded detached worker and the digest harvests whatever finished, without ever waiting. Same fixture: 0.84s. Nothing is dropped. fm-bootstrap.sh stays the single owner of every sweep and still runs all of them, through a new FM_BOOTSTRAP_NETWORK phase split whose `skip` and `only` halves are a partition of the unsplit run. Deferral is safe because the sweeps are idempotent detectors, the result is durable and always surfaces (inline, or as a `check: startup-network` wake), and the worker re-verifies that the fleet lock still names the session that asked before it mutates anything. While the worker is still running the digest names exactly what is unconfirmed rather than implying it passed. A relaunch performed by the deferred pass is now always reported, because the digest that printed the superseded endpoint record is already out. Also collapses the duplicate tasks-axi compatibility probe: the verdict is computed once and handed to the bootstrap child for one process hop, then consumed so it never reaches a spawned agent's environment. 10 tasks-axi invocations per startup become 7. Verified on Claude Code 2.1.222 that a worker detached by the session-open hook survives the hook returning, the one vendor behavior this design needs and no portable test can see. Re-landed on current main, superseding PR kunchenguid#1845, which was cut from a pre-kunchenguid#1842 base. The digest's section numbering in AGENTS.md section 3 now states the emission order directly - supervision block and its read-once contract, fleet state, network checks, then context - which keeps kunchenguid#1826's fleet-state-before-context ordering. The old-bin test shim keeps main's git-archive baseline from kunchenguid#1851, which already subsumes this branch's reason for widening that shim. * docs(verification): re-measure the deferred startup stage on the current base Re-runs the unreachable-remote latency fixture against default-branch tip 8398d31 rather than the now-historical 345de4e, and records the sweep-result comparison the deferral's safety argument rests on: the deferred worker's published report is byte-identical to the three sweep lines the blocking baseline printed, with the unreachable route preserved in both. * no-mistakes(review): Fail deferred startup when report publication fails * no-mistakes(document): Document deferred startup network behavior accurately
…tool-conflicts-upstream
* fix(procevent): apply a captured adapter result in code, not by instruction A remote secondmate's reply was captured and announced, but never applied. Nothing dispatched the reply adapter's `handle` on a `procevent remote-reply` wake, and the handling instruction named only the generic acknowledgement, so the wake was retired while everything it carried was dropped: the reply never reached the secondmate's local status mirror, the request it answered kept escalating as a missed report, and the relay - whose registration each capture retires, and which only that same handling re-arms - was left dead until the next session start armed it again. Applying such a result carries no judgement, so it belongs in code. After publishing, the runner now calls `bin/fm-procevent-<adapter>.sh autohandle <source-id> <sequence> <result-file>` and lets the adapter apply and acknowledge its own result, through the same kind of seam that already owns the terminal verdict. It runs strictly after terminal retirement, because a handling adapter re-arms its own next source and retiring afterwards would drop that fresh registration. An adapter with no such command, or one whose pass does not complete, leaves the result unacknowledged and therefore still announced, so a handler receives it exactly as before. Resolving the request was not enough on its own either. An escalation opens a durable keyed decision in the parent status log, and nothing ever closed it, so a request the remote had answered kept surfacing in every later open-decisions fold. The pending-reply library now owns both ends of that decision: it opens one under a per-request key rather than the shared default key, and closes it once the record resolves, appending the closing line only while that exact decision is still open in the fold so it can neither double-close nor clear an unrelated decision that has since taken the same key. The handling instruction still routes a wake to its adapter, now as the idempotent confirmation of what the runner already did rather than as the guarantee. Verified end to end in a throwaway isolated home driving the real armed source, blocking delta reader, runner, and wake queue, with the handler doing only the generic acknowledgement and no part of the ingest stubbed: before, seven failed observations reproducing the incident; after, none. Each half is independently load-bearing - without the runner change the reply never reaches the mirror, without the escalation close the settled request still surfaces as an open decision. * no-mistakes(review): Prevent legacy reply closure from masking decisions * no-mistakes(review): Serialize pending reply resolution and escalation closure * no-mistakes(review): Serialize pending reply escalation with resolution * no-mistakes(review): Clarify guarded legacy escalation closure behavior * no-mistakes(review): Guard legacy closure and reserve pending reply keys * no-mistakes(review): Match pending reply escalations by construction * no-mistakes(document): Document automatic remote reply resolution * no-mistakes(lint): Fix unused concurrent escalation loop variable * no-mistakes(lint): Fix unused concurrent resolution loop binding * no-mistakes(review): Version fold cache and gate autohandle on publication * no-mistakes(document): Clarify remote reply relay documentation
alexdancer
changed the title
Open
5 tasks
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
2 participants
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
Fix Firstmate Calm's Pi 0.84 startup collision with extension-owned replacements for any built-in tool. Reproduce and separate the startup trigger, Calm-on masking condition, Calm-off path, and smallest counterfactual; inspect Pi 0.84 extension-loading APIs and Calm history and seek disconfirming evidence. Calm must not register wrappers during extension discovery; after runtime binding it must wrap every uncontested built-in while preserving the earlier extension's complete implementation, execution, metadata, prompt behavior, and rendering for contested names. Preserve Calm-on startup, restored-row behavior to the extent Pi permits, Calm-off and first-activation behavior, and fail safely when ownership cannot be verified. Do not weaken duplicate-tool detection globally, disable unrelated extensions, or modify the external PDF reader. Add executable regression coverage through public Pi behavior for persisted Calm with an earlier replacement, ordinary built-ins, Calm off and activation, without asserting source bytes. Also eliminate Pi 0.84's headless Calm-off stale shutdown-context diagnostic by avoiding invalidated UI context after headless runs and cover that lifecycle behavior. Keep comments and maintained docs concise and in their authoritative owners. Update only existing PR #1857 on alexdancer:fix/calm-extension-tool-conflicts-upstream; preserve its a11c941 history, include current upstream main through fast-forwardable descendant history only, never rewrite or force-push, and never open a duplicate PR.
What Changed
Risk Assessment
✅ Low: Captain, the prior ownership and ancestry defects are resolved, and the remaining change is a bounded fail-closed extension-lifecycle fix with targeted regression coverage.
Testing
Against installed Pi 0.84.0, the real public-runtime regression demonstrated collision-free persisted-Calm startup while preserving the earlier PDF replacement’s execution, metadata, prompt behavior, and renderers; focused tests also passed for uncontested built-ins, Calm off and activation, restored-row limits, fail-closed ownership, and the headless shutdown lifecycle. The initial package lookup skipped several cases, so they were rerun successfully with Pi’s explicit pnpm path. No screenshot was produced because the changed end-user surface is startup/headless ownership behavior; evidence is the real Pi runtime transcript and renderer checks, while the native TUI regression completed without retaining image output.
Evidence: Pi 0.84 live conflict regression
ok - Pi 0.84.0 real runtime kept persisted Calm from contesting PDF read and preserved its execution, prompt metadata, and renderersEvidence: Complete Calm regression
Focused Pi 0.84 regression transcript covering discovery, ownership, activation, restored rows, Calm-off headless shutdown, persistence, and native TUI behavior.Evidence: Test-runner registration
ok - live family includes both Pi vendor guardsPipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
🔧 **Review** - 2 issues found → auto-fixed ✅
.pi/extensions/fm-calm.ts:323- The intent requires Calm to “fail safely when ownership cannot be verified” and “leave every contested name untouched.” Pi filtersgetAllTools()through--tools/--exclude-tools; when a built-in is omitted,owneris undefined here and the tool is treated as uncontested, so line 342 registers Calm’s wrapper despite having no ownership evidence. Treat missing or malformed inventory entries as unverified and skip those names at this shared ownership boundary.a11c941through fast-forwardable descendant history without rewriting. The target33c5becis not a descendant ofa11c941; its equivalent change chain begins ate5958ecdirectly ond8bb074, while the existing PR branch still containsa11c941. Rebuild the update as a descendant ofa11c941—for example by merging current main into that preserved chain—before delivery.🔧 Fix: Preserve Calm ancestry and fail closed on ownership
✅ Re-checked - no issues remain.
✅ **Test** - passed
✅ No issues found.
git diff --stat d8bb074f9f9a5ab4e21d243e8c172f03faff74ac..b1e834e793e8afa53456c5e0bb191f302dd33b4api --versionFM_PI_LIVE_E2E=1 tests/fm-calm-pi-conflict-live-e2e.test.shtests/fm-calm-pi-extension.test.sh(initial package-discovery run; setup skips identified)FM_PI_PACKAGE_DIR=/Users/alex/Library/pnpm/global/v11/f259-19fd905a8ad-441941e8f452438d/node_modules/@earendil-works/pi-coding-agent tests/fm-calm-pi-extension.test.shtests/fm-test-run.test.shgit status --short✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.