Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion bin/fm-test-run.sh
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,8 @@ family_for_basename() {
fm-test-run.test.sh|fm-test-isolation-proof.test.sh)
printf '%s\n' pure-contract-unit
;;
fm-daemon.test.sh|fm-guard-stale-banner.test.sh|fm-pi-watch-extension.test.sh|\
fm-daemon.test.sh|fm-guard-stale-banner.test.sh|fm-hash-pane.test.sh|\
fm-pi-watch-extension.test.sh|\
fm-supervision-events.test.sh|fm-turnend-guard.test.sh|fm-wake-daemon-lifecycle-e2e.test.sh|\
fm-wake-queue.test.sh|fm-watch-checkpoint.test.sh|fm-watch-triage.test.sh|\
fm-watcher-lock.test.sh)
Expand Down
20 changes: 17 additions & 3 deletions bin/fm-watch.sh
Original file line number Diff line number Diff line change
Expand Up @@ -174,14 +174,28 @@ _event_cap_fails=0
afk_present() { [ -e "$STATE/.afk" ]; }

hash_pane() {
local sbin_md5="${FM_MD5_SBIN_OVERRIDE:-/sbin/md5}"
# Only used for change-detection between polls, so any tool that reliably
# returns the checksum in its first whitespace-delimited field is
# interchangeable here. The chain degrades through progressively more
# universal tools rather than hard-erroring when a watcher's PATH is
# missing md5/md5sum (observed on a secondmate whose runtime PATH omitted
# both): openssl and cksum are near-universal fallbacks, and wc -c is a
# last-resort that cannot itself be absent.
if command -v md5 >/dev/null 2>&1; then
md5 -q
elif [ -x /sbin/md5 ]; then
/sbin/md5 -q
elif [ -x "$sbin_md5" ]; then
"$sbin_md5" -q
elif command -v md5sum >/dev/null 2>&1; then
md5sum | cut -d' ' -f1
else
elif command -v openssl >/dev/null 2>&1; then
openssl dgst -md5 -r | cut -d' ' -f1
elif command -v shasum >/dev/null 2>&1; then
shasum | cut -d' ' -f1
elif command -v cksum >/dev/null 2>&1; then
printf '%x\n' "$(cksum | cut -d' ' -f1)"
else
wc -c | tr -d ' '
Comment on lines +197 to +198

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

Use a content-sensitive final fallback.

wc -c returns only the byte count. Two different pane states with the same length produce the same token, so hash_pane() can miss a poll-to-poll change and suppress required watcher handling.

  • bin/fm-watch.sh#L197-L198: replace the byte-count fallback with a content-sensitive token, or define and handle an explicit unavailable result.
  • docs/configuration.md#L442-L442: document the selected behavior accurately. Do not describe a byte count as reliable pane hashing.
  • tests/fm-hash-pane.test.sh#L113-L123: assert that two different equal-length inputs produce distinct results when checksum tools are unavailable.
📍 Affects 3 files
  • bin/fm-watch.sh#L197-L198 (this comment)
  • docs/configuration.md#L442-L442
  • tests/fm-hash-pane.test.sh#L113-L123
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@bin/fm-watch.sh` around lines 197 - 198, Update the final fallback in
hash_pane within bin/fm-watch.sh at lines 197-198 to produce a content-sensitive
token, or explicitly return and handle an unavailable result instead of using wc
-c; update docs/configuration.md at line 442 to accurately describe the selected
behavior, and extend tests/fm-hash-pane.test.sh at lines 113-123 to verify that
equal-length, different inputs yield distinct results when checksum tools are
unavailable.

fi
}

Expand Down
1 change: 1 addition & 0 deletions docs/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -439,6 +439,7 @@ FM_BOOTSTRAP_DETECT_ONLY=0 # internal/read-only session-start mode: skip boots
FM_GUARD_READ_ONLY=0 # internal/read-only guard mode: keep alarms but suppress drain, supervision repair, and checkout repair commands
FM_GUARD_CONTINUE_LINE='This is a supervision warning only; the guarded operation WILL still run.' # banner continuation line; fm-send.sh overrides it to name the requested message specifically
FM_POLL=15 # seconds between watcher poll cycles
FM_MD5_SBIN_OVERRIDE=/sbin/md5 # override for hash_pane()'s hardcoded /sbin/md5 fallback tier, mainly for tests; hash_pane also falls back through md5sum, openssl, shasum, and cksum before a wc -c last resort that cannot itself be absent, so a watcher's poll-to-poll pane hashing never hard-errors when PATH lacks md5/md5sum
FM_HEARTBEAT=600 # base seconds between heartbeat scans; no-change heartbeats are absorbed while idle
FM_HEARTBEAT_MAX=7200 # heartbeat backoff cap
FM_CHECK_INTERVAL=300 # seconds between slow checks (authenticated merge polls, custom checks, or X-mode dispatch)
Expand Down
129 changes: 129 additions & 0 deletions tests/fm-hash-pane.test.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
#!/usr/bin/env bash
# tests/fm-hash-pane.test.sh - bin/fm-watch.sh's hash_pane() tool-resolution
# fallback chain. hash_pane() only feeds poll-to-poll change detection, so any
# tool that yields a stable token is interchangeable; these tests confirm the
# fallback chain never hard-errors when a watcher's PATH is missing md5 and
# md5sum (observed on a secondmate whose runtime PATH omitted both, causing
# repeated watcher FAILED cycles that needed manual restarts).
#
# Each case builds a minimal PATH from symlinks to individually resolved
# binaries rather than trimming directories out of the ambient PATH: on at
# least one dev machine md5, md5sum, and openssl are all symlinked from the
# same homebrew bin directory, so removing "the directory containing md5"
# would silently remove openssl too and defeat the tier being tested.
set -u

# shellcheck source=tests/lib.sh
. "$(dirname "${BASH_SOURCE[0]}")/lib.sh"

TMP_ROOT=$(fm_test_tmproot fm-hash-pane)

# Resolve real tool paths once, before any test narrows PATH.
REAL_CUT=$(command -v cut) || fail "cut not found on the test host"
REAL_TR=$(command -v tr) || fail "tr not found on the test host"
REAL_WC=$(command -v wc) || fail "wc not found on the test host"
REAL_SBIN_MD5=/sbin/md5
[ -x "$REAL_SBIN_MD5" ] || REAL_SBIN_MD5=$(command -v md5 || true)
Comment on lines +25 to +26

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Make the sbin-md5 test independent of host tools.

On hosts without /sbin/md5 or md5, Line 66 fails the whole test file. A resolved md5 path also does not prove that the executable supports -q.

Use a test-local executable that accepts -q, or feature-probe a real executable and skip this case when unavailable. The FM_MD5_SBIN_OVERRIDE seam supports a local fixture.

Also applies to: 66-70

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/fm-hash-pane.test.sh` around lines 25 - 26, Update the sbin-md5 test
setup around REAL_SBIN_MD5 and the FM_MD5_SBIN_OVERRIDE seam so it does not
depend on host availability or behavior of /sbin/md5 or md5. Provide a
test-local executable that supports -q, or feature-probe the resolved executable
and skip the affected test when unavailable; ensure the setup used by the test
at lines 66-70 never fails the entire file on unsupported hosts.

REAL_OPENSSL=$(command -v openssl || true)
REAL_SHASUM=$(command -v shasum || true)

# make_fakebin <dir> [tool=path ...]: symlink each named tool from its
# resolved absolute path into <dir>, plus cut/tr/wc always (hash_pane's
# fallback tiers pipe through them). Echoes <dir>.
make_fakebin() {
local dir=$1 spec tool src
shift
mkdir -p "$dir"
ln -sf "$REAL_CUT" "$dir/cut"
ln -sf "$REAL_TR" "$dir/tr"
ln -sf "$REAL_WC" "$dir/wc"
for spec in "$@"; do
tool=${spec%%=*}
src=${spec#*=}
[ -n "$src" ] || fail "make_fakebin: no resolved path for $tool on this host"
ln -sf "$src" "$dir/$tool"
done
printf '%s\n' "$dir"
}

# source_watch <home>: source fm-watch.sh's function definitions into the
# current shell without running its main-entry watcher loop (fm-watch.sh
# returns early when sourced; see its "Main entry" guard).
source_watch() {
local home=$1
FM_HOME="$home"
FM_STATE_OVERRIDE="$home/state"
export FM_HOME FM_STATE_OVERRIDE
# shellcheck source=/dev/null
. "$ROOT/bin/fm-watch.sh"
}

test_hash_pane_stable_and_distinct_via_sbin_md5() (
local home fakebin out status
home="$TMP_ROOT/sbin-md5"
mkdir -p "$home"
source_watch "$home"
[ -n "$REAL_SBIN_MD5" ] || fail "no BSD-compatible md5 -q binary found on this host"
fakebin=$(make_fakebin "$home/fakebin")
status=0
out=$(PATH=$fakebin FM_MD5_SBIN_OVERRIDE="$REAL_SBIN_MD5" hash_pane <<<"pane text a") || status=$?
expect_code 0 "$status" "hash_pane exit via the sbin md5 tier"
[ -n "$out" ] || fail "hash_pane produced empty output via the sbin md5 tier"
[ "$out" = "$(PATH=$fakebin FM_MD5_SBIN_OVERRIDE="$REAL_SBIN_MD5" hash_pane <<<"pane text a")" ] \
|| fail "hash_pane is not stable for identical input via the sbin md5 tier"
[ "$out" != "$(PATH=$fakebin FM_MD5_SBIN_OVERRIDE="$REAL_SBIN_MD5" hash_pane <<<"pane text b")" ] \
|| fail "hash_pane produced the same hash for different input via the sbin md5 tier"
pass "hash_pane returns a stable, input-sensitive hash via the sbin md5 tier"
)

test_hash_pane_falls_back_to_openssl_without_md5_or_md5sum() (
local home fakebin out expected status
home="$TMP_ROOT/no-md5-no-md5sum"
mkdir -p "$home"
source_watch "$home"
[ -n "$REAL_OPENSSL" ] || fail "openssl not available to exercise the fallback tier"
fakebin=$(make_fakebin "$home/fakebin" "openssl=$REAL_OPENSSL")
status=0
out=$(PATH=$fakebin FM_MD5_SBIN_OVERRIDE="$home/no-such-md5" hash_pane <<<"pane text a") || status=$?
expect_code 0 "$status" "hash_pane exit with md5/md5sum absent from PATH"
[ -n "$out" ] || fail "hash_pane produced empty output with md5/md5sum absent from PATH"
expected=$(printf 'pane text a\n' | "$REAL_OPENSSL" dgst -md5 -r | "$REAL_CUT" -d' ' -f1)
[ "$out" = "$expected" ] \
|| fail "hash_pane did not use the openssl fallback tier when md5/md5sum are unresolvable (got '$out', wanted '$expected')"
pass "hash_pane falls back to openssl and exits cleanly when md5 and md5sum are absent from PATH"
)

test_hash_pane_falls_back_past_openssl_to_shasum() (
local home fakebin out expected status
home="$TMP_ROOT/no-md5-no-openssl"
mkdir -p "$home"
source_watch "$home"
[ -n "$REAL_SHASUM" ] || fail "shasum not available to exercise the fallback tier"
fakebin=$(make_fakebin "$home/fakebin" "shasum=$REAL_SHASUM")
status=0
out=$(PATH=$fakebin FM_MD5_SBIN_OVERRIDE="$home/no-such-md5" hash_pane <<<"pane text a") || status=$?
expect_code 0 "$status" "hash_pane exit with md5/md5sum/openssl absent from PATH"
[ -n "$out" ] || fail "hash_pane produced empty output with md5/md5sum/openssl absent from PATH"
expected=$(printf 'pane text a\n' | "$REAL_SHASUM" | "$REAL_CUT" -d' ' -f1)
[ "$out" = "$expected" ] \
|| fail "hash_pane did not use the shasum fallback tier past openssl (got '$out', wanted '$expected')"
pass "hash_pane falls back past openssl to shasum and still exits cleanly"
)

test_hash_pane_never_hard_errors_with_no_hash_tool_at_all() (
local home fakebin out status
home="$TMP_ROOT/no-hash-tool"
mkdir -p "$home"
source_watch "$home"
fakebin=$(make_fakebin "$home/fakebin")
status=0
out=$(PATH=$fakebin FM_MD5_SBIN_OVERRIDE="$home/no-such-md5" hash_pane <<<"pane text a") || status=$?
expect_code 0 "$status" "hash_pane exit with no md5/md5sum/openssl/shasum/cksum on PATH"
[ -n "$out" ] || fail "hash_pane produced empty output with no hash tool at all on PATH"
pass "hash_pane never hard-errors even with no hashing tool at all on PATH"
)
Comment on lines +96 to +124

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Add direct coverage for the cksum tier.

Lines 195-196 in bin/fm-watch.sh add a distinct output conversion path. The tests exercise openssl, shasum, and no-tool behavior, but they never provide only cksum on PATH.

Add a fake-bin case with only cksum available. Assert that hash_pane() returns the expected normalized checksum.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/fm-hash-pane.test.sh` around lines 96 - 124, Add a dedicated test
alongside the existing hash_pane fallback tests that constructs a fake PATH
containing only cksum, invokes hash_pane with the md5 override unavailable, and
verifies successful execution with the expected normalized checksum output
produced by cksum. Keep the test focused on the cksum tier and mirror the setup
and assertions used by test_hash_pane_falls_back_past_openssl_to_shasum.


test_hash_pane_stable_and_distinct_via_sbin_md5
test_hash_pane_falls_back_to_openssl_without_md5_or_md5sum
test_hash_pane_falls_back_past_openssl_to_shasum
test_hash_pane_never_hard_errors_with_no_hash_tool_at_all
Loading