fix(backends): verify full Herdr payload before submit - #171
Merged
Merged
Conversation
Port upstream kunchenguid/firstmate 1d3ac67 (kunchenguid#5336) and widen its payload-proof gate to the fork's OMP away-mode supervisor. A Herdr submit used to type the literal and press Enter without proving the composer held the whole payload, so a long message could submit only its tail and still be reported delivered. Away-mode inject_msg had the same check-then-type race: an affirmative empty-composer read, then a human could type into the OMP supervisor before the escalation sent. The adapter now types only into a verified-empty composer and, before Enter, reads the selected composer back and requires it to show the typed payload. Comparison ignores whitespace and U+2063 (Claude's Herdr read-back drops the mark), and accepts pure Claude paste placeholders with no literal remainder. A suffix, a stale transcript head above a suffix, a placeholder with a literal remainder, or an unreadable composer withholds Enter; the draft is cleared with bounded Ctrl+U presses, reported send-failed when the clear is verified and unknown when it is not. The gate is identity-driven: native `agent get` identity `claude` for non-OMP sends, and the submit snapshot's proven `omp` identity on an idle or done baseline for OMP sends. Busy and blocked OMP baselines keep their exact session-event and ask-answer proofs; other harnesses and unidentified panes keep the type-then-Enter path. For the away-mode daemon, inject_msg's existing empty check stands and the send-time proof runs inside the backend call it already makes, so a refused escalation stays buffered with no daemon redesign. Composer-content extraction is fork-local: the shared Unicode-space normalization lands in bin/fm-composer-lib.sh, and the herdr adapter extracts the selected composer for bare `❯`/`›` prompts (unframed, or framed by `─` rules - verified as Claude's real composer on Herdr 0.9.0) and for the native OMP box.
… payload-proof contract: executable pane reads now expose verified empty/full composer states, and the Bun width stub returns measured row widths. The CI failures were caused by fixtures returning unstructured/empty reads before the turn-start assertions
… Herdr fixture now tracks composer text separately from the launch command, so pre-submit reads are genuinely empty and post-submit reads contain only typed payload; this restores the widened OMP payload-proof path and the remote lifecycle expectation. Added a narrowly scoped ShellCheck annotation for the generated Bun stub. Verified `shellcheck -x` on both changed fixtures, `git diff --check`, and the full `tests/fm-backend-herdr.test.sh` suite (including the previously failing literal-send case) pass
…: identity now occupies call 1, literal-send stderr is replayed, the regression test is added, OMP composer fixtures model verified empty/full states, and the generated Bun stub is ShellCheck-clean. `shellcheck`, syntax checks, and diff validation pass; the requested send-turn test still hits its pre-existing bounded wake-lock timeout (exit 142)
…lper now uses the Node width path for canonical Node runtimes even when the OMP entrypoint is separate, restoring valid remote OMP payload proof. The Herdr busy/blocked regression fixture now exports its typed-composer path on both sibling calls. Verified with composer, Herdr backend, and send-turn-start tests; syntax and diff checks pass
…fm-composer-lib.sh. Runtime detection now behaviorally identifies Node-compatible canonical runtimes instead of relying only on basename, preserving Bun handling and preventing valid remote OMP composers from being classified unknown. Verified with fm-composer-lib tests, bash syntax, shellcheck, and git diff checks
… standalone compiled OMP entrypoints are now routed to the Node width path before any `-e` probe, so they cannot receive unsupported Bun evaluation flags. Verified fm-composer-lib and fm-tmux-submit-busy tests, bash syntax, shellcheck, and diff checks. CI-2’s remote-secondmate failure was a transient delivery-verdict race (unknown instead of expected missing-turn-start) with no reproducible code defect identified
… box and composer widths with the shared terminal-width helper, avoiding locale-sensitive Bash character counts that produced malformed Unicode box widths and `unknown` composer verdicts. Verified with bash -n, shellcheck, git diff --check, and direct parser validation showing a valid OMP candidate
…n` through remote control output into parent route metadata, and requiring complete OMP runtime metadata. Removed the incomplete remote-only proof bypass. Added e2e assertions for the metadata. `fm-backend-herdr.test.sh`, syntax, shellcheck, and the isolated active-turn stall test pass. CI-2’s isolated stall case passes on this branch; no lock-related code change was warranted
…rs now dynamically match composer display width while preserving the parser-required `──╮` structure, and Ctrl+U clears the composer without marking it working. Verified bash syntax, shellcheck, diff checks, and `tests/fm-remote-secondmate-lifecycle-e2e.test.sh` (ALL TESTS PASSED). The fm-watch failure was non-deterministic: the standalone suite showed timing failures on repeated runs, while the reported turn-end case passed; no fm-watch code change was warranted
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
Port upstream kunchenguid/firstmate 1d3ac67 (fix(bin): refuse a Herdr Claude submit that would send only a message tail, kunchenguid#5336) into this fork, with the payload-proof gate WIDENED (captain decision 2026-09-25) to include the OMP away-mode supervisor, not only native Claude identity. Type only into a verified-empty composer; verify the composer shows the full payload (whitespace and U+2063 insensitive, pure Claude paste placeholders accepted) before Enter; refuse suffix-only, stale-head, placeholder-with-remainder, and unreadable composers; clear with bounded Ctrl+U and report send-failed when the clear is verified, else unknown. The same proof applies to idle/done OMP supervisor submits so human text typed between the daemon's empty check and the send makes the inject refuse and leaves the escalation buffered; busy and blocked OMP session-event and ask-answer confirmations unchanged. Non-Claude and non-OMP targets keep existing submit behavior. Do NOT reintroduce closed PR 84's blanket Herdr deferral. Out of scope: upstream kunchenguid#5554 and kunchenguid#5599 (daemon edits kept minimal) and issue #85. This is the captain-sanctioned rebuild '5336 fresh': the first run died on a transient missing-git infra error; this v2 branch carries the full port plus every accepted pipeline repair (probe-aware fixture ordering, literal-send stderr replay, wake-lock guards) re-applied onto current main including #165-#170.
Firstmate-Validation-Generation: 0e96e817839b2dd65bf4f27656d4ef8f
What Changed
Risk Assessment
✅ Low: The changed logic is bounded to Herdr payload verification and lock-failure propagation, with no source-verifiable correctness or security defects found in the reviewed paths.
Testing
Ran the targeted Herdr backend and composer regression suites plus the installed Herdr real-binary smoke suite. The smoke suite verified real session/task/send/capture/cleanup behavior; live Claude/OMP payload-proof scenarios remain untested because this run lacks an authenticated Claude session and a live OMP supervisor event stream.
bash tests/fm-backend-herdr-smoke.test.sh; herdr-smoke.logEvidence: Herdr real-binary smoke
Real Herdr smoke completed successfully; the suite also recorded that the authenticated real-Claude check was skipped becauseFM_HERDR_SMOKE_REAL_CLAUDE=1was not set.Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
🔧 **Review** - 1 issue found → auto-fixed (6) ✅
bin/fm-wake-lib.sh:1679-fm_wake_appendnow makesfm_lock_acquire_waitreturn 2 for an invalid lock shape, but the unbounded path ignores that status and proceeds tofm_wake_append_lockedanyway (line 1679). If.wake-queue.lockis an ordinary file, the queue and recovery marker can be written without holding the lock, allowing concurrent writers to corrupt sequencing or rows. Propagate the acquisition failure before entering the locked operation; the same fail-closed invariant also applies to other unconditionalfm_lock_acquire_waitcallers such asfm-watch.sh:1248,fm-wake-drain.sh:515, andfm-teardown.sh:336.🔧 Fix applied.
2 errors still open:
bin/fm-wake-lib.sh:1679-fm_wake_appendnow makesfm_lock_acquire_waitreturn 2 for an invalid lock shape, but the unbounded path ignores that status and proceeds tofm_wake_append_lockedanyway (line 1679). If.wake-queue.lockis an ordinary file, the queue and recovery marker can be written without holding the lock, allowing concurrent writers to corrupt sequencing or rows. Propagate the acquisition failure before entering the locked operation; the same fail-closed invariant also applies to other unconditionalfm_lock_acquire_waitcallers such asfm-watch.sh:1248,fm-wake-drain.sh:515, andfm-teardown.sh:336.bin/fm-wake-lib.sh:1006- The fix-round change makesfm_lock_acquire_waitreturn 2 immediately for an invalid lock shape, but many existing callers still ignore that status and proceed as if the lock were held. With an ordinary file at one of these lock paths, they can mutate shared state without synchronization. The same fail-closed invariant remains violated atbin/fm-afk-return.sh:242,bin/fm-spawn.sh:1252,4836,bin/fm-lease.sh:47,bin/fm-lease-lib.sh:187,bin/fm-procevent-lib.sh:379,bin/fm-remote-home-provision.sh:140,bin/fm-lock.sh:57,bin/fm-branch-outcome.sh:370,387,396,423,548,bin/fm-backlog-handoff.sh:420,427,461,465,bin/fm-wake-grant.sh:102,139,177,187, andbin/fm-captain-hold.sh:300,1795. Propagate the acquisition failure at every unconditional caller before any protected read/write or setting its HELD flag.🔧 Fix applied.
3 errors still open:
bin/fm-wake-lib.sh:1679-fm_wake_appendnow makesfm_lock_acquire_waitreturn 2 for an invalid lock shape, but the unbounded path ignores that status and proceeds tofm_wake_append_lockedanyway (line 1679). If.wake-queue.lockis an ordinary file, the queue and recovery marker can be written without holding the lock, allowing concurrent writers to corrupt sequencing or rows. Propagate the acquisition failure before entering the locked operation; the same fail-closed invariant also applies to other unconditionalfm_lock_acquire_waitcallers such asfm-watch.sh:1248,fm-wake-drain.sh:515, andfm-teardown.sh:336.bin/fm-wake-lib.sh:1006- The fix-round change makesfm_lock_acquire_waitreturn 2 immediately for an invalid lock shape, but many existing callers still ignore that status and proceed as if the lock were held. With an ordinary file at one of these lock paths, they can mutate shared state without synchronization. The same fail-closed invariant remains violated atbin/fm-afk-return.sh:242,bin/fm-spawn.sh:1252,4836,bin/fm-lease.sh:47,bin/fm-lease-lib.sh:187,bin/fm-procevent-lib.sh:379,bin/fm-remote-home-provision.sh:140,bin/fm-lock.sh:57,bin/fm-branch-outcome.sh:370,387,396,423,548,bin/fm-backlog-handoff.sh:420,427,461,465,bin/fm-wake-grant.sh:102,139,177,187, andbin/fm-captain-hold.sh:300,1795. Propagate the acquisition failure at every unconditional caller before any protected read/write or setting its HELD flag.bin/fm-wake-lib.sh:920- The fix-round lock contract is still broken:fm_lock_try_acquirereturns status 1 for an ordinary-file lock atbin/fm-wake-lib.sh:920, butfm_lock_acquire_waitonly treats status 2 as terminal atbin/fm-wake-lib.sh:1006. Therefore every unbounded caller spins forever on a malformed lock instead of failing closed. This remains reachable through the changed callers atbin/fm-wake-lib.sh:490,588,690,714,746,747,834,1676,1734,1761,bin/fm-watch.sh:1248,bin/fm-wake-drain.sh:515, andbin/fm-teardown.sh:336,529,531,536; the same primitive also affects existing unconditional callers such asbin/fm-lease.sh:47,bin/fm-spawn.sh:1252,4836,bin/fm-wake-grant.sh:102,139,177,187, andbin/fm-branch-outcome.sh:370,387,396,423,548. Make the invalid-shape result match the terminal failure status expected byfm_lock_acquire_wait(or otherwise make the wait primitive detect the shape) so no caller can hang indefinitely.🔧 Fix applied.
1 error still open:
bin/backends/herdr.sh:3254- The fix-round payload reader treats every non-empty line after an unframed bare❯/›row as composer content until a bordered row appears. A real idle Claude pane can have unbordered footer/status lines beneath its bare composer; those lines are then included incontent, so the pre-send emptiness check atbin/backends/herdr.sh:3388rejects an actually empty composer withsend-failedand every Claude submit is withheld. The changed loop atbin/backends/herdr.sh:3254-3263needs to isolate only the composer row/wrapped continuation rows (or otherwise stop at known footer structure) rather than treating arbitrary trailing pane text as payload.🔧 Fix applied.
4 errors still open:
bin/backends/herdr.sh:3254- The fix-round payload reader treats every non-empty line after an unframed bare❯/›row as composer content until a bordered row appears. A real idle Claude pane can have unbordered footer/status lines beneath its bare composer; those lines are then included incontent, so the pre-send emptiness check atbin/backends/herdr.sh:3388rejects an actually empty composer withsend-failedand every Claude submit is withheld. The changed loop atbin/backends/herdr.sh:3254-3263needs to isolate only the composer row/wrapped continuation rows (or otherwise stop at known footer structure) rather than treating arbitrary trailing pane text as payload.bin/fm-wake-grant.sh:102- The new malformed-lock guard is not fail-closed infm-wake-grant.sh: these are top-level case branches in an executed script, so|| return 1emits “return: can only return from a function” and then continues. It setsLOCK_HELD=trueand mutates grant state without holding the queue lock when acquisition returns nonzero. Replace each withexit 1(or otherwise abort the branch):bin/fm-wake-grant.sh:102,139,177,187.bin/fm-wake-lib.sh:938-fm_lock_try_acquirereturns status 2 for an invalid lock shape, but the stale-owner path collapses failure of the nested.steallock acquisition to status 1 atbin/fm-wake-lib.sh:938. If the primary lock is stale and its.stealpath is an ordinary file,fm_lock_acquire_waitrepeatedly retries forever instead of propagating the malformed-lock failure. Preserve and propagate the nested non-contention status so malformed primary or steal locks fail closed.bin/backends/herdr.sh:2633- The pure-braille boundary check does not work under the repository's C locale:[[ "$compact" != *[!$'\u2800'-$'\u28ff']* ]]treats the multibyte range as an invalid byte-level bracket expression, so a row such as⠋⠙is not recognized byfm_backend_herdr_bare_boundary. In an unframed Claude/Codex composer followed by the documented braille starfield footer, the footer is appended to payload content, causing the full-payload proof to reject/clear or classify incorrectly instead of stopping at the footer. Use a locale-independent code-point/shape test (and keep the boundary row excluded) at this shared boundary.🔧 Fix applied.
3 errors still open:
bin/backends/herdr.sh:3254- The fix-round payload reader treats every non-empty line after an unframed bare❯/›row as composer content until a bordered row appears. A real idle Claude pane can have unbordered footer/status lines beneath its bare composer; those lines are then included incontent, so the pre-send emptiness check atbin/backends/herdr.sh:3388rejects an actually empty composer withsend-failedand every Claude submit is withheld. The changed loop atbin/backends/herdr.sh:3254-3263needs to isolate only the composer row/wrapped continuation rows (or otherwise stop at known footer structure) rather than treating arbitrary trailing pane text as payload.bin/backends/herdr.sh:2633- The pure-braille boundary check does not work under the repository's C locale:[[ "$compact" != *[!$'\u2800'-$'\u28ff']* ]]treats the multibyte range as an invalid byte-level bracket expression, so a row such as⠋⠙is not recognized byfm_backend_herdr_bare_boundary. In an unframed Claude/Codex composer followed by the documented braille starfield footer, the footer is appended to payload content, causing the full-payload proof to reject/clear or classify incorrectly instead of stopping at the footer. Use a locale-independent code-point/shape test (and keep the boundary row excluded) at this shared boundary.bin/backends/herdr.sh:3269- The new unframed composer extractor stops at the first blank captured row ([ -n "${rows[$i]:-}" ] || break). A valid Claude payload containing a newline/blank line (for examplefoo\n\nbar) is captured as❯ foo, blank,bar; the extractor returns onlyfoo, so the full-payload proof rejects and clears an otherwise correctly typed payload instead of accepting it. This is in the footer-bounding logic introduced by the prior fix round; preserve blank rows as composer content while continuing to detect the documented footer boundaries.🔧 Fix applied.
✅ Re-checked - no issues remain.
bin/fm-composer-lib.sh:376- Live OMP supervisor submit validation fails under Bash nounset when the OMP composer path calls fm_composer_idle_matches with only two arguments; the function unconditionally readsbash tests/fm-backend-herdr-smoke.test.sh; artifactReal Herdr smokeFM_HERDR_SUBMIT_CONFIRM_LIVE=1 bash tests/fm-herdr-submit-confirm-live-e2e.test.sh; artifactLive Claude submit proofFM_OMP_HERDR_SUBMIT_LIVE_E2E=1 bash tests/fm-omp-herdr-live-e2e.test.sh; artifactLive OMP submit failurebash tests/fm-backend-herdr.test.shbash tests/fm-backend-herdr-smoke.test.shFM_HERDR_SUBMIT_CONFIRM_LIVE=1 bash tests/fm-herdr-submit-confirm-live-e2e.test.shFM_OMP_HERDR_SUBMIT_LIVE_E2E=1 bash tests/fm-omp-herdr-live-e2e.test.sh🔧 Fix applied.
1 warning still open:
bash tests/fm-backend-herdr-smoke.test.sh; herdr-smoke.logbash tests/fm-backend-herdr.test.shbash tests/fm-backend-herdr-smoke.test.shbash tests/fm-composer-lib.test.sh✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.