Repository navigation
Conversation
- backends/cmux: check `list-panes`' own exit status before handing off to jq. A bare `cli ... | jq -e ...` pipe (no pipefail) takes its exit status from jq alone, and jq 1.6 (still the apt-installed default on several supported Linux distros) reports success for `-e` on empty stdin where jq 1.7+ reports failure - so a failed, output-less list-panes call would misreport the surface as existing. - fm-test-run.sh --aggregate-json: skip an unreadable lane artifact instead of failing the whole aggregate. A lane artifact can be truncated when its own job is killed mid-write (host memory pressure, a hang timeout); the aggregate step runs with `if: always()` precisely so per-lane failures still get reported, so one bad artifact should not take the rest down with it. - fm-wake-lib.sh: bound fm_lock_acquire_wait instead of spinning forever. It retried fm_lock_try_acquire every 0.1s with no bound, so a malformed lock (e.g. a plain regular file where a symlink plus owner directory was expected) wedged callers indefinitely. Takes an optional timeout (default 30s) and returns 1 on expiry so callers can detect and handle it. Fixes kunchenguid#3279. - fm-install-herdr.sh / fm-install-treehouse.sh: capture and report the installed tool's own stderr on a version or protocol gate failure instead of discarding it with `2>/dev/null`. A version mismatch or protocol gate failure during install previously gave no clue why the tool's own `--version`/`status` call produced unexpected output. - fm-afk-return.sh: propagate a write failure out of print_evidence instead of silently swallowing it, so a caller relying on the function's exit status can detect a broken output stream. Each change is independent and self-contained; grouped into one PR because each is too small to warrant its own.
The coverage guard's set-comparison helpers sort their inputs with `LC_ALL=C sort` but then compare them with a bare `comm`, which uses the ambient collation. On a locale whose collation differs from the plain byte ordering `sort -u` used to build the inputs (for example anything that sorts case-insensitively or treats punctuation differently), `comm` can misreport lines as missing or extra even though the two files agree, because its own notion of "sorted" no longer matches the file's actual order. Nine call sites in the coverage guard were affected. All now run under `LC_ALL=C` to match the sort that produced their inputs.
…eouts Review found two real issues in the previous commit's bounded fm_lock_acquire_wait: - A timeout of 0 never actually returned: the expiry check was guarded by `[ "$timeout" -gt 0 ]`, so passing 0 (documented as "try once and return immediately") instead disabled the bound entirely and fell back to waiting forever, defeating the one caller shape most likely to want an immediate answer. Removed the guard so elapsed time alone decides. - On timeout the function returned 1 silently. Most existing callers predate this function ever being able to fail (the unbounded loop could only succeed) and do not check the return value, so a timeout let them proceed as if the lock had been acquired with no evidence anything went wrong. A full audit of every caller is a separate, much larger change; as a proportionate, in-scope mitigation the function now also prints a timeout diagnostic to stderr, so the failure is visible in logs even where a caller does not act on it. Added direct unit coverage for both: a live-held lock with timeout=0 returns immediately instead of hanging, and an expired timeout returns 1 and reports itself to stderr.
… lock Review found a genuine data-corruption risk in six call sites that predate fm_lock_acquire_wait ever being able to fail. fm-wake-grant.sh's four subcommands (activate, publish, release, deactivate) and two call sites in fm-wake-drain.sh called it bare, then unconditionally set LOCK_HELD/ DRAIN_LOCK_HELD=true and went on to mutate durable, concurrently-accessed wake-queue state (branch ownership, eligible rows, acknowledgement cutoffs) regardless of whether the lock was actually held. Before the bounded-wait change in this same PR, fm_lock_acquire_wait could only return after acquiring the lock, so these missing checks were latent - the failure path they don't guard against did not exist yet. The new default timeout makes that path reachable for the first time: under contention, a caller could now proceed to mutate shared wake-queue state without exclusive access, racing a concurrent writer and corrupting it. All six sites now exit 1 (with a diagnostic on the two fm-wake-drain.sh sites, matching that file's existing error convention) when the lock cannot be acquired, instead of proceeding as if it had been. Ran the test suites that exercise these two scripts directly: tests/fm-pi-branch-extension.test.sh, tests/fm-wake-queue.test.sh, tests/fm-wake-drain-outcome-backstop.test.sh, tests/fm-wake-drain-unread-status.test.sh, tests/fm-wake-drain-open-decisions.test.sh, tests/fm-wake-drain-open-decisions-cursor.test.sh, tests/fm-watcher-lock.test.sh. All pass, 0 failures.
…hanges - fm-wake-lib.sh: fm_wake_append and fm_wake_queued_keys called fm_lock_acquire_wait bare and then unconditionally mutated/read the durable wake queue and its sequence counter regardless of whether the lock was actually acquired - the same corruption class already fixed for fm-wake-grant.sh and fm-wake-drain.sh in the previous commit, just in the core library itself. Both now return 1 on a failed acquire instead of proceeding. - fm-wake-lib.sh: _fm_lock_acquire_wait_handoff (used by fm_lock_acquire_wait_bounded) called the now-bounded fm_lock_acquire_wait without forwarding the caller's configured wait budget, so it silently inherited the new 30s default instead of honoring a caller-configured budget above that. A caller passing more than 30 seconds (e.g. FM_STATUS_PRESENTATION_LOCK_TIMEOUT set above the default) would have its effective budget silently capped at 30s, changing which outcome path fm-wake-drain.sh takes. The handoff now forwards the caller's timeout so the outer fm_run_timed deadline stays authoritative, exactly as the bounded variant's own doc already promises. - fm-install-herdr.sh / fm-install-treehouse.sh: the previous commit's stderr capture (2>&1) merged stderr into the same string used for successful-path parsing (awk/tr/jq), so a healthy install that happens to write anything benign to stderr (a deprecation notice, update check, telemetry banner) could corrupt the parsed version or protocol on an otherwise-successful run. stderr is now captured to its own file, kept out of the success-path parse, and surfaced only in the die() message on an actual failure. All three were flagged by the PR's automated review after the previous commit; verified each against the actual code before fixing.
…ed fail-closed contract
…ve Windows fm_backend_zellij_server_ensure's `nohup zellij attach -b <name> &` never survives on native Windows (Git Bash/MSYS, not Cygwin): Git Bash's `&` backgrounding does not achieve real OS-level process detachment the way a Unix double-fork/setsid does, so the headless session dies with its one-shot invoking process before the poll loop below ever finds it. Verified live by elimination on Windows 11 (Zellij 0.45.1): the identical `zellij attach -b <name>` launched instead via PowerShell's Start-Process persists independently and is found by `zellij list-sessions` afterward, proving Zellij's own background-session support works fine there - only the script's Unix-style backgrounding idiom does not survive. On msys*/mingw* only (Cygwin keeps its real fork/setsid and the existing path), launch through `powershell.exe -NoProfile -NonInteractive -Command Start-Process ...` instead, behind a charset guard on the session name since it is interpolated into a single-quoted PowerShell string.
Three independent fixes to watcher and handoff behavior: - fm-watch-arm.sh / fm-watch.sh: a watched process that exits cleanly after absorbing a benign condition (nothing left to do) was reported as a false FAILED. Recognize a benign-absorb exit and report it as clean instead. - fm-backlog-handoff.sh: the backlog handoff's receiver-side wake did not name which items it actually routed, making the wake ambiguous when more than one item moved in the same handoff. The wake now names its own routed items directly, and covers every resume path uniformly instead of threading an extra parameter through each one. - fm-config-inherit-lib.sh / fm-remote-inherit-push.sh: shared captain header validation could not tolerate a mid-phrase reflow (a wrapped line whose whitespace differs only in reflow position), causing a spurious validation failure on an otherwise-identical header.
chmod alone only simulates an unreadable or unwritable path for a non-root caller. A root process holding CAP_DAC_OVERRIDE - the default inside a container, and how a CI runner commonly executes this suite - walks straight past the mode bits, so every assertion those fixtures were meant to gate on silently stopped testing anything. tests/lib.sh gains fm_run_without_dac_override, which drops CAP_DAC_OVERRIDE and CAP_DAC_READ_SEARCH from the bounding set for one exec'd command via setpriv, plus fm_dir_block_writes / fm_dir_unblock_writes / fm_run_dir_readonly built on it. The directory form proves the block actually holds before trusting it, rather than assuming the capability drop worked. A non-root caller runs the command unchanged. The seven suites that simulate an unreadable or unwritable path are converted to the new helpers.
Review found that fm_run_without_dac_override only removed CAP_DAC_OVERRIDE from the capability bounding set. CAP_DAC_OVERRIDE and CAP_DAC_READ_SEARCH each independently let a process bypass a file's read permission checks (capabilities(7)), so a fixture simulating an unreadable path (chmod 000) under this helper would still be readable by a root runner that retains CAP_DAC_READ_SEARCH, even with CAP_DAC_OVERRIDE dropped - silently defeating the exact coverage this rewrite exists to add. Both capabilities are now dropped together. Write-denial fixtures (fm_dir_block_writes/fm_run_dir_readonly, which only chmod away the write bit) are unaffected: write-permission bypass is governed solely by CAP_DAC_OVERRIDE, with no equivalent second capability.
…le unshare comments
… fixtures in lib.sh header
…etwork.test.sh: in test_a_report_publication_failure_is_failed_and_still_wakes, if `run_stage ... wait 30` failed, the code called `fail` (which exits the process) before `fm_dir_unblock_writes` ran, leaving the report directory at mode 0555. That breaks the EXIT trap's `rm -rf` cleanup for non-root test runners, since directory entries can't be removed from a write-blocked directory. Fixed by unblocking writes on the failure path before calling `fail`. Verified by running tests/fm-startup-network.test.sh directly — all 20 assertions pass, including the modified test, and shellcheck reports no new issues
…twork.test.sh:267 ("Signal cleanup leaves directory blocked"). The prior fix (358fc0f) only unblocked the report directory on the explicit `fail` path; a SIGINT/SIGTERM landing between `fm_dir_block_writes` and its matching `fm_dir_unblock_writes` still ran `fm_test_cleanup` via the trap while the directory remained write-blocked, and a non-root `rm -rf` can't unlink entries from an unwritable directory, leaving fixture cruft behind. Root-caused it at the shared-library level in tests/lib.sh rather than patching the one call site: `fm_dir_block_writes` now registers the directory it just blocked into a new in-process `FM_TEST_BLOCKED_DIRS` array, and `fm_test_cleanup` (already the EXIT/INT/TERM trap target) restores write access to every entry in that array before doing its `rm -rf` sweep. This covers every current and future caller of `fm_dir_block_writes`, not just the one test. Verified: (1) reproduced the exact bug pre-fix with a minimal repro script (block a dir containing a file, then self-SIGTERM before unblocking — left an unremovable write-blocked directory behind); (2) confirmed the same repro cleans up correctly with the fix applied; (3) ran tests/fm-startup-network.test.sh directly — all 20 assertions pass; (4) shellcheck on tests/lib.sh and tests/fm-startup-network.test.sh shows no new issues (only pre-existing info-level SC1091/SC2153)
…or unusable setpriv
The coverage guard's set-comparison helpers sort their inputs with `LC_ALL=C sort` but then compare them with a bare `comm`, which uses the ambient collation. On a locale whose collation differs from the plain byte ordering `sort -u` used to build the inputs (for example anything that sorts case-insensitively or treats punctuation differently), `comm` can misreport lines as missing or extra even though the two files agree, because its own notion of "sorted" no longer matches the file's actual order. Nine call sites in the coverage guard were affected. All now run under `LC_ALL=C` to match the sort that produced their inputs.
Git for Windows now ships a bash built for the Cygwin runtime (2.55.0: `bash --version` reports x86_64-pc-cygwin), so Git Bash reports OSTYPE=cygwin exactly like a real Cygwin install. The msys*/mingw* OSTYPE match therefore never fired there, and server_ensure fell back to the `nohup ... &` launch whose session dies with its launcher. Decide the branch with `uname -s` instead (MINGW*/MSYS* on Git for Windows, CYGWIN_NT-* on Cygwin) through fm_backend_zellij_native_windows. The tests stub uname and pin the real combination: OSTYPE=cygwin with a MINGW64 kernel name takes the PowerShell path, a CYGWIN_NT kernel keeps nohup, and Linux is unchanged.
…l locale Several scripts matched git's or awk's English-language output with a literal regex, so a non-English shell locale (e.g. de_DE.UTF-8) silently broke the guard: - fm-fleet-sync.sh's packed-refs.lock stale-lock detection never fired, because is_packed_refs_lock_error() only recognized git's English fetch-failure text. - fm-teardown.sh's index-lock retry path in treehouse_return() never engaged, for the same reason against git's localized error text. - The herdr backend's wait-for-working interval formatting used awk's locale-dependent decimal separator, breaking "%.4f" parsing under a comma-decimal locale. - The herdr backend's submit-confirmation budget computation had the identical unguarded awk "%.4f" call, feeding a comma-decimal value into the wait-for-working interval above and collapsing it to a zero-length sleep under the same locale. Fix: force LC_ALL=C on the specific git and awk invocations these guards inspect, so the English-text match stays reliable regardless of the operator's locale, without trying to recognize every git/awk translation.
An untracked-only working tree (e.g. an ignored tool cache that was never added to .gitignore) can never be silently clobbered by a checkout or ff-only merge - git itself refuses either if it would overwrite an untracked file - so treating untracked-only the same as real tracked-file dirt was stricter than needed and could deadlock a clone or secondmate home that most needed the very fast-forward it was blocking. Split "dirty" (tracked-file changes, still blocks) from "untracked" (reported, but no longer blocks) in both fm-fleet-sync.sh (project clones) and fm-ff-lib.sh (firstmate/secondmate self-sync), covering the on-default and detached-HEAD-recovery paths in fleet-sync. A genuine untracked collision that git's own overwrite protection still refuses is surfaced as a clean, reasoned skip rather than silently absorbed into STUCK. The two implementations stay parallel rather than converging to a shared helper: fleet-sync advances project clones under projects/, while ff-lib advances firstmate/secondmate checkouts in shared worktrees - different enough workloads that a shared helper would add cross-repo coupling without a caller count to justify it.
…mpleted re-attach
An Orca scout teardown, or any Orca teardown run with --force, skipped the early Orca path-match check and only re-verified it after reap_task_worktree_processes had already run against the recorded worktree path, so a stale or reassigned Orca worktree id could have its processes signalled before ownership was ever proven. Fold the Orca proof into require_owned_task_worktree_slot, the same early, single-owner ownership determination the treehouse pool-slot claim already uses, so it runs once, well before Fix 1/Fix 2, for every kind and --force alike, instead of being re-checked ad hoc at each later call site.
…PATH sanitization
…all destructive steps
test_untracked_only_advances_fast_forward, test_tracked_file_modification_blocks_fast_forward,
and test_untracked_collision_surfaces_git_error all called new_world with the
literal name "t12"/"t13"/"t14". "t12" collides with the world already used by
test_primary_update_rebinds_local_watch, so the second new_world call's
git clone into the non-empty directory fails silently, its later git commit
finds nothing new to stage, and git's own status text ("nothing to commit,
working tree clean") leaks onto stdout and gets captured into the world path,
breaking every git command that follows.
Rename all three fixture worlds to unique, subject-specific names instead of
sequential numbers, so a future test picking the next free number cannot
collide with them the same way.
… refusal sync_project reported every failed `git merge --ff-only` as a loud "STUCK: ... - needs attention" line, widening what used to be a quiet "skipped: fast-forward failed: <reason>". That over-reports: a transient, self-clearing failure (a held index.lock, a momentarily busy worktree) now raises the same alarm as the case a fast-forward genuinely cannot resolve on its own. Narrow it back: STUCK is reserved for a fast-forward git refuses because an untracked file actually sits at a path the advance touches - the one case the sync deliberately leaves for hands-on attention. Detection compares the diff's changed paths against the working tree's untracked paths directly, rather than matching git's own refusal text, which varies by git version and locale. Any other fast-forward failure goes back to the quiet skip. Adds coverage for a lock-blocked fast-forward with no untracked file in play, reproduced with a real held .git/index.lock: it must stay a quiet skip and never advance the clone, not read the reason text back out of the STUCK line.
The previous commit narrowed sync_project's loud STUCK reporting to only the untracked-collision case, detecting it by comparing the diff's changed paths against the working tree's untracked paths. That comparison turned out to miss the file-vs-directory shape of the same collision (an untracked plain file blocking an incoming path nested under it, or the reverse), so a permanent, non-self-clearing collision could still be misclassified as a benign transient failure. Rather than keep patching a path comparison to catch up with what git already knows, drop the classification altogether: every refused `git merge --ff-only` goes back to the plain `skipped: fast-forward failed: <reason>` it always was, with git's own reason carried through as before. The re-attach checkout path is untouched - it was already loud before any of this and stays that way. Updates the two fast-forward-collision tests to assert the restored quiet skip instead of STUCK, and aligns the two documentation sentences that described the now-removed behavior. The lock-based transient-failure test added alongside the narrowing needed no change: it never puts an untracked file in play, so it exercises the same quiet-skip path either way.
…ely" This reverts commit 2a6f2d4.
…ollision refusal" This reverts commit b25021f.
…up under a root test runner # Conflicts: # tests/lib.sh
…s, and test aggregation against silent failures # Conflicts: # bin/fm-wake-lib.sh # docs/watcher-continuity.md
…est-run coverage guard
…ions via PowerShell on native Windows
… output is parsed
…doff wakes and tolerate reflowed shared-captain headers # Conflicts: # bin/fm-config-inherit-lib.sh # tests/fm-shared-captain-inheritance.test.sh
MLA82
force-pushed
the
integration/heben-20260927
branch
from
September 28, 2026 01:36
5c12953 to
a68cc77
Compare
MLA82
changed the title
…e-launched Claude sessions # Conflicts: # bin/fm-spawn.sh # tests/fm-backend-orca.test.sh # tests/fm-spawn-dispatch-profile.test.sh
…fore any destructive teardown step
… fast-forward in fleet and firstmate sync # Conflicts: # .agents/skills/bootstrap-diagnostics/SKILL.md # bin/fm-ff-lib.sh # docs/configuration.md
…nt profiles # Conflicts: # AGENTS.md # bin/fm-config-inherit-lib.sh # bin/fm-remote-secondmate-control.sh # bin/fm-spawn.sh # docs/architecture.md # docs/configuration.md
…guid#4366 and PR kunchenguid#4683 - tests/fm-control-relaunch.test.sh: read the whole recorded launch command instead of the retired inline 'encode launch-brief' grep shape (needed by PR kunchenguid#5288's own new test). - tests/fm-backend-herdr-launcher-workspace-e2e.test.sh: pass FM_HOME to the teardown call explicitly, kept as a precaution. Dropped: the 10s->20s Herdr server poll-budget widening in bin/backends/herdr.sh. It was PR kunchenguid#4683's own fix for its own setsid-wrapped server launch; without kunchenguid#4683, bin/backends/herdr.sh reverts to the original, unmodified 10s poll and needs no widening.
MLA82
force-pushed
the
integration/heben-20260927
branch
from
September 28, 2026 02:48
a68cc77 to
3db100f
Compare
MLA82
changed the title
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.
Summary
Integration branch combining 10 currently open pull requests from this fork against
kunchenguid/firstmate, rebased onto a currentkunchenguid/firstmatemain (8c5493a). Each contribution was merged individually, conflicts resolved by hand against the newer upstream code, and validated withbash -nplusshellcheckon every touched file.Included contributions (kunchenguid/firstmate PR numbers): kunchenguid#3722, kunchenguid#3725, kunchenguid#3726, kunchenguid#4170, kunchenguid#4186, kunchenguid#4190, kunchenguid#4773, kunchenguid#4838, kunchenguid#4845, kunchenguid#5288.
Two contributions were deliberately left out of this lift, each backed by bisection evidence gathered on this fork's own CI runner:
bin/fm-teardown.shcan end a Herdr-projected task's pane before Herdr's own focus-preserving close path runs, occasionally landing the active workspace/tab on the wrong neighbor during an unrelated task's teardown. The same test scenario stayed green with only fix(bin): stop reissuing or tearing down Treehouse slots another task still owns kunchenguid/firstmate#4366's own test/fixture changes applied to a clean base, isolating the product code as the deciding factor.Keeping this fork on today's known-good behavior for both paths rather than adding a fresh regression.
For kunchenguid#5288 ("quota-balanced Claude account profiles"), the automatic inheritance of
config/claude-profiles.jsoninto secondmate homes was dropped; the quota-balanced selection mechanism itself was kept as a purely local, non-inherited per-home feature that coexists with the already-mergedconfig/claude-accountworker-account pin (kunchenguid#5358) instead of duplicating it.Test plan
bash -nandshellcheck -xon every file touched by conflict resolutionbin/fm-test-run.sh --check-coveragepasses on the merged tree