fix(bin): recognize kimi variants and pi launcher basenames in harness detection - #1363
Closed
sbracewell64 wants to merge 1 commit into
Closed
sbracewell64 wants to merge 1 commit into
sbracewell64 wants to merge 1 commit into
Conversation
Three places classify a process as a harness, and they disagreed. Measured against current main by running each rule over the same basenames: comm fm-harness session-lock tmux-alive kimi-nightly unknown yes alive pi-launcher unknown no alive Pi unknown no alive Two consequences, both real and both fail-safe rather than dangerous: kimi was matched EXACTLY in bin/fm-harness.sh while its four markerless siblings (claude, codex, opencode, grok) are substring globs, and while bin/fm-session-lock-lib.sh and bin/backends/tmux.sh both matched it as a substring. A kimi under a variant basename was therefore recognized by the other two consumers but self-detected as `unknown`. pi-launcher and Pi - the launcher wrapper and the npm shim basename, both already known to bin/backends/tmux.sh's pane classifier - were absent from self-detection and from the session-lock ancestry regex. A firstmate running under either reported `unknown`, and could not prove it owned its home's session lock, so bin/fm-claude-stop-autoarm.sh's ownership check would decline to arm. fm_session_lock_owned_by_self fails closed, so this was a false negative, never a false ownership claim. Align the two outliers: kimi becomes a substring match like its siblings, and the pi family gains the two missing basenames. The pi family stays ANCHORED on purpose - a bare `pi` substring would also match pip, pipenv, and any path component containing "pi" - so each launcher basename gets its own exact arm rather than relaxing the anchor. Deliberately NOT done: the originally-planned shared process-identity table. It would have replaced ~15 lines of clear in-place literals with 80-100 lines of table, dispatcher, three per-consumer generators, and two exceptions to encode exactly the divergences fixed above - failing the design's own stop condition, with its justifying defect (pi missing from the tmux alive list) already fixed on main by kunchenguid#1145. The inconsistencies were the finding worth shipping; the machinery was not. Coverage extends tests/fm-kimi-harness.test.sh rather than adding a runner, and asserts behavior by executing bin/fm-harness.sh and bin/fm-lock.sh against fake ancestry. Each case was verified to FAIL before the fix: reverting kimi to an exact arm, dropping the pi-family arms, and reverting the session-lock regex each reproduce the original miss. pip, pipenv, and piper are asserted to stay unrecognized so the anti-substring guard cannot be relaxed unnoticed.
Owner
|
Automated reminder: thanks for the PR! This branch currently has a merge conflict with the base branch. When you get a chance, please rebase onto (or merge) the latest base branch, resolve the conflict, and push. After that, checks will re-run and the PR will get looked at again. Noted for firstmate#1363 at |
Owner
|
Speaking as Kun's firstmate: closing this as stale. It has been waiting on a contributor update for 14+ days with no author push or comment. Reopen if you want to pick it back up. |
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
Fix two measured inconsistencies in how firstmate recognizes a harness process name, and deliberately NOT ship the refactor that was originally planned here.
BACKGROUND. This was scoped as increment C of a harness-interface effort: a shared per-harness process-identity table replacing three separate lists. I measured it against current main first and recommended dropping the refactor, which the captain approved. It would have replaced ~15 lines of clear in-place literals (12 case arms in bin/fm-harness.sh, one regex in bin/fm-session-lock-lib.sh, one glob arm in bin/backends/tmux.sh) with 80-100 lines of table, dispatcher, three per-consumer generators, and two exceptions - and the exceptions would have encoded exactly the divergences fixed below rather than removing them, because the captain's ruling required preserving all three matching rules byte-identically. Its justifying defect (pi missing from the tmux alive list) is already fixed on main by #1145. That trips the design's own stop condition, so the inconsistencies are shipped and the machinery is not. Do not suggest reintroducing the table.
THE MEASUREMENT, run over the same basenames through each rule:
comm fm-harness session-lock tmux-alive
kimi-nightly unknown yes alive
pi-launcher unknown no alive
Pi unknown no alive
FIX 1: kimi was matched EXACTLY in bin/fm-harness.sh while its four markerless siblings (claude, codex, opencode, grok) are substring globs, and while both other consumers matched it as a substring. A kimi under a variant basename was recognized by those two but self-detected as unknown. It becomes a substring match, consistent with its siblings.
FIX 2: pi-launcher and Pi - the launcher wrapper and the npm shim basename, both already known to bin/backends/tmux.sh's pane classifier - were missing from self-detection and from the session-lock ancestry regex. A firstmate under either reported unknown and could not prove session-lock ownership, so bin/fm-claude-stop-autoarm.sh's check would decline to arm. fm_session_lock_owned_by_self fails closed, so this was a false NEGATIVE, never a false ownership claim. Both basenames are added.
DELIBERATE: the pi family stays ANCHORED. A bare 'pi' substring would also match pip, pipenv, and any path component containing 'pi', so each launcher basename gets its own exact arm rather than relaxing the anchor to a glob. Do not 'simplify' those exact arms into pi.
VERIFICATION. Coverage extends tests/fm-kimi-harness.test.sh rather than adding a runner, and asserts behavior by executing bin/fm-harness.sh and bin/fm-lock.sh against fake ancestry - no source-content assertions, per the repo rule. Every case was verified to FAIL before the fix, not assumed: reverting kimi to an exact arm, dropping the pi-family arms, and reverting the session-lock regex each reproduce the original miss as a test failure. pip, pipenv, and piper are asserted to stay unrecognized so the anti-substring guard cannot be relaxed unnoticed.
LOCAL SUITE NOTE. tests/fm-watcher-lock.test.sh fails on this machine with 'restart did not attach to the verified healthy peer'. I verified it fails IDENTICALLY on a clean checkout of current origin/main with none of these changes present - same test, same message, only the pid differs - so it is environmental, not a regression from touching fm-session-lock-lib.sh. fm-kimi-harness and fm-secondmate-harness pass, and lint is clean.
What Changed
bin/fm-harness.shself-detection now matcheskimias a substring glob (*kimi*), consistent with its markerless siblings (claude, codex, opencode, grok) and with the other two consumers — a variant basename likekimi-nightlyno longer self-detects asunknown.pi-launcherandPi(the launcher wrapper and npm shim basenames, already known tobin/backends/tmux.sh's pane classifier) as exact arms inbin/fm-harness.shand as anchored alternatives inFM_HARNESS_REinbin/fm-session-lock-lib.sh, so a firstmate under either can self-identify and prove session-lock ownership. The pi family deliberately stays anchored/exact sopip,pipenv, and paths containing "pi" remain unrecognized.tests/fm-kimi-harness.test.shwith behavioral cases (21 total, executed against fake ancestry viabin/fm-harness.shandbin/fm-lock.sh); each new case was verified to fail against the pre-fix code, andpip/pipenv/piperare asserted to stay unrecognized so the anti-substring guard can't be relaxed unnoticed. The originally planned per-harness identity table is deliberately not shipped.Risk Assessment
✅ Low: A 21-line behavioral fix that brings three already-existing matching rules into agreement on measured basenames, preserves the deliberately anchored pi-family matching everywhere, adds no forbidden refactor machinery, and is covered by behavioral tests with explicit negative guards for the pip/pipenv near-misses.
Testing
Ran the extended fm-kimi-harness test suite (21/21 pass), proved the new assertions fail against the base-commit bin scripts by temporarily reverting them, and captured an end-to-end before/after CLI transcript of fm-harness.sh self-detection and fm-lock.sh lock ownership that reproduces the intent's measurement table exactly — kimi-nightly and pi-launcher/Pi are now recognized, pip/pipenv/piper stay rejected, and no table refactor was shipped. This is a CLI-only change, so CLI transcripts are the reviewer-visible product surface; no screenshot applies.
Evidence: Before/after CLI transcript: harness self-detection and session-lock ownership per ancestor basename
########## BEFORE FIX (base f7d0d0a) ########## == fm-harness.sh self-detection == ancestor kimi-nightly -> unknown ancestor pi-launcher -> unknown ancestor Pi -> unknown ancestor pip -> unknown == fm-lock.sh ownership == ancestor pi-launcher -> REFUSED: error: cannot locate harness process in ancestry ancestor Pi -> REFUSED: error: cannot locate harness process in ancestry ########## AFTER FIX (7896c83) ########## == fm-harness.sh self-detection == ancestor kimi-nightly -> kimi ancestor pi-launcher -> pi ancestor Pi -> pi ancestor pip -> unknown (pipenv, piper likewise) == fm-lock.sh ownership == ancestor pi-launcher -> lock acquired: harness pid 3260685 ancestor Pi -> lock acquired: harness pid 3260738 ancestor pip -> REFUSED: error: cannot locate harness process in ancestryEvidence: Full fm-kimi-harness test run on fixed code (21/21 pass)
Evidence: Pre-fix test failure (bin files reverted to base): new assertion reproduces the original miss
Evidence: End-to-end demo script used for the before/after transcript
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
bin/fm-session-lock-lib.sh:17- The anchored pi-family alternatives in FM_HARNESS_RE (^pi$|^pi-signed$|^pi-launcher$|^Pi$) can never match when the regex is grepped against a fullps -o args=line in the bare-interpreter fallback (fm-session-lock-lib.sh line 48 and 83), and the corresponding args fallback in fm-harness.sh line 77 only knows*" pi "*|*/pi. A pi-family harness presenting comm=node/python would still be unrecognized by these paths. This asymmetry is pre-existing (identical for ^pi$ before this change), theoretical (shebang shims present their own basename as comm), and outside the intent's measured scope, so no action is needed — recorded so a future consolidation doesn't mistake it for a regression from this change.✅ **Test** - passed
✅ No issues found.
bash tests/fm-kimi-harness.test.shon the fixed code — all 21 cases pass, including newtest_pi_launcher_basenames_detect_as_pi,test_pi_launcher_session_lock_identity, and the extended kimi variant-basename assertionRevertedbin/fm-harness.shandbin/fm-session-lock-lib.shto base f7d0d0a and re-ran the suite — it fails witha kimi variant basename detected as 'unknown', not kimi, proving the tests detect the pre-fix defect; then restored the fixed filesManual end-to-end before/after run ofbin/fm-harness.shagainst fake ancestry for kimi, kimi-nightly, pi, pi-signed, pi-launcher, Pi, pip, pipenv, piper — reproduces the intent's measurement table (unknown→kimi, unknown→pi) with the anti-substring guard intactManual end-to-end before/after run ofbin/fm-lock.shunder pi-launcher, Pi, and pip ancestry — lock refused before the fix, acquired after for the two real launchers, still refused for pipVerified the diff contains only the two literal fixes plus tests (no per-harness table/dispatcher machinery, per the forbidden constraint) and that the worktree is clean after testing✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.