From 85b7a29d0b96a1377ec68f4e997c66bd9018652b Mon Sep 17 00:00:00 2001 From: Kun Chen <3233006+kunchenguid@users.noreply.github.com> Date: Mon, 13 Jul 2026 14:20:40 -0700 Subject: [PATCH 001/131] fix: preserve secondmate routing markers in terminal sends (#533) * fix: preserve secondmate routing markers * no-mistakes(review): Captain, preserve trailing newlines in marked secondmate sends * no-mistakes(test): Captain, tolerate bootstrap timeout elapsed drift * no-mistakes(document): Refresh Herdr marker documentation --- bin/fm-marker-lib.sh | 45 ++-- bin/fm-send.sh | 16 +- docs/herdr-backend.md | 70 +++++- docs/scripts.md | 2 +- tests/fm-bootstrap.test.sh | 33 ++- ...m-send-secondmate-marker-herdr-e2e.test.sh | 199 ++++++++++++++++++ tests/fm-send-secondmate-marker.test.sh | 102 ++++++--- 7 files changed, 403 insertions(+), 64 deletions(-) create mode 100755 tests/fm-send-secondmate-marker-herdr-e2e.test.sh diff --git a/bin/fm-marker-lib.sh b/bin/fm-marker-lib.sh index 6cc69cd04f..2303f3a574 100644 --- a/bin/fm-marker-lib.sh +++ b/bin/fm-marker-lib.sh @@ -27,15 +27,15 @@ # Distinct from the afk daemon marker, on purpose. # The away-mode daemon (bin/fm-supervise-daemon.sh) marks its daemon->firstmate # escalations with a BARE leading unit separator (FM_INJECT_MARK, ASCII 0x1f). -# This from-firstmate marker mirrors that CONCEPT - it reuses the ASCII unit -# separator (0x1f), which is untypable on a normal keyboard, as the "a human can -# never forge this" guarantee - but it is a DISTINCT sequence: a human-readable -# label FOLLOWED by the separator, never a bare leading 0x1f. The afk contract -# keys on a LEADING 0x1f, which this marker never has, so the two cannot -# conflate: a secondmate's own afk machinery never mistakes a from-firstmate -# request for an internal daemon escalation, and vice versa. The visible label is -# also what the secondmate's LLM actually reads in its pane, since the separator -# byte itself is invisible. +# The from-firstmate marker instead uses U+2063 INVISIBLE SEPARATOR after its +# human-readable label. U+2063 has no normal keyboard keystroke but travels as +# UTF-8 text rather than a terminal control byte. The original ASCII 0x1f +# separator did not survive terminal input faithfully: on Herdr 0.7.3 feeding +# it to a real Pi composer removed the preceding label, so Pi received only the +# unmarked request (docs/herdr-backend.md records the incident and live proof). +# The afk contract keys on a LEADING 0x1f, while this marker begins with its +# label and contains no 0x1f, so the two cannot conflate. The visible label is +# what the secondmate's LLM reads; U+2063 remains invisible. # # Sourced by bin/fm-send.sh, bin/fm-brief.sh, and the tests. No side effects on # source. set -u / set -e safe. @@ -45,17 +45,32 @@ FM_FROMFIRST_LABEL='[fm-from-firstmate]' # The full marker fm-send prepends to a from-firstmate request: the label, then -# the ASCII unit separator (0x1f) as the untypable field separator. The request -# text follows the separator. -FM_FROMFIRST_MARK="${FM_FROMFIRST_LABEL}"$'\x1f' +# U+2063 INVISIBLE SEPARATOR (UTF-8 e2 81 a3). The request text follows it. +FM_FROMFIRST_SEPARATOR=$'\xE2\x81\xA3' +FM_FROMFIRST_MARK="${FM_FROMFIRST_LABEL}${FM_FROMFIRST_SEPARATOR}" # fm_message_from_firstmate: 0 (true) if carries the from-firstmate -# marker - it begins with the label immediately followed by the unit separator - -# and 1 otherwise. The unit separator is untypable, so a captain-typed message, -# even one that happens to start with the label text alone, is never matched. +# marker - it begins with the label immediately followed by U+2063 - and 1 +# otherwise. U+2063 has no normal keyboard keystroke, so captain-typed input, +# even when it starts with the visible label text alone, is never matched. fm_message_from_firstmate() { # case "$1" in "$FM_FROMFIRST_MARK"*) return 0 ;; esac return 1 } + +# fm_message_mark_from_firstmate: assign with exactly one leading +# from-firstmate marker. This is the single owner of marker transformation, so +# callers cannot drift on separator bytes or double-prefix an already-marked +# message. +fm_message_mark_from_firstmate() { # + local message=${1-} result_var=${2-} transformed + [ -n "$result_var" ] || return 2 + if fm_message_from_firstmate "$1"; then + transformed=$message + else + transformed="${FM_FROMFIRST_MARK}${message}" + fi + printf -v "$result_var" '%s' "$transformed" +} diff --git a/bin/fm-send.sh b/bin/fm-send.sh index 223e2f2bf0..cc16aff438 100755 --- a/bin/fm-send.sh +++ b/bin/fm-send.sh @@ -181,14 +181,14 @@ shift fm_backend_validate "$TARGET_BACKEND" || exit 1 -# Mark a from-firstmate -> secondmate request. Only a task selector resolved -# through this home's meta and recording kind=secondmate is marked: the +# Classify a from-firstmate -> secondmate request. Only a task selector resolved +# through this home's meta whose authoritative kind is secondmate is marked: the # secondmate then routes its reply via the status path (see fm-marker-lib.sh). # An explicit backend target (the escape hatch for endpoints outside this home) # and any crewmate/scout target are left unmarked, and so is the --key path. -MARK_PREFIX="" -if [ -n "$TARGET_SELECTOR" ] && [ -n "$TARGET_META" ] && grep -q '^kind=secondmate$' "$TARGET_META" 2>/dev/null; then - MARK_PREFIX="$FM_FROMFIRST_MARK" +MARK_FROM_FIRSTMATE=0 +if [ -n "$TARGET_SELECTOR" ] && [ -n "$TARGET_META" ] && [ "$(fm_meta_get "$TARGET_META" kind)" = secondmate ]; then + MARK_FROM_FIRSTMATE=1 fi # Resolve the target's harness from its meta (recorded by fm-spawn), used only to @@ -209,6 +209,10 @@ if [ "${1:-}" = "--key" ]; then exit 1 fi else + MESSAGE=$* + if [ "$MARK_FROM_FIRSTMATE" = 1 ]; then + fm_message_mark_from_firstmate "$MESSAGE" MESSAGE + fi # Slash commands open a completion popup in some TUIs (verified on codex); # submitting too fast selects nothing, so give the popup time to settle before # the (retried) Enter. Codex opens the same kind of popup for a `$` @@ -228,7 +232,7 @@ else sleep_s=${FM_SEND_SLEEP:-0.4} # Type once, submit, verify. Lenient: only a positively-confirmed swallow # (text still in the composer) is an error; an unreadable pane is assumed sent. - if ! verdict=$(fm_backend_send_text_submit "$TARGET_BACKEND" "$T" "$MARK_PREFIX$*" "$retries" "$sleep_s" "$settle" "$EXPECTED_LABEL"); then + if ! verdict=$(fm_backend_send_text_submit "$TARGET_BACKEND" "$T" "$MESSAGE" "$retries" "$sleep_s" "$settle" "$EXPECTED_LABEL"); then echo "error: text not sent to $T ($TARGET_BACKEND send failed; tried $RESOLUTION_TRIED)" >&2 exit 1 fi diff --git a/docs/herdr-backend.md b/docs/herdr-backend.md index fdd506d69c..0a0c15f769 100644 --- a/docs/herdr-backend.md +++ b/docs/herdr-backend.md @@ -4,7 +4,7 @@ This document records the empirical verification behind `bin/backends/herdr.sh`, It is the herdr equivalent of the tmux facts recorded in the `harness-adapters` skill and `docs/architecture.md`'s "Runtime session backends" section. Herdr is [an agent-native terminal multiplexer](https://herdr.dev) with a socket API, CLI wrappers, and native per-pane agent-state detection. -Verified against the real installed binary: herdr 0.7.1, protocol 14, macOS aarch64. +Originally verified against herdr 0.7.1, protocol 14, on macOS aarch64; the latest dated evidence below uses herdr 0.7.3, protocol 16. Current real-herdr verification uses isolated `HERDR_SESSION` names plus the guarded teardown helper in `tests/herdr-test-safety.sh`. A 2026-07-02 cleanup bug proved that `HERDR_SESSION` alone is not a safe way to target destructive session cleanup; see "Session targeting: the `--session` flag, not `HERDR_SESSION` alone" below. All real-herdr verification in this document uses isolated sessions and guarded cleanup; the captain's default herdr session and live tmux fleet were never intended targets. @@ -18,7 +18,7 @@ Firstmate only drives the `herdr` CLI as a separate process, which carries no AG Prerequisites: -- `herdr` itself, protocol 14 or newer (installed 0.7.1 verified) - see [herdr.dev](https://herdr.dev) for install instructions. +- `herdr` itself, protocol 14 or newer (0.7.1 and 0.7.3 verified) - see [herdr.dev](https://herdr.dev) for install instructions. - `jq`, required to parse herdr's JSON output: `brew install jq` (or your platform's package manager). - The universal firstmate prerequisites - a verified crew harness plus the required toolchain, owned by [`docs/configuration.md`](configuration.md) ("Harness support", "Toolchain"); treehouse still provides the worktree, herdr only provides the session. @@ -193,6 +193,72 @@ Herdr tasks additionally record: | Workspace create / tab create (focus) | `herdr workspace create --no-focus`, `herdr tab create --no-focus` | Verified: neither focuses by default once a workspace already exists in the session, matching pre-P3 (flagless) behavior; `--no-focus` is passed anyway for defense in depth, since the very first workspace ever created in a brand-new session focuses regardless of the flag. `--focus` was separately verified to reliably focus, confirming the flag has real effect. | | Session targeting for DESTRUCTIVE calls | `herdr session stop --session --json`, then `herdr session delete --session --json`; never `herdr server stop` | Owned by `bin/fm-herdr-lab.sh` (which `tests/herdr-test-safety.sh` sources), re-querying `herdr session list --json` before every destructive call. See "Session targeting" below - `HERDR_SESSION` alone is not reliably honored once another herdr server is already running on the machine. | +## Incident (2026-07-13): the ASCII request separator erased the secondmate marker + +A routed request reached a Pi/Herdr secondmate without the visible `[fm-from-firstmate]` label, so the secondmate correctly treated it as direct captain conversation and returned nothing to the parent status path. +The initial suspicion was selector classification, but a real isolated reproduction disproved that: exact-id lookup found the right metadata, read `kind=secondmate`, selected the recorded Herdr endpoint, and still delivered an unmarked Pi prompt. + +The reproduction used Herdr 0.7.3 (protocol 16), Pi 0.80.6, a task-local sender home, a real `fm-spawn.sh --secondmate --harness pi --backend herdr` endpoint, and a generated non-`default` session from `bin/fm-herdr-lab.sh`. +Every adapter call was routed through the lab helper, and teardown verified the default-session fleet-state tripwire. +The end-user command was run with normal `FM_SEND_SETTLE`: + +```sh +FM_HOME= bin/fm-send.sh marker-pi-sm \ + 'FM_MARKER_E2E_CURRENT exact-id request' +``` + +Immediately before submission, the authoritative selector helpers reported: + +```text +resolved-meta=/state/marker-pi-sm.meta +kind=secondmate +target=:w1:p2 +backend=herdr +expected-label=fm-marker-pi-sm +``` + +Pi's separator-only idle composer is outside the Herdr structural classifier's recognized bordered/bare shapes, so composer state was conservatively `unknown` both before and after the send. +The endpoint's native agent state was idle before submission, and the normal idle-to-working confirmation made `fm-send.sh` return successfully. +A task-local Pi `before_agent_start` hook then captured the exact received prompt and UTF-8 bytes: + +```json +{"prompt":"FM_MARKER_E2E_CURRENT exact-id request","hex":"464d5f4d41524b45525f4532455f43555252454e542065786163742d69642072657175657374"} +``` + +The old marker should instead have started with label bytes `5b666d2d66726f6d2d66697273746d6174655d`, followed by ASCII `1f` and then those request bytes. +The Pi transcript independently rendered only `FM_MARKER_E2E_CURRENT exact-id request`, and the agent answered it conversationally as captain input. + +The failure was in marker transport, not backend selection or metadata classification. +`fm-send.sh` correctly passed `[fm-from-firstmate]`, ASCII unit separator `0x1f`, and the request to `herdr pane send-text`. +Herdr's terminal input path treated the C0 byte as a control action rather than text, removing the preceding label before Pi submitted the remaining request. +A tmux-stub unit test could not expose this because it logged the string argument without driving a real terminal editor. + +The single marker owner, `bin/fm-marker-lib.sh`, now uses U+2063 INVISIBLE SEPARATOR (UTF-8 `e2 81 a3`) after the visible label. +U+2063 has no normal keyboard keystroke but travels through terminal input as text rather than a C0 control byte. +The same owner now provides the idempotent marker transformation, so an already-marked request is not prefixed twice. +No Herdr-specific injection or classification branch was added. + +The opt-in regression command is: + +```sh +FM_SEND_MARKER_HERDR_E2E=1 tests/fm-send-secondmate-marker-herdr-e2e.test.sh +``` + +The real post-fix Pi capture reported exactly one marker followed by the request: + +```text +evidence: exact-id received-hex=5b666d2d66726f6d2d66697273746d6174655de281a3464d5f4d41524b45525f48455244525f4532452065786163742d69642072657175657374 +``` + +The same run injected direct terminal text without `fm-send.sh` and captured it byte-exact with no marker: + +```text +evidence: direct-input received-hex=464d5f4d41524b45525f48455244525f444952454354206361707461696e20696e707574 +``` + +Unit coverage in `tests/fm-send-secondmate-marker.test.sh` pins exact-id and stable-label secondmates, exact-id and stable-label ordinary crewmates, explicit endpoints with and without local metadata, key-only sends, direct unmarked input, exact U+2063 bytes, and idempotence. +Strict unresolved-selector behavior remains covered by `tests/fm-send-strict.test.sh`. + ## Verified bug: `pane read --lines N` returns empty for small N This was the most significant finding of this verification pass. diff --git a/docs/scripts.md b/docs/scripts.md index 6ad6a4e544..a7d3030f90 100644 --- a/docs/scripts.md +++ b/docs/scripts.md @@ -39,7 +39,7 @@ The shared no-mistakes gate refusal used by `fm-spawn.sh`, `fm-send.sh`, and `fm | `fm-project-mode.sh` | Resolve a project's delivery mode and `+yolo` flag from `data/projects.md` | | `fm-merge-local.sh` | Fast-forward a `local-only` project's local default branch after approval | | `fm-review-diff.sh` | Review a crewmate branch or recorded PR head against the authoritative base | -| `fm-marker-lib.sh` | Shared from-firstmate request marker and detector | +| `fm-marker-lib.sh` | Shared from-firstmate request marker, detector, and idempotent transformation | | `fm-gate-refuse-lib.sh` | Shared no-mistakes gate-context refusal for fleet lifecycle entrypoints | | `fm-watch-arm.sh` | Verified home-scoped watcher arm wrapper with honest status reporting | | `fm-watch-checkpoint.sh` | Run one bounded foreground watcher checkpoint for Codex-style supervision | diff --git a/tests/fm-bootstrap.test.sh b/tests/fm-bootstrap.test.sh index 2c20707107..83a9842171 100755 --- a/tests/fm-bootstrap.test.sh +++ b/tests/fm-bootstrap.test.sh @@ -169,10 +169,10 @@ run_bootstrap_timeout_case() { sleep() { local inc=${1:-1} SECONDS=$((SECONDS + inc)) - if [ "${FM_FAKE_SLEEP_YIELDS:-0}" -lt 5 ]; then - FM_FAKE_SLEEP_YIELDS=$((${FM_FAKE_SLEEP_YIELDS:-0} + 1)) - command sleep 0.01 - fi + # Advance fake time quickly, but yield on every tick so the background + # fleet-sync process can deterministically write its partial output before + # the simulated timeout kills it, even on a busy full-suite runner. + command sleep 0.01 } # shellcheck disable=SC2317,SC2329 # Exported and invoked by the bootstrap subprocess. git() { @@ -208,6 +208,16 @@ run_bootstrap_timeout_case() { ) } +assert_timeout_report() { + local out=$1 expected_timeout=$2 timing timeout elapsed + timing=$(printf '%s\n' "$out" | sed -n 's/^FLEET_SYNC: fleet: skipped: bootstrap refresh timed out (timeout=\([0-9][0-9]*\)s elapsed=\([0-9][0-9]*\)s)$/\1 \2/p') + [ -n "$timing" ] || fail "missing fleet-sync timeout report" + timeout=${timing%% *} + elapsed=${timing#* } + [ "$timeout" -eq "$expected_timeout" ] || fail "expected timeout=${expected_timeout}s, got timeout=${timeout}s" + [ "$elapsed" -ge "$timeout" ] || fail "expected elapsed >= timeout, got elapsed=${elapsed}s timeout=${timeout}s" +} + # Each row (fields are '^'-separated; the install URL contains a literal '|'): #