fix(bin): sync upstream startup and lifecycle safeguards - #14
Merged
Merged
Conversation
…d#1724) * fix(pi): stop Calm claiming a built-in tool name another extension owns fm-calm.ts claimed bash/read/edit/write/grep/find/ls unconditionally at extension load, regardless of whether Calm was on. Pi resolves two extensions registering the same built-in name by first-registered-wins with no merge and no unregister call, and Calm's project-local .pi/extensions/ position beats any global or CLI-configured extension, so a user who never even enabled Calm could have their own bash/read/etc override silently replaced. Captain-approved plan implemented: - Registration is now gated on config/calm already being "on" at load time. A Calm-off session or reload registers nothing, so a non-Calm user never contests a name. This stays synchronous during the factory's own load, not deferred to session_start: /reload (and ctx.newSession/fork/switchSession) render the restored transcript from a pre-session_start snapshot of the tool registry, so a deferred claim would miss that render - confirmed by tests/fm-calm-pi-extension .test.sh's hidden-block-geometry E2E when trialed. - The first time Calm turns on in a session that started off (activateBuiltInsIfNeeded, from the /calm command handler), Calm calls pi.getAllTools() - safe only once every extension has finished loading, unlike the load-time path above - to see whether a different extension already owns a name, and skips claiming only that one, leaving it and its owning extension fully intact and callable. - A contested name found this way prints a prominent ctx.ui.notify() warning naming the tool, plus a console diagnostic. - reportBuiltInLosses() remains the backstop for the one case neither of the above can reach: a session that starts or reloads with Calm already on, where the registry snapshot is taken before Calm gets any chance to check ownership. A symlink-safe realpath comparison avoids misreporting Calm's own registration as foreign when its path crosses a symlink (macOS /tmp, /var). Confirmed, bounded trade-off: the very first time a session that started Calm-off turns Calm on, tool-call rows already on screen from before that toggle do not retroactively collapse, because Pi never lets an extension re-point an already-rendered row at a definition registered later. Every session after that first toggle starts with the preference already on and takes the synchronous load-time path, so the guarantee is intact from then on. docs/calm.md and the file's own header document this in full. tests/fm-calm-pi-extension.test.sh gains test_builtin_gate_load_time (config/calm off registers nothing, on registers all 7 synchronously at load) and test_calm_activation_collision_and_regression_bound (first activation claims every uncontested built-in, leaves a foreign bash tool fully intact and callable, warns and logs the contested name, and locks in the documented pre-activation bound against real ToolExecutionComponent rendering). test_rendering_and_session_lifecycle and the live interactive E2E are updated for the new gate-at-load and first-activation-bound contract. * no-mistakes(document): Document Calm tool collision boundaries * no-mistakes: apply CI fixes
…#1727) * fix(bin): give secondmate homes a durable parent binding record Finished-worker cleanup on a remote second mate refused forever with "cannot resolve the primary home ... durable parent binding". The remote launch hands the child the remote code checkout as its parent home (fm-spawn.sh's sole writer of FM_PUBLIC_FOLLOWUP_PRIMARY_HOME receives FM_HOME=$FM_ROOT from fm-remote-secondmate-control.sh's host-local launch), and that path can never carry the parent's real records, so the guard refused unconditionally once relay looked active anywhere on that host. fm-home-seed.sh and fm-remote-home-provision.sh now write a durable .fm-secondmate-parent record next to the .fm-secondmate-home identity marker, naming the home's route to its parent as local (with the real parent path) or remote (with the parent's SSH alias for diagnostics only). fm-teardown.sh's cleanup gate reads it: a remote parent is out of scope for the delegated-promise check (the whole promised-public- reply subsystem is same-filesystem by construction, so a remote parent can never hold one), while a token committed directly to the child's own .env file - never the process environment - still refuses, so an unrelated export in the remote host's login shell can no longer mask in. For a local secondmate, the durable parent_home now also backs up the launch-time env var, closing a silent fail-open where a restart that dropped the launch prefix made the guard treat a genuinely active parent relay as off. Regression coverage drives the real remote route (SSH boundary + Herdr fixture) and real fm-home-seed.sh seeding rather than hand-crafted markers. * no-mistakes(review): Captain: fail closed on unsafe durable parent records * no-mistakes(review): Captain: enforce durable parent binding commit protocol * no-mistakes(review): Captain: publish local parent binding before identity * no-mistakes(review): Captain: refuse conflicting local parent bindings * no-mistakes(review): Captain: reject non-regular secondmate seed leaves * no-mistakes(review): Captain: enforce unique durable parent bindings * no-mistakes(review): Captain: reject route-incompatible durable parent fields * no-mistakes(document): Document durable secondmate parent bindings * no-mistakes(lint): Fix secondmate parent parser ShellCheck warnings * no-mistakes: apply CI fixes
* feat(bin): gate lavish-axi at its session_ended floor in bootstrap bin/fm-procevent-lavish.sh decides that a human "Send & End" review is terminal by reading session_ended from the poll response's leading session block. That field first shipped in lavish-axi 0.1.35, so an older installed build silently leaves every ended review source armed forever and captures an empty ended result on each later cycle. The same release is what makes a plain reopen refuse a session the human deliberately ended. Add LAVISH_AXI_MIN=0.1.35 to the existing axi-family floor structure in bin/fm-bootstrap.sh, reusing tool_version_at_least and the same MISSING diagnostic gh-axi already emits, so an incompatible build is reported as an upgrade request before any review surface is armed. Later lavish-axi releases only add artifact-authoring surface the adapter never reads, so the floor is the feature-introduction point rather than latest. Fixtures that stubbed lavish-axi as a bare exit-0 tool would now be read as unparseable builds, so tests/lib.sh gains fm_fake_version_tool and every bootstrap-running suite uses it for lavish-axi. * no-mistakes(review): Clarify lavish-axi version floor rationale * no-mistakes: apply CI fixes * feat(bin): set axi-family floors to current latest under the bump policy The axi-family bootstrap floors are the CURRENT LATEST published version of each tool, captain-bumped periodically to move the whole fleet onto the newest axi tools. They are not the minimum feature-introduced version. The earlier lavish-axi work set a feature-minimum floor, which is the opposite of this policy, so replace it along with the older feature-minimum rationale carried by tasks-axi and quota-axi. State the policy explicitly in bin/fm-bootstrap.sh's header, which owns it, and in each per-tool floor owner, so no future change argues a floor back down to the earliest release that happens to satisfy some behavior. Remove the lavish-axi session_ended and upstream-PR citation, the tasks-axi multi-ID-mv minimum argument, and the quota-axi credential-source argument as floor rationale; the tasks-axi feature probes remain as a separate defense-in-depth concern. Floors: lavish-axi 0.1.45 (was 0.1.35), tasks-axi 0.2.4 (was 0.2.2), quota-axi 0.1.17 (was 0.1.16), gh-axi 0.1.29 unchanged and already latest. Each was verified against the tool's current published version. The mechanism is unchanged: the same shared version helper and the same MISSING diagnostic path. The below-fires and at-or-above-silent regression rows move to the new floors, keeping each boundary genuine by pinning the patch immediately below each floor rather than a version that was only below the old one. Fleet fixtures move to the new floors so a bootstrap- running suite is not reported as an out-of-date build. Three operator-facing backlog handoff and receipt errors named "0.2.2+" while the enforced floor moved, so they now point at the floor's owner instead of duplicating a version number that drifts. * no-mistakes(review): Centralize AXI floor policy beside constants * no-mistakes(review): Clarify bootstrap boundary test comment * no-mistakes(document): Centralize AXI floor policy rationale
…guid#1737) * fix(bin): bound OPEN DECISIONS scan cost with a per-status-file cursor The fleet-wide OPEN DECISIONS scan added in kunchenguid#1711 re-reads and refolds every task's entire lifetime status log on every drain, so its cost grows unbounded with total log size. Add status_open_decisions_incremental and scan_open_decisions_incremental to fm-classify-lib.sh: they persist a per-status-file byte cursor plus the folded open-decision set, and fold only newly appended bytes on each call, reusing status_open_decisions' exact fold-line rule (extracted into _fm_decision_fold_line) so the two strategies can never disagree on what is open. A missing or invalidated cursor (new task, truncated/rewritten/shrunk log) falls back to a full re-fold. bin/fm-wake-drain.sh now calls the incremental wrapper instead of the whole-file scan. * fix(bin): add O(1) rotation detection and read-failure guarding to the cursor fold Add the two pieces the incremental open-decisions cursor was missing, scoped to this repo's actual status-file usage (create-once, append-only, never replaced or rewritten in place): - An O(1) device+inode identity check (one stat call) alongside the existing size-shrink check, so a status file replaced/rotated/recreated at the same path is detected and falls back to a full re-fold, even when the replacement is the same size. A same-inode, same-size, in-place byte edit is a deliberately accepted gap: no code path in this repo ever does that to a status file. - Checked reads: a stat/wc/tail failure is a genuine I/O error, not "the file is empty" - it now reports the already-trusted persisted open set unchanged instead of risking a silent invalidation. Both stay O(1) plus new bytes per call, matching the cursor's bounded- cost design; no content hashing or pending-fragment machinery. * no-mistakes(review): Preserve cursor state across failed incremental reads * no-mistakes(review): Refold status when cursor cache reads fail * no-mistakes(document): Document cursor-backed open-decision scanning * no-mistakes: apply CI fixes
…guid#1754) * fix(bin): preempt remote reply long-polls for queued short jobs Session start on a home with live remote second mates could stall silently for many minutes: the single serial remote job worker ran each armed fm-remote-delta-read.sh reply poll to its full 55s window while bootstrap's short sync, inherit, state, and route commands sat queued behind it, and non-FIFO queue pickup let re-armed polls keep winning the lane. Measured end to end, a trivial short job took 31s behind one 30s poll window. The worker now preempts a running preemptible job (the read-only, cursor- anchored delta read is the only member of that class) as soon as a non-preemptible job is queued, publishing exit 75 with emptied output - byte-identical to the poll's own elapsed-window-with-no-data result - so the parent runner takes its existing no-result path and the watcher re-arms from the same cursor with nothing lost. The delta read translates SIGTERM into that same exit after removing its staging directory. Sibling polls never preempt each other, so two armed monitors cannot churn. The same measured scenario now completes in 1s. * no-mistakes(document): Clarify remote poll preemption documentation
Sync the fork with the canonical template, incorporating 5 upstream commits while retaining the fork's own omp/herdr/secondmate/Pi-Calm work: - 6f0f87f fix(bin): prevent remote polls from blocking session startup (kunchenguid#1754) - bf01a42 fix(bin): bound open decision scans with incremental cursors (kunchenguid#1737) - ef2c3a2 feat(bin): enforce latest AXI-family tool floors (kunchenguid#1733) - 30b18b9 fix(bin): persist secondmate parent bindings for cleanup (kunchenguid#1727) - 71f0b3f fix(pi): gate Calm built-in overrides by activation state (kunchenguid#1724) Four files conflicted; each was resolved as a union that keeps both intents: - AGENTS.md: kept both new state-file inventory lines - the fork's .omp-primary-extension-loaded marker and upstream's .<id>.open-decisions-cursor. - bin/fm-teardown.sh: unioned the state-file cleanup rm list, keeping the fork's omp-ext.ts/omp-ready/omp-started removals and adding upstream's .<id>.open-decisions-cursor removal. - tests/fm-watcher-lock.test.sh: kept the fork's subshell form and EXIT-trap cleanup for the healthy-peer test and added upstream's peer_ready readiness handshake (declared peer_ready in the locals so the merged body resolves). - tests/fm-calm-pi-extension.test.sh: two hunks. In the renderer test, kept upstream's output-file redirect (the post-heredoc consumer auto-merged to out=$(cat "$output_file")) with the fork's .pi/extensions/ WATCH_EXT path (the fixture copies there). In the /calm E2E redraw loop, took upstream's corrected detection - scrollback capture (-S -600) and waiting for the collapsed-thinking row to hide rather than for CALM_E2E_OUTPUT to vanish, since the merged activation-gated fm-calm.ts keeps pre-activation built-in rows visible - and preserved the fork's config/calm=on persistence gate.
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.
Intent
Sync this firstmate fork with its upstream template by merging upstream/main into fm/upstream-sync-v1, resolving all conflicts, and shipping the result as a no-mistakes PR against origin (dnth/firstmate). This is a real 2-parent merge (fork was ~78 ahead / 5 behind), NOT a fast-forward. The 5 incoming upstream commits, all of which must land: 6f0f87f prevent remote polls from blocking session startup (kunchenguid#1754); bf01a42 bound open decision scans with incremental cursors (kunchenguid#1737); ef2c3a2 enforce latest AXI-family tool floors (kunchenguid#1733); 30b18b9 persist secondmate parent bindings for cleanup (kunchenguid#1727); 71f0b3f gate Calm built-in overrides by activation state (kunchenguid#1724). Every conflict was resolved by COMBINING both intents (keep the fork's own omp/herdr/secondmate/Pi-Calm work AND incorporate upstream's fix), never by dropping one side. Four files conflicted: (1) AGENTS.md - kept both new state-file inventory lines (fork's .omp-primary-extension-loaded marker and upstream's ..open-decisions-cursor). (2) bin/fm-teardown.sh - unioned the state-file cleanup rm list, keeping the fork's omp-ext.ts/omp-ready/omp-started removals and adding upstream's ..open-decisions-cursor removal. (3) tests/fm-watcher-lock.test.sh - kept the fork's subshell function form and EXIT-trap cleanup for the healthy-peer test and added upstream's peer_ready readiness handshake, declaring peer_ready in the locals so the auto-merged body resolves. (4) tests/fm-calm-pi-extension.test.sh, two hunks: in the renderer test kept upstream's output-file redirect (the post-heredoc consumer auto-merged to out=cat output_file) with the fork's .pi/extensions/ WATCH_EXT path where the fixture actually copies the file; in the /calm E2E redraw loop took upstream's corrected detection (scrollback capture -S -600 and waiting for the collapsed-thinking row to hide rather than for CALM_E2E_OUTPUT to vanish, because the merged activation-gated fm-calm.ts keeps pre-activation built-in tool rows visible per its documented bound) and preserved the fork's config/calm=on persistence gate. Deliberate scope constraint: the diff is limited to the merge plus conflict resolution plus any test/lint fixes the merge itself requires; no project code was changed outside the sync. Note: one local teardown test (content-landed) fails only because this host runs git 2.34.1 while the fork's content_in_default uses git merge-tree --write-tree (needs git >= 2.38); that check is byte-identical on both sides of the merge, so it is a pre-existing environment limitation unrelated to this sync, and CI runs modern git.
What Changed
Risk Assessment
✅ Low: Captain, the five upstream fixes land through the required two-parent merge, and the four conflict resolutions preserve both upstream and fork behavior without source-verifiable defects.
Testing
The merge structure, all five incoming fixes, and every conflict-resolution surface were exercised through six targeted suites, a focused teardown selector, manual CLI verification, and rendered Pi terminal evidence; all passed. Git 2.34.1 was confirmed, so the documented unrelated full-teardown merge-tree case was intentionally excluded, and one transient post-pass fixture directory was removed.
/tmp/no-mistakes-evidence/01KZAK2H4GF2SRRFKMZ3HY1FAX/calm-active-hidden.png)/tmp/no-mistakes-evidence/01KZAK2H4GF2SRRFKMZ3HY1FAX/calm-restarted.png)/tmp/no-mistakes-evidence/01KZAK2H4GF2SRRFKMZ3HY1FAX/calm-restored-off.png)Evidence: Incremental open-decisions CLI transcript
Evidence: Merge ancestry and combined conflict-resolution inventory
/tmp/no-mistakes-evidence/01KZAK2H4GF2SRRFKMZ3HY1FAX)Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
✅ **Review** - passed
✅ No issues found.
✅ **Test** - passed
✅ No issues found.
git show -s --format='%H%nparents:%P%nsubject:%s' HEADandgit merge-base --is-ancestor <incoming> HEADfor all five required commitsbash tests/fm-wake-drain-open-decisions-cursor.test.shbash tests/fm-remote-job.test.shbash tests/fm-remote-secondmate-parent-binding.test.shbash tests/fm-bootstrap.test.shbash tests/fm-calm-pi-extension.test.shbash tests/fm-watcher-lock.test.shFocusedtest_no_mistakes_origin_remote_allowsextraction fromtests/fm-teardown.test.shManual repeatedbin/fm-wake-drain.shsession demonstrating a buried decision persists across empty drains, clears after resolution, and persists its cursorRendered and inspected Pi terminal captures for Calm active, restart persistence, and Calm-off restorationCheckedgit status --short, evidence inventory, and removal of transient test fixtures✅ **Document** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.