From 58cb76ef5f0a376cb82b5f4eb7013769c6e9f534 Mon Sep 17 00:00:00 2001 From: kunchenguid Date: Mon, 13 Jul 2026 12:21:11 -0700 Subject: [PATCH 1/4] fix: preserve secondmate routing markers --- bin/fm-marker-lib.sh | 42 ++-- bin/fm-send.sh | 16 +- docs/herdr-backend.md | 66 ++++++ tests/fm-bootstrap.test.sh | 8 +- ...m-send-secondmate-marker-herdr-e2e.test.sh | 192 ++++++++++++++++++ tests/fm-send-secondmate-marker.test.sh | 85 +++++--- 6 files changed, 355 insertions(+), 54 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 6cc69cd04f2..7f37fc4b708 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,29 @@ 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: print 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. Prints no trailing newline. +fm_message_mark_from_firstmate() { # + if fm_message_from_firstmate "$1"; then + printf '%s' "$1" + else + printf '%s%s' "$FM_FROMFIRST_MARK" "$1" + fi +} diff --git a/bin/fm-send.sh b/bin/fm-send.sh index 223e2f2bf0a..4f030f7a6ee 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 + MESSAGE=$(fm_message_mark_from_firstmate "$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 fdd506d69c5..7528a7c4e50 100644 --- a/docs/herdr-backend.md +++ b/docs/herdr-backend.md @@ -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/tests/fm-bootstrap.test.sh b/tests/fm-bootstrap.test.sh index 2c20707107e..992306a8391 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() { diff --git a/tests/fm-send-secondmate-marker-herdr-e2e.test.sh b/tests/fm-send-secondmate-marker-herdr-e2e.test.sh new file mode 100755 index 00000000000..426510ba0b2 --- /dev/null +++ b/tests/fm-send-secondmate-marker-herdr-e2e.test.sh @@ -0,0 +1,192 @@ +#!/usr/bin/env bash +# Real Pi/Herdr regression for exact-id secondmate marker delivery. +# +# This is opt-in because it launches a real interactive Pi process and a real +# isolated Herdr lab session. +# It exercises the end-user command shape against metadata written by a real +# fm-spawn.sh --secondmate launch, captures Pi's before_agent_start prompt bytes, +# and proves both sides of the routing boundary: +# - exact task id through explicit FM_HOME receives exactly one marker; +# - direct terminal input remains unmarked. +# +# Every Herdr call, including calls made inside the production backend adapter, +# is routed through bin/fm-herdr-lab.sh. The PATH shim strips only the adapter's +# already-validated trailing --session pair, then delegates to the lab helper, +# which appends its own required trailing --session before invoking real Herdr. +set -u + +# shellcheck source=tests/lib.sh +. "$(dirname "${BASH_SOURCE[0]}")/lib.sh" +# shellcheck source=bin/fm-marker-lib.sh +. "$ROOT/bin/fm-marker-lib.sh" +# shellcheck source=bin/fm-backend.sh +. "$ROOT/bin/fm-backend.sh" + +if [ "${FM_SEND_MARKER_HERDR_E2E:-0}" != 1 ]; then + echo "skip: set FM_SEND_MARKER_HERDR_E2E=1 to run the real Pi/Herdr secondmate-marker regression" + exit 0 +fi + +for tool in git herdr jq pi python3; do + command -v "$tool" >/dev/null 2>&1 || { echo "skip: $tool not found"; exit 0; } +done + +LAB_HELPER=${HERDR_LAB_HELPER:-$ROOT/bin/fm-herdr-lab.sh} +SESSION=$("$LAB_HELPER" name fm-send-secondmate-marker-v7) +TMP_ROOT=$(mktemp -d "${TMPDIR:-/tmp}/fm-send-marker-herdr-e2e.XXXXXX") +SENDER_HOME="$TMP_ROOT/sender-home" +SECOND_HOME="$TMP_ROOT/secondmate-home" +CAPTURE="$TMP_ROOT/pi-before-agent.jsonl" +FAKEBIN="$TMP_ROOT/fakebin" +ORIGINAL_PATH=$PATH +ID='marker-pi-sm' +REQUEST='FM_MARKER_HERDR_E2E exact-id request' +DIRECT='FM_MARKER_HERDR_DIRECT captain input' + +cleanup() { + local rc=$? + trap - EXIT + if ! "$LAB_HELPER" teardown "$SESSION"; then + rc=1 + fi + rm -rf "$TMP_ROOT" + exit "$rc" +} +trap cleanup EXIT + +mkdir -p "$SENDER_HOME/state" "$SENDER_HOME/data" "$SENDER_HOME/config" "$SENDER_HOME/projects" "$FAKEBIN" + +# Route production adapter invocations through the same guarded helper as every +# explicit E2E probe. The helper itself runs with the original PATH, preventing +# recursion into this shim. +cat > "$FAKEBIN/herdr" <&2; exit 97; } + args=("\${args[@]:0:\$((n-2))}") +else + [ "\${HERDR_SESSION:-}" = "\$session" ] || { echo "wrapper requires the isolated lab session" >&2; exit 98; } + for arg in "\${args[@]}"; do + case "\$arg" in + --session|--session=*) echo "wrapper refused non-trailing session flag" >&2; exit 99 ;; + esac + done +fi +PATH="\$real_path" exec "\$helper" run "\$session" "\${args[@]}" +EOF +chmod +x "$FAKEBIN/herdr" + +git clone -q --no-hardlinks "$ROOT" "$SECOND_HOME" +git -C "$SECOND_HOME" checkout -q --detach HEAD +mkdir -p "$SECOND_HOME/state" "$SECOND_HOME/data" "$SECOND_HOME/config" "$SECOND_HOME/projects" +printf '%s\n' "$ID" > "$SECOND_HOME/.fm-secondmate-home" +cat > "$SECOND_HOME/data/charter.md" <<'EOF' +# Isolated marker capture secondmate + +You are a task-local secondmate used only for the marker transport regression. +Stay idle and do not initiate work. +EOF + +# The extension is already an explicit Pi -e resource in the real secondmate +# launch template, so its project_trust hook can grant session-only trust before +# project resources load. before_agent_start records the exact prompt bytes and +# aborts before any provider request, keeping this transport regression local. +CAPTURE_JSON=$(printf '%s' "$CAPTURE" | jq -Rs .) +python3 - "$SECOND_HOME/.pi/extensions/fm-primary-turnend-guard.ts" "$CAPTURE_JSON" <<'PY' +from pathlib import Path +import sys + +path = Path(sys.argv[1]) +capture_json = sys.argv[2] +source = path.read_text() +import_anchor = 'import type { ExtensionAPI } from "@earendil-works/pi-coding-agent";\n' +source = source.replace( + import_anchor, + import_anchor + + 'import { appendFileSync as fmAppendFileSync } from "node:fs";\n' + + f'const fmCapturePath = {capture_json};\n', + 1, +) +factory_anchor = 'export default function (pi: ExtensionAPI) {\n' +replacement = '''export default function (pi: ExtensionAPI) { + pi.on("project_trust", () => ({ trusted: "yes", remember: false })); + pi.on("before_agent_start", (event, ctx) => { + fmAppendFileSync(fmCapturePath, `${JSON.stringify({ prompt: event.prompt, hex: Buffer.from(event.prompt, "utf8").toString("hex") })}\\n`); + ctx.abort(); + }); +''' +if import_anchor not in source or factory_anchor not in source: + raise SystemExit("Pi extension insertion point missing") +path.write_text(source.replace(factory_anchor, replacement, 1)) +PY + +"$LAB_HELPER" provision "$SESSION" +PATH="$FAKEBIN:$ORIGINAL_PATH" FM_GATE_REFUSE_BYPASS=1 FM_HOME="$SENDER_HOME" HERDR_SESSION="$SESSION" \ + "$ROOT/bin/fm-spawn.sh" "$ID" "$SECOND_HOME" --secondmate --harness pi --backend herdr >/dev/null + +META="$SENDER_HOME/state/$ID.meta" +[ -f "$META" ] || fail "real secondmate spawn did not write exact-id metadata" +[ "$(fm_meta_get "$META" kind)" = secondmate ] || fail "real secondmate metadata did not record kind=secondmate" +TARGET=$(fm_backend_target_of_meta "$META") +PANE=${TARGET#*:} +case "$TARGET" in + "$SESSION":w*:p*) : ;; + *) fail "real secondmate metadata recorded an unexpected Herdr target: $TARGET" ;; +esac + +wait_for_prompt() { # + local needle=$1 _ + for _ in $(seq 1 240); do + if [ -s "$CAPTURE" ] && jq -e --arg needle "$needle" 'select(.prompt | contains($needle))' "$CAPTURE" >/dev/null 2>&1; then + return 0 + fi + sleep 0.25 + done + return 1 +} + +wait_for_idle() { + local status _ + for _ in $(seq 1 240); do + status=$("$LAB_HELPER" run "$SESSION" agent get "$PANE" 2>/dev/null \ + | jq -r '.result.agent.agent_status // empty' 2>/dev/null || true) + case "$status" in idle|done) return 0 ;; esac + sleep 0.25 + done + return 1 +} + +# The startup charter proves the CLI extension loaded. Wait until ctx.abort() +# has fully settled before exercising the idle Pi composer. +wait_for_prompt 'Isolated marker capture secondmate' \ + || fail "real Pi before_agent_start capture did not load for the startup charter" +wait_for_idle || fail "real Pi did not become idle after the startup capture" + +PATH="$FAKEBIN:$ORIGINAL_PATH" FM_GATE_REFUSE_BYPASS=1 FM_HOME="$SENDER_HOME" \ + "$ROOT/bin/fm-send.sh" "$ID" "$REQUEST" >/dev/null +wait_for_prompt "$REQUEST" || fail "real Pi did not receive the exact-id fm-send request" +GOT=$(jq -r --arg needle "$REQUEST" 'select(.prompt | contains($needle)) | .prompt' "$CAPTURE" | tail -1) +[ "$GOT" = "${FM_FROMFIRST_MARK}${REQUEST}" ] \ + || fail "real Pi exact-id prompt did not contain exactly one terminal-safe marker"$'\n'"--- bytes ---"$'\n'"$(printf '%s' "$GOT" | od -An -tx1)" +printf 'evidence: exact-id received-hex=%s\n' "$(printf '%s' "$GOT" | od -An -tx1 | tr -d ' \n')" +pass "real Pi/Herdr: exact-id FM_HOME send delivers exactly one from-firstmate marker" +wait_for_idle || fail "real Pi did not become idle after the exact-id capture" + +# Direct terminal input bypasses fm-send's metadata-routed transformation and +# therefore remains conversational captain input. +"$LAB_HELPER" run "$SESSION" pane send-text "$PANE" "$DIRECT" >/dev/null +"$LAB_HELPER" run "$SESSION" pane send-keys "$PANE" enter >/dev/null +wait_for_prompt "$DIRECT" || fail "real Pi did not receive direct terminal input" +GOT=$(jq -r --arg needle "$DIRECT" 'select(.prompt | contains($needle)) | .prompt' "$CAPTURE" | tail -1) +[ "$GOT" = "$DIRECT" ] || fail "direct captain input was changed or marked"$'\n'"--- bytes ---"$'\n'"$(printf '%s' "$GOT" | od -An -tx1)" +if fm_message_from_firstmate "$GOT"; then + fail "direct captain input was classified as from-firstmate" +fi +printf 'evidence: direct-input received-hex=%s\n' "$(printf '%s' "$GOT" | od -An -tx1 | tr -d ' \n')" +pass "real Pi/Herdr: direct captain terminal input stays unmarked" diff --git a/tests/fm-send-secondmate-marker.test.sh b/tests/fm-send-secondmate-marker.test.sh index ea7ef925a95..75c2d3387a4 100755 --- a/tests/fm-send-secondmate-marker.test.sh +++ b/tests/fm-send-secondmate-marker.test.sh @@ -8,12 +8,12 @@ # selector whose meta records kind=secondmate, so the secondmate can recognize # the request and route its reply via the status path. These tests pin that # behavior hermetically (stubbed tmux, no real agent): -# 1. A send to a kind=secondmate task selector prepends the marker to the text. -# 2. A send to a crewmate (kind=ship) target sends the bare text, no marker. -# 3. An explicit session:window target (no meta) is never marked. +# 1. Exact-id and stable-label kind=secondmate selectors prepend the marker. +# 2. Exact-id and stable-label ordinary crewmate selectors stay unmarked. +# 3. Explicit endpoints stay unmarked, with or without matching local meta. # 4. The --key path never carries the marker. -# 5. The marker is exactly the label "[fm-from-firstmate]" + ASCII 0x1f, and the -# fm_message_from_firstmate detector keys on that untypable sequence. +# 5. Direct captain text stays unmarked, and already-marked text is idempotent. +# 6. The marker is the label plus terminal-safe U+2063 INVISIBLE SEPARATOR. set -u # shellcheck source=tests/lib.sh @@ -107,7 +107,7 @@ test_secondmate_target_is_marked() { } test_exact_secondmate_task_id_is_marked() { - local dir fb log home rc got + local dir fb log home rc got already_marked dir="$TMP_ROOT/sm-exact"; mkdir -p "$dir" fb=$(make_stubs "$dir"); log="$dir/send.log" home=$(setup_home sm-exact) @@ -119,7 +119,13 @@ test_exact_secondmate_task_id_is_marked() { "$FM_FROMFIRST_MARK"audit\ the\ build) : ;; *) fail "exact secondmate send: literal text should be marker+text"$'\n'"--- bytes ---"$'\n'"$(printf '%s' "$got" | od -An -c)" ;; esac - pass "fm-send: an exact kind=secondmate task id gets the from-firstmate marker prepended" + already_marked="${FM_FROMFIRST_MARK}already routed" + run_send "$fb" "$home" "$log" "domain" "$already_marked"; rc=$? + expect_code 0 "$rc" "send of already-marked exact-id content should succeed" + got=$(cat "$log") + [ "$got" = "$already_marked" ] \ + || fail "exact secondmate send double-prefixed already-marked content"$'\n'"--- bytes ---"$'\n'"$(printf '%s' "$got" | od -An -tx1)" + pass "fm-send: an exact kind=secondmate task id is marked exactly once" } test_crewmate_target_is_not_marked() { @@ -131,11 +137,16 @@ test_crewmate_target_is_not_marked() { "window=sess:fm-build" "worktree=$home/wt" "project=$home/p" \ "harness=echo" "kind=ship" "mode=no-mistakes" "yolo=off" run_send "$fb" "$home" "$log" "fm-build" "fix the test"; rc=$? - expect_code 0 "$rc" "send to a crewmate target should succeed" + expect_code 0 "$rc" "send to a stable-label crewmate target should succeed" got=$(cat "$log") [ "$got" = "fix the test" ] \ - || fail "crewmate send: expected bare text, got marker or other"$'\n'"--- bytes ---"$'\n'"$(printf '%s' "$got" | od -An -c)" - pass "fm-send: a kind=ship (crewmate) target is sent unmarked" + || fail "stable-label crewmate send: expected bare text, got marker or other"$'\n'"--- bytes ---"$'\n'"$(printf '%s' "$got" | od -An -c)" + run_send "$fb" "$home" "$log" "build" "fix the exact test"; rc=$? + expect_code 0 "$rc" "send to an exact-id crewmate target should succeed" + got=$(cat "$log") + [ "$got" = "fix the exact test" ] \ + || fail "exact-id crewmate send: expected bare text, got marker or other"$'\n'"--- bytes ---"$'\n'"$(printf '%s' "$got" | od -An -c)" + pass "fm-send: exact-id and stable-label kind=ship selectors are sent unmarked" } test_explicit_window_is_not_marked() { @@ -143,15 +154,22 @@ test_explicit_window_is_not_marked() { dir="$TMP_ROOT/explicit"; mkdir -p "$dir" fb=$(make_stubs "$dir"); log="$dir/send.log" home=$(setup_home explicit) - # No meta lookup happens for an explicit session:window target, so even with a - # same-named secondmate meta present it must stay unmarked (escape hatch). + # An explicit endpoint is not a task selector, so even matching secondmate + # metadata must not make fm-send guess the caller's intent and mark it. fm_write_secondmate_meta "$home/state/win.meta" "$home" "other:win" run_send "$fb" "$home" "$log" "other:win" "ping"; rc=$? - expect_code 0 "$rc" "send to an explicit window should succeed" + expect_code 0 "$rc" "send to an explicit window with matching meta should succeed" got=$(cat "$log") [ "$got" = "ping" ] \ - || fail "explicit session:window send: expected bare text, got marker"$'\n'"--- bytes ---"$'\n'"$(printf '%s' "$got" | od -An -c)" - pass "fm-send: an explicit session:window target is never marked" + || fail "explicit session:window send with meta: expected bare text, got marker"$'\n'"--- bytes ---"$'\n'"$(printf '%s' "$got" | od -An -c)" + + home=$(setup_home explicit-no-meta) + run_send "$fb" "$home" "$log" "outside:window" "outside ping"; rc=$? + expect_code 0 "$rc" "send to an explicit window with no local meta should succeed" + got=$(cat "$log") + [ "$got" = "outside ping" ] \ + || fail "explicit session:window send without meta: expected bare text, got marker"$'\n'"--- bytes ---"$'\n'"$(printf '%s' "$got" | od -An -c)" + pass "fm-send: explicit endpoints stay unmarked with or without local metadata" } test_key_path_is_not_marked() { @@ -167,26 +185,34 @@ test_key_path_is_not_marked() { pass "fm-send: the --key path carries no marker (no literal text is typed)" } -test_marker_is_label_plus_unit_separator() { - local us hex - us=$(printf '\037') - [ "$FM_FROMFIRST_MARK" = "[fm-from-firstmate]$us" ] \ - || fail "marker is not the expected label + 0x1f sequence"$'\n'"--- bytes ---"$'\n'"$(printf '%s' "$FM_FROMFIRST_MARK" | od -An -c)" - # The last byte must be ASCII unit separator 0x1f, the untypable guarantee. +test_marker_is_label_plus_invisible_separator() { + local separator hex + separator=$(printf '\342\201\243') + [ "$FM_FROMFIRST_MARK" = "[fm-from-firstmate]$separator" ] \ + || fail "marker is not the expected label + U+2063 sequence"$'\n'"--- bytes ---"$'\n'"$(printf '%s' "$FM_FROMFIRST_MARK" | od -An -tx1)" hex=$(printf '%s' "$FM_FROMFIRST_MARK" | od -An -tx1 | tr -d ' \n') case "$hex" in - *1f) : ;; - *) fail "marker does not end in a 0x1f byte; bytes were: $hex" ;; + *e281a3) : ;; + *) fail "marker does not end in UTF-8 U+2063 bytes e2 81 a3; bytes were: $hex" ;; esac - # The detector keys on that exact untypable sequence. fm_message_from_firstmate "${FM_FROMFIRST_MARK}do the work" \ || fail "detector should recognize a marked message" fm_message_from_firstmate "do the work" \ - && fail "detector must reject an unmarked message" - # The bare label without the separator (the typable part) is NOT a match. + && fail "direct captain input must remain unmarked" fm_message_from_firstmate "[fm-from-firstmate]do the work" \ - && fail "detector must reject the label without the 0x1f separator" - pass "fm-send: the marker is exactly '[fm-from-firstmate]' + ASCII 0x1f, detector keys on it" + && fail "detector must reject the label without U+2063" + pass "fm-send: the marker is '[fm-from-firstmate]' + terminal-safe U+2063, while direct captain text stays unmarked" +} + +test_marker_transformation_is_idempotent() { + local once twice + once=$(fm_message_mark_from_firstmate "do the work") + twice=$(fm_message_mark_from_firstmate "$once") + [ "$once" = "$twice" ] \ + || fail "already-marked content was double-prefixed"$'\n'"--- once ---"$'\n'"$(printf '%s' "$once" | od -An -tx1)"$'\n'"--- twice ---"$'\n'"$(printf '%s' "$twice" | od -An -tx1)" + [ "$once" = "${FM_FROMFIRST_MARK}do the work" ] \ + || fail "marker transformation did not prefix bare content exactly once" + pass "fm-marker: from-firstmate transformation is idempotent" } test_secondmate_target_is_marked @@ -194,4 +220,5 @@ test_exact_secondmate_task_id_is_marked test_crewmate_target_is_not_marked test_explicit_window_is_not_marked test_key_path_is_not_marked -test_marker_is_label_plus_unit_separator +test_marker_is_label_plus_invisible_separator +test_marker_transformation_is_idempotent From c6e0da1d27fedf9667bd3a06b4bbb8ec8ba38718 Mon Sep 17 00:00:00 2001 From: kunchenguid Date: Mon, 13 Jul 2026 12:27:25 -0700 Subject: [PATCH 2/4] no-mistakes(review): Captain, preserve trailing newlines in marked secondmate sends --- bin/fm-marker-lib.sh | 13 ++++++++----- bin/fm-send.sh | 2 +- tests/fm-send-secondmate-marker.test.sh | 21 +++++++++++++++++++-- 3 files changed, 28 insertions(+), 8 deletions(-) diff --git a/bin/fm-marker-lib.sh b/bin/fm-marker-lib.sh index 7f37fc4b708..2303f3a5748 100644 --- a/bin/fm-marker-lib.sh +++ b/bin/fm-marker-lib.sh @@ -60,14 +60,17 @@ fm_message_from_firstmate() { # return 1 } -# fm_message_mark_from_firstmate: print with exactly one leading +# 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. Prints no trailing newline. -fm_message_mark_from_firstmate() { # +# 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 - printf '%s' "$1" + transformed=$message else - printf '%s%s' "$FM_FROMFIRST_MARK" "$1" + 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 4f030f7a6ee..cc16aff4380 100755 --- a/bin/fm-send.sh +++ b/bin/fm-send.sh @@ -211,7 +211,7 @@ if [ "${1:-}" = "--key" ]; then else MESSAGE=$* if [ "$MARK_FROM_FIRSTMATE" = 1 ]; then - MESSAGE=$(fm_message_mark_from_firstmate "$MESSAGE") + 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 diff --git a/tests/fm-send-secondmate-marker.test.sh b/tests/fm-send-secondmate-marker.test.sh index 75c2d3387a4..2e60d937585 100755 --- a/tests/fm-send-secondmate-marker.test.sh +++ b/tests/fm-send-secondmate-marker.test.sh @@ -206,8 +206,8 @@ test_marker_is_label_plus_invisible_separator() { test_marker_transformation_is_idempotent() { local once twice - once=$(fm_message_mark_from_firstmate "do the work") - twice=$(fm_message_mark_from_firstmate "$once") + fm_message_mark_from_firstmate "do the work" once + fm_message_mark_from_firstmate "$once" twice [ "$once" = "$twice" ] \ || fail "already-marked content was double-prefixed"$'\n'"--- once ---"$'\n'"$(printf '%s' "$once" | od -An -tx1)"$'\n'"--- twice ---"$'\n'"$(printf '%s' "$twice" | od -An -tx1)" [ "$once" = "${FM_FROMFIRST_MARK}do the work" ] \ @@ -215,6 +215,22 @@ test_marker_transformation_is_idempotent() { pass "fm-marker: from-firstmate transformation is idempotent" } +test_marked_send_preserves_trailing_newlines() { + local dir fb log home rc payload expected_hex got_hex + dir="$TMP_ROOT/sm-trailing-newlines"; mkdir -p "$dir" + fb=$(make_stubs "$dir"); log="$dir/send.log" + home=$(setup_home sm-trailing-newlines) + fm_write_secondmate_meta "$home/state/domain.meta" "$home" "sess:fm-domain" + payload=$'audit the build\n\n' + run_send "$fb" "$home" "$log" "domain" "$payload"; rc=$? + expect_code 0 "$rc" "marked send with trailing newlines should succeed" + expected_hex=$(printf '%s%s' "$FM_FROMFIRST_MARK" "$payload" | od -An -tx1 | tr -d ' \n') + got_hex=$(od -An -tx1 "$log" | tr -d ' \n') + [ "$got_hex" = "$expected_hex" ] \ + || fail "marked send changed trailing newline bytes: expected $expected_hex, got $got_hex" + pass "fm-send: marked secondmate payload preserves trailing newline bytes" +} + test_secondmate_target_is_marked test_exact_secondmate_task_id_is_marked test_crewmate_target_is_not_marked @@ -222,3 +238,4 @@ test_explicit_window_is_not_marked test_key_path_is_not_marked test_marker_is_label_plus_invisible_separator test_marker_transformation_is_idempotent +test_marked_send_preserves_trailing_newlines From b750b5aca8ac920cab72915b92293601d6461b42 Mon Sep 17 00:00:00 2001 From: kunchenguid Date: Mon, 13 Jul 2026 13:07:00 -0700 Subject: [PATCH 3/4] no-mistakes(test): Captain, tolerate bootstrap timeout elapsed drift --- tests/fm-bootstrap.test.sh | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/tests/fm-bootstrap.test.sh b/tests/fm-bootstrap.test.sh index 992306a8391..6a12d01af1e 100755 --- a/tests/fm-bootstrap.test.sh +++ b/tests/fm-bootstrap.test.sh @@ -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 '|'): #