fix(bin): tmux next-free window index, symlink-safe spawn wait, crew-state lookup retries#257
Open
DQ4443 wants to merge 6 commits into
Open
fix(bin): tmux next-free window index, symlink-safe spawn wait, crew-state lookup retries#257DQ4443 wants to merge 6 commits into
DQ4443 wants to merge 6 commits into
Conversation
…rew-state race retry - tmux.sh: target new-window at "<session>:" so tmux picks the next free index. A bare "<session>" resolves to the session's current window and can hit "index in use" on tmux >=3.7 with renumber-windows on; the trailing colon is the documented next-free-index form and removes the need for the external PATH shim. - fm-spawn.sh: canonicalize the treehouse-worktree wait-loop path comparison (pwd -P both sides) so a symlinked project path (/tmp -> /private/tmp, /var -> /private/var) no longer misdetects the project as the worktree and trips validate_spawn_worktree. - fm-crew-state.sh: bounded retry/backoff around the run-attribution lookup so a race against an active no-mistakes pipeline no longer falls through to the stale status log, which defeated the watcher's provably-working absorption (per-minute false stale wakes during long validations). Supersedes state/.crew-state-retry.sh; byte-identical with FM_CREW_STATE_RETRIES=0. Tests: new tests/fm-spawn-worktree-symlink.test.sh (canonicalization regression); fm-backend-tmux-smoke trailing-colon args-capture and renumber-windows behavioral guards; fm-crew-state retry-recovery regression. Bugs #1 (fm-brief heredoc parse) and kunchenguid#2 (batch spawn set -u unbound array) were already fixed by kunchenguid#173 and verified non-reproducing under bash 3.2; the FM_STALE_ESCALATE_SECS-for-provably-working concern is already correct and covered by fm-watch-triage.
An authoritative non-empty answer (another branch's run, no runs-list row for this branch) now breaks the attribution loop without retrying, so the steady-state implementing crew no longer pays retry backoff and extra CLI calls on the watcher's hot triage path. Only the timed-out-to-empty signature - the race the retry exists to cover - is re-attempted.
… resolution, test index
…ndex Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
4 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
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.
What Changed
bin/backends/tmux.shnow creates task windows with the$ses:(trailing colon) target, the documented "next free window index" form, instead of a bare session target that could resolve to the current window and fail with "index in use" underrenumber-windows on; this replaces the previous external PATH shim.bin/fm-spawn.shcanonicalizes both sides (pwd -P) when waiting for the pane's cwd to move from the project to the treehouse worktree, so a symlinked project path (e.g. macOS/tmp->/private/tmp) no longer reads as "already moved" and trips the worktree-isolation check.bin/fm-crew-state.shretries the no-mistakes run lookup (configurableFM_CREW_STATE_RETRIES/FM_CREW_STATE_RETRY_DELAYknobs) when a busy CLI returns an emptyaxi statusanswer or a timed-out coarserunslisting, instead of surfacing a falsestale; answered-but-no-matching-run responses remain authoritative and are not retried. New tests cover the tmux target, the symlinked spawn path, and the retry paths, and the pipeline's review round hardened the timed-out-coarse-lookup case.Risk Assessment
✅ Low: Well-bounded shell fixes with dedicated regression tests for each; the prior round's retry-gate finding was fixed exactly per the given instructions (NM_RUNS_RESPONDED timeout signature, authoritative no-run fast path preserved) and no new issues surfaced on a full re-pass.
Testing
Completed 1 recorded test check.
Pipeline
Updates from git push no-mistakes
⏭️ **intent** - skipped
✅ No issues found.
🔧 **Rebase** - 1 issue found → auto-fixed ✅
docs/scripts.md- merge conflict rebasing onto origin/main🔧 Fix applied.
✅ Re-checked - no issues remain.
🔧 **Review** - 2 issues found → auto-fixed ✅
bin/fm-crew-state.sh:394- The retry loop only retries when the primaryaxi statuscall is empty, treating any non-empty answer plus an empty coarse fallback as an authoritative "no run for this branch". Butnm_runs_status_for_branchrunsno-mistakes runsthrough the samenm_runtimeout wrapper against the same busy CLI, and returns empty both for "list answered, no matching row" (authoritative) and "runs call timed out to empty" (the exact race this loop exists to cover). For a crew whose active run is not the most recent one (concurrent multi-crew validations, the motivating busy-CLI scenario), a responsiveaxi statusreporting another branch's run plus a timed-outrunscall breaks without retrying, so the false-stale surfacing can still recur on that path. Fix: distinguish empty-runs-output (retryable, like an emptyaxi status) from answered-but-no-row (authoritative) and feed that into thecli_respondeddecision.bin/fm-crew-state.sh:363- Worst-case latency of a crew-state read grows from ~1×FM_CREW_STATE_NM_TIMEOUT (10s) to (1+FM_CREW_STATE_RETRIES)×timeout + retries×delay ≈ 34s at defaults when the CLI is persistently unresponsive. This runs on the watcher's provably-working triage and heartbeat paths, so a hung no-mistakes CLI can stall a poll cycle noticeably longer than before. It is bounded, documented, and configurable (and replaces an equivalent production shim), so no action needed; just noting the tradeoff.🔧 Fix: retry crew-state lookup on timed-out coarse runs list
✅ Re-checked - no issues remain.
✅ **Test** - passed
✅ No issues found.
command -v tmux >/dev/null || { echo "tmux is required for e2e tests" >&2; exit 1; }; tmux -V; rc=0; for t in tests/*.test.sh; do echo "== $t =="; bash "$t" || rc=1; done; exit "$rc"✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.