fix(fm-send): fail loudly on unresolvable send targets#254
Open
mielyemitchell wants to merge 10 commits into
Open
fix(fm-send): fail loudly on unresolvable send targets#254mielyemitchell wants to merge 10 commits into
mielyemitchell wants to merge 10 commits into
Conversation
4 tasks
Owner
|
Thanks for the PR! It looks like this branch has a merge conflict with the base branch right now. When you get a chance, could you rebase onto (or merge in) the latest base branch, resolve the conflict, and push? Once GitHub shows the PR as mergeable again, it'll be picked back up for review. Noted for firstmate#254 at |
…ions missing from doc inventories
8f4f72d to
f3a2ec4
Compare
Open
4 tasks
Author
|
Rebased onto main — all checks green and mergeable now. Thanks for the nudge! |
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
Make bin/fm-send.sh fail loudly so messages that cannot be provably delivered are hard errors, never silent fallbacks. Preserve healthy fm-send behavior while adding strict FM_HOME sanity, bare-id diagnostics, resolvable fm- meta/backend validation, explicit backend target validation including herdr pane shape, backend failure propagation, and clearer watcher warning text. Update the fm-send usage comment and AGENTS.md send guidance, add focused bash 3.2-compatible tests covering bare id, unset FM_HOME, unresolvable targets, prefixless herdr pane ids, and healthy sends. This PR is PR #254; the known prior failures were environment-only herdr/treehouse smoke flakes under treehouse pool contention and not caused by this branch.
What Changed
bin/fm-send.shnow requiresFM_HOMEto be set, rejects bare task ids without thefm-prefix with a "did you mean fm-?" diagnostic, validates that anfm-<id>target resolves through the home'sstate/<id>.meta(with backend/window checks), validates explicit backend targets including herdr's prefixless pane-id shape, and propagates backend send failures as hard errors instead of silently no-oping.bin/fm-guard.shand related fm-send output was scoped/clarified so the message specifically calls out that the requested send will still proceed.tests/fm-send-strict.test.sh(bash 3.2-compatible) covering bare id, unsetFM_HOME, unresolvable targets, prefixless herdr pane ids, dead explicit targets, and a healthyfm-<id>send; adjustedtests/fm-backend*.test.sh,tests/fm-gotmp.test.sh,tests/fm-daemon.test.sh, andtests/fm-turnend-guard.test.shfor home-explicit test isolation.AGENTS.md,CONTRIBUTING.md, and thedocs/*-backend.md/docs/configuration.md/docs/scripts.mdreference docs to describe the new fail-closedfm-sendcontract (FM_HOME requirement, target resolution rules) and fixed related doc inconsistencies for cmux/herdr/zellij backends and X-mode skill metadata;bin/fm-promote.shupdated for a corresponding fm-send usage change.Risk Assessment
✅ Low: The change is well-scoped to bin/fm-send.sh (plus matching docs/tests): it adds strict FM_HOME/target validation with clear diagnostics, preserves the existing send/verify/backend-dispatch behavior byte-for-byte (verified by the old-vs-new conformance test with an explicit preflight-call filter), and every doc/skill reference to fm-send.sh call sites was updated to include the new FM_HOME requirement. No internal script shells out to fm-send.sh without FM_HOME already available, so no real caller breaks. The one latent gap (ad hoc explicit targets with a single colon are assumed tmux, which would misclassify a bare zellij escape-hatch target) is a pre-existing limitation carried over unchanged from the old fm_backend_resolve_selector/fm_backend_of_selector default, not a regression introduced by this branch, and the docs already documented zellij's session:pane shape as deliberately indistinguishable from tmux's.
Testing
The configured baseline suite had already passed; I additionally ran the new tests/fm-send-strict.test.sh (all 6 pass), which is the direct product-level evidence for the intent — a CLI transcript showing fm-send.sh now hard-errors with named diagnostics on bare ids, missing FM_HOME, unresolvable targets, and prefixless herdr pane ids, with zero fallback tmux sends in each failure case, while the healthy fm-<id> path still sends correctly. I also re-ran every other test touching the modified files (fm-guard.sh, fm-promote.sh, fm-backend.sh usage) with no regressions, and confirmed the one failure I hit (fm-backend-autodetect-smoke) is a known environment-only herdr/treehouse flake by re-running it in isolation, where it passed cleanly. No source changes were needed and the worktree is clean.Evidence: fm-send-strict.test.sh CLI transcript (all 6 fail-loudly scenarios)
ok - fm-send strict: bare task/lane id fails with a did-you-mean fm-<id> diagnostic ok - fm-send strict: unset FM_HOME fails before target resolution ok - fm-send strict: unresolvable selectors do not fall back to tmux ok - fm-send strict: prefixless herdr pane ids are rejected before tmux fallback ok - fm-send strict: unmatched single-colon explicit targets must verify live before sending ok - fm-send strict: healthy fm-<id> sends still type once and submitPipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
🔧 **Rebase** - 1 issue found → auto-fixed ✅
docs/configuration.md- merge conflict rebasing onto origin/main🔧 Fix applied.
✅ Re-checked - no issues remain.
✅ **Review** - passed
✅ No issues found.
✅ **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"bash tests/fm-send-strict.test.sh(new fail-loudly test file: bare id, unset FM_HOME, unresolvable target, prefixless herdr pane, dead explicit target, healthy fm-<id> send)bash tests/fm-send-popup-settle.test.shbash tests/fm-send-secondmate-marker.test.shbash tests/fm-send-settle.test.shbash tests/fm-backend.test.shbash tests/fm-watcher-lock.test.shbash tests/fm-gotmp.test.shbash tests/fm-turnend-guard.test.shbash tests/fm-daemon.test.shbash tests/fm-tangle-guard.test.shRetriedbash tests/fm-backend-autodetect-smoke.test.shafter an initial failure — passed on retry, confirming it is the pre-existing environment/treehouse-pool-contention flake noted in the task description and unrelated to this diff (no changes to that test file or fm-spawn.sh in this branch)✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.